fix(agent): hold input during a guarded submit instead of refusing it

The input lease is held across the awaited tmux Enter, so WebSocket frames
keep arriving while it is held. A tagged frame (seq N) was refused and its
seq rolled back, but the browser's WebSocket path does not wait for ACKs:
seq N+1 landed after the lease and moved the watermark past N, and the
redelivered N was then dropped as a duplicate. The keystroke was lost.

Input that arrives during the lease is now held and written in arrival order
when the lease is released, whatever the submit's outcome. A WebSocket frame
is applied and ACKed only once written; managed writes (writeViaMux) and
named keys (send-key) wait for their turn the same way, and a held write that
is still being sent holds everything behind it. A keystroke held before the
guard runs still moves the input revision, so it still invalidates the Enter.
The hold is capped per lease (256 inputs, 1 MiB): past it the WebSocket is
closed with 1013 and nothing later from it is applied, so the browser resends
every unACKed frame in order, and send-key answers 409 CONFLICT instead of
logging a tmux failure as INTERNAL_ERROR. POST .../input keeps its 409 for the
lease plus the held input's drain; its browser drainer is serialized.

Docs: correct the api-reference paragraph that said these keystrokes are
redelivered, and describe the capture fallback and the per-scope cursor cap
from the previous two commits in api-reference, architecture-invariants and
the changeset.
This commit is contained in:
Aamer Akhter committed 2026-10-26 09:19:05 -04:00
1 parent ecaddb40ae
commit 2aad93914e
9 files changed
+727 -100

No files matched your search

+19 -10
View File
@@ -50,7 +50,7 @@ it('fences writes and reused-pid pane lifecycles without persisting prompt conte
expect(session.guardedInputSnapshot()).toBeNull();
});
it('checks conditional Enter at the dispatch boundary and rejects concurrent browser input', async () => {
it('checks conditional Enter at the dispatch boundary and holds concurrent input until after it', async () => {
let dispatch: (() => void) | undefined;
const writes: string[] = [];
const mux = {
@@ -84,32 +84,41 @@ it('checks conditional Enter at the dispatch boundary and rejects concurrent bro
muxSession: { muxName: 'codeman-test', sessionId: 'test', createdAt: 1, pid: 1 },
} as unknown as ConstructorParameters<typeof Session>[0]);
try {
const settle = () => new Promise<void>((resolve) => setImmediate(resolve));
const expected = session.guardedInputSnapshot()!;
const sending = session.submitGuardedInput(expected);
expect(await session.writeViaMux('other')).toBe(false);
// Held behind the lease, not refused: it still invalidates the pending Enter, and
// it goes out once the lease is released.
const other = session.writeViaMux('other');
dispatch!();
expect(await sending).toBe(false);
expect(writes).toEqual([]);
await settle();
dispatch!();
expect(await other).toBe(true);
expect(writes).toEqual(['other']);
const next = session.submitGuardedInput(session.guardedInputSnapshot()!);
dispatch!();
expect(await next).toBe(true);
expect(writes).toEqual(['\r']);
expect(writes).toEqual(['other', '\r']);
const restarting = session.submitGuardedInput(session.guardedInputSnapshot()!);
await (session as unknown as { _withPaneLifecycle(op: () => Promise<void>): Promise<void> })._withPaneLifecycle(
async () => {}
);
dispatch!();
expect(await restarting).toBe(false);
expect(writes).toEqual(['\r']);
expect(writes).toEqual(['other', '\r']);
const auxiliary = session.submitGuardedInput(session.guardedInputSnapshot()!);
await expect(
session.performAuxiliaryInput(async () => {
throw new Error('must not execute');
})
).rejects.toThrow('Guarded submission');
const key = session.performAuxiliaryInput(async () => {
writes.push('aux');
});
await settle();
// The named key waits for the lease instead of failing, and never runs inside it.
expect(writes).toEqual(['other', '\r']);
dispatch!();
expect(await auxiliary).toBe(false);
expect(writes).toEqual(['\r']);
await key;
expect(writes).toEqual(['other', '\r', 'aux']);
} finally {
await session.stop(false);
}
+395
View File
@@ -0,0 +1,395 @@
/**
* @fileoverview Input that lands while a guarded submit holds the input lease is held
* and applied in arrival order once the lease is released, never refused and lost.
*
* Shaped like the review repro: a real `Session`, a mux that runs `beforeDispatch` and
* then awaits the Enter the way `TmuxManager.sendInput` does, and the real WebSocket
* route driven through a fake socket. The browser's WebSocket path does not wait for
* ACKs, so the client model below sends every frame at once and redelivers whatever
* is still unACKed afterwards, in seq order, the way `_redeliverSweep` does.
*/
import { EventEmitter } from 'node:events';
import { afterEach, describe, expect, it } from 'vitest';
import type { FastifyInstance } from 'fastify';
import { formatPaneSnapshot } from '../src/tmux-manager.js';
import { GuardedInputBusyError, Session } from '../src/session.js';
import { registerWsRoutes } from '../src/web/routes/ws-routes.js';
import type { SessionPort } from '../src/web/ports/session-port.js';
type Frame = { t: 'i'; d: string; seq?: number; cid?: string };
class FakeSocket extends EventEmitter {
readyState = 1;
sent: Array<Record<string, unknown>> = [];
closedWith: number | null = null;
send(data: string): void {
this.sent.push(JSON.parse(data));
}
close(code?: number): void {
this.closedWith = code ?? 1000;
this.readyState = 3;
}
ping(): void {}
terminate(): void {
this.readyState = 3;
}
frame(msg: Frame): void {
this.emit('message', Buffer.from(JSON.stringify(msg)));
}
acks(): number[] {
return this.sent.filter((m) => m.t === 'ia').map((m) => m.seq as number);
}
}
interface Harness {
session: Session;
pty: string[];
/** Resolve the in-flight Enter (true = tmux accepted it, false = it failed). */
finishEnter(ok?: boolean | Error): void;
enterDispatched(): boolean;
connect(cid: string): FakeSocket;
stop(): Promise<void>;
}
const sockets: FakeSocket[] = [];
function harness(awaitBeforeGuard = false): Harness {
const pty: string[] = [];
let finish: ((ok: boolean | Error) => void) | undefined;
let dispatched = false;
const mux = {
isAvailable: () => true,
capturePaneBuffer: (
_name: string,
_pane?: string,
opts?: { capturedGeometry?: { cols: number; rows: number } }
) => {
if (opts) opts.capturedGeometry = { cols: 100, rows: 20 };
return formatPaneSnapshot(['› hello', ' GPT-6-Luna default · ~/case'], {
cols: 100,
rows: 20,
cursorX: 7,
cursorY: 0,
});
},
// Like TmuxManager.sendInput's Enter-only branch: the guard runs synchronously,
// then the `send-keys Enter` child process is awaited with the lease still held.
sendInput: async (_id: string, text: string, options?: { beforeDispatch?: () => boolean }) => {
if (options?.beforeDispatch && text !== '\r') return false;
const settle = () =>
new Promise<boolean | Error>((resolve) => {
finish = resolve;
});
// A transport that awaits before its guard leaves a window in which held
// keystrokes must still invalidate the Enter.
const early = awaitBeforeGuard ? await settle() : undefined;
if (options?.beforeDispatch && !options.beforeDispatch()) return false;
dispatched = true;
const outcome = early ?? (await settle());
if (outcome instanceof Error) throw outcome;
if (outcome) pty.push(text);
return outcome;
},
};
const session = new Session({
workingDir: '/tmp',
mode: 'codex',
mux,
muxSession: { muxName: 'codeman-test', sessionId: 'test', createdAt: 1, pid: 1 },
} as unknown as ConstructorParameters<typeof Session>[0]);
const access = session as unknown as { ptyProcess: { write(data: string): void } | null };
access.ptyProcess = { write: (data: string) => pty.push(data) };
let handler: ((socket: FakeSocket, req: unknown) => void) | undefined;
const app = {
get: (_path: string, _opts: unknown, h: (socket: FakeSocket, req: unknown) => void) => {
handler = h;
},
} as unknown as FastifyInstance;
const ctx = { sessions: new Map([[session.id, session]]) } as unknown as SessionPort;
registerWsRoutes(app, ctx, () => ({ bindHost: '127.0.0.1', allowedHosts: [] }));
return {
session,
pty,
finishEnter: (ok = true) => finish!(ok),
enterDispatched: () => dispatched,
connect: (cid: string) => {
const socket = new FakeSocket();
sockets.push(socket);
handler!(socket, { headers: { host: 'localhost:3000' }, params: { id: session.id }, query: { cid } });
return socket;
},
stop: async () => {
access.ptyProcess = null;
await session.stop(false);
},
};
}
/** Redeliver every still-unACKed frame in seq order, as the browser's sweep does. */
function redeliverUnacked(socket: FakeSocket, frames: Frame[]): void {
const acked = new Set(socket.acks());
for (const f of frames) if (f.seq !== undefined && !acked.has(f.seq)) socket.frame(f);
}
afterEach(() => {
for (const s of sockets.splice(0)) s.emit('close', 1000, Buffer.from(''));
});
describe('input during a guarded submit lease', () => {
it('delivers a keystroke that landed during the lease before the one typed after it (review repro)', async () => {
const h = harness();
try {
const socket = h.connect('c1');
const a: Frame = { t: 'i', d: 'a', seq: 1, cid: 'c1' };
const b: Frame = { t: 'i', d: 'b', seq: 2, cid: 'c1' };
const submit = h.session.submitGuardedInput(h.session.guardedInputSnapshot()!);
await Promise.resolve();
expect(h.enterDispatched()).toBe(true);
expect(h.session.guardedSubmitInFlight).toBe(true);
socket.frame(a);
// Held, not refused: no ACK yet and nothing written while the lease is held.
expect(socket.acks()).toEqual([]);
expect(h.pty).toEqual([]);
h.finishEnter(true);
await expect(submit).resolves.toBe(true);
socket.frame(b);
redeliverUnacked(socket, [a, b]);
expect(h.pty).toEqual(['\r', 'a', 'b']);
expect(socket.acks()).toEqual([1, 2]);
expect(socket.sent.filter((m) => m.dup)).toEqual([]);
} finally {
await h.stop();
}
});
it('keeps arrival order for several frames during the lease and one after it', async () => {
const h = harness();
try {
const socket = h.connect('c1');
const frames: Frame[] = ['a', 'b', 'c', 'd'].map((d, i) => ({ t: 'i', d, seq: i + 1, cid: 'c1' }));
const submit = h.session.submitGuardedInput(h.session.guardedInputSnapshot()!);
await Promise.resolve();
for (const f of frames.slice(0, 3)) socket.frame(f);
expect(h.pty).toEqual([]);
h.finishEnter(true);
await submit;
socket.frame(frames[3]);
redeliverUnacked(socket, frames);
expect(h.pty).toEqual(['\r', 'a', 'b', 'c', 'd']);
expect(socket.acks()).toEqual([1, 2, 3, 4]);
} finally {
await h.stop();
}
});
it('interleaves a second client by arrival, each with its own seq space', async () => {
const h = harness();
try {
const one = h.connect('c1');
const two = h.connect('c2');
const submit = h.session.submitGuardedInput(h.session.guardedInputSnapshot()!);
await Promise.resolve();
one.frame({ t: 'i', d: 'a', seq: 1, cid: 'c1' });
two.frame({ t: 'i', d: 'x', seq: 1, cid: 'c2' });
one.frame({ t: 'i', d: 'b', seq: 2, cid: 'c1' });
h.finishEnter(true);
await submit;
two.frame({ t: 'i', d: 'y', seq: 2, cid: 'c2' });
expect(h.pty).toEqual(['\r', 'a', 'x', 'b', 'y']);
expect(one.acks()).toEqual([1, 2]);
expect(two.acks()).toEqual([1, 2]);
} finally {
await h.stop();
}
});
it('flushes held input when the lease ends in failure, refused Enter or thrown', async () => {
for (const outcome of [false, new Error('tmux send-keys failed')]) {
const h = harness();
try {
const socket = h.connect('c1');
const submit = h.session.submitGuardedInput(h.session.guardedInputSnapshot()!);
await Promise.resolve();
socket.frame({ t: 'i', d: 'a', seq: 1, cid: 'c1' });
h.finishEnter(outcome);
if (outcome instanceof Error) await expect(submit).rejects.toThrow('tmux send-keys failed');
else await expect(submit).resolves.toBe(false);
socket.frame({ t: 'i', d: 'b', seq: 2, cid: 'c1' });
expect(h.pty).toEqual(['a', 'b']);
expect(socket.acks()).toEqual([1, 2]);
expect(h.session.guardedSubmitInFlight).toBe(false);
} finally {
await h.stop();
}
}
});
it('holds untagged frames too, which have no redelivery to fall back on', async () => {
const h = harness();
try {
const socket = h.connect('c1');
const submit = h.session.submitGuardedInput(h.session.guardedInputSnapshot()!);
await Promise.resolve();
socket.frame({ t: 'i', d: 'a' });
h.finishEnter(true);
await submit;
socket.frame({ t: 'i', d: 'b' });
expect(h.pty).toEqual(['\r', 'a', 'b']);
} finally {
await h.stop();
}
});
it('a frame held for a socket that closed is still applied once; its redelivery is a duplicate', async () => {
const h = harness();
try {
const first = h.connect('c1');
const a: Frame = { t: 'i', d: 'a', seq: 1, cid: 'c1' };
const submit = h.session.submitGuardedInput(h.session.guardedInputSnapshot()!);
await Promise.resolve();
first.frame(a);
first.close();
first.emit('close', 1006, Buffer.from(''));
h.finishEnter(true);
await submit;
expect(first.sent).toEqual([]);
const second = h.connect('c1');
second.frame(a);
second.frame({ t: 'i', d: 'b', seq: 2, cid: 'c1' });
expect(h.pty).toEqual(['\r', 'a', 'b']);
expect(second.sent.filter((m) => m.t === 'ia')).toEqual([
{ t: 'ia', seq: 1, dup: true, last: 1 },
{ t: 'ia', seq: 2 },
]);
} finally {
await h.stop();
}
});
it('closes the socket instead of holding past the cap, and the reconnect resends in order', async () => {
const h = harness();
try {
const socket = h.connect('c1');
const cap = Session.MAX_GUARDED_HELD_INPUTS;
const frames: Frame[] = Array.from({ length: cap + 3 }, (_, i) => ({
t: 'i',
d: `k${i}`,
seq: i + 1,
cid: 'c1',
}));
const submit = h.session.submitGuardedInput(h.session.guardedInputSnapshot()!);
await Promise.resolve();
for (const f of frames) socket.frame(f);
// Frames past the cap are not held, not applied and not ACKed; the socket is
// closed so the browser reconnects and resends everything still unACKed.
expect(socket.closedWith).toBe(1013);
h.finishEnter(true);
await submit;
expect(h.pty.length).toBe(1 + cap);
expect(socket.acks()).toEqual([]);
socket.emit('close', 1013, Buffer.from(''));
const again = h.connect('c1');
for (const f of frames) again.frame(f);
expect(h.pty).toEqual(['\r', ...frames.map((f) => f.d)]);
expect(again.acks()).toEqual(frames.map((f) => f.seq));
} finally {
await h.stop();
}
});
it('holds managed writes and named keys behind the lease, and refuses them only when the hold queue is full', async () => {
const h = harness();
try {
const submit = h.session.submitGuardedInput(h.session.guardedInputSnapshot()!);
await Promise.resolve();
const order: string[] = [];
const key = h.session.performAuxiliaryInput(async () => {
order.push('key');
});
await new Promise<void>((resolve) => setImmediate(resolve));
expect(order).toEqual([]);
for (let i = 1; i < Session.MAX_GUARDED_HELD_INPUTS; i++) {
expect(h.session.holdForGuardedLease(() => order.push('frame'))).toBe('held');
}
expect(h.session.holdForGuardedLease(() => order.push('lost'))).toBe('full');
await expect(h.session.performAuxiliaryInput(async () => order.push('lost'))).rejects.toBeInstanceOf(
GuardedInputBusyError
);
await expect(h.session.writeViaMux('lost')).resolves.toBe(false);
h.finishEnter(true);
await submit;
await key;
expect(order[0]).toBe('key');
expect(order.filter((o) => o === 'frame')).toHaveLength(Session.MAX_GUARDED_HELD_INPUTS - 1);
expect(order).not.toContain('lost');
} finally {
await h.stop();
}
});
it('caps held bytes as well as held frames', async () => {
const h = harness();
try {
const submit = h.session.submitGuardedInput(h.session.guardedInputSnapshot()!);
await Promise.resolve();
expect(h.session.holdForGuardedLease(() => {}, Session.MAX_GUARDED_HELD_BYTES)).toBe('held');
expect(h.session.holdForGuardedLease(() => {}, 1)).toBe('full');
h.finishEnter(true);
await submit;
expect(h.session.holdForGuardedLease(() => {}, Session.MAX_GUARDED_HELD_BYTES + 1)).toBe('free');
} finally {
await h.stop();
}
});
it('a frame arriving while a held named key is still being sent waits for it', async () => {
const h = harness();
try {
const socket = h.connect('c1');
let keySent!: () => void;
const submit = h.session.submitGuardedInput(h.session.guardedInputSnapshot()!);
await Promise.resolve();
const key = h.session.performAuxiliaryInput(async () => {
await new Promise<void>((resolve) => {
keySent = resolve;
});
h.pty.push('<newline>');
});
socket.frame({ t: 'i', d: 'a', seq: 1, cid: 'c1' });
h.finishEnter(true);
await submit;
// The lease is over, but the held key is still in flight: input stays held.
expect(h.session.guardedSubmitInFlight).toBe(false);
expect(h.session.guardedInputHeld).toBe(true);
socket.frame({ t: 'i', d: 'b', seq: 2, cid: 'c1' });
expect(h.pty).toEqual(['\r']);
keySent();
await key;
await new Promise<void>((resolve) => setImmediate(resolve));
expect(h.pty).toEqual(['\r', '<newline>', 'a', 'b']);
expect(socket.acks()).toEqual([1, 2]);
expect(h.session.guardedInputHeld).toBe(false);
} finally {
await h.stop();
}
});
it('a keystroke held before the guard runs still invalidates the Enter, then lands', async () => {
const h = harness(true);
try {
const socket = h.connect('c1');
const submit = h.session.submitGuardedInput(h.session.guardedInputSnapshot()!);
await Promise.resolve();
socket.frame({ t: 'i', d: 'a', seq: 1, cid: 'c1' });
h.finishEnter(true);
await expect(submit).resolves.toBe(false);
expect(h.enterDispatched()).toBe(false);
expect(h.pty).toEqual(['a']);
expect(socket.acks()).toEqual([1]);
} finally {
await h.stop();
}
});
});
+30 -3
View File
@@ -25,7 +25,7 @@ import { tmpdir } from 'node:os';
import { createMockRouteContext, type MockRouteContext } from '../mocks/index.js';
import { installRouteErrorHandler } from '../../src/web/route-error-handler.js';
import { ApiErrorCode, httpStatusForErrorCode } from '../../src/types.js';
import { Session } from '../../src/session.js';
import { GuardedInputBusyError, Session } from '../../src/session.js';
import { agentInbox } from '../../src/web/agent-inbox.js';
// Mock execFile so the send-key route's `tmux` invocation is observable (not run for real).
@@ -265,6 +265,33 @@ describe('session-routes', () => {
});
});
it('answers 409 CONFLICT, without an error log, when a guarded submit cannot hold the key', async () => {
execFile.mockReset();
const session = harness.ctx._session as unknown as {
performAuxiliaryInput: (op: () => Promise<void>) => Promise<void>;
};
const original = session.performAuxiliaryInput;
session.performAuxiliaryInput = async () => {
throw new GuardedInputBusyError();
};
const errors = vi.spyOn(console, 'error').mockImplementation(() => {});
try {
const res = await harness.app.inject({
method: 'POST',
url: '/api/sessions/test-session-1/send-key',
payload: { key: 'S-Enter' },
});
expect(res.statusCode).toBe(409);
expect(res.json()).toMatchObject({ success: false, errorCode: 'CONFLICT' });
expect(res.json().error).toMatch(/guarded submit is in progress.*nothing was written/);
expect(errors).not.toHaveBeenCalled();
expect(execFile).not.toHaveBeenCalled();
} finally {
errors.mockRestore();
session.performAuxiliaryInput = original;
}
});
it('rejects keys outside the hex allowlist without invoking tmux', async () => {
execFile.mockReset();
const res = await harness.app.inject({
@@ -1203,7 +1230,7 @@ describe('session-routes', () => {
const session = harness.ctx.sessions.get(harness.ctx._sessionId) as any;
session.writeBuffer.length = 0;
const post = (payload: unknown) => harness.app.inject({ method: 'POST', url, payload });
session.guardedSubmitInFlight = true;
session.guardedInputHeld = true;
for (const payload of [
{ input: 'x' },
{ input: 'x', useMux: true },
@@ -1218,7 +1245,7 @@ describe('session-routes', () => {
expect(session.writeBuffer).toEqual([]);
// Once the lease is released the browser's retry of the same seq is applied, not
// rejected as a duplicate.
session.guardedSubmitInFlight = false;
session.guardedInputHeld = false;
expect((await post({ input: 'tagged', seq: 7, clientId: 'cid-guard' })).statusCode).toBe(200);
expect(session.writeBuffer).toEqual(['tagged']);
});