From ac36ea9ce35b57f9469f50db5d1a809a816c59f4 Mon Sep 17 00:00:00 2001 From: chiga0 Date: Fri, 28 Aug 2026 19:49:38 +0800 Subject: [PATCH 1/5] fix(serve): classify channel initialization timeouts --- docs/developers/qwen-serve-protocol.md | 16 +++++++++++ packages/cli/src/serve/server.test.ts | 27 +++++++++++++++++++ .../cli/src/serve/server/error-response.ts | 18 +++++++++++++ 3 files changed, 61 insertions(+) diff --git a/docs/developers/qwen-serve-protocol.md b/docs/developers/qwen-serve-protocol.md index 63753d2dd7e..41c24906230 100644 --- a/docs/developers/qwen-serve-protocol.md +++ b/docs/developers/qwen-serve-protocol.md @@ -106,6 +106,22 @@ When `--max-total-sessions` rejects a fresh session, the same response shape is Attaches to existing sessions are NOT counted toward the cap, so an idle daemon's reconnects keep working even when at-capacity. +If the ACP channel initialization budget expires before `newSession` is dispatched, `POST /session` returns `504` with `Retry-After: 5` and: + +```json +{ + "error": "AcpSessionBridge initialize timed out after 10000ms", + "code": "init_timeout", + "errorKind": "init_timeout", + "retryable": true, + "sideEffectPossible": false, + "phase": "channel.initialize", + "timeoutMs": 10000 +} +``` + +The `sideEffectPossible: false` field is authoritative because channel initialization precedes the ACP `newSession` request. A client that understands this structured contract may retry after the advertised delay without risking a duplicate Session. Clients that classify all 5xx mutation responses as ambiguous remain conservatively fail-closed. Other timeouts do not inherit this contract. + `RestoreInProgressError` — emitted by `POST /session/:id/load`, `POST /session/:id/resume`, or a caller-supplied-id `POST /session` when another registration already owns that id — returns `409` and: ```json diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index 3b6ee795c98..8b4c86d2292 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -100,6 +100,7 @@ import { } from '@qwen-code/qwen-code-core'; import * as qwenCore from '@qwen-code/qwen-code-core'; import type { DaemonStatusProvider } from '@qwen-code/acp-bridge'; +import { BridgeTimeoutError } from '@qwen-code/acp-bridge/status'; import { CancelSentinelCollisionError, InvalidClientIdError, @@ -11150,6 +11151,32 @@ describe('createServeApp', () => { }); describe('POST /session', () => { + it('returns a typed safe-retry error when channel initialization times out', async () => { + const bridge = fakeBridge({ + spawnImpl: async () => { + throw new BridgeTimeoutError('initialize', 10_000); + }, + }); + const app = createServeApp(baseOpts, undefined, { bridge }); + + const res = await request(app) + .post('/session') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ cwd: WS_BOUND }); + + expect(res.status).toBe(504); + expect(res.headers['retry-after']).toBe('5'); + expect(res.body).toEqual({ + error: 'AcpSessionBridge initialize timed out after 10000ms', + code: 'init_timeout', + errorKind: 'init_timeout', + retryable: true, + sideEffectPossible: false, + phase: 'channel.initialize', + timeoutMs: 10_000, + }); + }); + it('200 when cwd is omitted (falls back to bound workspace, #3803 §02)', async () => { // Legacy primary compatibility: clients may omit `cwd`, in which case // the route falls back to `opts.workspace ?? process.cwd()`. diff --git a/packages/cli/src/serve/server/error-response.ts b/packages/cli/src/serve/server/error-response.ts index 47cd9f45576..f90e081fc19 100644 --- a/packages/cli/src/serve/server/error-response.ts +++ b/packages/cli/src/serve/server/error-response.ts @@ -18,6 +18,7 @@ import { } from '@qwen-code/qwen-code-core'; import type { Response } from 'express'; import { restoreRetryAfterSeconds } from '@qwen-code/acp-bridge/sessionRestoreTimeout'; +import { BridgeTimeoutError } from '@qwen-code/acp-bridge/status'; import { writeStderrLine } from '../../utils/stdioHelpers.js'; import { BranchWhilePromptActiveError, @@ -196,6 +197,23 @@ export function sendBridgeError( ctx?: BridgeErrorContext, daemonLog?: DaemonLogger, ): void { + if (err instanceof BridgeTimeoutError && err.label === 'initialize') { + recordExpectedBridgeError(err, ctx, daemonLog); + res.set('Retry-After', '5'); + // Initialization is attempted before newSession is dispatched, so clients + // that understand the structured body can safely distinguish this timeout + // from an ambiguous mutation outcome. + res.status(504).json({ + error: err.message, + code: 'init_timeout', + errorKind: 'init_timeout', + retryable: true, + sideEffectPossible: false, + phase: 'channel.initialize', + timeoutMs: err.timeoutMs, + }); + return; + } if (err instanceof SessionRestoreTimeoutError) { recordExpectedBridgeError(err, ctx, daemonLog); // The state this 504 leaves behind is the abandoned-restore fence, which From 365de1658957defc61f9fb6ed0709d408b190df9 Mon Sep 17 00:00:00 2001 From: chiga0 Date: Mon, 31 Aug 2026 10:53:23 +0800 Subject: [PATCH 2/5] fix(serve): scope the init-timeout safe-retry contract to plain creation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The initialize-timeout mapping in sendBridgeError promised retryable:true / sideEffectPossible:false for every route, but two paths mutate before or around the initialize handshake: - POST /session with branch/worktree runs createBranch (moving the shared HEAD) or creates a worktree before spawn, and the rollback on failure is best-effort — a failed checkout rollback leaves the repo on the new branch while the response claims no side effect. - POST /session/:id/branch and /side-task can time out on a replacement channel's initialize after the fork was already durably committed; a contract-trusting retry would commit a duplicate fork. The safe-retry shape is now emitted only when the caller asserts via the new initPrecedesMutations context flag that initialization strictly precedes every durable mutation — POST /session sets it only when no branch/worktree was prepared. All other paths keep the typed init_timeout code, phase, and timeoutMs but omit Retry-After, retryable, and sideEffectPossible, reporting an unknown outcome. The protocol doc narrows the authoritative claim accordingly and notes that timeoutMs reflects the configured --initialize-timeout-ms budget (values shown are the default). Tests pin the branch-body reduced shape (red without the route guard) and the non-initialize label falling through to the generic 500. --- docs/developers/qwen-serve-protocol.md | 4 +- packages/cli/src/serve/routes/session.ts | 10 ++- packages/cli/src/serve/server.test.ts | 69 +++++++++++++++++++ .../cli/src/serve/server/error-response.ts | 46 +++++++++++-- 4 files changed, 120 insertions(+), 9 deletions(-) diff --git a/docs/developers/qwen-serve-protocol.md b/docs/developers/qwen-serve-protocol.md index 41c24906230..d2b4788bfeb 100644 --- a/docs/developers/qwen-serve-protocol.md +++ b/docs/developers/qwen-serve-protocol.md @@ -120,7 +120,9 @@ If the ACP channel initialization budget expires before `newSession` is dispatch } ``` -The `sideEffectPossible: false` field is authoritative because channel initialization precedes the ACP `newSession` request. A client that understands this structured contract may retry after the advertised delay without risking a duplicate Session. Clients that classify all 5xx mutation responses as ambiguous remain conservatively fail-closed. Other timeouts do not inherit this contract. +`timeoutMs` — and the numeric suffix of `error` — reflect the daemon's configured `--initialize-timeout-ms` budget (the values above are the default). The full safe-retry shape above is emitted only for plain session creation, where channel initialization strictly precedes every durable mutation: on that path the `sideEffectPossible: false` field is authoritative because initialization precedes the ACP `newSession` request, and a client that understands this structured contract may retry after the advertised delay without risking a duplicate Session. + +Requests carrying `branch` or `worktree`, and initialize timeouts surfaced by any other route (for example `POST /session/:id/branch` and `POST /session/:id/side-task`, where a committed fork can outlive the failed handshake), return the same `504` with `code: "init_timeout"`, `phase`, and `timeoutMs` — but WITHOUT `Retry-After`, `retryable`, or `sideEffectPossible`. For those the mutation outcome is unknown: branch and worktree preparation mutates git before the channel initializes, and the rollback attempted on failure is best-effort (a failed checkout rollback leaves the workspace on the new branch, and a retry may surface a branch-already-exists conflict). Clients that classify all 5xx mutation responses as ambiguous remain conservatively fail-closed, and should apply the same policy to the reduced shape. Other timeouts do not inherit this contract. `RestoreInProgressError` — emitted by `POST /session/:id/load`, `POST /session/:id/resume`, or a caller-supplied-id `POST /session` when another registration already owns that id — returns `409` and: diff --git a/packages/cli/src/serve/routes/session.ts b/packages/cli/src/serve/routes/session.ts index 3ff9d3e2e15..a1042ef4c16 100644 --- a/packages/cli/src/serve/routes/session.ts +++ b/packages/cli/src/serve/routes/session.ts @@ -3344,7 +3344,15 @@ export function registerSessionRoutes( daemonLog, ); } - sendBridgeError(res, err, { route: 'POST /session' }); + // Only the plain creation path can promise that the initialize + // handshake preceded every durable mutation: `branch`/`worktree` + // bodies mutate git BEFORE spawn, and their rollback above is + // best-effort (a failed checkout rollback leaves the workspace on + // the new branch while the error response goes out). + sendBridgeError(res, err, { + route: 'POST /session', + ...(branchMeta || worktreeMeta ? {} : { initPrecedesMutations: true }), + }); } finally { sessionIdReservation?.release(); } diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index 8b4c86d2292..01728b2cb70 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -11177,6 +11177,75 @@ describe('createServeApp', () => { }); }); + it('does not promise sideEffectPossible:false when a branch body mutated git first', async () => { + // The safe-retry contract is scoped to plain creation: a `branch` + // body runs createBranch (moving the shared HEAD) BEFORE the channel + // initialize handshake, and the rollback on failure is best-effort. + // The init-timeout response must not tell a contract-trusting client + // that no side effect is possible. + const bridge = fakeBridge({ + spawnImpl: async () => { + throw new BridgeTimeoutError('initialize', 10_000); + }, + }); + const app = createServeApp( + { ...baseOpts, workspace: WS_BOUND }, + undefined, + { bridge }, + ); + mockWt.impl = () => ({ + isGitRepository: () => Promise.resolve(true), + getCurrentBranch: () => Promise.resolve('main'), + }); + mockBranchOps.branchExists = () => Promise.resolve(false); + mockBranchOps.isDirtyTree = () => Promise.resolve(false); + mockBranchOps.getHeadCommit = () => Promise.resolve('a'.repeat(40)); + + try { + const res = await request(app) + .post('/session') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ cwd: WS_BOUND, branch: { name: 'feat/x' } }); + + expect(res.status).toBe(504); + expect(res.headers['retry-after']).toBeUndefined(); + expect(res.body.code).toBe('init_timeout'); + expect(res.body.phase).toBe('channel.initialize'); + expect(res.body.timeoutMs).toBe(10_000); + expect(res.body.retryable).toBeUndefined(); + expect(res.body.sideEffectPossible).toBeUndefined(); + } finally { + mockWt.impl = undefined; + mockBranchOps.branchExists = undefined; + mockBranchOps.isDirtyTree = undefined; + mockBranchOps.getHeadCommit = undefined; + } + }); + + it('does not map a non-initialize bridge timeout to the init_timeout contract', async () => { + // `newSession` is the label on the dispatch that follows a successful + // initialize: a timeout there leaves the session-creation outcome + // ambiguous. It must fall through to the generic 500, not the typed + // 504 with retry guidance. + const bridge = fakeBridge({ + spawnImpl: async () => { + throw new BridgeTimeoutError('newSession', 10_000); + }, + }); + const app = createServeApp(baseOpts, undefined, { bridge }); + + const res = await request(app) + .post('/session') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ cwd: WS_BOUND }); + + expect(res.status).toBe(500); + expect(res.headers['retry-after']).toBeUndefined(); + expect(res.body.code).toBeUndefined(); + expect(res.body.retryable).toBeUndefined(); + expect(res.body.sideEffectPossible).toBeUndefined(); + }); + it('200 when cwd is omitted (falls back to bound workspace, #3803 §02)', async () => { // Legacy primary compatibility: clients may omit `cwd`, in which case // the route falls back to `opts.workspace ?? process.cwd()`. diff --git a/packages/cli/src/serve/server/error-response.ts b/packages/cli/src/serve/server/error-response.ts index f90e081fc19..62b9bda74b5 100644 --- a/packages/cli/src/serve/server/error-response.ts +++ b/packages/cli/src/serve/server/error-response.ts @@ -65,6 +65,15 @@ import { ConversationRuntimeOwnershipError } from '../conversations/conversation export type BridgeErrorContext = { route?: string; sessionId?: string; + /** + * The caller asserts that, on this request path, the channel-initialize + * handshake strictly precedes every durable mutation (git branch/worktree + * prep, committed forks, dispatched ACP requests). Only then may the + * `init_timeout` response promise `sideEffectPossible: false`. Routes + * that mutate before or around initialization must leave it unset so the + * response reports an unknown outcome instead. + */ + initPrecedesMutations?: boolean; [key: string]: string | number | boolean | undefined; }; @@ -88,7 +97,12 @@ function bridgeErrorExtraContext( ): Record { const extra: Record = {}; for (const [key, value] of Object.entries(ctx ?? {})) { - if (key === 'route' || key === 'sessionId' || value === undefined) { + if ( + key === 'route' || + key === 'sessionId' || + key === 'initPrecedesMutations' || + value === undefined + ) { continue; } extra[key] = value; @@ -199,16 +213,34 @@ export function sendBridgeError( ): void { if (err instanceof BridgeTimeoutError && err.label === 'initialize') { recordExpectedBridgeError(err, ctx, daemonLog); - res.set('Retry-After', '5'); - // Initialization is attempted before newSession is dispatched, so clients - // that understand the structured body can safely distinguish this timeout - // from an ambiguous mutation outcome. + if (ctx?.initPrecedesMutations === true) { + res.set('Retry-After', '5'); + // The caller asserted initialization strictly precedes every durable + // mutation on this path (plain session creation: the initialize + // handshake runs before the ACP newSession request is dispatched), so + // clients that understand the structured body can safely distinguish + // this timeout from an ambiguous mutation outcome. + res.status(504).json({ + error: err.message, + code: 'init_timeout', + errorKind: 'init_timeout', + retryable: true, + sideEffectPossible: false, + phase: 'channel.initialize', + timeoutMs: err.timeoutMs, + }); + return; + } + // Mutations may precede or interleave with initialization on this path + // (branch/worktree preparation on POST /session, committed forks on the + // branch and side-task restore flows). Report the timeout WITHOUT the + // safe-retry contract: no Retry-After, no retryable, and no + // sideEffectPossible claim — the mutation outcome is unknown, and a + // contract-trusting client must not auto-retry. res.status(504).json({ error: err.message, code: 'init_timeout', errorKind: 'init_timeout', - retryable: true, - sideEffectPossible: false, phase: 'channel.initialize', timeoutMs: err.timeoutMs, }); From 453d38de8c07c5a8763370f92c67bf9a4be59887 Mon Sep 17 00:00:00 2001 From: chiga0 Date: Mon, 31 Aug 2026 13:05:09 +0800 Subject: [PATCH 3/5] docs(serve): clarify init_timeout 504 on load/resume routes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The restore Errors list previously described every 504 as session_restore_timeout (retryable, fenced), but an init-budget expiry during ensureChannel returns init_timeout without Retry-After, retryable, or fence — the restore was never dispatched. Add the init_timeout 504 entry and narrow the "any other route" paragraph to mutation-bearing routes, explicitly calling out load/resume. --- docs/developers/qwen-serve-protocol.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/developers/qwen-serve-protocol.md b/docs/developers/qwen-serve-protocol.md index d2b4788bfeb..af1542d5d08 100644 --- a/docs/developers/qwen-serve-protocol.md +++ b/docs/developers/qwen-serve-protocol.md @@ -122,7 +122,7 @@ If the ACP channel initialization budget expires before `newSession` is dispatch `timeoutMs` — and the numeric suffix of `error` — reflect the daemon's configured `--initialize-timeout-ms` budget (the values above are the default). The full safe-retry shape above is emitted only for plain session creation, where channel initialization strictly precedes every durable mutation: on that path the `sideEffectPossible: false` field is authoritative because initialization precedes the ACP `newSession` request, and a client that understands this structured contract may retry after the advertised delay without risking a duplicate Session. -Requests carrying `branch` or `worktree`, and initialize timeouts surfaced by any other route (for example `POST /session/:id/branch` and `POST /session/:id/side-task`, where a committed fork can outlive the failed handshake), return the same `504` with `code: "init_timeout"`, `phase`, and `timeoutMs` — but WITHOUT `Retry-After`, `retryable`, or `sideEffectPossible`. For those the mutation outcome is unknown: branch and worktree preparation mutates git before the channel initializes, and the rollback attempted on failure is best-effort (a failed checkout rollback leaves the workspace on the new branch, and a retry may surface a branch-already-exists conflict). Clients that classify all 5xx mutation responses as ambiguous remain conservatively fail-closed, and should apply the same policy to the reduced shape. Other timeouts do not inherit this contract. +Requests carrying `branch` or `worktree`, and initialize timeouts surfaced by mutation-bearing routes other than plain creation (for example `POST /session/:id/branch` and `POST /session/:id/side-task`, where a committed fork can outlive the failed handshake), return the same `504` with `code: "init_timeout"`, `phase`, and `timeoutMs` — but WITHOUT `Retry-After`, `retryable`, or `sideEffectPossible`. For those the mutation outcome is unknown: branch and worktree preparation mutates git before the channel initializes, and the rollback attempted on failure is best-effort (a failed checkout rollback leaves the workspace on the new branch, and a retry may surface a branch-already-exists conflict). Clients that classify all 5xx mutation responses as ambiguous remain conservatively fail-closed, and should apply the same policy to the reduced shape. `POST /session/:id/load` and `POST /session/:id/resume` surface the same reduced `init_timeout` `504` when channel initialization times out before the restore request is dispatched (the `ensureChannel` stage); unlike the `session_restore_timeout` `504` documented in their Errors lists, this shape carries no `Retry-After`, no `retryable`, and installs no fence because the restore was never dispatched. Other timeouts do not inherit this contract. `RestoreInProgressError` — emitted by `POST /session/:id/load`, `POST /session/:id/resume`, or a caller-supplied-id `POST /session` when another registration already owns that id — returns `409` and: @@ -2201,6 +2201,7 @@ The replay-window byte caps apply after the child has reconstructed the persiste - `403` — `untrusted_workspace` when `cwd` targets an untrusted non-primary workspace. - `503` — `session_limit_exceeded` (counts against `--max-sessions`; in-flight restores are accounted for too). - `504` — `session_restore_timeout`; retryable, with a `Retry-After` derived from the restore budget (clamped to 5-120s) because the same session id stays fenced until late cleanup settles. +- `504` — `init_timeout`; NOT retryable, no `Retry-After`, no `sideEffectPossible`, no fence installed. Emitted when channel initialization times out before the restore request is dispatched (the `ensureChannel` stage); the restore was never attempted, so no session id is fenced and no cleanup is pending. - `503` — `acp_channel_unavailable` when the workspace channel is closed to new session work. `reason` says why: `restore_cleanup_failed` when an abandoned restore could not be cleaned up conclusively, or `restore_settlement_overdue` when an abandoned restore has still not settled one full restore budget after its deadline. In both cases existing sessions remain available, and new session work may be retried after the workspace channel drains — the body carries `retryAfterSeconds` and the header a matching budget-derived `Retry-After`, because quarantine outlives the fence and a fresh id never sees the 409 that would carry the hint. - `409` — `restore_in_progress` (a `session/resume` for the same id is already in flight, or a fresh spawn supplied an id a restore owns). `Retry-After: 5` while the restore is active; a budget-derived hint once it is fenced as `awaiting_abandoned_cleanup`. Same-action races (two concurrent `session/load` for the same id) coalesce — exactly one returns `attached: false`, the rest return `attached: true` with the same `state`. - `409` — `session_workspace_conflict` when the same session id is already live or being restored by another workspace runtime. From 7717b8a5a477c193d0f290a1061a20ffa58bbaf1 Mon Sep 17 00:00:00 2001 From: chiga0 Date: Mon, 31 Aug 2026 16:46:53 +0800 Subject: [PATCH 4/5] fix(cli): remove duplicate BridgeTimeoutError import in error-response The file imported BridgeTimeoutError from both @qwen-code/acp-bridge/status and ../acp-session-bridge.js, which caused a TS2300 duplicate identifier build failure. Drop the bridge/status import; the local acp-session-bridge re-export is the one used across the route error handling. --- packages/cli/src/serve/server/error-response.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/cli/src/serve/server/error-response.ts b/packages/cli/src/serve/server/error-response.ts index ee0a4eb8352..15158b2db73 100644 --- a/packages/cli/src/serve/server/error-response.ts +++ b/packages/cli/src/serve/server/error-response.ts @@ -18,7 +18,6 @@ import { } from '@qwen-code/qwen-code-core'; import type { Response } from 'express'; import { restoreRetryAfterSeconds } from '@qwen-code/acp-bridge/sessionRestoreTimeout'; -import { BridgeTimeoutError } from '@qwen-code/acp-bridge/status'; import { writeStderrLine } from '../../utils/stdioHelpers.js'; import { BranchWhilePromptActiveError, From 83498f314a74d0054463e3cf49ae95d3d7ae6a6d Mon Sep 17 00:00:00 2001 From: chiga0 Date: Mon, 31 Aug 2026 16:53:28 +0800 Subject: [PATCH 5/5] fix(cli): reconcile init_timeout tests and docs with merged #10268 behavior Update error-response and server tests plus the qwen-serve protocol doc to reflect that `newSession` dispatch timeouts now map to a retryable `init_timeout` 504, while `initialize` timeouts without caller context use the reduced contract. --- docs/developers/qwen-serve-protocol.md | 2 +- packages/cli/src/serve/server.test.ts | 18 +++++++++++------- .../src/serve/server/error-response.test.ts | 12 +++++++++--- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/docs/developers/qwen-serve-protocol.md b/docs/developers/qwen-serve-protocol.md index 0d309a96c73..312e182372c 100644 --- a/docs/developers/qwen-serve-protocol.md +++ b/docs/developers/qwen-serve-protocol.md @@ -124,7 +124,7 @@ If the ACP channel initialization budget expires before `newSession` is dispatch `timeoutMs` — and the numeric suffix of `error` — reflect the daemon's configured `--initialize-timeout-ms` budget (the values above are the default). The full safe-retry shape above is emitted only for plain session creation, where channel initialization strictly precedes every durable mutation: on that path the `sideEffectPossible: false` field is authoritative because initialization precedes the ACP `newSession` request, and a client that understands this structured contract may retry after the advertised delay without risking a duplicate Session. -Requests carrying `branch` or `worktree`, and initialize timeouts surfaced by mutation-bearing routes other than plain creation (for example `POST /session/:id/branch` and `POST /session/:id/side-task`, where a committed fork can outlive the failed handshake), return the same `504` with `code: "init_timeout"`, `phase`, and `timeoutMs` — but WITHOUT `Retry-After`, `retryable`, or `sideEffectPossible`. For those the mutation outcome is unknown: branch and worktree preparation mutates git before the channel initializes, and the rollback attempted on failure is best-effort (a failed checkout rollback leaves the workspace on the new branch, and a retry may surface a branch-already-exists conflict). Clients that classify all 5xx mutation responses as ambiguous remain conservatively fail-closed, and should apply the same policy to the reduced shape. `POST /session/:id/load` and `POST /session/:id/resume` surface the same reduced `init_timeout` `504` when channel initialization times out before the restore request is dispatched (the `ensureChannel` stage); unlike the `session_restore_timeout` `504` documented in their Errors lists, this shape carries no `Retry-After`, no `retryable`, and installs no fence because the restore was never dispatched. Other timeouts do not inherit this contract. +Requests carrying `branch` or `worktree`, and initialize timeouts surfaced by mutation-bearing routes other than plain creation (for example `POST /session/:id/branch` and `POST /session/:id/side-task`, where a committed fork can outlive the failed handshake), return the same `504` with `code: "init_timeout"`, `phase`, and `timeoutMs` — but WITHOUT `Retry-After`, `retryable`, or `sideEffectPossible`. For those the mutation outcome is unknown: branch and worktree preparation mutates git before the channel initializes, and the rollback attempted on failure is best-effort (a failed checkout rollback leaves the workspace on the new branch, and a retry may surface a branch-already-exists conflict). Clients that classify all 5xx mutation responses as ambiguous remain conservatively fail-closed, and should apply the same policy to the reduced shape. `POST /session/:id/load` and `POST /session/:id/resume` surface the same reduced `init_timeout` `504` when channel initialization times out before the restore request is dispatched (the `ensureChannel` stage); unlike the `session_restore_timeout` `504` documented in their Errors lists, this shape carries no `Retry-After`, no `retryable`, and installs no fence because the restore was never dispatched. The `newSession` dispatch timeout on `POST /session` is an exception: it returns `504` with `code: "init_timeout"`, `retryable: true`, and a budget-derived `Retry-After`, but without `phase` or `sideEffectPossible`, because the creation outcome is ambiguous. All other timeout labels keep the generic mapping. `RestoreInProgressError` — emitted by `POST /session/:id/load`, `POST /session/:id/resume`, or a caller-supplied-id `POST /session` when another registration already owns that id — returns `409` and: diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index 84bb3b4ef90..78878bf34ad 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -11690,11 +11690,12 @@ describe('createServeApp', () => { } }); - it('does not map a non-initialize bridge timeout to the init_timeout contract', async () => { + it('maps the newSession dispatch timeout to its own retryable init_timeout contract', async () => { // `newSession` is the label on the dispatch that follows a successful // initialize: a timeout there leaves the session-creation outcome - // ambiguous. It must fall through to the generic 500, not the typed - // 504 with retry guidance. + // ambiguous, but the daemon still classifies it as `init_timeout` + // with a budget-derived Retry-After so fail-closed clients can back + // off without treating it as an unrecoverable server error. const bridge = fakeBridge({ spawnImpl: async () => { throw new BridgeTimeoutError('newSession', 10_000); @@ -11707,10 +11708,13 @@ describe('createServeApp', () => { .set('Host', `127.0.0.1:${baseOpts.port}`) .send({ cwd: WS_BOUND }); - expect(res.status).toBe(500); - expect(res.headers['retry-after']).toBeUndefined(); - expect(res.body.code).toBeUndefined(); - expect(res.body.retryable).toBeUndefined(); + expect(res.status).toBe(504); + expect(res.headers['retry-after']).toBe('10'); + expect(res.body.code).toBe('init_timeout'); + expect(res.body.errorKind).toBe('init_timeout'); + expect(res.body.retryable).toBe(true); + expect(res.body.timeoutMs).toBe(10_000); + expect(res.body.phase).toBeUndefined(); expect(res.body.sideEffectPossible).toBeUndefined(); }); diff --git a/packages/cli/src/serve/server/error-response.test.ts b/packages/cli/src/serve/server/error-response.test.ts index 890e1f4f69d..5736162313c 100644 --- a/packages/cli/src/serve/server/error-response.test.ts +++ b/packages/cli/src/serve/server/error-response.test.ts @@ -89,15 +89,21 @@ describe('sendBridgeError session writer errors', () => { }); }); - it('leaves non-session-initialization bridge timeouts on the generic path', () => { + it('maps channel initialization timeouts without caller context to the reduced contract', () => { const { response, status, json, set } = responseMock(); const error = new BridgeTimeoutError('initialize', 10_000); sendBridgeError(response, error); expect(set).not.toHaveBeenCalled(); - expect(status).toHaveBeenCalledWith(500); - expect(json).toHaveBeenCalledWith({ error: error.message }); + expect(status).toHaveBeenCalledWith(504); + expect(json).toHaveBeenCalledWith({ + error: error.message, + code: 'init_timeout', + errorKind: 'init_timeout', + phase: 'channel.initialize', + timeoutMs: 10_000, + }); }); it.each([