Thanks for the PR!

Template looks good ✓ — all required sections present including bilingual description.

On direction: this solves a real problem — stale client IDs after daemon restarts cause accepted prompts to silently fail, leaving clients stuck waiting for events that never arrive. The fix is squarely within the daemon reliability work the project has been doing (session management, prompt admission contracts). Aligned.

On approach: the scope is tight and well-contained. Three production files changed with a total of ~10 lines of logic, plus three test files adding/updating coverage across bridge, daemon route, and SDK. The core insight is simple: resolveTrustedClientId already throws synchronously, and the existing try/catch → Promise.reject wrapper was the only thing preventing the synchronous throw from reaching the route's catch block. Removing that wrapper aligns the behavior with the documented admission contract (non-async, synchronous admission failures). This is the minimal change needed.

One observation: the PR also adds a daemonLog.warn entry for InvalidClientIdError in the route handler, matching the existing PromptQueueFullError logging pattern — reasonable for debugging stale client issues in production.

Moving on to code review. 🔍

— Qwen Code · qwen3.7-max

Code Review

Independent proposal before reading the diff: the bug is that resolveTrustedClientId throws synchronously but the sendPrompt method wraps it in try/catch → Promise.reject, making the error asynchronous. The fix should remove the wrapper so the synchronous throw propagates to the route handler's catch block, which already knows how to map InvalidClientIdError to 400 invalid_client_id. The sendPrompt type docs should also be updated to list InvalidClientIdError alongside the existing PromptQueueFullError.

The PR's approach matches this exactly. No simpler path missed.

Review findings:

• No critical blockers. The logic change is correct: removing the try/catch → Promise.reject wrapper around resolveTrustedClientId makes InvalidClientIdError throw synchronously, matching the non-async admission contract already documented in bridgeTypes.ts.
• The existing sendBridgeError in server.ts (line ~5640) already handles InvalidClientIdError → 400 { code: 'invalid_client_id', sessionId, clientId } without a promptId, so no route-level changes were needed beyond adding the daemonLog.warn entry.
• Test changes are thorough: bridge tests updated from rejects.toBeInstanceOf to toThrow (synchronous), new server route test verifies the 400 response shape and daemon log, new SDK tests verify both non-blocking and blocking prompt error mapping plus prompt slot release.
• The promptId included in the daemonLog.warn payload is intentional for debugging correlation, even though the HTTP response body omits it. Consistent with the PromptQueueFullError logging pattern.

Reuse check: no new utility code or abstractions introduced. The PR reuses existing error types (InvalidClientIdError), existing response shape (invalid_client_id), and existing logging pattern (daemonLog.warn). Clean.

Test Results

All existing tests pass, plus 7 new/updated test assertions across 3 packages:

Before/After

Before (installed build — curl against running daemon)

Created a session, then sent a prompt with a stale client ID:

$ curl -s -w '\nHTTP_STATUS:%{http_code}' \
  -X POST http://127.0.0.1:14170/session/46b8b7c8-.../prompt \
  -H 'Content-Type: application/json' \
  -H 'X-Qwen-Client-Id: stale-client-123' \
  -d '{"prompt":[{"type":"text","text":"hello"}]}'

{"promptId":"31ae264e-49d9-49ad-8778-c961ceb8e155","lastEventId":1}
HTTP_STATUS:202

Bug confirmed: stale client ID accepted with 202, returns a promptId that will never complete.

After (PR fix — vitest evidence, tmux capture-pane)

npm run dev for full daemon start is blocked by a pre-existing ink/dom package export issue in this environment (Ink 7.0.3 doesn't export ./dom), unrelated to this PR. Using vitest as the after-behavior evidence:

=== BEFORE (installed qwen, stale client ID) ===
POST /session/<id>/prompt with X-Qwen-Client-Id: stale-client-123
Response: 202 Accepted with promptId - BUG: client hangs waiting

=== AFTER (PR fix, unit test evidence) ===

 ✓ src/serve/server.test.ts (517 tests | 516 skipped) 54ms
 Test Files  1 passed (1)
      Tests  1 passed | 516 skipped (517)

--- bridge admission tests ---
 ✓ src/bridge.test.ts (296 tests | 292 skipped) 30ms
 Test Files  1 passed (1)
      Tests  4 passed | 292 skipped (296)

--- SDK client tests ---
 ✓ test/unit/DaemonClient.test.ts (168 tests | 166 skipped) 13ms
 Test Files  1 passed (1)
      Tests  2 passed | 166 skipped (168)

The focused tests verify:

• Daemon route: stale client ID → 400, response body has code: 'invalid_client_id', no promptId
• Bridge: sendPrompt throws InvalidClientIdError synchronously (not rejected promise), activePromptCount stays at 0
• SDK: non-blocking prompts map to DaemonHttpError, blocking prompts release the local slot so a subsequent valid prompt succeeds

— Qwen Code · qwen3.7-max

Stepping back to think about this as a whole.

My independent proposal for the fix was identical to what the PR does: remove the try/catch → Promise.reject wrapper around resolveTrustedClientId so the synchronous throw reaches the route handler's existing error mapping. The PR didn't miss a simpler path — this is the simplest path.

Does it solve something users care about? Yes. The before-test curl output clearly shows the bug: a stale client ID gets 202 with a promptId, and the client waits forever. The after-behavior (verified through focused tests) returns 400 invalid_client_id immediately, letting clients clear their pending state.

The code is straightforward — 10 lines of production logic change, no abstractions, no drive-by refactors. The daemonLog.warn addition follows the existing pattern. Test coverage is solid across bridge, daemon route, and SDK.

Every change in the diff is needed for the stated goal. The bridge test updates (rejects → toThrow) are necessary because the behavior changed from async rejection to sync throw. The new tests at each layer verify a different aspect of the fix. Nothing extraneous.

If I had to maintain this in six months, I'd thank the author — the change is minimal, well-tested, and the motivation is clearly documented.

Approving. ✅

— Qwen Code · qwen3.7-max

✅ Maintainer local verification — build, 981 unit tests, an independent real-HTTP e2e, and a mutation test

Verified PR #5784 at head 6a3bd53cf in an isolated worktree (fresh npm ci, Node 22.22.2). Beyond re-running the PR's suites, I wrote an independent supertest harness that drives the real createServeApp over real HTTP to demonstrate the bug→fix contract directly. Summary first, evidence below.

Verdict: safe to merge. Clean, focused fix; build + typecheck exit 0; all touched suites pass (981 tests); my independent e2e reproduces the stuck-client bug and confirms the fix; a mutation test proves the change is guarded. CI is already green on macOS/Ubuntu/Windows.

1. Root cause & the fix (one-line behavioral pivot)

The whole fix is making AcpSessionBridge.sendPrompt throw synchronously for an invalid/stale client id instead of returning a rejected promise:

// packages/acp-bridge/src/bridge.ts
-      let originatorClientId: string | undefined;
-      try {
-        originatorClientId = resolveTrustedClientId(entry, context?.clientId);
-      } catch (err) {
-        return Promise.reject(err);   // ← async: HTTP route had already sent 202
-      }
+      const originatorClientId = resolveTrustedClientId(entry, context?.clientId);
//        ↑ throws synchronously, like PromptQueueFullError → route can reject before 202

The /prompt route wraps sendPrompt in a synchronous try/catch; only a synchronous throw is caught before the 202 is written. sendBridgeError already maps InvalidClientIdError → 400 invalid_client_id, and bridgeTypes.ts now documents InvalidClientIdError alongside PromptQueueFullError as a synchronous admission failure. server.ts adds one daemonLog.warn line. The SDK change is test-only (no DaemonClient.ts source change) — it pins the existing DaemonHttpError mapping.

2. Build + typecheck — exit 0

3. Unit tests — 981 passed across the 3 touched suites

4. Independent real-HTTP e2e (my own harness, not the PR's) — the bug→fix, demonstrated

A standalone supertest spec driving the real createServeApp with a minimal injected bridge, toggling only the admission behavior:

For the same stale client id, the pre-fix async-reject path returns 202 + orphaned promptId (the client waits forever) while the fixed sync-throw path returns 400 (the client recovers immediately). This is exactly the user-visible contract the PR restores, proven end-to-end over real HTTP. 4 passed (4).

5. Mutation test — the change is non-vacuous

Reverted the production files to base (ac3c8ea13), kept the PR's tests:

Restoring the PR source → all green again.

Honest note: the full-suite mutation run also showed 2 other red tests (workspace_mismatch…, permission voter invalid_client_id). I re-ran each in isolation against base server.ts and both pass — they are test-ordering/pollution flakes in the reverted full-suite run, not a dependency on this PR. At clean head the suite is a stable 517/517.

Risk notes (low)

• Behavioral change is narrow and matches the documented contract: a previously async admission failure for invalid client ids is now a synchronous throw. The one caller-facing consequence (direct sendPrompt callers now get a sync throw instead of a rejected promise) is called out in the PR and matches the PromptQueueFullError precedent.
• No wire-shape change; the daemon reuses the existing invalid_client_id response.

ci-bot is at LGTM, no unresolved review threads, CI green on all three platforms; this independent local run agrees.

// packages/acp-bridge/src/bridge.ts
-      let originatorClientId: string | undefined;
-      try {
-        originatorClientId = resolveTrustedClientId(entry, context?.clientId);
-      } catch (err) {
-        return Promise.reject(err);   // ← 异步:HTTP 路由此时已经发了 202
-      }
+      const originatorClientId = resolveTrustedClientId(entry, context?.clientId);
//        ↑ 同步抛出,和 PromptQueueFullError 一样 → 路由能在 202 之前拒绝

@qwen-code /triage