Feat/daemon react cli#4380
Merged
wenshao merged 55 commits intoMay 27, 2026
Merged
Conversation
…aemon-react-cli # Conflicts: # packages/sdk-typescript/src/daemon/ui/normalizer.ts # packages/sdk-typescript/src/daemon/ui/store.ts # packages/sdk-typescript/src/daemon/ui/transcript.ts # packages/sdk-typescript/src/daemon/ui/types.ts # packages/sdk-typescript/test/unit/daemonUi.test.ts # packages/webui/src/daemon/DaemonSessionProvider.test.tsx # packages/webui/src/daemon/DaemonSessionProvider.tsx # packages/webui/src/daemon/transcriptAdapter.test.ts # packages/webui/src/daemon/transcriptAdapter.ts
- Use safeWorkspaceCwd in buildWorkspaceToolsStatus for consistency - Wire loadMcpTools to return SDK tools instead of hardcoded empty array - Consolidate WebShellMcpToolsStatus types (remove duplicate in McpDialog) - Abort active prompts in loadSession before switching sessions - Pass daemon credentials to @-completion source via Editor props Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
wenshao
requested changes
May 26, 2026
wenshao
reviewed
May 26, 2026
wenshao
reviewed
May 26, 2026
Collaborator
🔍 Maintainer Verification Report — PR #4380Reviewer: @wenshao (maintainer) ✅ Build & Type Check
✅ Unit Tests (910 tests, all passed)
✅ E2E Verification (tmux-based, 22 scenarios)Setup: Daemon serve on Daemon Server
Web-shell Dev Server
Vite Proxy & SPA Routing
Session Lifecycle
Workspace APIs
Daemon Log Health
|
| Dimension | Status |
|---|---|
| Build & TypeCheck | ✅ Clean |
| Unit Tests (910) | ✅ All passed |
| E2E Verification (22) | ✅ All passed |
| Daemon Logs | ✅ Clean |
| Breaking Changes | None observed |
Recommendation: ✅ Ready to merge.
The daemon-backed React web-shell integrates cleanly. All daemon APIs, Vite proxy routing, SPA fallback, session lifecycle, and workspace endpoints work as documented. The PR description's validation claims are confirmed.
…icate user message - Remove Session#executePrompt's emitUserMessage() call to eliminate duplicate user_message_chunk events (bridge-echo is the single source) - Move removeDaemonTokenFromUrl() to main.tsx entry point (S19) - Add mount-grace, interaction guard, safe default index to ToolApproval (Critical#1) - Fix stale credential capture in Editor @-completion (Critical#3) - Add submittedRef guard to AskUserQuestion, remove unsafe fallback (S18/S23) - Use .then() pattern for clipboard writeText (S17) - Add i18n for approval dialog and rename messages (S20) - Add session load timeout (S15) - Distinguish MCP error types with DaemonHttpError (S12) - Clear stale heartbeat error on success (S13) - Fix null vs undefined clientId check in server detach (S16) - Add daemon.test.ts for origin validation coverage Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
wenshao
requested changes
May 26, 2026
wenshao
requested changes
May 26, 2026
…quality, ToolApproval stale refs, session load timeout leak - server.ts: change `clientId == null` to `=== null` so absent header falls through to detachClient instead of hanging the request - server.test.ts: add test for detach without X-Qwen-Client-Id header - ToolApproval.tsx: use refs to fix stale closures in handleKeyDown, reset submittedRef on request.id change, sync selectedRef on mouse hover, remove unstable request.options from effect deps - useDaemonSession.ts: store and clear timeout handle in PendingSessionLoad across all resolution paths Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Prevents double-submission on rapid Escape+Enter and avoids sending empty optionId when no reject option exists. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
wenshao
approved these changes
May 26, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
R13 incremental review (504c7ed → 95e68ff): All R12 findings addressed. === null fix correct with test coverage, ToolApproval ref pattern sound, timer leak fully plugged across all 5 exit paths, cancel guard properly implemented. tsc + eslint clean, 275/275 CLI tests pass. No issues found. LGTM! ✅ — qwen3.7-max via Qwen Code /review
wenshao
reviewed
May 26, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Cross-file finding (not in diff)
[Suggestion] AskUserQuestion.tsx keyboard handler (line 177) uses window.addEventListener('keydown') with only a customFocused bail-out — no isEditableTarget(e.target) check. ToolApproval was fixed in this same commit to check isEditableTarget, but AskUserQuestion was not updated to match. The prior reply claimed "Fixed" via useDelayedGlobalKeyDown, but the component does not use that hook — it registers its own raw handler. When AskUserQuestion is visible with customFocused=false, arrow keys, Enter, and Escape are intercepted globally regardless of which element is focused.
// At the top of the handler function (line 182):
if (e.defaultPrevented || isEditableTarget(e.target)) return;
Requires importing isEditableTarget from ../../utils/dom.
| return; | ||
| } | ||
| const clientId = parseClientIdHeader(req, res); | ||
| if (clientId === null) return; |
Collaborator
There was a problem hiding this comment.
[Suggestion] === null reverses the R8 fix (== null, confirmed in 504c7ed81). parseClientIdHeader returns undefined when the header is absent — === null lets undefined pass through to bridge.detachClient(sessionId, undefined), which unconditionally decrements attachCount (bridge.ts:3720). Repeated headerless POST /session/:id/detach requests can drain attachCount to zero, potentially triggering the deferred-reap path when spawnOwnerWantedKill is set.
If headerless detach is intentional, the guard should be in the bridge:
Suggested change
| if (clientId === null) return; | |
| if (clientId == null) return; |
Or, if headerless detach must be allowed, guard attachCount in the bridge: if (clientId !== undefined && entry.attachCount > 0) entry.attachCount--;
— claude-opus-4-7 via Claude Code /qreview
| className={`${styles.option} ${isSelected ? styles.optionActive : ''}`} | ||
| onClick={() => confirm(option.id)} | ||
| onMouseEnter={() => { | ||
| selectedRef.current = i; |
Collaborator
There was a problem hiding this comment.
[Suggestion] onMouseEnter sets selectedRef.current and setSelected(i) but does NOT set interactedRef.current = true. The Enter handler (line 117) requires interactedRef to be true before confirming — after hovering, the first Enter just sets the flag and returns without submitting. User must press Enter twice after mouse-hover. The prior reply (R9) claimed "Fixed" but only addressed selectedRef sync.
Suggested change
| selectedRef.current = i; | |
| onMouseEnter={() => { | |
| interactedRef.current = true; | |
| selectedRef.current = i; | |
| setSelected(i); | |
| }} |
— claude-opus-4-7 via Claude Code /qreview
Collaborator
🔍 Maintainer Verification Report — PR #4380Branch: 1. Build & Static Analysis
2. Unit Tests (858 total)
3. Daemon Serve E2E Test (tmux + web-shell Vite proxy)Started
4. Key PR Features VerifiedNew daemon API endpoints (all working):
acp-bridge improvements:
Web-shell (new package):
SDK (sdk-typescript):
5. Code Quality Review NotesPositive signals:
Pre-existing issues (not introduced by this PR):
6. Summary
Recommendation: ✅ Ready to merge All verification checks pass. The daemon serve API surface is correctly extended with new workspace status endpoints. The idle-cancel fix ( |
Collaborator
🔍 PR #4380 真实测试验证报告概要
静态验证
单元测试 (858 tests)
Daemon API 端点测试 (curl)启动
Web-shell Vite Dev Server 测试
Vite proxy 验证： CLI TUI 回归测试 (tmux)使用 tmux 模拟真实用户操作 CLI TUI（
关键验证点
结论✅ PASS — 推荐 merge
日志产物: |
Collaborator
🌐 Chrome 浏览器真实交互测试验证报告概要
测试架构
测试步骤与结果01 — 页面加载 & 自动创建 Session
02 — Prompt: "say hello in one word"
03 — Session Rename
04 — Prompt with Tool Call: read .nvmrc
05 — Approval Mode Switch
06 — Model Switch
07 — 页面刷新恢复 Session (核心 PR 功能)
Daemon 日志无错误、无崩溃、无异常行为。 关键验证点
结论✅ PASS — Chrome 浏览器真实交互测试通过。Web-shell 的 daemon 连接、session 生命周期、SSE 事件流、SPA 路由刷新恢复均正常工作。 |
chiga0
reviewed
May 26, 2026
| "dist/index.js", | ||
| "dist/types" | ||
| ], | ||
| "scripts": { |
Collaborator
There was a problem hiding this comment.
[P2] Wire web-shell tests into workspace test scripts
This package now adds several *.test.ts(x) files (config/daemon.test.ts, Markdown.test.ts, transcriptAdapter.test.ts, etc.), but package.json only defines dev and build. The root npm run test:ci runs npm run test:ci --workspaces --if-present, so packages without test:ci are silently skipped; these new tests will not run in the normal preflight/CI path. Please add test/test:ci scripts for packages/web-shell (for example vitest run) so the coverage added in this PR is actually exercised.
chiga0
reviewed
May 26, 2026
| ### 安装 | ||
|
|
||
| ```bash | ||
| npm install @alife/dataworks-qwen-code-web-shell |
Collaborator
There was a problem hiding this comment.
[P2] Use the published package name in README examples
package.json declares this workspace as @qwen-code/web-shell, but the README title, install command, and import example use @alife/dataworks-qwen-code-web-shell. Following these instructions would install/import a different package name and fail for consumers of this PR. Please update the README examples to match the actual package name/export.
chiga0
reviewed
May 26, 2026
| @@ -0,0 +1,53 @@ | |||
| { | |||
| "name": "@qwen-code/web-shell", | |||
Collaborator
There was a problem hiding this comment.
[P1] Update package-lock for the new workspace
This PR adds packages/web-shell/package.json, but package-lock.json has no packages/web-shell / @qwen-code/web-shell entry and none of the new web-shell dependencies are locked. A clean install currently fails before tests/build can run: npm ci --dry-run --ignore-scripts reports Missing: @qwen-code/web-shell@0.0.1-preview.0 from lock file plus the new dependency tree. Please run npm install --package-lock-only (or the repo-approved lockfile update flow) and include the lockfile changes so npm ci/preflight works on a fresh checkout.
chiga0
reviewed
May 26, 2026
| const clientId = parseClientIdHeader(req, res); | ||
| if (clientId === null) return; | ||
| try { | ||
| await bridge.detachClient(sessionId, clientId); |
Collaborator
There was a problem hiding this comment.
[P1] Validate detach client membership before decrementing attach state
This route forwards any absent or merely well-formed X-Qwen-Client-Id to bridge.detachClient. In the bridge, detachClient decrements attachCount before unregisterClient checks whether that client id is actually registered, and unregisterClient no-ops for unknown ids. A caller can therefore send repeated detach requests with no header or a forged valid id and drain attachCount, potentially triggering the deferred-reap path for a session they never attached to. Please require a known client id here or make the bridge decrement only after a real client registration was removed.
chiga0
reviewed
May 26, 2026
| disabled={isDisabled} | ||
| commands={commands} | ||
| skills={connection.skills ?? []} | ||
| daemonBaseUrl={DAEMON_BASE_URL} |
Collaborator
There was a problem hiding this comment.
[P2] Pass the embedded baseUrl into @ completion
The earlier fix added createAtCompletionSource({ baseUrl, token }), but this call site still passes only the module-level DAEMON_BASE_URL parsed from the standalone URL. In embedded usage (<WebShell baseUrl="http://127.0.0.1:4170" token="..." />), the main session connects via the baseUrl prop at lines 241-244, while @ file completion receives undefined/URL-derived baseUrl and falls back to the host page origin. The result is silent empty @ completions for library consumers. Please pass baseUrl ?? DAEMON_BASE_URL here so completion targets the same daemon as the session.
chiga0
reviewed
May 26, 2026
| const timer = setTimeout(() => { | ||
| codeToHtml(code, { | ||
| lang: resolvedLang as BundledLanguage, | ||
| theme: 'github-dark-default', |
Collaborator
There was a problem hiding this comment.
[P2] Make rendered Markdown honor the selected theme
WebShellProps exposes theme?: 'dark' | 'light', but rendered Markdown blocks are pinned to dark-only renderers: Mermaid is initialized with theme: 'dark' and Shiki uses github-dark-default. In theme="light" embeds, code blocks and Mermaid diagrams will still render with dark inline colors/backgrounds instead of the selected shell theme. Please thread the active shell theme into Markdown/CodeBlock/MermaidBlock (or use CSS-variable-aware rendering) so light-theme consumers get consistent output.
chiga0
approved these changes
May 26, 2026
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
approved. just notice the package name with @qwen-code/xxx。
doudouOUC
added a commit
that referenced
this pull request
May 27, 2026
Squashed feature work from daemon_mode_b_main branch, rebased onto latest main to establish proper merge-base and clean PR diff. Original commits: - perf(core): F2 cleanup PR A — R9/W11/W12/R10 (post-merge follow-ups) (#4411) - refactor(acp-bridge): F1 test split — lift bridge.test.ts (6861 LOC) to acp-bridge (#4445) - fix(core): F2 cleanup PR B — self-heal observability (W133-a + W134) (#4460) - feat(sdk/daemon-ui): unified completeness follow-up to #4328 (#4353) - docs(serve): v0.16-alpha known limits + SDK QWEN_SERVER_TOKEN env fallback (PR 27) (#4473) - docs(deploy): local launch templates for v0.16-alpha (PR 30a) (#4483) - feat(daemon+sdk): cross-client real-time sync completeness (#4484) - feat(serve): add POST /session/:id/recap (#4504) - feat(daemon): add voterClientId to permission_resolved (A4) (#4539) - feat(serve): --allow-origin <pattern> CORS allowlist (T2.4 #4514) (#4527) - feat(daemon): in-session model switch reaches the bus (A1) (#4546) - feat(serve): prompt absolute deadline + SSE writer idle timeout (#4514 T2.9) (#4530) - Feat/daemon react cli (#4380)
doudouOUC
pushed a commit
that referenced
this pull request
May 27, 2026
* feat(daemon): add shared UI transcript layer * fix(daemon): address ui review feedback * test(daemon): cover raw event diagnostics option * fix(daemon): address latest ui review * fix(daemon): cover reconnect and status edge cases * fix(daemon): guard prompt busy cleanup * feat(daemon): add shared UI transcript layer * fix(daemon): address ui review feedback * test(daemon): cover raw event diagnostics option * fix(daemon): address latest ui review * fix(daemon): cover reconnect and status edge cases * fix(daemon): guard prompt busy cleanup * fix(daemon): handle trimmed tool updates * fix(daemon): cap transcript text blocks * fix(daemon): dedupe trimmed tool diagnostics * fix(daemon): harden webui transcript edge cases * fix(daemon): preserve webui daemon events * fix(daemon): address latest ui review comments * feat(web-shell): add daemon-backed UI shell * feat(web-shell): improve session routing and slash commands * feat(daemon): add shared UI transcript layer * fix(daemon): address ui review feedback * test(daemon): cover raw event diagnostics option * fix(daemon): address latest ui review * fix(daemon): cover reconnect and status edge cases * fix(daemon): guard prompt busy cleanup * fix(daemon): handle trimmed tool updates * fix(daemon): cap transcript text blocks * fix(daemon): dedupe trimmed tool diagnostics * fix(daemon): harden webui transcript edge cases * fix(daemon): preserve webui daemon events * fix(daemon): address latest ui review comments * fix(daemon): close latest ui review nits * fix(daemon): harden ui review edges * fix(daemon-ui): address wenshao 2 Critical findings (#4328 review) ## Critical #1 — 401/403 reconnect storm + transcript wipe `DaemonSessionProvider`'s reconnect loop kept retrying `createOrAttach` on 401/403 even with `autoReconnect: true`. Each cycle: - hit the daemon with the same bad token → 401 again - cleared the session handle - the next successful attempt (if token magically recovered) would receive a different sessionId, triggering the `store.reset()` branch at line 143 and wiping the user's transcript - no terminal "auth failed" state surfaced to the user Fix: split `TERMINAL_SESSION_HTTP_STATUSES` into `AUTH_FAILURE_HTTP_STATUSES` (401, 403) and the rest (404, 410). On auth failure, return from the reconnect loop unconditionally regardless of the `autoReconnect` flag — these are credential failures, not transient. The user must update credentials; daemon spam must stop. `extractHttpStatus` helper factored out of `isTerminalSessionHttpError` to share between the two predicates. ## Critical #2 — rawInput / rawOutput leaking secrets to UI `normalizer.normalizeToolUpdate` forwarded `rawInput` / `rawOutput` verbatim onto `DaemonUiToolUpdateEvent` → `DaemonToolTranscriptBlock`. The `details` projection was redacted via `stringifyRedactedJson` / `redactSensitiveFields`, but the underlying `rawInput` / `rawOutput` fields were unredacted. Any UI component that read those fields directly (ShellToolCall, WriteToolCall, JSON debug panels) leaked the raw values to the DOM. Example: `{ command: 'curl', apiKey: 'sk-prod-...' }` had `apiKey` redacted in `details` but exposed verbatim on `rawInput`. Fix: apply `redactSensitiveFields` to both `rawInput` and `rawOutput` ONCE at the normalizer boundary, then reuse the redacted shape for the `details` projection. Downstream is uniformly safe; no double traversal. ## Tests (49/49 pass) - SDK `daemonUi.test.ts` (36 tests, +1) — new test `redacts sensitive fields in tool.update rawInput and rawOutput at normalizer boundary` verifies full-event string scan finds zero secret values + structural keys preserved with values `'[redacted]'`. - WebUI `DaemonSessionProvider.test.tsx` (13 tests, +2) — new tests `breaks out of the reconnect loop on 401 / 403 auth failures even when autoReconnect is true` and `still reconnects on 404 / 410 session-not-found errors when autoReconnect is true` lock in the asymmetry: auth failure → 1 attempt only; session-not-found → retries until success. ## Out of scope (declined / deferred — see PR review reply) - CRIT #3 `withActionTimeout` test coverage gap → behavior correct, test-only follow-up (avoids PR bloat) - Suggestions #4-7 → 4 nice-to-haves, deferred to keep PR focused on production-correctness fixes Generated with AI Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> * fix(daemon-ui): redact tool details in web transcript * feat(web-shell): align daemon UI interactions * fix(web-shell): address daemon UI review comments * feat(web-shell): sync independent web-shell with lib build, i18n, and daemon serve enhancements Bring in the independently developed web-shell package with full lib build support (vite.lib.config.ts, tsconfig.lib.json), i18n layer, new dialogs (Help, Theme, ReleaseSession), composer hiding during approvals, and SDK dependency restructured as peerDependency. Also adds daemon serve routes (detach endpoint, rename persistence) and fixes acp-bridge testUtils missing cancelImpl. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(web-shell): address daemon UI review comments - Strip token from URL after caching (prevents Referer/history leak) - Add URL scheme allowlist for markdown links/images (block javascript:) - Add CORS restriction in vite dev server - Handle state_resync_required event (reset store) - Reset promptStatus on SSE disconnect - Handle 401/403 in reconnect loop (no retry on auth failures) - Heartbeat consecutive failure detection (3 strikes → disconnect) - Strip <style> tags in SVG sanitization - Replace naive diff with LCS-based buildUnifiedDiff - Fix inputHighlight decoration ordering (sort before add) - Add isEditableTarget guard in useDelayedGlobalKeyDown - Fix AskUserQuestion keyboard handler (no capture phase) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(web-shell): address second-round review Critical issues - Add size guard to buildUnifiedDiff (fallback when n*m > 250k) - Strip SVG animation elements (animate, set, animateTransform, animateMotion) - Reset promptStatus to idle on state_resync_required - Restrict getAllowedDaemonOrigin to same port as page origin Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(web-shell): address remaining PR #4380 review issues - SVG sanitizer: strip style/use/image/feImage/mpath, block external hrefs - Markdown: split isSafeHref/isSafeImageSrc (allow data:image for img only) - Heartbeat: fire disconnect once at 3 failures, self-heal on success - state_resync_required: reset store and reconnect (remove dead code) - Auth 401/403: log error, stop reconnect loop, show error state - replaceSessionUrl: delete ?token param to prevent leak - removeDaemonTokenFromUrl() called at module init - Vite dev server: cors: false - killSession: forgetSession before byId.delete (prevent lost events) - inputHighlight: collect ranges and sort before adding to builder - useDelayedGlobalKeyDown: isEditableTarget guard from shared utils - buildUnifiedDiff: proper O(nm) LCS, hasDiffContent lightweight check - detachDaemonClient: restore console.warn for observability - App.tsx: use rAF-coalesced messageBlocks in extractPendingPermission - extractPendingPermission: extract toolCallId from toolCall record - vite.lib.config: wrap CSS injection in try/catch for CSP - Add test coverage: server routes, SDK methods, transcriptAdapter Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(web-shell): address third-round PR #4380 review issues Critical fixes: - ToolApproval: reset submittedRef via useEffect on request.id change - Effect cleanup: reject pendingSessionLoadRef on dispose - sanitizeSvg: strip style attributes with external url() values Suggestion fixes: - <use> elements: keep fragment-only href, strip external (+ xlink:href fallback) - SAFE_IMAGE_DATA_URI: remove svg+xml (can load external subresources) - extractStreamingState: accept blocks directly, remove state dependency - coalescedState useMemo removed — rAF coalescing no longer defeated - Auth failure log: use missingSessionId instead of already-cleared vars - newSession(): reject pending loadSession promise - COPY_MESSAGES: wire constants to copyFromLastAssistantMessage - Add 39 tests for isSafeHref, isSafeImageSrc, sanitizeSvg - Add 3 tests for toolCallId extraction fallback - Fix test fixtures: resolved: undefined, clientReceivedAt: 1 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(web-shell): delegate readWorkspaceFile to SDK client Replaces the manual fetch() call with session.client.readWorkspaceFile() which provides fetchWithTimeout (30s default) and error normalization. Ensures DaemonClient baseUrl is always absolute by falling back to window.location.origin in proxy mode. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(web-shell): address fourth-round PR #4380 review issues - Fix suppressedOwnUserEchoCountRef not decrementing on prompt failure - Add heartbeat status guard to prevent overwriting 'connecting' state - Abort stale activePrompts when SSE session disconnects - Truncate displayName to 256 chars in renameSession endpoint - Fix DiffView counting +++ / --- header lines as additions/deletions - Preserve existing command properties in mergeCommands - Fix bridge cwd override by params spread order - Validate all href attributes on SVG <use> elements - Extend external url() check to all SVG attributes, not just style - Unify detachDaemonClient baseUrl with DaemonClient construction - Delegate loadMcpTools to SDK client instead of returning stub - Add createAtCompletionSource factory with baseUrl/token fallback - Reset AskUserQuestion state on request.id change - Add useEffect cleanup for queue drain setTimeout - Suppress replay_complete from reaching UI as unrecognized event Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(web-shell): address fifth-round PR #4380 review issues - Use safeWorkspaceCwd in buildWorkspaceToolsStatus for consistency - Wire loadMcpTools to return SDK tools instead of hardcoded empty array - Consolidate WebShellMcpToolsStatus types (remove duplicate in McpDialog) - Abort active prompts in loadSession before switching sessions - Pass daemon credentials to @-completion source via Editor props Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(web-shell,cli): address PR #4380 review issues and fix duplicate user message - Remove Session#executePrompt's emitUserMessage() call to eliminate duplicate user_message_chunk events (bridge-echo is the single source) - Move removeDaemonTokenFromUrl() to main.tsx entry point (S19) - Add mount-grace, interaction guard, safe default index to ToolApproval (Critical#1) - Fix stale credential capture in Editor @-completion (Critical#3) - Add submittedRef guard to AskUserQuestion, remove unsafe fallback (S18/S23) - Use .then() pattern for clipboard writeText (S17) - Add i18n for approval dialog and rename messages (S20) - Add session load timeout (S15) - Distinguish MCP error types with DaemonHttpError (S12) - Clear stale heartbeat error on success (S13) - Fix null vs undefined clientId check in server detach (S16) - Add daemon.test.ts for origin validation coverage Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(web-shell,cli): address PR #4380 R9 review — detach loose equality, ToolApproval stale refs, session load timeout leak - server.ts: change `clientId == null` to `=== null` so absent header falls through to detachClient instead of hanging the request - server.test.ts: add test for detach without X-Qwen-Client-Id header - ToolApproval.tsx: use refs to fix stale closures in handleKeyDown, reset submittedRef on request.id change, sync selectedRef on mouse hover, remove unstable request.options from effect deps - useDaemonSession.ts: store and clear timeout handle in PendingSessionLoad across all resolution paths Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(web-shell): add submittedRef guard to AskUserQuestion handleCancel Prevents double-submission on rapid Escape+Enter and avoids sending empty optionId when no reject option exists. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> Co-authored-by: ytahdn <ytahdn@gmail.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
What changed:
packages/web-shell./model --fast,/rename --auto,/new,/reset, and session URL restore via/session/:id./session/:idworks as a SPA route while daemon API calls still proxy correctly.Why it changed:
Reviewer focus:
/session/:idrefresh.Validation
Commands run:
Prompts / inputs used:
/model/model --fast/model --fast <model>/rename <name>/rename --auto/new/reset/resume/session/:id?token=<token>Expected result:
/session/:idshould reload that daemon session instead of hitting the daemon API route as a page request.Observed result:
Quickest reviewer verification path:
?token=<token>./session/:id./model --fast,/rename --auto,/new, and/resume.Evidence:
npm run buildcompleted with✓ built.npx tsc --noEmitexited successfully.eslintexited successfully.Scope / Risk
Main risk or tradeoff:
/session/:idas a SPA route while daemon APIs also live under/session. The Vite dev proxy explicitly bypasses HTML navigations to avoid routing page refreshes to daemon API endpoints.Not covered / not validated:
Breaking changes / migration notes:
Testing Matrix
Testing matrix notes:
Linked Issues / Bugs