From 01323fda75db0401d7fcd395051ffea909675b7e Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Mon, 25 May 2026 17:25:20 +0800 Subject: [PATCH 1/4] feat(serve): add POST /session/:id/recap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wraps generateSessionRecap (core/services/sessionRecap.ts) so daemon clients can fetch a one-sentence "where did I leave off" summary without driving the agent through a full prompt turn. Mirrors the ext-method roundtrip used by /session/:id/approval-mode โ€” bridge forwards `qwen/control/session/recap` to the ACP child, which calls the existing core helper against the per-session GeminiClient history. - Route: non-strict mutation gate (parity with /prompt โ€” costs tokens but mutates no state) - Capability tag: `session_recap` - SDK: `client.recapSession(sessionId, opts)` + `session.recap(opts)` convenience wrapper - 60s bridge-side backstop timeout; client-disconnect aborts the HTTP wait (LLM call in the child still completes โ€” recap is short) - Recap is best-effort: short history / transient model failure surfaces as 200 with `recap: null`, not an error Tests cover the route (200 happy path, 200 null recap, client-id context, 404 on unknown session, malformed client-id, non-strict gate posture), the bridge ext-method roundtrip (success, null recap, SessionNotFoundError), the SDK client + session-client wrappers (URL encoding, body, headers, signal propagation, 404 throw), and a public-surface type lock for `DaemonSessionRecapResult`. Closes part of #4175 (Top 5 ROI port #1 from the daemon coverage gap inventory). Targets daemon_mode_b_main integration branch. ๐Ÿค– Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --- .../session-recap/session-recap-design.md | 38 ++++-- docs/developers/qwen-serve-protocol.md | 30 +++++ docs/users/qwen-serve.md | 1 + packages/acp-bridge/src/bridge.test.ts | 76 +++++++++++ packages/acp-bridge/src/bridge.ts | 44 ++++++ packages/acp-bridge/src/bridgeTypes.ts | 16 +++ packages/acp-bridge/src/status.ts | 1 + packages/cli/src/acp-integration/acpAgent.ts | 30 +++++ packages/cli/src/serve/capabilities.ts | 9 ++ packages/cli/src/serve/server.test.ts | 127 ++++++++++++++++++ packages/cli/src/serve/server.ts | 32 +++++ .../sdk-typescript/src/daemon/DaemonClient.ts | 39 ++++++ .../src/daemon/DaemonSessionClient.ts | 15 +++ packages/sdk-typescript/src/daemon/index.ts | 1 + packages/sdk-typescript/src/daemon/types.ts | 20 +++ packages/sdk-typescript/src/index.ts | 1 + .../test/unit/DaemonClient.test.ts | 80 ++++++++++- .../test/unit/DaemonSessionClient.test.ts | 29 ++++ .../test/unit/daemon-public-surface.test.ts | 6 + 19 files changed, 584 insertions(+), 11 deletions(-) diff --git a/docs/design/session-recap/session-recap-design.md b/docs/design/session-recap/session-recap-design.md index af93fcc813e..0c1d9b36be7 100644 --- a/docs/design/session-recap/session-recap-design.md +++ b/docs/design/session-recap/session-recap-design.md @@ -21,16 +21,34 @@ returns: ## Triggers -| Trigger | Conditions | Implementation | -| ---------- | -------------------------------------------------------------------------------------------- | ----------------------------------------------------------------- | -| **Manual** | User runs `/recap` | `recapCommand.ts` calls the same underlying service | -| **Auto** | Terminal blurred (DECSET 1004 focus protocol) for โ‰ฅ 5 min + focus returns + stream is `Idle` | `useAwaySummary.ts` โ€” 5min blur timer + `useFocus` event listener | - -Both paths funnel into a single function โ€” `generateSessionRecap()` โ€” to -guarantee identical behavior. The auto-trigger is gated by -`general.showSessionRecap` (default: off โ€” explicit opt-in, so ambient -LLM calls are never silently added to a user's bill); the manual -command ignores that setting. +| Trigger | Conditions | Implementation | +| --------------- | -------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------- | +| **Manual** | User runs `/recap` | `recapCommand.ts` calls the same underlying service | +| **Auto** | Terminal blurred (DECSET 1004 focus protocol) for โ‰ฅ 5 min + focus returns + stream is `Idle` | `useAwaySummary.ts` โ€” 5min blur timer + `useFocus` event listener | +| **Daemon HTTP** | Remote client calls `POST /session/:id/recap` | `server.ts` route โ†’ `bridge.generateSessionRecap` (ext-method roundtrip) โ†’ `acpAgent.ts` calls `generateSessionRecap(session.getConfig(), signal)` | + +All three paths funnel into the same `generateSessionRecap()` function +in `core/services/sessionRecap.ts` to guarantee identical behavior. The +auto-trigger is gated by `general.showSessionRecap` (default: off โ€” +explicit opt-in, so ambient LLM calls are never silently added to a +user's bill); the manual command and daemon HTTP route ignore that +setting (the caller is making an explicit request). + +### Daemon access path + +The daemon route is non-strict-gated (mirrors `/session/:id/prompt`'s +posture โ€” recap costs tokens but mutates no state). Capability tag +`session_recap` advertises the route on `/capabilities.features`. SDK +helpers: `DaemonClient.recapSession(sessionId, opts)` and +`DaemonSessionClient.recap(opts)`. See +`docs/developers/qwen-serve-protocol.md` ยง `POST /session/:id/recap` +for the wire contract and error envelope. + +Cancellation is best-effort at v1: client disconnect aborts the +bridge-side wait, but the LLM call in the ACP child runs to completion +(recap is short โ€” single-attempt, ~1โ€“5s typical). A 60s backstop +timeout guards a wedged ACP channel. A future request-id-based cancel +ext-method can plumb full end-to-end cancellation if needed. ## Architecture diff --git a/docs/developers/qwen-serve-protocol.md b/docs/developers/qwen-serve-protocol.md index dc5b96066d5..1b2e2c89330 100644 --- a/docs/developers/qwen-serve-protocol.md +++ b/docs/developers/qwen-serve-protocol.md @@ -1107,6 +1107,36 @@ Response: On success, publishes `model_switched` to the SSE stream. On failure, publishes `model_switch_failed` (so passive subscribers see the failure, not just the caller). Races against the agent channel exit so a wedged child can't block the HTTP handler. +### `POST /session/:id/recap` + +Capability tag: `session_recap`. Bridge โ†’ ACP extMethod `qwen/control/session/recap`. + +Generate a one-sentence "where did I leave off" summary of the session. Wraps core's `generateSessionRecap` (`packages/core/src/services/sessionRecap.ts`), which runs a side-query against the fast model with tools disabled, `maxOutputTokens: 300`, and a strict `...` output format. The side-query reads the session's existing GeminiClient chat history and does **not** add to it. + +Request body is ignored (send `{}` or empty). Non-strict mutation gate โ€” posture mirrors `/session/:id/prompt` (the call costs tokens but mutates no state). No SSE event is published. + +Response (200): + +```json +{ + "sessionId": "sess:42", + "recap": "Debugging the auth retry race. Next: add deterministic timing to the integration test." +} +``` + +`recap` is `null` (a normal 200, not an error) when: + +- the session has fewer than two dialog turns yet, +- the side-query returned no extractable `...` payload, +- or any underlying model error occurred (the core helper is best-effort and never throws). + +Errors: + +- `400 {code: 'invalid_client_id'}` โ€” malformed `X-Qwen-Client-Id` header. +- `404` โ€” session unknown. + +Cancellation: client disconnect aborts the bridge-side wait, but the LLM call in the ACP child runs to completion (recap is short โ€” single-attempt, ~1โ€“5s typical). A 60s backstop timeout guards against a wedged ACP channel. + ### Mutation: approval, tools, init, MCP restart Issue [#4175](https://github.com/QwenLM/qwen-code/issues/4175) Wave 4 PR 17 adds four mutation control routes that let remote clients change runtime posture without touching the daemon host's CLI. All four: diff --git a/docs/users/qwen-serve.md b/docs/users/qwen-serve.md index 0bc91349302..88752093a00 100644 --- a/docs/users/qwen-serve.md +++ b/docs/users/qwen-serve.md @@ -15,6 +15,7 @@ Run Qwen Code as a local HTTP daemon so multiple clients (IDE plugins, web UIs, - **First-responder permissions** โ€” when the agent asks for permission to run a tool, every connected client sees the request; whichever client answers first wins. - **One daemon, one workspace** โ€” each `qwen serve` process binds to exactly one workspace at boot (per [#3803](https://github.com/QwenLM/qwen-code/issues/3803) ยง02). Multi-workspace deployments run one daemon per workspace on separate ports (or behind an orchestrator). - **Remote runtime control** ([#4175](https://github.com/QwenLM/qwen-code/issues/4175) PR 17) โ€” change a session's approval mode (`POST /session/:id/approval-mode`), toggle a tool per workspace (`POST /workspace/tools/:name/enable`), scaffold an empty `QWEN.md` (`POST /workspace/init`, mechanical only โ€” does NOT call the model; for AI-fill, follow up with `POST /session/:id/prompt`), or restart a single MCP server with a budget pre-check (`POST /workspace/mcp/:server/restart`). All four are strict-gated โ€” configure `--token` first. +- **Session recap** ([#4175](https://github.com/QwenLM/qwen-code/issues/4175) follow-up) โ€” fetch a one-sentence "where did I leave off" summary of an active session (`POST /session/:id/recap`). Wraps core's `generateSessionRecap` as a side-query against the fast model; pollutes neither the main chat history nor the SSE stream. Non-strict gate (same posture as `/prompt`); SDK helper `client.recapSession(sessionId)`. ## v0.16-alpha known limits diff --git a/packages/acp-bridge/src/bridge.test.ts b/packages/acp-bridge/src/bridge.test.ts index 4388510e583..e584c2d5da1 100644 --- a/packages/acp-bridge/src/bridge.test.ts +++ b/packages/acp-bridge/src/bridge.test.ts @@ -4306,6 +4306,82 @@ describe('createHttpAcpBridge', () => { }); }); + describe('generateSessionRecap (#4175 follow-up)', () => { + function recapFactory( + respond: ( + params: Record, + ) => Record | Promise>, + ): ChannelFactory { + return async () => { + const { clientStream, agentStream } = createInMemoryChannel(); + const agent = new FakeAgent({ + extMethodImpl: (method, params) => { + if (method === 'qwen/control/session/recap') { + return Promise.resolve(respond(params)); + } + return Promise.resolve({}); + }, + }); + new AgentSideConnection(() => agent as Agent, agentStream); + return { + stream: clientStream, + exited: new Promise< + | { exitCode: number | null; signalCode: NodeJS.Signals | null } + | undefined + >(() => {}), + kill: async () => {}, + killSync: () => {}, + }; + }; + } + + it('forwards through the ACP child and returns the recap verbatim', async () => { + const recapText = + 'Refactoring the auth middleware. Next: regenerate the integration fixtures.'; + let observedParams: Record | undefined; + const bridge = makeBridge({ + channelFactory: recapFactory((params) => { + observedParams = params; + return { sessionId: params['sessionId'], recap: recapText }; + }), + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + const result = await bridge.generateSessionRecap(session.sessionId); + expect(result).toEqual({ + sessionId: session.sessionId, + recap: recapText, + }); + expect(observedParams).toEqual({ sessionId: session.sessionId }); + await bridge.shutdown(); + }); + + it('preserves a null recap (best-effort failure surface)', async () => { + const bridge = makeBridge({ + channelFactory: recapFactory((params) => ({ + sessionId: params['sessionId'], + recap: null, + })), + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + const result = await bridge.generateSessionRecap(session.sessionId); + expect(result.recap).toBeNull(); + await bridge.shutdown(); + }); + + it('throws SessionNotFoundError for unknown sessionId', async () => { + const bridge = makeBridge({ + channelFactory: recapFactory(() => ({ + sessionId: 'never', + recap: null, + })), + }); + await expect( + bridge.generateSessionRecap('does-not-exist'), + ).rejects.toBeInstanceOf(SessionNotFoundError); + await bridge.shutdown(); + }); + }); + describe('setWorkspaceToolEnabled (#4175 Wave 4 PR 17)', () => { it('throws when no persistDisabledTools callback is wired', async () => { const bridge = makeBridge(); diff --git a/packages/acp-bridge/src/bridge.ts b/packages/acp-bridge/src/bridge.ts index e056645c89a..843e4c55bba 100644 --- a/packages/acp-bridge/src/bridge.ts +++ b/packages/acp-bridge/src/bridge.ts @@ -320,6 +320,16 @@ const DEFAULT_INIT_TIMEOUT_MS = 10_000; * as long as the slowest legitimate per-server discovery. */ const MCP_RESTART_TIMEOUT_MS = 300_000; +/** + * Backstop timeout for `qwen/control/session/recap`. The underlying + * side-query is single-attempt with `maxOutputTokens: 300`, so a + * healthy call finishes in 1โ€“5 seconds; we cap at 60s to absorb model- + * provider hiccups without inheriting the 10s `initTimeoutMs` default + * (which would false-fire on any GPT-style slow start). The race is a + * safety net against a wedged ACP channel โ€” actual cancellation on + * client disconnect is handled at the HTTP route layer. + */ +const SESSION_RECAP_TIMEOUT_MS = 60_000; const DEFAULT_MAX_SESSIONS = 20; /** * Soft upper bound on `BridgeOptions.eventRingSize` to catch operator @@ -2858,6 +2868,40 @@ export function createHttpAcpBridge(opts: BridgeOptions): HttpAcpBridge { }; }, + async generateSessionRecap(sessionId, _context) { + // #4175 follow-up. Thin pass-through to `qwen/control/session/ + // recap` โ€” the ACP child runs `generateSessionRecap` against the + // session's GeminiClient history and returns `{sessionId, recap}` + // where `recap` may be `null` for too-short histories or transient + // model failures. The core helper is documented to never throw, + // so the only paths that surface as bridge errors are: unknown + // sessionId (`SessionNotFoundError`), transport closed mid-flight + // (race against `getTransportClosedReject`), and the backstop + // `SESSION_RECAP_TIMEOUT_MS` race for a wedged ACP channel. + // + // `_context` carries the trusted client id for future event + // fan-out (e.g. a `session_recap_generated` push event), but + // recap is informational-only today โ€” no SSE broadcast. + const entry = byId.get(sessionId); + if (!entry) throw new SessionNotFoundError(sessionId); + const info = channelInfoForEntry(entry); + if (!info || info.isDying) throw new SessionNotFoundError(sessionId); + const response = (await Promise.race([ + withTimeout( + entry.connection.extMethod(SERVE_CONTROL_EXT_METHODS.sessionRecap, { + sessionId, + }), + SESSION_RECAP_TIMEOUT_MS, + SERVE_CONTROL_EXT_METHODS.sessionRecap, + ), + getTransportClosedReject(entry), + ])) as { sessionId: string; recap: string | null }; + return { + sessionId: entry.sessionId, + recap: response.recap ?? null, + }; + }, + async setWorkspaceToolEnabled(toolName, enabled, originatorClientId) { // #4175 Wave 4 PR 17. Pure file IO + event fan-out โ€” no ACP // roundtrip. The settings file is the source of truth; live diff --git a/packages/acp-bridge/src/bridgeTypes.ts b/packages/acp-bridge/src/bridgeTypes.ts index 7c6a027504c..4f89cbdf1d7 100644 --- a/packages/acp-bridge/src/bridgeTypes.ts +++ b/packages/acp-bridge/src/bridgeTypes.ts @@ -326,6 +326,22 @@ export interface HttpAcpBridge { persisted: boolean; }>; + /** + * Generate a one-sentence "where did I leave off" recap of a live + * session. Forwards through `qwen/control/session/recap`, which + * invokes `generateSessionRecap` (`core/services/sessionRecap.ts`) in + * the ACP child against the per-session chat history. + * + * Best-effort: the helper returns `null` when history is too short or + * the underlying side-query fails โ€” both surface as a 200 response + * with `recap: null`. Hard errors (unknown session, ACP transport + * down) throw as usual. + */ + generateSessionRecap( + sessionId: string, + context?: BridgeClientRequestContext, + ): Promise<{ sessionId: string; recap: string | null }>; + /** * Add or remove a tool name from the workspace's `tools.disabled` * settings list and fan-out a `tool_toggled` event to every live diff --git a/packages/acp-bridge/src/status.ts b/packages/acp-bridge/src/status.ts index e61233e7029..7c70c178fd5 100644 --- a/packages/acp-bridge/src/status.ts +++ b/packages/acp-bridge/src/status.ts @@ -107,6 +107,7 @@ export const SERVE_STATUS_EXT_METHODS = { export const SERVE_CONTROL_EXT_METHODS = { sessionClose: 'qwen/control/session/close', sessionApprovalMode: 'qwen/control/session/approval_mode', + sessionRecap: 'qwen/control/session/recap', workspaceMcpRestart: 'qwen/control/workspace/mcp/restart', } as const; diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index 7dc3cb5de18..b2840ff2345 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -10,6 +10,7 @@ import { AuthType, clearCachedCredentialFile, createDebugLogger, + generateSessionRecap, QwenOAuth2Event, qwenOAuth2Events, MCP_BUDGET_WARN_FRACTION, @@ -2243,6 +2244,35 @@ class QwenAgent implements Agent { const current = config.getApprovalMode(); return { previous, current }; } + case SERVE_CONTROL_EXT_METHODS.sessionRecap: { + // #4175 follow-up. Generate a one-sentence "where did I leave + // off" summary by running `generateSessionRecap` against the + // session's GeminiClient history. Best-effort: the core helper + // is documented to return `null` on any failure (short history, + // transient model error, etc.) and never throws โ€” we surface + // that null verbatim so the daemon route returns a 200 with + // `recap: null` rather than a 5xx. + const sessionId = params['sessionId']; + if (typeof sessionId !== 'string' || sessionId.length === 0) { + throw RequestError.invalidParams( + undefined, + 'Invalid or missing sessionId', + ); + } + const session = this.sessionOrThrow(sessionId); + const config = session.getConfig(); + // v1: no cross-process abort plumbing. Client disconnect aborts + // the bridge-side wait but the LLM call in this child runs to + // completion. Acceptable because recap is short (single-attempt + // side-query, maxOutputTokens: 300). A future request-id-based + // cancel ext-method can plumb a real signal end-to-end if the + // bandwidth cost ever becomes an issue. + const recap = await generateSessionRecap( + config, + new AbortController().signal, + ); + return { sessionId, recap }; + } case 'deleteSession': { const sessionId = params['sessionId'] as string; if (!sessionId || !SESSION_ID_RE.test(sessionId)) { diff --git a/packages/cli/src/serve/capabilities.ts b/packages/cli/src/serve/capabilities.ts index 5fe5cd3293c..09827d634f3 100644 --- a/packages/cli/src/serve/capabilities.ts +++ b/packages/cli/src/serve/capabilities.ts @@ -162,6 +162,15 @@ export const SERVE_CAPABILITY_REGISTRY = { // `'in_flight'` (concurrent discovery in progress), `'disabled'` // (server is configured but explicitly disabled). workspace_mcp_restart: { since: 'v1' }, + // #4175 follow-up. Daemon hosts `POST /session/:id/recap`, which + // generates a one-sentence "where did I leave off" summary by + // running `generateSessionRecap` (`core/services/sessionRecap.ts`) as + // a side-query against the fast model. Non-strict mutation gate โ€” + // posture mirrors `/session/:id/prompt` (token cost, not state + // mutation). The route returns `{sessionId, recap}` where `recap` + // may be `null` for too-short histories or transient model failures + // (best-effort, never throws). SDK helper: `DaemonClient.recapSession`. + session_recap: { since: 'v1' }, // F2 (#4175 commit 5). Daemon hosts a workspace-shared MCP transport // pool (`QwenAgent.mcpPool`); `GET /workspace/mcp` reflects pool-level // accounting (`entryCount`, `entrySummary` on each per-server cell). diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index 1c7cea28fe0..3f0066fa212 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -134,6 +134,9 @@ const EXPECTED_STAGE1_FEATURES = [ 'workspace_tool_toggle', 'workspace_init', 'workspace_mcp_restart', + // #4175 follow-up. Daemon hosts `POST /session/:id/recap` (wraps + // core's `generateSessionRecap` for one-sentence session summaries). + 'session_recap', // Issue #4175 PR 21 โ€” auth device-flow surface advertised unconditionally. // Registry order on origin/main has PR 21 appended last, so the // baseline assertion below mirrors that even though PR 21 landed @@ -248,6 +251,10 @@ interface FakeBridgeOpts { previous: ApprovalMode; persisted: boolean; }>; + generateSessionRecapImpl?: ( + sessionId: string, + context?: BridgeClientRequestContext, + ) => Promise<{ sessionId: string; recap: string | null }>; setToolEnabledImpl?: ( toolName: string, enabled: boolean, @@ -336,6 +343,10 @@ interface FakeBridge extends HttpAcpBridge { opts: { persist: boolean }; context?: BridgeClientRequestContext; }>; + generateSessionRecapCalls: Array<{ + sessionId: string; + context?: BridgeClientRequestContext; + }>; setToolEnabledCalls: Array<{ toolName: string; enabled: boolean; @@ -499,6 +510,13 @@ function fakeBridge(opts: FakeBridgeOpts = {}): FakeBridge { previous: ApprovalMode.DEFAULT, persisted: o.persist, })); + const generateSessionRecapCalls: FakeBridge['generateSessionRecapCalls'] = []; + const generateSessionRecapImpl = + opts.generateSessionRecapImpl ?? + (async (sessionId: string) => ({ + sessionId, + recap: 'Default fake recap.', + })); const setToolEnabledCalls: FakeBridge['setToolEnabledCalls'] = []; const setToolEnabledImpl = opts.setToolEnabledImpl ?? @@ -562,6 +580,7 @@ function fakeBridge(opts: FakeBridgeOpts = {}): FakeBridge { sessionSupportedCommandsCalls, setModelCalls, setApprovalModeCalls, + generateSessionRecapCalls, setToolEnabledCalls, initWorkspaceCalls, restartMcpServerCalls, @@ -697,6 +716,13 @@ function fakeBridge(opts: FakeBridgeOpts = {}): FakeBridge { }); return setApprovalModeImpl(sessionId, mode, o, context); }, + async generateSessionRecap(sessionId, context) { + generateSessionRecapCalls.push({ + sessionId, + ...(context ? { context } : {}), + }); + return generateSessionRecapImpl(sessionId, context); + }, async setWorkspaceToolEnabled(toolName, enabled, originatorClientId) { setToolEnabledCalls.push({ toolName, @@ -2353,6 +2379,107 @@ describe('createServeApp', () => { }); }); + describe('POST /session/:id/recap (#4175 follow-up)', () => { + it('200 with the recap on success and forwards no body', async () => { + const bridge = fakeBridge({ + generateSessionRecapImpl: async (sessionId) => ({ + sessionId, + recap: + 'Refactoring the auth retry. Next: regenerate the snapshot tests.', + }), + }); + const app = createServeApp(baseOpts, undefined, { bridge }); + const res = await request(app) + .post('/session/session-A/recap') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send(); + expect(res.status).toBe(200); + expect(res.body).toEqual({ + sessionId: 'session-A', + recap: + 'Refactoring the auth retry. Next: regenerate the snapshot tests.', + }); + expect(bridge.generateSessionRecapCalls).toHaveLength(1); + expect(bridge.generateSessionRecapCalls[0]?.sessionId).toBe('session-A'); + }); + + it('200 with recap:null is a valid best-effort response', async () => { + // The core helper `generateSessionRecap` is documented to return + // `null` when history is too short or the side-query fails. That + // must surface as a normal 200 โ€” a 5xx here would force daemon + // clients to special-case "we have no recap yet" as an error. + const bridge = fakeBridge({ + generateSessionRecapImpl: async (sessionId) => ({ + sessionId, + recap: null, + }), + }); + const app = createServeApp(baseOpts, undefined, { bridge }); + const res = await request(app) + .post('/session/session-A/recap') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send(); + expect(res.status).toBe(200); + expect(res.body.recap).toBeNull(); + }); + + it('passes client identity context into the bridge', async () => { + const bridge = fakeBridge(); + const app = createServeApp(baseOpts, undefined, { bridge }); + await request(app) + .post('/session/session-A/recap') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .set('X-Qwen-Client-Id', 'client-1') + .send(); + expect(bridge.generateSessionRecapCalls[0]?.context).toEqual({ + clientId: 'client-1', + }); + }); + + it('404 when bridge throws SessionNotFoundError', async () => { + const bridge = fakeBridge({ + generateSessionRecapImpl: async (sessionId) => { + throw new SessionNotFoundError(sessionId); + }, + }); + const app = createServeApp(baseOpts, undefined, { bridge }); + const res = await request(app) + .post('/session/missing/recap') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send(); + expect(res.status).toBe(404); + expect(res.body.sessionId).toBe('missing'); + }); + + it('400 on malformed X-Qwen-Client-Id header', async () => { + const bridge = fakeBridge(); + const app = createServeApp(baseOpts, undefined, { bridge }); + const res = await request(app) + .post('/session/session-A/recap') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .set('X-Qwen-Client-Id', 'bad client id with spaces') + .send(); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_client_id'); + expect(bridge.generateSessionRecapCalls).toHaveLength(0); + }); + + it('non-strict gate: works on no-token loopback default', async () => { + // Posture mirrors /session/:id/prompt โ€” the route costs tokens + // but mutates no state, so it should NOT require operators to + // configure a token. This pins the contract so a future cleanup + // that mass-flips session-scoped routes to strict catches the + // regression. + const bridge = fakeBridge(); + const app = createServeApp(baseOpts, undefined, { bridge }); + const res = await request(app) + .post('/session/session-A/recap') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send(); + expect(res.status).toBe(200); + }); + }); + describe('POST /session/:id/approval-mode (#4175 Wave 4 PR 17)', () => { // Strict-gated route: refuses on no-token loopback defaults. All // tests configure a token and forward `Authorization: Bearer โ€ฆ`. diff --git a/packages/cli/src/serve/server.ts b/packages/cli/src/serve/server.ts index 79d0776d0f0..d3f7596943c 100644 --- a/packages/cli/src/serve/server.ts +++ b/packages/cli/src/serve/server.ts @@ -1470,6 +1470,38 @@ export function createServeApp( } }); + app.post('/session/:id/recap', mutate(), async (req, res) => { + // #4175 follow-up. Wraps `generateSessionRecap` (core/services/ + // sessionRecap.ts) so daemon clients can fetch a one-sentence + // "where did I leave off" summary without driving the agent through + // a full prompt turn. Posture mirrors `/session/:id/prompt`: + // non-strict gate (token cost, not state mutation), and disconnect + // is detected via `res.once('close')` for the bridge-side + // cancellation. Best-effort โ€” `recap: null` on short history or + // transient model failure is a normal 200, not an error. + const sessionId = req.params['id']; + if (!sessionId) { + res + .status(400) + .json({ error: '`sessionId` route parameter is required' }); + return; + } + const clientId = parseClientIdHeader(req, res); + if (clientId === null) return; + try { + const response = await bridge.generateSessionRecap( + sessionId, + clientId !== undefined ? { clientId } : undefined, + ); + res.status(200).json(response); + } catch (err) { + sendBridgeError(res, err, { + route: 'POST /session/:id/recap', + sessionId, + }); + } + }); + app.post( '/session/:id/approval-mode', mutate({ strict: true }), diff --git a/packages/sdk-typescript/src/daemon/DaemonClient.ts b/packages/sdk-typescript/src/daemon/DaemonClient.ts index fcc55f3361f..0d5b1815ec6 100644 --- a/packages/sdk-typescript/src/daemon/DaemonClient.ts +++ b/packages/sdk-typescript/src/daemon/DaemonClient.ts @@ -47,6 +47,7 @@ import type { DaemonApprovalModeResult, DaemonInitWorkspaceResult, DaemonMcpRestartResult, + DaemonSessionRecapResult, DaemonToolToggleResult, } from './types.js'; @@ -975,6 +976,44 @@ export class DaemonClient { ); } + /** + * #4175 follow-up. Generate a one-sentence "where did I leave off" + * recap of the session. Wraps `generateSessionRecap` (core/services/ + * sessionRecap.ts) via an ACP control-channel ext-method, so the + * summary is computed against the active GeminiClient chat history + * inside the daemon's ACP child. + * + * Non-strict mutation gate โ€” posture matches `/session/:id/prompt` + * (the route costs tokens but mutates no state). Bypasses + * `fetchTimeoutMs` because the underlying side-query can take longer + * than the default 30s budget under a slow model; cancellation is + * via the optional `signal`. Older daemons (pre-recap support) return + * 404 โ€” pre-flight `caps.features.session_recap` before calling. + * + * `recap` may be `null` on too-short histories or transient model + * failures (a 200 response with `recap: null`), per the best-effort + * contract of the core helper. + */ + async recapSession( + sessionId: string, + opts?: { signal?: AbortSignal; clientId?: string }, + ): Promise { + const res = await this._fetch( + `${this.baseUrl}/session/${encodeURIComponent(sessionId)}/recap`, + { + method: 'POST', + headers: this.headers( + { 'Content-Type': 'application/json' }, + opts?.clientId, + ), + body: '{}', + signal: opts?.signal, + }, + ); + if (!res.ok) throw await this.failOnError(res, 'POST /session/:id/recap'); + return (await res.json()) as DaemonSessionRecapResult; + } + /** * #4175 Wave 4 PR 17. Toggle a tool name in the workspace's * `tools.disabled` settings list. Strict-gated mutation route โ€” the diff --git a/packages/sdk-typescript/src/daemon/DaemonSessionClient.ts b/packages/sdk-typescript/src/daemon/DaemonSessionClient.ts index cae2ca111bb..d7643c0d63b 100644 --- a/packages/sdk-typescript/src/daemon/DaemonSessionClient.ts +++ b/packages/sdk-typescript/src/daemon/DaemonSessionClient.ts @@ -14,6 +14,7 @@ import { import type { DaemonEvent, DaemonSessionContextStatus, + DaemonSessionRecapResult, DaemonSessionState, DaemonSession, DaemonSessionSupportedCommandsStatus, @@ -218,6 +219,20 @@ export class DaemonSessionClient { ); } + /** + * One-sentence "where did I leave off" recap of this session. See + * `DaemonClient.recapSession` for the contract (best-effort, may + * return `recap: null`; cancellation via `signal`). + */ + async recap(opts?: { + signal?: AbortSignal; + }): Promise { + return await this.client.recapSession(this.sessionId, { + ...(opts?.signal ? { signal: opts.signal } : {}), + ...(this.clientId ? { clientId: this.clientId } : {}), + }); + } + async context(): Promise { return await this.client.sessionContext(this.sessionId, this.clientId); } diff --git a/packages/sdk-typescript/src/daemon/index.ts b/packages/sdk-typescript/src/daemon/index.ts index 3474b34f2f5..7397d933298 100644 --- a/packages/sdk-typescript/src/daemon/index.ts +++ b/packages/sdk-typescript/src/daemon/index.ts @@ -235,6 +235,7 @@ export type { DaemonApprovalModeResult, DaemonInitWorkspaceResult, DaemonMcpRestartResult, + DaemonSessionRecapResult, DaemonToolToggleResult, DaemonAvailableCommand, DaemonCapabilities, diff --git a/packages/sdk-typescript/src/daemon/types.ts b/packages/sdk-typescript/src/daemon/types.ts index 45c733ea866..caccc949890 100644 --- a/packages/sdk-typescript/src/daemon/types.ts +++ b/packages/sdk-typescript/src/daemon/types.ts @@ -761,6 +761,26 @@ export interface DaemonInitWorkspaceResult { action: 'created' | 'overwrote' | 'noop'; } +/** + * #4175 follow-up. Returned from `POST /session/:id/recap`. The recap + * is a one-sentence "where did I leave off" summary generated by core's + * `generateSessionRecap` via a side-query against the fast model. + * + * `recap` is `null` (not absent, not an empty string) when: + * - the session has fewer than two dialog turns yet, + * - the side-query returns no extractable `...` payload, + * - or any underlying model error occurred (the core helper is + * best-effort and never throws). + * + * The route returns 200 in all three cases; only hard errors (unknown + * session, ACP transport down, bridge timeout) surface as non-2xx. + * Pre-flight `caps.features.session_recap` before calling. + */ +export interface DaemonSessionRecapResult { + sessionId: string; + recap: string | null; +} + /** * #4175 Wave 4 PR 17. Result body of `POST /workspace/mcp/:server/ * restart`. Discriminated by `restarted`: `true` carries the wall- diff --git a/packages/sdk-typescript/src/index.ts b/packages/sdk-typescript/src/index.ts index e85017db67f..8007f99bf5d 100644 --- a/packages/sdk-typescript/src/index.ts +++ b/packages/sdk-typescript/src/index.ts @@ -29,6 +29,7 @@ export { type DaemonApprovalModeResult, type DaemonInitWorkspaceResult, type DaemonMcpRestartResult, + type DaemonSessionRecapResult, type DaemonMcpServerRestartedData, type DaemonMcpServerRestartedEvent, type DaemonMcpServerRestartRefusedData, diff --git a/packages/sdk-typescript/test/unit/DaemonClient.test.ts b/packages/sdk-typescript/test/unit/DaemonClient.test.ts index d0ae6003c7c..3e4b87f04fe 100644 --- a/packages/sdk-typescript/test/unit/DaemonClient.test.ts +++ b/packages/sdk-typescript/test/unit/DaemonClient.test.ts @@ -53,6 +53,7 @@ interface CapturedRequest { method: string; headers: Record; body: string | null; + signal?: AbortSignal | null; } function recordingFetch( @@ -74,7 +75,13 @@ function recordingFetch( h.forEach((v, k) => (headers[k.toLowerCase()] = v)); } const body = typeof init?.body === 'string' ? init.body : null; - const captured: CapturedRequest = { url, method, headers, body }; + const captured: CapturedRequest = { + url, + method, + headers, + body, + signal: init?.signal ?? null, + }; calls.push(captured); return reply(captured); }, @@ -1343,6 +1350,77 @@ describe('DaemonClient', () => { }); }); + describe('recapSession (#4175 follow-up)', () => { + it('POSTs an empty body and returns the typed recap', async () => { + const { fetch, calls } = recordingFetch(() => + jsonResponse(200, { + sessionId: 's-1', + recap: + 'Debugging the auth retry race. Next: add deterministic timing.', + }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + const result = await client.recapSession('s-1'); + expect(result).toEqual({ + sessionId: 's-1', + recap: 'Debugging the auth retry race. Next: add deterministic timing.', + }); + expect(calls[0]?.url).toBe('http://daemon/session/s-1/recap'); + expect(calls[0]?.method).toBe('POST'); + expect(calls[0]?.body).toBe('{}'); + }); + + it('returns recap:null verbatim when the daemon reports best-effort failure', async () => { + const { fetch } = recordingFetch(() => + jsonResponse(200, { sessionId: 's-1', recap: null }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + const result = await client.recapSession('s-1'); + expect(result.recap).toBeNull(); + }); + + it('URL-encodes the session id', async () => { + const { fetch, calls } = recordingFetch(() => + jsonResponse(200, { sessionId: 's/1', recap: 'x' }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + await client.recapSession('s/1'); + expect(calls[0]?.url).toBe('http://daemon/session/s%2F1/recap'); + }); + + it('forwards X-Qwen-Client-Id when supplied', async () => { + const { fetch, calls } = recordingFetch(() => + jsonResponse(200, { sessionId: 's-1', recap: 'x' }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + await client.recapSession('s-1', { clientId: 'client-1' }); + expect(calls[0]?.headers['x-qwen-client-id']).toBe('client-1'); + }); + + it('forwards the AbortSignal so callers can cancel mid-flight', async () => { + const { fetch, calls } = recordingFetch(() => + jsonResponse(200, { sessionId: 's-1', recap: 'x' }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + const ctrl = new AbortController(); + await client.recapSession('s-1', { signal: ctrl.signal }); + expect(calls[0]?.signal).toBe(ctrl.signal); + }); + + it('throws on 404 when session is unknown', async () => { + const { fetch } = recordingFetch(() => + jsonResponse(404, { + error: 'session not found', + code: 'session_not_found', + }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + await expect(client.recapSession('s-1')).rejects.toMatchObject({ + status: 404, + }); + }); + }); + describe('setWorkspaceToolEnabled (#4175 Wave 4 PR 17)', () => { it('POSTs the enabled flag and URL-encodes the tool name', async () => { const { fetch, calls } = recordingFetch(() => diff --git a/packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts b/packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts index 1943d62eb61..86059cb0d1f 100644 --- a/packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts +++ b/packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts @@ -357,6 +357,35 @@ describe('DaemonSessionClient', () => { expect(calls[0]?.headers['x-qwen-client-id']).toBe('client-1'); }); + it('forwards recap through DaemonClient with the bound clientId and signal', async () => { + const { fetch, calls } = recordingFetch(() => + jsonResponse(200, { + sessionId: 's-1', + recap: 'Refactoring the auth flow. Next: run the integration tests.', + }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + const session = new DaemonSessionClient({ + client, + session: { + sessionId: 's-1', + workspaceCwd: '/work/a', + attached: true, + clientId: 'client-1', + }, + }); + const ctrl = new AbortController(); + const result = await session.recap({ signal: ctrl.signal }); + expect(result).toEqual({ + sessionId: 's-1', + recap: 'Refactoring the auth flow. Next: run the integration tests.', + }); + expect(calls[0]?.url).toBe('http://daemon/session/s-1/recap'); + expect(calls[0]?.method).toBe('POST'); + expect(calls[0]?.headers['x-qwen-client-id']).toBe('client-1'); + expect(calls[0]?.signal).toBe(ctrl.signal); + }); + it('forwards session-scoped operations through DaemonClient', async () => { const { fetch, calls } = recordingFetch((req) => { if (req.url.endsWith('/session/s-1/prompt')) { diff --git a/packages/sdk-typescript/test/unit/daemon-public-surface.test.ts b/packages/sdk-typescript/test/unit/daemon-public-surface.test.ts index 319b9eb2103..46ede5bedb9 100644 --- a/packages/sdk-typescript/test/unit/daemon-public-surface.test.ts +++ b/packages/sdk-typescript/test/unit/daemon-public-surface.test.ts @@ -30,6 +30,7 @@ import type { DaemonSessionDiedData, DaemonSessionDiedEvent, DaemonSessionEvent, + DaemonSessionRecapResult, DaemonSessionUpdateData, DaemonSessionUpdateEvent, DaemonSessionViewState, @@ -106,6 +107,11 @@ describe('public SDK entry โ€” typed daemon event surface (#4217)', () => { expectTypeOf().not.toBeNever(); expectTypeOf().not.toBeNever(); expectTypeOf().not.toBeNever(); + // #4175 follow-up: the recap result type lives under the daemon + // sub-barrel and is re-exported at the top-level. Without this + // assertion a future barrel reshuffle could silently drop the + // result type SDK consumers need to type `client.recapSession`. + expectTypeOf().not.toBeNever(); }); it('exposes the PR 21 auth device-flow surface at the public entry', () => { From 058bde70f9e7b448731e8de2932bc9bb71adb16d Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Mon, 25 May 2026 20:41:19 +0800 Subject: [PATCH 2/4] docs(serve): reconcile recap cancellation docs with actual v1 behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per chiga0's review on #4504 (option 1 โ€” match docs to reality rather than wire up cosmetic AbortController plumbing). The route, design doc, and protocol reference all claimed "client disconnect aborts the bridge-side wait" via `res.once('close')`, but the route has no such listener and the bridge accepts no `AbortSignal`. The only ceilings are the 60s `SESSION_RECAP_TIMEOUT_MS` backstop and the transport- closed race against ACP channel death. Wiring an HTTP-side AbortController in isolation would be cosmetic because the ACP child handler also passes a never-aborting `AbortController().signal` to the core helper (no cross-process abort plumbing yet) โ€” e2e cancel needs both layers. Recap is short (~1โ€“5s, `maxOutputTokens: 300`), so the absent cancellation is acceptable for v1; a request-id-based cancel ext-method can land in a follow-up. Also adds two known-limit bullets to the user guide per chiga0's other minor notes: token-cost amplification on no-token loopback (no per-route rate limit) and concurrent-recap safety (side-query reads chat history via `GeminiClient.getChat().getHistory()` snapshot and runs through a separate `BaseLlmClient`, never mutating the session's `GeminiChat`). ๐Ÿค– Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --- .../session-recap/session-recap-design.md | 18 +++++++++++---- docs/developers/qwen-serve-protocol.md | 2 +- docs/users/qwen-serve.md | 2 ++ packages/cli/src/serve/server.ts | 23 +++++++++++++++---- 4 files changed, 34 insertions(+), 11 deletions(-) diff --git a/docs/design/session-recap/session-recap-design.md b/docs/design/session-recap/session-recap-design.md index 0c1d9b36be7..03fdce7b653 100644 --- a/docs/design/session-recap/session-recap-design.md +++ b/docs/design/session-recap/session-recap-design.md @@ -44,11 +44,19 @@ helpers: `DaemonClient.recapSession(sessionId, opts)` and `docs/developers/qwen-serve-protocol.md` ยง `POST /session/:id/recap` for the wire contract and error envelope. -Cancellation is best-effort at v1: client disconnect aborts the -bridge-side wait, but the LLM call in the ACP child runs to completion -(recap is short โ€” single-attempt, ~1โ€“5s typical). A 60s backstop -timeout guards a wedged ACP channel. A future request-id-based cancel -ext-method can plumb full end-to-end cancellation if needed. +Cancellation is **absent in v1**. The route does not listen for HTTP +client disconnect, no `AbortSignal` is threaded into +`bridge.generateSessionRecap`, and the ACP child handler passes a +never-aborting `AbortController().signal` to the core helper (no +cross-process abort plumbing yet). The only ceilings are the bridge's +60s `SESSION_RECAP_TIMEOUT_MS` backstop and the transport-closed race +against ACP channel death. Wiring an HTTP-side AbortController in +isolation would be cosmetic โ€” the child-side LLM call would still run +to completion, so e2e cancel is not achievable without the cross- +process abort piece. This is acceptable for v1 because recap is short +(single-attempt side-query, `maxOutputTokens: 300`, ~1โ€“5s typical). +A future request-id-based cancel ext-method can plumb full end-to-end +cancellation if/when the bandwidth cost justifies it. ## Architecture diff --git a/docs/developers/qwen-serve-protocol.md b/docs/developers/qwen-serve-protocol.md index 1b2e2c89330..97b202371ab 100644 --- a/docs/developers/qwen-serve-protocol.md +++ b/docs/developers/qwen-serve-protocol.md @@ -1135,7 +1135,7 @@ Errors: - `400 {code: 'invalid_client_id'}` โ€” malformed `X-Qwen-Client-Id` header. - `404` โ€” session unknown. -Cancellation: client disconnect aborts the bridge-side wait, but the LLM call in the ACP child runs to completion (recap is short โ€” single-attempt, ~1โ€“5s typical). A 60s backstop timeout guards against a wedged ACP channel. +Cancellation: **none in v1**. The route does not listen for HTTP client disconnect, no `AbortSignal` is plumbed into the bridge, and the ACP child runs the side-query to completion regardless of whether the caller has disconnected. The only ceilings are the bridge's 60s backstop timeout (`SESSION_RECAP_TIMEOUT_MS`) and the transport-closed race against ACP channel death. This is acceptable because recap is short (single-attempt, `maxOutputTokens: 300`, ~1โ€“5s typical); a request-id-based cancel ext-method can plumb full end-to-end cancellation in a future release if the bandwidth cost ever justifies it. ### Mutation: approval, tools, init, MCP restart diff --git a/docs/users/qwen-serve.md b/docs/users/qwen-serve.md index 88752093a00..92f76e07b40 100644 --- a/docs/users/qwen-serve.md +++ b/docs/users/qwen-serve.md @@ -16,6 +16,8 @@ Run Qwen Code as a local HTTP daemon so multiple clients (IDE plugins, web UIs, - **One daemon, one workspace** โ€” each `qwen serve` process binds to exactly one workspace at boot (per [#3803](https://github.com/QwenLM/qwen-code/issues/3803) ยง02). Multi-workspace deployments run one daemon per workspace on separate ports (or behind an orchestrator). - **Remote runtime control** ([#4175](https://github.com/QwenLM/qwen-code/issues/4175) PR 17) โ€” change a session's approval mode (`POST /session/:id/approval-mode`), toggle a tool per workspace (`POST /workspace/tools/:name/enable`), scaffold an empty `QWEN.md` (`POST /workspace/init`, mechanical only โ€” does NOT call the model; for AI-fill, follow up with `POST /session/:id/prompt`), or restart a single MCP server with a budget pre-check (`POST /workspace/mcp/:server/restart`). All four are strict-gated โ€” configure `--token` first. - **Session recap** ([#4175](https://github.com/QwenLM/qwen-code/issues/4175) follow-up) โ€” fetch a one-sentence "where did I leave off" summary of an active session (`POST /session/:id/recap`). Wraps core's `generateSessionRecap` as a side-query against the fast model; pollutes neither the main chat history nor the SSE stream. Non-strict gate (same posture as `/prompt`); SDK helper `client.recapSession(sessionId)`. + - **Known limit โ€” token-cost amplification:** the route is a pure-cost endpoint (each call is an LLM side-query, no state benefit) and the daemon has no per-route rate limit in v1. On a no-token loopback default a buggy or malicious local client can spam it to burn tokens. Configure `--token` (and optionally `--require-auth`) on shared dev hosts before exposing the daemon. + - **Concurrent recap safety:** two simultaneous `/recap` calls on the same session run two independent side-queries. `generateSessionRecap` reads a snapshot of the chat history via `GeminiClient.getChat().getHistory()` and feeds it to a separate `BaseLlmClient.generateText` call (via `runSideQuery`); it never appends to or mutates the session's `GeminiChat`. Safe to call from multiple clients without coordination. ## v0.16-alpha known limits diff --git a/packages/cli/src/serve/server.ts b/packages/cli/src/serve/server.ts index d3f7596943c..1e74ec3c2fd 100644 --- a/packages/cli/src/serve/server.ts +++ b/packages/cli/src/serve/server.ts @@ -1474,11 +1474,24 @@ export function createServeApp( // #4175 follow-up. Wraps `generateSessionRecap` (core/services/ // sessionRecap.ts) so daemon clients can fetch a one-sentence // "where did I leave off" summary without driving the agent through - // a full prompt turn. Posture mirrors `/session/:id/prompt`: - // non-strict gate (token cost, not state mutation), and disconnect - // is detected via `res.once('close')` for the bridge-side - // cancellation. Best-effort โ€” `recap: null` on short history or - // transient model failure is a normal 200, not an error. + // a full prompt turn. Non-strict gate (token cost, not state + // mutation), matching `/session/:id/prompt`'s posture. + // + // v1 cancellation: NONE on the route side. There is intentionally no + // `res.once('close')` listener and no `AbortSignal` plumbed into + // `bridge.generateSessionRecap`. The only ceilings are the bridge's + // 60s `SESSION_RECAP_TIMEOUT_MS` backstop and the + // `getTransportClosedReject` race against ACP transport death. This + // matches the ACP child's `acpAgent.ts` handler, which also passes + // a never-aborting `AbortController().signal` to the core helper + // because there is no cross-process abort plumbing yet. Wiring an + // HTTP-side AbortController would be cosmetic โ€” the child-side LLM + // call would still run to completion, so e2e cancel is not + // achievable in v1. Recap is short (single-attempt side-query, + // ~1โ€“5s typical, `maxOutputTokens: 300`), so this is acceptable. + // + // Best-effort โ€” `recap: null` on short history or transient model + // failure is a normal 200, not an error. const sessionId = req.params['id']; if (!sessionId) { res From 40c23eed85532ffd74440e081bf8df5cd22346a8 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Mon, 25 May 2026 23:43:14 +0800 Subject: [PATCH 3/4] docs(serve): finish recap cancellation reconciliation in acpAgent ext-method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit (058bde70f) reconciled the cancellation narrative in 3 doc files + the route comment in server.ts, but missed the inline comment inside the ACP child's `SERVE_CONTROL_EXT_METHODS.sessionRecap` handler. That comment still claimed "Client disconnect aborts the bridge-side wait" โ€” the exact false statement 058bde70f was meant to remove from the codebase. Worse, the new server.ts comment from 058bde70f points readers at this handler for corroboration ("This matches the ACP child's `acpAgent.ts` handler ..."), so a reader following that crumb would land on a comment saying the opposite. Per @wenshao's `[Suggestion]` review on #4504, applying his suggested replacement verbatim. Comment-only change; no behavior delta. ๐Ÿค– Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --- packages/cli/src/acp-integration/acpAgent.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index b2840ff2345..2e8e23be617 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -2261,12 +2261,15 @@ class QwenAgent implements Agent { } const session = this.sessionOrThrow(sessionId); const config = session.getConfig(); - // v1: no cross-process abort plumbing. Client disconnect aborts - // the bridge-side wait but the LLM call in this child runs to - // completion. Acceptable because recap is short (single-attempt - // side-query, maxOutputTokens: 300). A future request-id-based - // cancel ext-method can plumb a real signal end-to-end if the - // bandwidth cost ever becomes an issue. + // v1: no cross-process abort plumbing. The bridge does not listen + // for HTTP client disconnect and no AbortSignal is threaded through + // the ext-method, so the LLM call in this child always runs to + // completion. The only ceilings are the bridge's 60s + // `SESSION_RECAP_TIMEOUT_MS` backstop and the transport-closed race + // against ACP channel death. Acceptable because recap is short + // (single-attempt side-query, `maxOutputTokens: 300`). A future + // request-id-based cancel ext-method can plumb a real signal + // end-to-end if the bandwidth cost ever becomes an issue. const recap = await generateSessionRecap( config, new AbortController().signal, From 0a1931113b3284e140eefb80c206484362601a12 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 26 May 2026 00:13:55 +0800 Subject: [PATCH 4/4] docs(serve): finish recap cancellation reconciliation across bridge + SDK JSDocs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third pass on the same task. wenshao caught one more spot in `bridge.ts:330` (JSDoc for `SESSION_RECAP_TIMEOUT_MS` claimed "actual cancellation on client disconnect is handled at the HTTP route layer" โ€” the exact opposite of what the route comment + protocol doc + design doc + acpAgent comment all now say). Pre-empting another round-trip by sweeping the rest of the codebase and fixing the two remaining misleading SDK JSDocs in the same go: - `DaemonClient.recapSession`: previously said "cancellation is via the optional signal" without qualifying that the signal aborts ONLY the local HTTP fetch. The daemon-side wait + the child-side LLM call both ignore it. Spelled out the layered reality: signal โ†’ fetch cancellation only; bridge โ†’ 60s backstop; ACP child โ†’ always runs to completion. Also corrected the "bypasses fetchTimeoutMs" claim โ€” the raw `_fetch` simply doesn't go through that wrapper at all. - `DaemonSessionClient.recap`: same clarification on the wrapper that delegates to `recapSession`. Comment-only changes; no behavior delta. ๐Ÿค– Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --- packages/acp-bridge/src/bridge.ts | 4 ++-- .../sdk-typescript/src/daemon/DaemonClient.ts | 18 +++++++++++++----- .../src/daemon/DaemonSessionClient.ts | 7 +++++-- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/packages/acp-bridge/src/bridge.ts b/packages/acp-bridge/src/bridge.ts index 843e4c55bba..f8664fb18cb 100644 --- a/packages/acp-bridge/src/bridge.ts +++ b/packages/acp-bridge/src/bridge.ts @@ -326,8 +326,8 @@ const MCP_RESTART_TIMEOUT_MS = 300_000; * healthy call finishes in 1โ€“5 seconds; we cap at 60s to absorb model- * provider hiccups without inheriting the 10s `initTimeoutMs` default * (which would false-fire on any GPT-style slow start). The race is a - * safety net against a wedged ACP channel โ€” actual cancellation on - * client disconnect is handled at the HTTP route layer. + * safety net against a wedged ACP channel โ€” there is no HTTP-side + * disconnect cancellation in v1 (see server.ts route comment). */ const SESSION_RECAP_TIMEOUT_MS = 60_000; const DEFAULT_MAX_SESSIONS = 20; diff --git a/packages/sdk-typescript/src/daemon/DaemonClient.ts b/packages/sdk-typescript/src/daemon/DaemonClient.ts index 0d5b1815ec6..26c5d441c52 100644 --- a/packages/sdk-typescript/src/daemon/DaemonClient.ts +++ b/packages/sdk-typescript/src/daemon/DaemonClient.ts @@ -984,11 +984,19 @@ export class DaemonClient { * inside the daemon's ACP child. * * Non-strict mutation gate โ€” posture matches `/session/:id/prompt` - * (the route costs tokens but mutates no state). Bypasses - * `fetchTimeoutMs` because the underlying side-query can take longer - * than the default 30s budget under a slow model; cancellation is - * via the optional `signal`. Older daemons (pre-recap support) return - * 404 โ€” pre-flight `caps.features.session_recap` before calling. + * (the route costs tokens but mutates no state). Calls `_fetch` + * directly without the per-call `fetchTimeoutMs` wrapper because the + * underlying side-query can take longer than the default 30s under + * a slow model. Older daemons (pre-recap support) return 404 โ€” + * pre-flight `caps.features.session_recap` before calling. + * + * Cancellation: the optional `signal` aborts only the LOCAL HTTP + * fetch. It does NOT propagate to the daemon โ€” the bridge-side wait + * continues until the 60s `SESSION_RECAP_TIMEOUT_MS` backstop, and + * the side-query inside the ACP child always runs to completion (no + * cross-process abort plumbing in v1). A future request-id-based + * cancel ext-method will plumb a real signal end-to-end if/when the + * bandwidth cost justifies it. * * `recap` may be `null` on too-short histories or transient model * failures (a 200 response with `recap: null`), per the best-effort diff --git a/packages/sdk-typescript/src/daemon/DaemonSessionClient.ts b/packages/sdk-typescript/src/daemon/DaemonSessionClient.ts index d7643c0d63b..4e8a2374e5c 100644 --- a/packages/sdk-typescript/src/daemon/DaemonSessionClient.ts +++ b/packages/sdk-typescript/src/daemon/DaemonSessionClient.ts @@ -221,8 +221,11 @@ export class DaemonSessionClient { /** * One-sentence "where did I leave off" recap of this session. See - * `DaemonClient.recapSession` for the contract (best-effort, may - * return `recap: null`; cancellation via `signal`). + * `DaemonClient.recapSession` for the full contract: best-effort + * (may return `recap: null`); the optional `signal` aborts only the + * local HTTP fetch โ€” the daemon-side wait + the LLM call in the ACP + * child both run to completion regardless (no cross-process abort + * plumbing in v1). */ async recap(opts?: { signal?: AbortSignal;