From 71647e090dd8725ea3d0779d991346897583b0ff Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Tue, 13 Oct 2026 17:46:08 +0200 Subject: [PATCH] fix(mcp): start_session returns the worker's id when only the readiness call fails (#615 merge-time fix) After quick-start succeeded, a failed wait-output readiness call (a 429 from the waiter cap, a network blip) became an isError result without the session id, so the model could neither prompt nor delete the worker it had just started, and a retry started a second one. It now returns the id with ready: null and a note, as `codeman agent spawn` does. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/mcp-server.ts | 29 ++++++++++++++++++++++------- test/mcp-server.test.ts | 14 ++++++++++++++ 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/src/mcp-server.ts b/src/mcp-server.ts index 692977b4..aaf942b5 100644 --- a/src/mcp-server.ts +++ b/src/mcp-server.ts @@ -624,13 +624,28 @@ export function createToolRunner(deps: ToolRunnerDeps) { let note = 'Ready for send_prompt.'; const mark = composerReadyMark(mode); if (waitReady && mark) { - const { wait } = await waitCall( - data.sessionId, - { match: mark, from: 'buffer' }, - 'wait-output', - timeoutMs, - signal - ); + let wait: WaitResult | undefined; + try { + ({ wait } = await waitCall( + data.sessionId, + { match: mark, from: 'buffer' }, + 'wait-output', + timeoutMs, + signal + )); + } catch (err) { + // The worker is already running: a failed readiness CALL (429, a network blip) + // must still hand the model its id, or it cannot prompt or delete it. + if (!(err instanceof ToolError)) throw err; + return structuredResult({ + session_id: data.sessionId, + mode, + case: data.caseName ?? caseName, + case_path: data.casePath ?? '', + ready: null, + note: `Started, but the readiness check failed (${err.message}): read_output with source terminal before send_prompt.`, + }); + } const outcome = waitOutcome(wait); if (outcome === 'exited' || outcome === 'ended') { ready = false; diff --git a/test/mcp-server.test.ts b/test/mcp-server.test.ts index ce34a217..fc552cf2 100644 --- a/test/mcp-server.test.ts +++ b/test/mcp-server.test.ts @@ -366,6 +366,20 @@ describe('the tools', () => { expect(result.structured).toMatchObject({ session_id: MINE, ready: true }); }); + it('start_session still returns the session id when only the readiness call fails', async () => { + const busy = fakeCodeman(undefined, { + 'GET /api/v1/sessions/*': () => ({ + status: 429, + json: { success: false, error: 'too many waiters', errorCode: 'WAIT_CAPACITY' }, + text: '', + }), + }); + const result = await runner(config(), busy).tools.run('start_session', { case: 'review-2', mode: 'claude' }); + expect(result.isError).toBeFalsy(); + expect(result.structured).toMatchObject({ session_id: MINE, ready: null }); + expect(String(result.structured?.note)).toContain('readiness check failed'); + }); + it('send_prompt refuses a newline or control byte before any request', async () => { const { tools, calls } = runner(config()); for (const text of ['line one\nline two', 'esc \u001b[A', 'tab\there']) {