diff --git a/docs/reliable-input-delivery.md b/docs/reliable-input-delivery.md index 213dd834..99f65beb 100644 --- a/docs/reliable-input-delivery.md +++ b/docs/reliable-input-delivery.md @@ -66,6 +66,31 @@ each `(clientId, seq)` at most once, so a resend can't type the prompt twice. (the 200 is the client's ACK). `curl`/legacy callers omit the fields and always apply. +## Oversized input (issue #484) + +Delivery has a third outcome besides "applied" and "retry": **refused for good**. +Both transports refuse a frame longer than `MAX_INPUT_LENGTH` (64 KiB, +`src/config/terminal-limits.ts`; the POST schema uses the same constant). Before +#484 the client treated that like a transient failure, so an oversized paste sat +at the head of the queue, was re-sent every 2 s forever, blocked every later +input for the session, and came back from localStorage on each reload. + +- `_sendInputAsync()` splits a paste over the frame limit into in-limit frames + (`CodemanInputLimit.split`, constants.js, never cutting a surrogate pair). They + go out in seq order, so the PTY sees one contiguous stream. A paste over + `PASTE_MAX_CHARS` (1 MiB), or an oversized `useMux` write (line-oriented, never + split), is refused with a toast and never queued. +- The WebSocket answers an oversized sequenced frame with + `{t:'ia', seq, err:'too_large', max}`; the client drops it with a toast. A + client that predates `err` reads it as a plain ACK and drops it too. +- The POST drain drops a frame answered `400`/`413` (`401`/`403` stay transient: + an expired login delivers once the user signs in again). +- `_loadReliableState()` prunes persisted frames over the limit, so a queue + poisoned by an older build heals on the first load after upgrading. +- ⚠️ The frontend limit (`INPUT_FRAME_MAX_CHARS`) and the composer's + `COMPOSER_INPUT_FRAME_LIMIT` must equal `MAX_INPUT_LENGTH`; pinned by + `test/input-size-limit.test.ts`. + ## Known limitation Dedup state is in-memory on the server. A **server restart** between a write and @@ -79,3 +104,5 @@ across the narrow restart window. semantics (monotonic, per-client, gap-tolerant, eviction-safe). - `test/routes/session-routes.test.ts` — POST `/input` applies a tagged `(clientId, seq)` once on redelivery; untagged input always applies. +- `test/input-size-limit.test.ts`: one input limit on both sides, frame + splitting, and dropping (never retrying) a frame refused for good (#484). diff --git a/src/web/public/app.js b/src/web/public/app.js index 5bfdfe43..9f2ba5b5 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -3427,7 +3427,26 @@ class CodemanApp { */ _sendInputAsync(sessionId, input, opts) { if (!sessionId || !input) return; - this._reliableSend(sessionId, input, opts?.useMux === true); + const useMux = opts?.useMux === true; + // Both transports refuse a frame over the server's limit (issue #484), and a + // refused frame used to sit at the head of the durable queue for good. So an + // oversized paste goes out as several in-limit frames, delivered in seq order + // as one contiguous stream. A mux write is line-oriented (it strips newlines + // and sends Enter on its own), so it is never split: refuse it instead. + const limit = window.CodemanInputLimit; + if (limit && input.length > limit.FRAME_MAX_CHARS) { + if (useMux || input.length > limit.PASTE_MAX_CHARS) { + const max = useMux ? limit.FRAME_MAX_CHARS : limit.PASTE_MAX_CHARS; + this.showToast?.( + `Input too large (${Math.ceil(input.length / 1024)} KB, limit ${Math.floor(max / 1024)} KB); not sent`, + 'error' + ); + return; + } + for (const frame of limit.split(input)) this._reliableSend(sessionId, frame, false); + return; + } + this._reliableSend(sessionId, input, useMux); } /** @@ -3547,6 +3566,12 @@ class CodemanApp { // Session no longer exists — the input can never land. Drop it // rather than retry forever (not a "lost" prompt: the target is gone). this._ackDelivery(sessionId, rec.seq); + } else if (resp && (resp.status === 400 || resp.status === 413)) { + // The frame itself was refused, so a retry gets the same answer. Kept + // queued, it was re-POSTed every 2 s forever and blocked every later + // input for this session behind it (issue #484). 401/403 stay + // transient: an expired login delivers fine once the user signs in. + this._dropRejectedInput(sessionId, rec); } else { break; // offline / 5xx — leave queued; sweep + reconnect retry later } @@ -3557,6 +3582,12 @@ class CodemanApp { })(); } + /** Drop a frame the server refused for good, and say so once. */ + _dropRejectedInput(sessionId, rec) { + this._ackDelivery(sessionId, rec.seq); + this.showToast?.(`Input refused by the server (${Math.ceil(rec.data.length / 1024)} KB); not sent`, 'error'); + } + /** Drop an ACKed record (by exact seq) and persist. */ _ackDelivery(sessionId, seq) { const list = this._pendingDeliveries.get(sessionId); @@ -3601,6 +3632,13 @@ class CodemanApp { _onWsInputAck(seq, msg) { const sessionId = this._wsSessionId; if (!sessionId || !Number.isInteger(seq)) return; + if (msg && msg.err) { + // Refused for good (e.g. over the size limit): retrying cannot help. + const rec = (this._pendingDeliveries.get(sessionId) || []).find((r) => r.seq === seq); + if (rec) this._dropRejectedInput(sessionId, rec); + else this._ackDelivery(sessionId, seq); + return; + } if (msg && msg.dup) { const list = this._pendingDeliveries.get(sessionId); const rec = list && list.find((r) => r.seq === seq); @@ -3714,20 +3752,22 @@ class CodemanApp { if (saved && saved.pending) { for (const [s, recs] of Object.entries(saved.pending)) { if (Array.isArray(recs) && recs.length) { - // Reset sentAt so they re-deliver promptly on this fresh load. - this._pendingDeliveries.set( - s, - recs - .filter((r) => r && typeof r.data === 'string' && Number.isInteger(r.seq)) - .map((r) => ({ - seq: r.seq, - data: r.data, - useMux: !!r.useMux, - ts: r.ts || Date.now(), - tries: 0, - sentAt: 0, - })) - ); + const frameMax = window.CodemanInputLimit?.FRAME_MAX_CHARS ?? Infinity; + const kept = recs + .filter((r) => r && typeof r.data === 'string' && Number.isInteger(r.seq)) + // A frame over the server's limit can never be ACKed; one persisted + // by an older build would otherwise come back on every load (#484). + .filter((r) => r.data.length <= frameMax) + // Reset sentAt so they re-deliver promptly on this fresh load. + .map((r) => ({ + seq: r.seq, + data: r.data, + useMux: !!r.useMux, + ts: r.ts || Date.now(), + tries: 0, + sentAt: 0, + })); + if (kept.length) this._pendingDeliveries.set(s, kept); } } } diff --git a/src/web/public/constants.js b/src/web/public/constants.js index 15f24368..6f9d85c8 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -762,6 +762,49 @@ function resolveTerminalFontWeights(settings) { // without a terminal, a clipboard, or a browser. // --------------------------------------------------------------------------- +/** + * Largest single input frame the server accepts, in UTF-16 code units. + * ⚠️ Must equal MAX_INPUT_LENGTH in src/config/terminal-limits.ts (pinned by + * test/input-size-limit.test.ts). Both transports reject a longer frame, and + * before issue #484 the durable input queue retried such a frame forever. + */ +const INPUT_FRAME_MAX_CHARS = 64 * 1024; + +/** + * Largest paste the client will deliver at all. Anything up to this is split + * into INPUT_FRAME_MAX_CHARS frames that go out in seq order, so the PTY sees + * one contiguous byte stream (bracketed-paste markers included). Past it the + * input is refused with a toast rather than queued: every frame is persisted + * and retried until ACKed, so a multi-megabyte paste would pin the queue. + */ +const INPUT_PASTE_MAX_CHARS = 1024 * 1024; + +/** + * Split input into frames no longer than `max` code units, never cutting a + * surrogate pair in half (a lone surrogate reaches the PTY as U+FFFD). + * + * @param {string} data + * @param {number} [max] + * @returns {string[]} + */ +function splitInputFrames(data, max = INPUT_FRAME_MAX_CHARS) { + if (typeof data !== 'string' || data.length === 0) return []; + if (!(max >= 2)) max = 2; + if (data.length <= max) return [data]; + const frames = []; + let start = 0; + while (start < data.length) { + let end = Math.min(start + max, data.length); + if (end < data.length) { + const code = data.charCodeAt(end - 1); + if (code >= 0xd800 && code <= 0xdbff) end--; // keep the pair together + } + frames.push(data.slice(start, end)); + start = end; + } + return frames; +} + /** * Upper bound on an AUTO-copied selection. * @@ -940,6 +983,11 @@ if (typeof window !== 'undefined') { compare: compareSessionActivity, sort: sortSessionsByActivity, }; + window.CodemanInputLimit = { + FRAME_MAX_CHARS: INPUT_FRAME_MAX_CHARS, + PASTE_MAX_CHARS: INPUT_PASTE_MAX_CHARS, + split: splitInputFrames, + }; window.CodemanAutoCopy = { decide: decideAutoCopy, MAX_CHARS: AUTO_COPY_MAX_CHARS, diff --git a/src/web/routes/ws-routes.ts b/src/web/routes/ws-routes.ts index 9cefcb21..19c2ddca 100644 --- a/src/web/routes/ws-routes.ts +++ b/src/web/routes/ws-routes.ts @@ -175,7 +175,16 @@ export function registerWsRoutes(app: FastifyInstance, ctx: SessionPort, getHost try { const msg = JSON.parse(String(raw)); if (msg.t === 'i' && typeof msg.d === 'string') { - if (msg.d.length > MAX_INPUT_LENGTH) return; + if (msg.d.length > MAX_INPUT_LENGTH) { + // Refused for good, so say so: a silent return left the frame + // unACKed and the client redelivered it every few seconds forever + // (issue #484). A client that predates `err` reads this as a plain + // ACK and drops the frame, which is also the right outcome. + if (Number.isInteger(msg.seq) && socket.readyState === 1) { + socket.send(`{"t":"ia","seq":${msg.seq as number},"err":"too_large","max":${MAX_INPUT_LENGTH}}`); + } + return; + } // Reliable delivery: when the frame carries a clientId + seq, apply it // exactly once (skip a duplicate redelivery) but ACK it regardless so // the client can drop it from its durable queue. Frames without seq diff --git a/src/web/schemas.ts b/src/web/schemas.ts index 400552c0..c12a83a9 100644 --- a/src/web/schemas.ts +++ b/src/web/schemas.ts @@ -20,6 +20,7 @@ import { import { MAX_EDITABLE_BYTES } from '../config/file-editing.js'; import { MIN_MATCH_LENGTH, MAX_MATCH_LENGTH } from '../config/agent-wait.js'; import { MAX_WAKE_MACS } from '../config/remote-wake-limits.js'; +import { MAX_INPUT_LENGTH } from '../config/terminal-limits.js'; import { enabledCliIds, enabledClis } from '../config/cli-registry/registry.js'; import type { SessionMode } from '../types.js'; @@ -1500,7 +1501,10 @@ export const SettingsUpdateSchema = z * Schema for POST /api/sessions/:id/input with length limit */ export const SessionInputWithLimitSchema = z.object({ - input: z.string().max(100000), // 100KB max input + // One limit for both transports (issue #484): the route's own length check and + // ws-routes.ts read the same constant, so a schema cap above it only hid which + // check refused the input. + input: z.string().max(MAX_INPUT_LENGTH), useMux: z.boolean().optional(), // Reliable-delivery dedup (optional; absent for curl/legacy clients). The web // client tags each input with a stable clientId + a monotonic per-session seq diff --git a/test/input-size-limit.test.ts b/test/input-size-limit.test.ts new file mode 100644 index 00000000..6c7c5269 --- /dev/null +++ b/test/input-size-limit.test.ts @@ -0,0 +1,108 @@ +/** + * @fileoverview Oversized input must never poison the durable input queue (#484). + * + * The failure: a paste over MAX_INPUT_LENGTH was queued for reliable delivery, + * refused by both transports (the WebSocket silently, the POST with a 400), and + * never dropped by the client, which treated a 400 like a transient failure. It + * was re-sent every 2 s forever, blocked every later input for that session + * behind it, and came back from localStorage on every reload. + * + * The fix, pinned here: the client splits a large paste into in-limit frames + * (one limit, shared with the server), drops a frame the server refused for + * good, and prunes oversized frames persisted by an older build. + */ +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import vm from 'node:vm'; +import { describe, expect, it } from 'vitest'; +import { MAX_INPUT_LENGTH } from '../src/config/terminal-limits.js'; +import { SessionInputWithLimitSchema } from '../src/web/schemas.js'; + +const pub = (f: string) => readFileSync(resolve(import.meta.dirname, '../src/web/public', f), 'utf8'); +const appSource = pub('app.js'); + +type InputLimit = { FRAME_MAX_CHARS: number; PASTE_MAX_CHARS: number; split: (d: string, max?: number) => string[] }; + +function loadLimit(): InputLimit { + const context = vm.createContext({ window: {}, globalThis: {} }); + vm.runInContext(pub('constants.js'), context, { filename: 'constants.js' }); + return (context.window as { CodemanInputLimit: InputLimit }).CodemanInputLimit; +} + +describe('one input limit on both sides', () => { + it('the frontend frame limit equals the server MAX_INPUT_LENGTH', () => { + expect(loadLimit().FRAME_MAX_CHARS).toBe(MAX_INPUT_LENGTH); + }); + + it('the composer budget is derived from the same number', () => { + expect(pub('keyboard-accessory.js')).toMatch( + new RegExp(`COMPOSER_INPUT_FRAME_LIMIT = (64 \\* 1024|${MAX_INPUT_LENGTH});`) + ); + expect(MAX_INPUT_LENGTH).toBe(64 * 1024); + }); + + it('the POST schema caps input at MAX_INPUT_LENGTH, not a second number', () => { + expect(SessionInputWithLimitSchema.safeParse({ input: 'x'.repeat(MAX_INPUT_LENGTH) }).success).toBe(true); + expect(SessionInputWithLimitSchema.safeParse({ input: 'x'.repeat(MAX_INPUT_LENGTH + 1) }).success).toBe(false); + }); +}); + +describe('splitInputFrames', () => { + const { split } = loadLimit(); + + it('returns a short input as one frame and nothing for empty input', () => { + expect(split('abc')).toEqual(['abc']); + expect(split('')).toEqual([]); + }); + + it('splits a large paste into in-limit frames that rejoin byte-identically', () => { + const paste = '\x1b[200~' + 'log line\n'.repeat(15000) + '\x1b[201~'; + const frames = split(paste); + expect(frames.length).toBeGreaterThan(1); + for (const f of frames) expect(f.length).toBeLessThanOrEqual(MAX_INPUT_LENGTH); + expect(frames.join('')).toBe(paste); + }); + + it('never cuts a surrogate pair in half', () => { + const paste = 'a' + '😀'.repeat(10); // pairs start at odd offsets + const frames = split(paste, 4); + expect(frames.join('')).toBe(paste); + for (const f of frames) { + const last = f.charCodeAt(f.length - 1); + expect(last >= 0xd800 && last <= 0xdbff).toBe(false); + const first = f.charCodeAt(0); + expect(first >= 0xdc00 && first <= 0xdfff).toBe(false); + } + }); +}); + +describe('the client never queues or keeps an undeliverable frame', () => { + const sendAsync = appSource.slice( + appSource.indexOf('_sendInputAsync(sessionId, input, opts) {'), + appSource.indexOf('_sendInputEphemeral(sessionId, input) {') + ); + + it('splits an oversized paste, and refuses an oversized mux write or giant paste with a toast', () => { + expect(sendAsync).toContain('limit.split(input)'); + expect(sendAsync).toMatch(/useMux \|\| input\.length > limit\.PASTE_MAX_CHARS/); + expect(sendAsync).toContain('showToast'); + }); + + it('drops a POST the server refused as invalid instead of retrying it forever', () => { + const drain = appSource.slice(appSource.indexOf('_drainSession(sessionId) {')); + const body = drain.slice(0, drain.indexOf('_dropRejectedInput(sessionId, rec) {')); + expect(body).toMatch( + /resp\.status === 400 \|\| resp\.status === 413\)\) \{\s*[\s\S]{0,400}this\._dropRejectedInput\(sessionId, rec\);/ + ); + }); + + it('drops a frame the WebSocket refused with an error ACK', () => { + const handler = appSource.slice(appSource.indexOf('_onWsInputAck(seq, msg) {')); + expect(handler.slice(0, 600)).toMatch(/if \(msg && msg\.err\)/); + }); + + it('prunes oversized frames persisted by an older build on load', () => { + const load = appSource.slice(appSource.indexOf('_loadReliableState() {')); + expect(load.slice(0, 3000)).toMatch(/r\.data\.length <= frameMax/); + }); +}); diff --git a/test/routes/ws-routes.test.ts b/test/routes/ws-routes.test.ts index 0d7c3bd0..2ca5110d 100644 --- a/test/routes/ws-routes.test.ts +++ b/test/routes/ws-routes.test.ts @@ -282,6 +282,22 @@ describe('ws-routes', () => { } }); + it('refuses an oversized sequenced frame with an error ACK, so the client can drop it', async () => { + // Issue #484: a silent return left the frame unACKed, and the client's + // durable queue re-sent it every few seconds forever. + const ws = await connectWs('/ws/sessions/ws-test-session/terminal'); + try { + const session = ctx._session; + const hugeInput = 'y'.repeat(MAX_INPUT_LENGTH + 1); + ws.send(JSON.stringify({ t: 'i', d: hugeInput, cid: 'c1', seq: 3 })); + + expect(await nextMessage(ws)).toEqual({ t: 'ia', seq: 3, err: 'too_large', max: MAX_INPUT_LENGTH }); + expect(session.writeBuffer).not.toContain(hugeInput); + } finally { + ws.close(); + } + }); + it('ignores malformed JSON messages', async () => { const ws = await connectWs('/ws/sessions/ws-test-session/terminal'); try {