diff --git a/.changeset/tidy-workers-return.md b/.changeset/tidy-workers-return.md new file mode 100644 index 00000000..812810a0 --- /dev/null +++ b/.changeset/tidy-workers-return.md @@ -0,0 +1,5 @@ +--- +'aicodeman': patch +--- + +Preserve the created session ID in agent spawn output when the readiness request fails with a transport error. diff --git a/src/cli-agent.ts b/src/cli-agent.ts index 6a5fad47..efacd654 100644 --- a/src/cli-agent.ts +++ b/src/cli-agent.ts @@ -343,6 +343,18 @@ export const httpRequest: ApiRequest = (ctx, options) => { }); }; +/** + * One line describing a thrown request error. Node 22 rejects a refused connection to a host + * that resolves to both ::1 and 127.0.0.1 with an AggregateError whose `message` is empty, so + * fall back to its `code` (ECONNREFUSED) before a generic line rather than printing nothing. + */ +export function describeRequestError(error: unknown): string { + const message = getErrorMessage(error); + if (message) return message; + const code = (error as { code?: unknown } | null | undefined)?.code; + return code ? String(code) : 'request failed without a message'; +} + /** One line describing a failed response, for humans. Plain-text guards (401/403/429) have no envelope. */ export function describeFailure(res: ApiResponse): string { if (res.json && !res.json.success) { @@ -555,21 +567,25 @@ export async function agentSpawn(deps: AgentDeps, options: SpawnOptions): Promis let dead = false; const mark = composerReadyMark(options.mode); if (options.ready && mark) { - const wait = await deps.request(deps.ctx, { - method: 'GET', - path: `/api/v1/sessions/${encodeURIComponent(sid)}/wait-output`, - query: { match: mark, from: 'buffer', timeout: options.timeoutMs }, - timeoutMs: options.timeoutMs + 10_000, - }); - // A failed readiness call (waiter cap, 400, network) is its own error, not "the - // composer never showed up": report the real reason instead of the trust-dialog hint. - if (!wait.json?.success) readinessError = describeFailure(wait); - else { - const result = (wait.json.data as { wait?: WaitResult } | undefined)?.wait; - // A worker that died while we waited is exit 3 like every other wait, not a - // "composer not seen" timeout that sends the caller looking for a dialog. - dead = waitExitCode(result) === EXIT.dead; - ready = !dead && Boolean(result?.matched); + try { + const wait = await deps.request(deps.ctx, { + method: 'GET', + path: `/api/v1/sessions/${encodeURIComponent(sid)}/wait-output`, + query: { match: mark, from: 'buffer', timeout: options.timeoutMs }, + timeoutMs: options.timeoutMs + 10_000, + }); + // A failed readiness call (waiter cap, 400, network) is its own error, not "the + // composer never showed up": report the real reason instead of the trust-dialog hint. + if (!wait.json?.success) readinessError = describeFailure(wait); + else { + const result = (wait.json.data as { wait?: WaitResult } | undefined)?.wait; + // A worker that died while we waited is exit 3 like every other wait, not a + // "composer not seen" timeout that sends the caller looking for a dialog. + dead = waitExitCode(result) === EXIT.dead; + ready = !dead && Boolean(result?.matched); + } + } catch (error) { + readinessError = describeRequestError(error); } } @@ -1038,7 +1054,7 @@ async function run(json: boolean, verb: (deps: AgentDeps) => Promise): P try { process.exitCode = await verb(deps); } catch (err) { - console.error(palette.err(`${GLYPH.fail} ${getErrorMessage(err)}`)); + console.error(palette.err(`${GLYPH.fail} ${describeRequestError(err)}`)); process.exitCode = EXIT.error; } } diff --git a/src/mcp-server.ts b/src/mcp-server.ts index c79b2b4a..7d9a1b81 100644 --- a/src/mcp-server.ts +++ b/src/mcp-server.ts @@ -38,6 +38,7 @@ import { buildSendBody, composerReadyMark, describeFailure, + describeRequestError, httpRequest, inputRefusal, isSelfSession, @@ -497,7 +498,7 @@ export function createToolRunner(deps: ToolRunnerDeps) { } catch (err) { if (signal?.aborted) throw err; throw new ToolError( - `Cannot reach Codeman at ${ctx.apiUrl}: ${getErrorMessage(err)}. Check that it is running. If every call ` + + `Cannot reach Codeman at ${ctx.apiUrl}: ${describeRequestError(err)}. Check that it is running. If every call ` + 'fails at once, a sandbox around this MCP server may be blocking network access to this machine.' ); } diff --git a/test/cli-agent.test.ts b/test/cli-agent.test.ts index c6331aad..7f579696 100644 --- a/test/cli-agent.test.ts +++ b/test/cli-agent.test.ts @@ -30,6 +30,7 @@ import { buildSendBody, deleteRefusal, describeFailure, + describeRequestError, httpRequest, inboxCell, inputRefusal, @@ -548,6 +549,75 @@ describe('agent spawn', () => { expect(deps.calls).toHaveLength(1); expect(deps.err.join('')).toMatch(/SESSION_BUSY/); }); + + it.each([false, true])('preserves the created id when readiness throws (json=%s)', async (json) => { + const deps = fakeDeps((options) => { + if (options.path === '/api/v1/quick-start') return ok({ sessionId: OTHER, caseName: 'c' }); + throw new Error('readiness connection reset'); + }, json); + const exit = await agentSpawn(deps, { caseName: 'c', mode: 'claude', ready: true, timeoutMs: 1000 }).catch( + () => undefined + ); + + if (json) { + expect(deps.out).toHaveLength(1); + expect(JSON.parse(deps.out[0])).toEqual({ + sessionId: OTHER, + caseName: 'c', + readinessError: 'readiness connection reset', + }); + expect(deps.err).toEqual([]); + } else { + expect(deps.out).toEqual([OTHER]); + expect(deps.err.join('')).toContain('readiness check failed: readiness connection reset'); + expect(deps.err.join('')).not.toMatch(/composer not seen|startup dialog/); + } + expect(exit).toBe(EXIT.error); + expect(deps.calls.map(({ method, path }) => ({ method, path }))).toEqual([ + { method: 'POST', path: '/api/v1/quick-start' }, + { method: 'GET', path: `/api/v1/sessions/${OTHER}/wait-output` }, + ]); + }); + + it.each([ + { error: new Error(''), kind: 'Error' }, + { error: '', kind: 'string' }, + ])('reports a readiness failure when the thrown $kind has no message', async ({ error }) => { + const deps = fakeDeps((options) => { + if (options.path === '/api/v1/quick-start') return ok({ sessionId: OTHER }); + throw error; + }, true); + + expect(await agentSpawn(deps, { caseName: 'c', mode: 'claude', ready: true, timeoutMs: 1000 })).toBe(EXIT.error); + expect(deps.out).toHaveLength(1); + expect(JSON.parse(deps.out[0])).toEqual({ sessionId: OTHER, readinessError: 'request failed without a message' }); + expect(deps.calls).toHaveLength(2); + }); + + it('keeps the error code when a refused connection rejects with an empty AggregateError', async () => { + // Node 22 on a host resolving to ::1 and 127.0.0.1: message '', code ECONNREFUSED. + const refused = Object.assign(new AggregateError([new Error('connect ECONNREFUSED ::1:3000')], ''), { + code: 'ECONNREFUSED', + }); + const deps = fakeDeps((options) => { + if (options.path === '/api/v1/quick-start') return ok({ sessionId: OTHER }); + throw refused; + }, true); + + expect(await agentSpawn(deps, { caseName: 'c', mode: 'claude', ready: true, timeoutMs: 1000 })).toBe(EXIT.error); + expect(JSON.parse(deps.out[0])).toEqual({ sessionId: OTHER, readinessError: 'ECONNREFUSED' }); + }); +}); + +describe('describeRequestError', () => { + it('prefers the message, then the code, then a generic line', () => { + expect(describeRequestError(new Error('socket hang up'))).toBe('socket hang up'); + expect(describeRequestError(Object.assign(new AggregateError([], ''), { code: 'ECONNREFUSED' }))).toBe( + 'ECONNREFUSED' + ); + expect(describeRequestError(new Error(''))).toBe('request failed without a message'); + expect(describeRequestError('')).toBe('request failed without a message'); + }); }); describe('session id prefixes', () => { diff --git a/test/mcp-server.test.ts b/test/mcp-server.test.ts index fc552cf2..a6f47e88 100644 --- a/test/mcp-server.test.ts +++ b/test/mcp-server.test.ts @@ -248,6 +248,17 @@ describe('protocol: both eras', () => { expect(result.isError).toBe(true); expect(result.content[0].text).toContain('CODEMAN_API_URL'); }); + + it('a refused connection with an empty AggregateError still names the error code', async () => { + const refused: ApiRequest = async () => { + throw Object.assign(new AggregateError([new Error('connect ECONNREFUSED ::1:9')], ''), { code: 'ECONNREFUSED' }); + }; + const { out, send } = server(config(), { request: refused, calls: [] }); + await send({ jsonrpc: '2.0', id: 1, method: 'tools/call', params: { name: 'list_sessions', arguments: {} } }); + const result = (out[0] as { result: { isError: boolean; content: Array<{ text: string }> } }).result; + expect(result.isError).toBe(true); + expect(result.content[0].text).toContain('Cannot reach Codeman at http://127.0.0.1:9: ECONNREFUSED.'); + }); }); describe('tool definitions', () => {