Skip to content
19 changes: 19 additions & 0 deletions docs/developers/qwen-serve-protocol.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,24 @@ 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",
Comment thread
chiga0 marked this conversation as resolved.
"code": "init_timeout",
"errorKind": "init_timeout",
"retryable": true,
"sideEffectPossible": false,
"phase": "channel.initialize",
"timeoutMs": 10000
}
```

`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.
Comment thread
chiga0 marked this conversation as resolved.

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.
Comment thread
chiga0 marked this conversation as resolved.

`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
Expand Down Expand Up @@ -2220,6 +2238,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; `restore_settlement_overdue` when an abandoned restore has still not settled one full restore budget after its deadline; `new_session_cleanup_failed` when a session created after the public initialization timeout could not be closed conclusively; or `new_session_settlement_overdue` when the timed-out initialization has still not settled one further initialization budget after its deadline. Existing sessions remain available. A settlement-overdue state clears after a late failure settles or a late success completes its exact-ID cleanup; inconclusive cleanup transitions to the matching cleanup-failed state, which requires the workspace channel to drain and recycle. The body carries `retryAfterSeconds` and the header a matching budget-derived `Retry-After` so clients use an operation-budget-scale backoff instead of polling at the ordinary 5-second cadence.
- `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.
Expand Down
10 changes: 9 additions & 1 deletion packages/cli/src/serve/routes/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3377,7 +3377,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 }),
Comment thread
chiga0 marked this conversation as resolved.
});
} finally {
sessionIdReservation?.release();
}
Expand Down
100 changes: 100 additions & 0 deletions packages/cli/src/serve/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,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,
Expand Down Expand Up @@ -11689,6 +11690,105 @@ 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 });

Comment thread
chiga0 marked this conversation as resolved.
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('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('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, 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);
},
});
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('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();
});

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()`.
Expand Down
12 changes: 9 additions & 3 deletions packages/cli/src/serve/server/error-response.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,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([
Expand Down
51 changes: 50 additions & 1 deletion packages/cli/src/serve/server/error-response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,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;
};

Expand Down Expand Up @@ -136,7 +145,12 @@ function bridgeErrorExtraContext(
): Record<string, string | number | boolean> {
const extra: Record<string, string | number | boolean> = {};
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;
Expand Down Expand Up @@ -245,6 +259,41 @@ export function sendBridgeError(
ctx?: BridgeErrorContext,
daemonLog?: DaemonLogger,
): void {
if (err instanceof BridgeTimeoutError && err.label === 'initialize') {
Comment thread
chiga0 marked this conversation as resolved.
Comment thread
chiga0 marked this conversation as resolved.
Comment thread
chiga0 marked this conversation as resolved.
recordExpectedBridgeError(err, ctx, daemonLog);
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',
phase: 'channel.initialize',
timeoutMs: err.timeoutMs,
});
Comment thread
chiga0 marked this conversation as resolved.
return;
}
if (err instanceof SessionRestoreTimeoutError) {
recordExpectedBridgeError(err, ctx, daemonLog);
// The state this 504 leaves behind is the abandoned-restore fence, which
Expand Down
Loading