mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-15 02:49:33 +01:00
fix: apply the review items for #694 at merge
- src/web/routes/session-routes.ts:2102: the guarded-submit 409 now applies to tagged input (clientId + seq) only; untagged input is held like a WebSocket frame. The raw write (:2228) goes through holdForGuardedLease and answers 200 once held (a send-and-wait reads `delivered` from the held write; a full queue releases the waiter and answers 409); a mux write takes its own turn in writeViaMux. - test/guarded-lease-held-input.test.ts: real-Session route tests for an untagged raw POST during the lease (written after the Enter) and a held send-and-wait (delivered: true); both fail without the fix. - test/routes/session-routes.test.ts + test/mocks/mock-session.ts: the 409 test now covers tagged input only; new tests for untagged raw (held), untagged mux (own turn) and a full hold queue (409); MockSession gains holdForGuardedLease/releaseGuardedLease and a lease-aware writeViaMux. - src/session.ts:4787 (guardedInputHeld JSDoc) + write() + docs/architecture-invariants.md: write() refuses during the lease only, so cron's paste mode can land ahead of input still draining. - docs/api-reference.md: the input route's 409 is for tagged input; untagged input is held. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
2aad93914e
commit
6368128f1e
7 files changed
+221
-32
No files matched your search
@@ -9,12 +9,14 @@
|
||||
* is still unACKed afterwards, in seq order, the way `_redeliverSweep` does.
|
||||
*/
|
||||
import { EventEmitter } from 'node:events';
|
||||
import { afterEach, describe, expect, it } from 'vitest';
|
||||
import { afterEach, describe, expect, it, vi } 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 { registerSessionRoutes } from '../src/web/routes/session-routes.js';
|
||||
import type { SessionPort } from '../src/web/ports/session-port.js';
|
||||
import { createRouteTestHarness } from './routes/_route-test-utils.js';
|
||||
|
||||
type Frame = { t: 'i'; d: string; seq?: number; cid?: string };
|
||||
|
||||
@@ -376,6 +378,59 @@ describe('input during a guarded submit lease', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('holds an untagged raw POST .../input during the lease and writes it after the Enter', async () => {
|
||||
const h = harness();
|
||||
const route = await createRouteTestHarness(registerSessionRoutes);
|
||||
try {
|
||||
(route.ctx.sessions as unknown as Map<string, Session>).set(h.session.id, h.session);
|
||||
const submit = h.session.submitGuardedInput(h.session.guardedInputSnapshot()!);
|
||||
await Promise.resolve();
|
||||
expect(h.session.guardedSubmitInFlight).toBe(true);
|
||||
// The Stop button's Ctrl+C: an untagged POST whose answer nobody reads.
|
||||
const res = await route.app.inject({
|
||||
method: 'POST',
|
||||
url: `/api/sessions/${h.session.id}/input`,
|
||||
payload: { input: '\x03' },
|
||||
});
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(h.pty).toEqual([]);
|
||||
h.finishEnter(true);
|
||||
await expect(submit).resolves.toBe(true);
|
||||
expect(h.pty).toEqual(['\r', '\x03']);
|
||||
} finally {
|
||||
await route.app.close();
|
||||
await h.stop();
|
||||
}
|
||||
});
|
||||
|
||||
it('a held untagged raw send-and-wait reports the held write as delivered', async () => {
|
||||
const h = harness();
|
||||
const route = await createRouteTestHarness(registerSessionRoutes);
|
||||
try {
|
||||
(route.ctx.sessions as unknown as Map<string, Session>).set(h.session.id, h.session);
|
||||
const submit = h.session.submitGuardedInput(h.session.guardedInputSnapshot()!);
|
||||
await Promise.resolve();
|
||||
const answer = route.app.inject({
|
||||
method: 'POST',
|
||||
url: `/api/sessions/${h.session.id}/input`,
|
||||
payload: { input: '\x03', useMux: false, wait: 'exit', waitTimeout: 1000 },
|
||||
});
|
||||
// Private, but the only proof the route reached the hold before the lease ends.
|
||||
const queue = (h.session as unknown as { _guardedHeld: unknown[] })._guardedHeld;
|
||||
await vi.waitFor(() => expect(queue).toHaveLength(1));
|
||||
expect(h.pty).toEqual([]);
|
||||
h.finishEnter(true);
|
||||
await submit;
|
||||
const res = await answer;
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(res.json().data).toMatchObject({ delivered: true, duplicate: false });
|
||||
expect(h.pty).toEqual(['\r', '\x03']);
|
||||
} finally {
|
||||
await route.app.close();
|
||||
await h.stop();
|
||||
}
|
||||
});
|
||||
|
||||
it('a keystroke held before the guard runs still invalidates the Enter, then lands', async () => {
|
||||
const h = harness(true);
|
||||
try {
|
||||
|
||||
@@ -111,6 +111,22 @@ export class MockSession extends EventEmitter {
|
||||
await operation();
|
||||
}
|
||||
|
||||
/** Mirrors Session.guardedInputHeld; a route test sets it to stand in for a guarded submit's lease. */
|
||||
guardedInputHeld = false;
|
||||
/** Input held behind that lease, in arrival order (see releaseGuardedLease). */
|
||||
heldGuardedInput: Array<() => void | Promise<unknown>> = [];
|
||||
/** Mirrors Session.holdForGuardedLease, without the cap (a test replaces it to model a full queue). */
|
||||
holdForGuardedLease(run: () => void | Promise<unknown>, _bytes = 0, _data?: string): 'free' | 'held' | 'full' {
|
||||
if (!this.guardedInputHeld) return 'free';
|
||||
this.heldGuardedInput.push(run);
|
||||
return 'held';
|
||||
}
|
||||
/** Ends the stand-in lease and runs the held input in arrival order, as Session does. */
|
||||
releaseGuardedLease(): void {
|
||||
this.guardedInputHeld = false;
|
||||
for (const run of this.heldGuardedInput.splice(0)) void run();
|
||||
}
|
||||
|
||||
private _muxName: string | null = null;
|
||||
|
||||
constructor(id: string = 'mock-session-id') {
|
||||
@@ -134,9 +150,14 @@ export class MockSession extends EventEmitter {
|
||||
|
||||
/** Write via mux (used by respawn controller) */
|
||||
async writeViaMux(data: string): Promise<boolean> {
|
||||
if (this.failWrites) return false;
|
||||
this.writeBuffer.push(data);
|
||||
return true;
|
||||
const write = (): boolean => {
|
||||
if (this.failWrites) return false;
|
||||
this.writeBuffer.push(data);
|
||||
return true;
|
||||
};
|
||||
// Like Session.writeViaMux, it waits its turn behind a stand-in guarded lease.
|
||||
if (!this.guardedInputHeld) return write();
|
||||
return new Promise<boolean>((resolve) => this.heldGuardedInput.push(() => resolve(write())));
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -1224,7 +1224,7 @@ describe('session-routes', () => {
|
||||
expect(session.writeBuffer).toEqual(['prompt', '\r']);
|
||||
});
|
||||
|
||||
it('refuses with 409 and gives the seq back while a guarded submit holds the input lease', async () => {
|
||||
it('refuses tagged input with 409 and gives the seq back while a guarded submit holds the input lease', async () => {
|
||||
const url = `/api/sessions/${harness.ctx._sessionId}/input`;
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
const session = harness.ctx.sessions.get(harness.ctx._sessionId) as any;
|
||||
@@ -1232,10 +1232,8 @@ describe('session-routes', () => {
|
||||
const post = (payload: unknown) => harness.app.inject({ method: 'POST', url, payload });
|
||||
session.guardedInputHeld = true;
|
||||
for (const payload of [
|
||||
{ input: 'x' },
|
||||
{ input: 'x', useMux: true },
|
||||
{ input: 'prompt\r' },
|
||||
{ input: 'tagged', seq: 7, clientId: 'cid-guard' },
|
||||
{ input: 'prompt\r', useMux: true, seq: 1, clientId: 'cid-guard-mux' },
|
||||
]) {
|
||||
const refused = await post(payload);
|
||||
expect(refused.statusCode, JSON.stringify(payload)).toBe(409);
|
||||
@@ -1243,13 +1241,76 @@ describe('session-routes', () => {
|
||||
expect(refused.json().error).toMatch(/guarded submit is in progress.*nothing was written/);
|
||||
}
|
||||
expect(session.writeBuffer).toEqual([]);
|
||||
expect(session.heldGuardedInput).toEqual([]);
|
||||
// Once the lease is released the browser's retry of the same seq is applied, not
|
||||
// rejected as a duplicate.
|
||||
session.guardedInputHeld = false;
|
||||
session.releaseGuardedLease();
|
||||
expect((await post({ input: 'tagged', seq: 7, clientId: 'cid-guard' })).statusCode).toBe(200);
|
||||
expect(session.writeBuffer).toEqual(['tagged']);
|
||||
});
|
||||
|
||||
it('holds an untagged raw POST during a guarded submit and writes it once the lease is released', async () => {
|
||||
// The phone accessory keys and the Stop button's Ctrl+C are untagged, fire-and-forget
|
||||
// POSTs that never read the answer, so a 409 here silently lost the tap.
|
||||
const url = `/api/sessions/${harness.ctx._sessionId}/input`;
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
const session = harness.ctx.sessions.get(harness.ctx._sessionId) as any;
|
||||
session.writeBuffer.length = 0;
|
||||
session.guardedInputHeld = true;
|
||||
try {
|
||||
for (const input of ['\x03', '\x1b[A']) {
|
||||
const res = await harness.app.inject({ method: 'POST', url, payload: { input } });
|
||||
expect(res.statusCode, JSON.stringify(input)).toBe(200);
|
||||
expect(res.json()).toMatchObject({ success: true });
|
||||
}
|
||||
expect(session.writeBuffer).toEqual([]);
|
||||
session.releaseGuardedLease();
|
||||
expect(session.writeBuffer).toEqual(['\x03', '\x1b[A']);
|
||||
} finally {
|
||||
session.releaseGuardedLease();
|
||||
}
|
||||
});
|
||||
|
||||
it('lets untagged mux input wait its own turn behind the lease instead of refusing it', async () => {
|
||||
const url = `/api/sessions/${harness.ctx._sessionId}/input`;
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
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.guardedInputHeld = true;
|
||||
try {
|
||||
// Fire-and-forget `useMux`: answered at once, written in its turn.
|
||||
expect((await post({ input: 'x', useMux: true })).statusCode).toBe(200);
|
||||
// A plain prompt's mux write is awaited, so it answers once written.
|
||||
const prompt = post({ input: 'prompt\r' });
|
||||
await vi.waitFor(() => expect(session.heldGuardedInput).toHaveLength(2));
|
||||
expect(session.writeBuffer).toEqual([]);
|
||||
session.releaseGuardedLease();
|
||||
expect((await prompt).statusCode).toBe(200);
|
||||
expect(session.writeBuffer).toEqual(['x', 'prompt\r']);
|
||||
} finally {
|
||||
session.releaseGuardedLease();
|
||||
}
|
||||
});
|
||||
|
||||
it('refuses an untagged raw POST with 409 only when the hold queue is full', async () => {
|
||||
const url = `/api/sessions/${harness.ctx._sessionId}/input`;
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
const session = harness.ctx.sessions.get(harness.ctx._sessionId) as any;
|
||||
session.writeBuffer.length = 0;
|
||||
session.guardedInputHeld = true;
|
||||
session.holdForGuardedLease = () => 'full';
|
||||
try {
|
||||
const res = await harness.app.inject({ method: 'POST', url, payload: { input: '\x03' } });
|
||||
expect(res.statusCode).toBe(409);
|
||||
expect(res.json()).toMatchObject({ success: false, errorCode: 'CONFLICT' });
|
||||
expect(session.writeBuffer).toEqual([]);
|
||||
} finally {
|
||||
delete session.holdForGuardedLease;
|
||||
session.releaseGuardedLease();
|
||||
}
|
||||
});
|
||||
|
||||
it('always applies untagged input (curl/legacy, no dedup)', async () => {
|
||||
const url = `/api/sessions/${harness.ctx._sessionId}/input`;
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
|
||||
Reference in new issue
Block a user