feat(web-shell,webui,sdk): context-usage API + daemon-react-sdk refactor + dialog UX#4573
Conversation
📋 Review SummaryThis PR introduces three major feature areas: (1) a complete context-usage API链路 across SDK/acp-bridge/CLI/web-shell, (2) a comprehensive modular refactor of the daemon React providers into session/workspace submodules with a new 🔍 General Feedback
🎯 Specific Feedback🔴 Critical
🟡 High
🟢 Medium
🔵 Low
✅ Highlights
Summary Statistics
Recommendation: Address the critical security issues in |
ytahdn
left a comment
There was a problem hiding this comment.
wenshao
left a comment
There was a problem hiding this comment.
Cross-file findings (not anchorable to the PR diff)
[Suggestion] logSafe in packages/cli/src/serve/acpHttp/jsonRpc.ts:128 strips only ASCII C0/C1 controls + DEL, but sanitizeForStderr in deviceFlow.ts was upgraded to also strip Unicode LINE SEPARATOR (U+2028), bidi controls, zero-width chars, and BOM. logSafe sanitizes client-controlled values (sessionId, method names) before they reach writeStderrLine — a client could use U+2028 to forge daemon log lines. Consider aligning with sanitizeForStderr's character set.
[Suggestion] cancelAbandonedPermission in packages/cli/src/serve/acpHttp/dispatch.ts:237 uses /not found|unknown session/i.test(msg) to detect session-gone errors. If the bridge changes error wording, this silently fails and the mediator stays stuck permanently. Prefer matching on err.name === 'SessionNotFoundError' with the regex as fallback.
[Suggestion] After a 504 deadline timeout in packages/cli/src/serve/server.ts:1589, bridgePromise.catch(() => undefined) silently swallows all rejections from the orphaned bridge prompt. The session's prompt FIFO stays occupied with no logging. Consider logging when the orphaned promise settles for debugging.
[Suggestion] In packages/cli/src/ui/commands/contextCommand.ts:40, the breakdown categories use estimateTokens() (character-based heuristic) while totalTokens uses the actual API count. The breakdown sum won't match totalTokens — especially for code-heavy or multilingual content where the heuristic can be 20-40% off. Consider labeling as "estimated" or normalizing proportionally.
| autoConnect, | ||
| autoReconnect, | ||
| resolvedBaseUrl, | ||
| resolvedToken, |
There was a problem hiding this comment.
[Suggestion] resolvedToken in the useEffect dependency array causes a full teardown-and-reconnect cycle on every token refresh — dropping the SSE stream, transcript state, and in-flight prompts. An OAuth token rotation forces a visible disconnect/reconnect flash in the UI.
Consider storing the token in a ref and reading it at call-time instead of including it in the dependency array.
— claude-opus-4-7 via Claude Code /qreview
There was a problem hiding this comment.
Token changes require a full reconnect — the existing SSE connection is authenticated with the old token and will be rejected by the daemon. Storing the token in a ref and reading it at call-time would cause requests to fail silently with stale credentials. The reconnect flash is the correct behavior; it can be improved with a loading state overlay if UX is a concern.
| ); | ||
| }, | ||
|
|
||
| async globWorkspace(pattern, opts) { |
There was a problem hiding this comment.
[Suggestion] globWorkspace (and stat, listDirectory) use raw fetch with manual URL/header construction, bypassing DaemonClient. Every other action in this file consistently uses client.* methods.
Glob/stat/list requests miss error normalization, timeout configuration, and any future middleware that DaemonClient provides. Consider adding corresponding methods to DaemonClient.
— claude-opus-4-7 via Claude Code /qreview
There was a problem hiding this comment.
Agreed that these should ideally go through DaemonClient. However, DaemonClient currently lacks glob/stat/listDirectory methods. Adding them to the SDK is a separate scope item — tracked for a follow-up PR.
| ); | ||
| } | ||
|
|
||
| async sessionContextUsage( |
There was a problem hiding this comment.
[Suggestion] sessionContextUsage() and DaemonSessionClient.contextUsage() are new public SDK methods with zero unit tests. Sibling methods like recapSession(), workspaceMcpTools(), and workspaceTools() all have dedicated tests.
URL encoding, the detail query parameter serialization, and error deserialization are untested — a regression would ship silently.
— claude-opus-4-7 via Claude Code /qreview
There was a problem hiding this comment.
Acknowledged. sessionContextUsage() follows the same pattern as existing SDK methods (recapSession, workspaceMcpTools, etc.) with straightforward URL construction and JSON deserialization. Will track test coverage as a follow-up item alongside the broader daemon SDK test effort.
…llel agents display Security fixes: - Mermaid securityLevel reverted to 'strict', strip foreignObject/style from SVG sanitizer - Shift+Tab no longer silently sets yolo mode (only approves current request) - clientLifecycle uses sessionStorage for per-tab client ID isolation Bug fixes: - cancel() finally block guards setPromptStatus with session-ID check - lastRecapBlockCountRef resets on session switch - collectContextData wrapped in try/catch with field stripping - useDaemonResource: request sequence counter prevents stale response overwrite - ResumeDialog: shows error state when session list fails to load - detachDaemonClient: adds keepalive:true for tab-close reliability - server.test.ts: adds session_context_usage to EXPECTED_STAGE1_FEATURES Performance: - useSyncExternalStore selector hoisted via useCallback Feature: - Parallel agents merged display (ParallelAgentsGroup component) Tests: - clientLifecycle.test.ts (9 tests): sessionStorage, keepalive, detach behavior - useDaemonResource.test.tsx (5 tests): stale response race condition coverage - Markdown.test.ts: updated foreignObject/style assertions to expect stripping Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
wenshao
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
The incremental diff (9 files) adds a submit confirmation tab to AskUserQuestion and forwards permission vote metadata through the ACP bridge. The implementation is correct: metadata pipeline is consistent across all 4 resolution paths, keyboard handling properly accounts for the submit tab state, and the buildResult key change (question text → numeric indices) is actually a bug fix that aligns with the agent-side parseInt key parsing.
— DeepSeek/deepseek-v4-pro via Qwen Code /review
Verification Report — PR #4573Reviewer: wenshao 1. Build
2. Unit TestsPR-targeted tests (as listed in validation):
Full suite per package:
3. Code Review — Context-Usage API ChainReviewed the full data flow: CLI route → bridge → ACP ext-method → core
4. Code Review — Webui Daemon RefactorThe monolithic
5. Issues Found
6. SummaryThis is a well-executed PR covering three distinct concerns:
2152 tests pass across 73 files with zero failures. The two low-severity issues are non-blocking. Recommendation: ✅ Approve. Verified locally on 2026-05-28 |
wenshao
left a comment
There was a problem hiding this comment.
Cross-file findings (not anchorable to this PR diff)
[Critical] packages/cli/src/serve/runQwenServe.ts:590 — writerIdleTimeoutMs passes isPositiveIntegerMs but skips assertTimerDelayInRange (while promptDeadlineMs at line 582 correctly calls both). A value exceeding 2³¹−1 ms passes the integer check but Node's setInterval silently clamps it to 1 ms — every SSE client would be evicted 1 ms after connecting, causing an infinite reconnect storm. Fix: add assertTimerDelayInRange('writerIdleTimeoutMs', opts.writerIdleTimeoutMs); after the isPositiveIntegerMs check.
[Suggestion] packages/web-shell/client/components/messages/Markdown.tsx:384 — remarkPlugins={[remarkGfm, remarkMath]} and rehypePlugins={[rehypeKatex]} allocate new array instances on every render. Hoist to module-scope constants (const REMARK_PLUGINS = [remarkGfm, remarkMath]) to ensure referential stability.
[Suggestion] packages/cli/src/serve/daemonLogger.ts:167 — The daemon log file grows without bound — no rotation, no size cap. A long-lived daemon will fill the disk. Consider adding a max-file-size check (e.g., 50 MB) with rotation.
— claude-opus-4-7 via Claude Code /qreview
chiga0
left a comment
There was a problem hiding this comment.
Review — Part 1
PR Classification: New Feature + Refactor (feat + refactor)
Scope reviewed: Backend API (context-usage endpoint chain), security (SVG sanitization), permission metadata forwarding, AskUserQuestion UX rewrite, DaemonSessionProvider refactor, SDK types, App.tsx integration.
Summary
This is a large, multi-faceted PR with well-structured changes across the stack. The context-usage API follows a clean layered pattern (SDK → bridge → server → agent). The DaemonSessionProvider modular refactor is well-organized. The permission metadata forwarding through the mediator is clean and covers all resolution paths. The SVG sanitization approach (keeping <style> and <foreignObject> while stripping dangerous constructs) is well-reasoned with good tests.
Phantom Fix Status (post-HEAD 54bc655f4)
Verified wenshao's 9 phantom fixes against HEAD:
- ✅
fmtCategoryRowzero-guard — fixed incontextCommand.ts:335 - ✅
releaseSessioncallssession.close()before detach — fixed inactions.ts - ✅
newSessionaborts in-flight prompts — fixed inactions.ts - ✅ Auto-recap error logging — fixed (console.warn)
- ✅
collectContextDatacatch logging — fixed (console.warn) - ✅
parseContextUsageMessageruntime validation — fixed (typeof totalTokens !== 'number'check) - ✅
storageKeyRefupdated on prop change — fixed (storageKeyRef.current = storageKey) - ❌
buildResultkeyString(i)→q.question— STILL BROKEN (see finding #1) - ✅
ParallelAgentsGrouppendingApproval/onConfirm — fixed (props accepted and forwarded in MessageList.tsx)
8/9 phantom fixes are now present. 1 critical regression remains.
Findings (4 items)
| # | Severity | File | Issue |
|---|---|---|---|
| 1 | Major | AskUserQuestion.tsx:98 |
buildResult uses String(i) as answer key — regression from old q.question key, contradicts commit 675e8f698 |
| 2 | Minor | bridge.ts:354 |
extractPermissionResponseMetadata accepts any object-type answers without string value validation |
| 3 | Minor | Markdown.tsx:167 |
lastMermaidTheme module-level cache is fragile and non-invalidateable |
| 4 | Minor | actions.ts:456 |
getModeFromSessionContext uses Record<string, unknown> cast |
This review was generated by QoderWork AI
| const custom = customInputs[i]; | ||
| const all = custom ? [...multi, custom] : multi; | ||
| result[q.question] = all.join(', '); | ||
| result[String(i)] = all.join(', '); |
There was a problem hiding this comment.
[Major] buildResult answer key regression — String(i) vs q.question
The old code used q.question (question text) as the answer key:
result[q.question] = answers[i] || customInputs[i] || '';The new code uses String(i) (numeric index):
result[String(i)] = answers[i] || customInputs[i] || '';This is:
- A regression from old behavior — any downstream consumer matching answers by question text (e.g., the agent's permission handler) will now get empty strings instead of the expected values
- Contradicts commit
675e8f698which says "restore question-text answer keys" / "use q.question as answer key instead of numeric index" — but the code at HEAD still usesString(i) - Flagged by wenshao's phantom fix review as still absent from HEAD
If the intent is to use numeric indices, downstream consumers must be updated to match. If the intent is question-text keys (as the commit message claims), this code must be reverted to q.question.
This review was generated by QoderWork AI
There was a problem hiding this comment.
This is an intentional revert. Commit 675e8f6 changed to q.question, but it was manually reverted back to String(i) because downstream consumers (the ACP permission handler) use numeric index keys to match answers. The numeric index is the correct and current contract.
| // Track last initialized theme to avoid redundant mermaid.initialize() calls. | ||
| // mermaid.initialize() is idempotent but runs per-block; with N diagrams in a | ||
| // transcript this saves N-1 redundant calls per render cycle. | ||
| let lastMermaidTheme: string | undefined; |
There was a problem hiding this comment.
[Minor] Module-level lastMermaidTheme cache is fragile
let lastMermaidTheme: string | undefined;This module-level variable serves as a cache to avoid redundant mermaid.initialize() calls. Two concerns:
-
Cannot be invalidated: If the theme changes (e.g., from a prop or user toggle), all MermaidBlock instances share this global cache. In a single-instance app this works, but if multiple shells render simultaneously (micro-frontends, storybook, tests), only the first instance triggers initialization and others may render with a stale theme.
-
Test pollution: Since it persists across test cases, tests that render mermaid blocks with different themes may produce order-dependent results.
Consider using a useRef per-component instance, or accepting the minor performance cost of always calling mermaid.initialize() (it's idempotent).
This review was generated by QoderWork AI
There was a problem hiding this comment.
mermaid.initialize() is itself a global singleton API — it maintains a single global config regardless of how many call sites exist. Using useRef per component would not solve the multi-instance concern because mermaid internally only has one config. The module-level cache simply avoids redundant calls to an already-global API. In tests, mermaid is lazily imported and not rendered, so test pollution does not apply.
| ): string | undefined { | ||
| const modes = | ||
| typeof context.state.modes === 'object' && context.state.modes !== null | ||
| ? (context.state.modes as Record<string, unknown>) |
There was a problem hiding this comment.
[Minor] getModeFromSessionContext uses loose Record<string, unknown> cast
const modes =
typeof context.state.modes === 'object' && context.state.modes !== null
? (context.state.modes as Record<string, unknown>)
: undefined;
const mode = modes?.['currentModeId'] ?? modes?.['currentMode'];The as Record<string, unknown> cast assumes modes is a plain object with string keys. If the daemon returns an unexpected shape (e.g., an array, a class instance, or nested objects), the ?. access still returns undefined gracefully — so no runtime error. But the dual property lookup (currentModeId ?? currentMode) suggests uncertainty about the actual API contract.
Consider defining a proper interface:
interface SessionModes {
currentModeId?: string;
currentMode?: string;
}And casting to that instead of a loose record.
This review was generated by QoderWork AI
There was a problem hiding this comment.
The dual key lookup (currentModeId ?? currentMode) is intentional compatibility code — the daemon API field name differs across versions. Defining a strict interface would need to track daemon API evolution and provides little safety gain since the fallback chain already returns undefined gracefully for any unexpected shape. Will revisit once the daemon API stabilizes on a single field name.
…llel agents display Security fixes: - Mermaid securityLevel reverted to 'strict', strip foreignObject/style from SVG sanitizer - Shift+Tab no longer silently sets yolo mode (only approves current request) - clientLifecycle uses sessionStorage for per-tab client ID isolation Bug fixes: - cancel() finally block guards setPromptStatus with session-ID check - lastRecapBlockCountRef resets on session switch - collectContextData wrapped in try/catch with field stripping - useDaemonResource: request sequence counter prevents stale response overwrite - ResumeDialog: shows error state when session list fails to load - detachDaemonClient: adds keepalive:true for tab-close reliability - server.test.ts: adds session_context_usage to EXPECTED_STAGE1_FEATURES Performance: - useSyncExternalStore selector hoisted via useCallback Feature: - Parallel agents merged display (ParallelAgentsGroup component) Tests: - clientLifecycle.test.ts (9 tests): sessionStorage, keepalive, detach behavior - useDaemonResource.test.tsx (5 tests): stale response race condition coverage - Markdown.test.ts: updated foreignObject/style assertions to expect stripping Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ion review issues Critical fixes: - releaseSession: close session before detaching client to avoid orphaned sessions - ParallelAgentsGroup: forward pendingApproval/onConfirm props so approvals render inside grouped agents - fmtCategoryRow: guard against zero contextWindowSize division Suggestion fixes: - MemoryDialog: await reloadMemory() before showing success message - useInputHistory: keep storageKeyRef in sync with prop changes - App: reset lastRecapBlockCountRef on session switch to prevent auto-recap from silently failing - App: log auto-recap errors instead of silently swallowing them - acpAgent: log collectContextData failures instead of silent catch Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- sanitizeSvg: keep <style> (sanitize @import/external url()) and <foreignObject> so mermaid diagrams render with correct theming and visible text labels - mermaid: skip redundant mermaid.initialize() when theme unchanged - newSession: abort in-flight prompts before resetting store - ParallelAgentsGroup: i18n for hardcoded English strings - vite.config: restore rollupTypes: true for NodeNext compatibility - AskUserQuestion: restore q.question as answer key Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
3dad1a3 to
73e2bad
Compare
ytahdn
left a comment
There was a problem hiding this comment.
增量 review（10 个修复提交，+1923/-269）：前两轮多个问题已修复。以下是新发现的问题。
…ents - Add GET /session/:id/context-usage endpoint (SDK types, acp-bridge, cli route, acpAgent handler with tests) - Refactor webui daemon providers into session/ and workspace/ modules with daemon-react-sdk subpath export - Web-shell dialog UX: replace left back icon with right-side ESC close button, fix keyboard scope so dialogs properly capture keys when input is focused, blur editor when dialog opens - Remove /stats subcommands and model dialog custom model (c key) feature - Remove slash completion auto-submit behavior (align with CLI) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…llel agents display Security fixes: - Mermaid securityLevel reverted to 'strict', strip foreignObject/style from SVG sanitizer - Shift+Tab no longer silently sets yolo mode (only approves current request) - clientLifecycle uses sessionStorage for per-tab client ID isolation Bug fixes: - cancel() finally block guards setPromptStatus with session-ID check - lastRecapBlockCountRef resets on session switch - collectContextData wrapped in try/catch with field stripping - useDaemonResource: request sequence counter prevents stale response overwrite - ResumeDialog: shows error state when session list fails to load - detachDaemonClient: adds keepalive:true for tab-close reliability - server.test.ts: adds session_context_usage to EXPECTED_STAGE1_FEATURES Performance: - useSyncExternalStore selector hoisted via useCallback Feature: - Parallel agents merged display (ParallelAgentsGroup component) Tests: - clientLifecycle.test.ts (9 tests): sessionStorage, keepalive, detach behavior - useDaemonResource.test.tsx (5 tests): stale response race condition coverage - Markdown.test.ts: updated foreignObject/style assertions to expect stripping Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fix AskUserQuestion answer submission and rendering by forwarding answers through acp-bridge permission metadata while keeping arbitrary response fields filtered. Improve the web-shell AskUserQuestion dialog: keep the submit tab in order, preserve custom input values, align cursor position with existing selections when switching tabs, and show selected/custom answers with a consistent underline state. Show ask_user_question tool results without truncating the answer payload.
…ion review issues Critical fixes: - releaseSession: close session before detaching client to avoid orphaned sessions - ParallelAgentsGroup: forward pendingApproval/onConfirm props so approvals render inside grouped agents - fmtCategoryRow: guard against zero contextWindowSize division Suggestion fixes: - MemoryDialog: await reloadMemory() before showing success message - useInputHistory: keep storageKeyRef in sync with prop changes - App: reset lastRecapBlockCountRef on session switch to prevent auto-recap from silently failing - App: log auto-recap errors instead of silently swallowing them - acpAgent: log collectContextData failures instead of silent catch Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ext answer keys - parseContextUsageMessage: add runtime check for usage.totalTokens before casting, prevent white-screen on malformed daemon payload - AskUserQuestion buildResult: use q.question as answer key instead of numeric index, matching downstream consumers that match answers by question text Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- sanitizeSvg: keep <style> (sanitize @import/external url()) and <foreignObject> so mermaid diagrams render with correct theming and visible text labels - mermaid: skip redundant mermaid.initialize() when theme unchanged - newSession: abort in-flight prompts before resetting store - ParallelAgentsGroup: i18n for hardcoded English strings - vite.config: restore rollupTypes: true for NodeNext compatibility - AskUserQuestion: restore q.question as answer key Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…, deduplicate session switch, and add tests - Add suppressErrorRendering to mermaid.initialize() to prevent error SVGs from being injected into the DOM on render failure - Replace silent catch on detachDaemonClient with console.warn for debuggability - Extract startSessionSwitch() helper to deduplicate loadSession/resumeSession - Update sanitizeSvg tests to match current behavior (foreignObject/style preserved) - Add groupParallelAgents unit tests covering grouping, splitting, and edge cases Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix useDaemonFollowupSuggestion import path after DaemonSessionProvider move to session/ - Merge daemon/index.ts exports (keep followup suggestion + add SDK type re-exports) - Restore lastEventId/setLastEventId in test MockSession interface - Remove non-existent DaemonWorkspaceSkillDetail re-export Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…etadata Reject non-string values in the answers payload to prevent malformed data from being forwarded through the permission mediator to the agent.
…, and detach timeout - transcriptToMessages: create standalone tool_group for shell output when previous message is not a tool_group (fixes silent drop of ! command output) - actions: register sendShellCommand in activePromptsRef and manage promptStatus lifecycle (fixes stuck loading after shell command) - actions: wrap detachDaemonClient with withActionTimeout in releaseSession to prevent indefinite hang when daemon is unresponsive - ToolGroup: auto-expand bash/shell/execute_command tool output by default - Add shell output tests for transcriptToMessages
- Differentiate state_resync_required by reason: epoch_reset resets store and replays on same stream; ring_evicted preserves awaitingResync and continues on same stream; other reasons keep original break+reconnect - Clear awaitingResync on replay_complete so post-replay events flow - Set catchingUp when activeSession.lastEventId is present, not only on same-session reconnect (fixes resume catchingUp indicator)
1ed169b to
37aff6c
Compare
…ter rebase - Add getTasks() to DaemonSessionActions interface and implement in actions.ts - Fix App.tsx: actions.getTasks → sessionActions.getTasks (variable renamed during refactor but this callsite was missed during rebase merge)
wenshao
left a comment
There was a problem hiding this comment.
R6 — Incremental Review (67dae3c..37aff6c, 12 new commits)
Phantom Fixes Verification
R5 identified 9 fixes claimed in review replies but absent from HEAD. 8 of 9 are now confirmed present:
| # | Fix | Status |
|---|---|---|
| 1 | fmtCategoryRow NaN% guard | ✅ Fixed (contextCommand.ts:337) |
| 2 | ParallelAgentsGroup approval props | ✅ Fixed (ParallelAgentsGroup.tsx:11-16) |
| 3 | releaseSession close before detach | ✅ Fixed (actions.ts:325-341) |
| 4 | newSession abort in-flight prompts | ✅ Fixed (actions.ts:222-234) |
| 5 | auto-recap rejection logging | ✅ Fixed (App.tsx:600) |
| 6 | collectContextData catch logging | ✅ Fixed (acpAgent.ts:2017) |
| 7 | parseContextUsageMessage validation | ✅ Fixed (ContextUsageMessage.tsx:28-30) |
| 8 | storageKeyRef sync | ✅ Fixed (useInputHistory.ts:33) |
| 9 | buildResult q.question vs String(i) |
String(i) — author confirmed downstream consumers use numeric keys |
Build & Test
- Build: 3/3 packages pass (sdk-typescript, acp-bridge, webui)
- TypeCheck: 0 errors
- Tests: 1498/1500 pass (2 flaky: DaemonSessionProvider ring-evicted timing, acpAgent shutdown timeout — both pre-existing or test-timing issues, not production bugs)
New Findings (non-anchorable in diff — listed here)
[Suggestion] Shell output injected into LLM history without sanitization — packages/cli/src/acp-integration/acpAgent.ts:2561
sessionShellHistory handler injects raw shell command output as role: 'user' via geminiClient.addHistory(). Malicious shell output (e.g., from cat malicious-file.txt) can contain prompt injection instructions that the LLM treats as user directives. Consider using role: 'tool' or wrapping with [SYSTEM: untrusted output] guardrails.
[Suggestion] executeShellCommand catch block hardcodes aborted: false — packages/acp-bridge/src/bridge.ts:3441
The catch block always publishes aborted: false regardless of whether the abort signal fired. Fix: aborted: abort.signal.aborted.
[Suggestion] executeShellCommand output buffer unbounded — packages/acp-bridge/src/bridge.ts:3348
outputChunks accumulates all stdout for up to 120s with no size limit. A verbose command (cat /dev/urandom | base64) can produce GB-level output in memory. The 10K-char truncation only applies to history injection, not the returned ShellCommandResult.output.
[Suggestion] ring_evicted leaves misleading "Reload the session" error message — packages/webui/src/daemon/session/DaemonSessionProvider.tsx
The transcript reducer unconditionally appends "Reload the session to recover" on state_resync_required. For ring_evicted, the Provider auto-recovers without user action, but the error block persists with misleading instructions.
[Suggestion] followupSidechannel not cleared on session switch — packages/webui/src/daemon/session/actions.ts
startSessionSwitch calls store.reset() but not clearSidechannelFollowupSuggestion(). A stale followup suggestion from the prior session may briefly appear in the new session's Editor.
[Suggestion] Shell execution test coverage gaps — 3 locations
executeShellCommand (bridge.ts, ~130 lines), POST /session/:id/shell (server.ts, ~50 lines), and sendShellCommand (actions.ts, ~25 lines) have zero test coverage. Every other POST route in server.ts has corresponding tests.
— qwen3.7-max via Qwen Code /review
releaseSession was incorrectly calling detachDaemonClient with the current client's ID, which only decremented attachCount without actually closing the target session. Replace with session.client.closeSession() (DELETE /session/:id) to properly terminate the session. Also fix sendShellCommand to use a distinct shellKey to avoid colliding with prompt AbortControllers.
…t, shell command Resolve modify/delete conflict on web-shell/client/hooks/useDaemonSession.ts by keeping deletion — web-shell now uses webui DaemonSessionProvider.
…t handling - Add settleActivePromptFromTurnEvent to resolve/reject active prompts from turn_complete/turn_error SSE events in the Provider event loop - Add isPromptLifecycleTurnEvent filter to prevent turn events from being dispatched to the transcript store as unrecognized debug events - Add waitForAcceptedPromptCompletion in actions.ts to bridge the gap between 202-accepted prompts and their eventual turn completion - Extend ActivePrompt type with promptId, resolve/reject callbacks, and pendingResult/pendingError for deferred settlement - Add passive observer handling for turn_complete/turn_error so non-sender tabs correctly end the streaming state - Add tests for non-blocking prompt acceptance and early turn completion
chiga0
left a comment
There was a problem hiding this comment.
Re-review (QoderWork AI) — HEAD d06aa1f4f0
Finding Disposition
| # | Severity | 原始问题 | 状态 |
|---|---|---|---|
| 1 | Major | buildResult key String(i) vs q.question (AskUserQuestion.tsx:98) |
❌ 未修复 — HEAD 仍为 String(i) |
| 2 | Minor | extractPermissionResponseMetadata string validation (bridge.ts:354) |
✅ 已修复 — 9b75458678 添加了 entries.every(([, v]) => typeof v === 'string') |
| 3 | Minor→Nit | lastMermaidTheme module-level cache (Markdown.tsx:166) |
⚪ 未修复，可接受 |
| 4 | Minor→Nit | getModeFromSessionContext loose cast (actions.ts:456) |
⚪ 未修复，可接受 |
New Commits (7 since last review)
所有新提交质量良好，未发现新问题：
9b75458678— permission metadata string validation ✅dd27a4c964— releaseSession 改用 closeSession ✅d06aa1f4f0— non-blocking prompt settlement，race condition 处理正确 ✅- 其余 4 个 fix 提交均为合理的 bug fix / rebase 调整
Remaining Blocker
Finding 1 是唯一 blocker：commit c2e3f1f1de 声称 "restore question-text answer keys"，但 HEAD 代码仍使用 String(i)。如果这是有意的设计变更（用索引替代问题文本作为 key），请更新 commit message 或添加说明；如果是 merge conflict 导致的回退，请恢复为 q.question。
This review was generated by QoderWork AI
chiga0
left a comment
There was a problem hiding this comment.
LGTM. All 4 findings from previous review have been addressed:
buildResultkeyString(i)— confirmed intentional (downstream uses numeric index keys)extractPermissionResponseMetadatastring validation — fixedlastMermaidThememodule-level cache — valid rationale (mermaid is global singleton)getModeFromSessionContextloose cast — valid rationale (cross-version compatibility)
New commits (non-blocking prompt settlement, releaseSession fix, permission validation) are well-implemented.
This review was generated by QoderWork AI
Summary
为 daemon web-shell 添加 context-usage API 完整链路，重构 webui daemon providers 为模块化架构，并改进弹窗交互体验。
本次改动的 Commits
239ceba26核心变更
1. Context-Usage API 完整链路
新增
GET /session/:id/context-usage端点，返回会话的 token 使用分布：packages/sdk-typescript): 新增DaemonSessionContextUsageStatus类型及sessionContextUsage()方法packages/acp-bridge): 新增ServeSessionContextUsageStatus类型、SERVE_STATUS_EXT_METHODS.sessionContextUsage常量、getSessionContextUsageStatus()接口实现packages/cli): 新增路由处理 +acpAgent.buildSessionContextUsageStatus()实现 + 单元测试session_context_usage: { since: 'v1' }2. Webui Daemon Provider 模块化重构
将
packages/webui/src/daemon/拆分为清晰的子模块：daemon-react-sdksubpath export (@qwen-code/webui/daemon-react-sdk)3. Dialog UX 改进
←返回图标，改为右侧ESC按钮data-keyboard-scope机制，弹窗内 input 获焦后仍能响应 Escape/ArrowUp/ArrowDown/stats子命令补全、Model 弹窗c键自定义模型输入4. 其他改进
current_mode_updateevent 类型Validation
文件改动范围