fix(daemon): Handle settings reload events outside transcript - #6407
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the daemon event → UI event normalization path so settings_reloaded is treated as a structured workspace settings refresh signal (rather than an unknown/debug transcript entry), and adds a targeted browser debug log to aid reload diagnostics in WebUI sessions.
Changes:
- Normalize
settings_reloadedintoworkspace.settings.changed(scopeworkspace) in the TypeScript SDK normalizer. - Add WebUI-side debug logging for
settings_reloadedwith sanitized key-name metadata (no values). - Add unit tests to ensure reloads bump the settings signal and do not inject debug transcript blocks.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/webui/src/daemon/session/DaemonSessionProvider.tsx | Logs settings_reloaded metadata via console.debug and ensures normalization happens without transcript noise. |
| packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx | Adds a test asserting settings reloads trigger a settings signal + debug log without creating transcript debug blocks. |
| packages/sdk-typescript/src/daemon/ui/normalizer.ts | Normalizes settings_reloaded into a workspace settings change signal event. |
| packages/sdk-typescript/test/unit/daemonUi.test.ts | Adds coverage verifying settings_reloaded produces a settings refresh signal and no debug UI event. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await renderWithProvider(<Harness />, { autoConnect: true }); | ||
| await act(async () => { | ||
| await flushPromises(); | ||
| }); | ||
|
|
||
| expect(signals?.settingsVersion).toBe(1); | ||
| expect(blocks).not.toContainEqual( | ||
| expect.objectContaining({ | ||
| kind: 'debug', | ||
| text: expect.stringContaining( | ||
| 'settings_reloaded (unrecognized daemon event)', | ||
| ) as string, | ||
| }), | ||
| ); | ||
| expect(debug).toHaveBeenCalledWith( | ||
| '[DaemonSessionProvider] settings reloaded:', | ||
| expect.objectContaining({ | ||
| childReloaded: true, | ||
| changedKeys: ['env', 'hooks'], | ||
| env: { updatedKeys: ['OPENAI_API_KEY'], removedKeys: [] }, | ||
| sessionsRefreshed: ['session-1'], | ||
| sessionsSkipped: [], | ||
| }), | ||
| ); | ||
| debug.mockRestore(); |
|
Thanks for the PR! Template looks good ✓ Problem: observed bug — the daemon emits Direction: aligned. Mapping Size: 69 production logic lines (normalizer +21, provider +48), 82 test lines (sdk +27, webui +55). Not a core-module path ( Approach: scope feels right — two surgical changes that directly address the problem. The logging helpers in the provider ( 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的 bug — daemon 会发出 方向：对齐。将 规模：69 行生产逻辑（normalizer +21，provider +48），82 行测试（sdk +27，webui +55）。不涉及核心模块路径，远低于任何阈值。 方案：范围合理 — 两处精准修改直接解决问题。provider 中的日志辅助函数比一行 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal before reading the diff: I would have added a The implementation is clean and correct:
No critical issues found. No AGENTS.md violations. TestingUnit tests (PR branch in worktree): Focused tests for Build + typecheck: both pass clean. tmux E2E: not applicable for this change. The behavior being fixed — daemon event normalization in the web shell UI — runs in the browser, not the terminal CLI. There's no way to trigger a The relevant verification is the unit test suite, which comprehensively covers the normalizer switch, the transcript filter, and the session provider's event handling. 中文说明代码审查读 diff 前的独立方案：在 normalizer switch 中添加 实现干净且正确：
无关键问题，无 AGENTS.md 违规。 测试单元测试（worktree 中的 PR 分支）：
Build + typecheck： 均通过。 tmux E2E： 不适用。被修复的行为 — web shell UI 中的 daemon 事件归一化 — 运行在浏览器中，而非终端 CLI。无法通过 tmux 中的 CLI 触发 — Qwen Code · qwen3.7-max |
|
This is a clean, focused bug fix. The daemon was emitting The implementation matches what I'd write independently. It follows the existing The one caveat is tmux E2E testing — the change lives in the browser-based web shell, not the terminal CLI, so daemon event normalization can't be exercised from a tmux session. The unit test coverage is thorough though: 252 SDK normalizer tests and 136 provider tests, including focused assertions for the new behavior. Approving. ✅ 中文说明这是一个干净、聚焦的 bug fix。daemon 发出的 实现与我独立方案一致。复用了已有的 唯一的限制是 tmux E2E 测试 — 变更在浏览器端 web shell 中，而非终端 CLI，因此无法通过 tmux 会话触发 daemon 事件归一化。但单元测试覆盖全面：252 个 SDK normalizer 测试和 136 个 provider 测试，包含针对新行为的聚焦断言。 批准合并。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running.
— qwen3.7-max via Qwen Code /review
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
What this PR does
This PR treats daemon settings reload notifications as structured workspace settings refresh signals instead of unknown debug transcript entries, so reload metadata no longer appears as a chat message in web shell sessions. It also emits a concise debug log for reload diagnostics with the changed settings keys, environment key names, child reload status, refreshed or skipped sessions, and reload errors when present.
Why it's needed
When daemon reloads were broadcast to active sessions, the UI layer did not recognize the reload event and rendered the raw event payload into the conversation transcript. That made operational reload noise visible to users and still did not give developers a clean log line for troubleshooting reload behavior.
Reviewer Test Plan
How to verify
Trigger a daemon settings reload while a web shell session is open. The transcript should not show a
settings_reloaded (unrecognized daemon event)system/debug message, settings-dependent UI state should refresh through the workspace settings signal, and browser DevTools should show a[DaemonSessionProvider] settings reloaded:debug entry containing only diagnostic key names and session status fields.Run
cd packages/sdk-typescript && npx vitest run test/unit/daemonUi.test.ts --testNamePattern "settings_reloaded"and expect one focused test to pass. Runcd packages/webui && npx vitest run src/daemon/session/DaemonSessionProvider.test.tsx --testNamePattern "logs settings reloads"and expect one focused test to pass. Runnpm run buildandnpm run typecheckfrom the repository root and expect both commands to exit successfully.Evidence (Before & After)
Before: a daemon settings reload could appear in the web shell transcript as
settings_reloaded (unrecognized daemon event)with the reload payload. After: the same reload is normalized as a workspace settings refresh signal, is omitted from the transcript, and is available as a concise debug log for troubleshooting.Tested on
Environment (optional)
Node.js v22.22.3, npm 10.9.8, local repository build and unit test commands.
Risk & Scope
Linked Issues
N/A
中文说明
What this PR does
这个 PR 将 daemon settings reload 通知处理为结构化的 workspace settings refresh signal，而不是未知 debug transcript 条目，因此 reload 元数据不再作为 web shell 会话中的聊天消息展示。同时，它会为 reload 排障输出一条简洁的 debug log，包含变更的 settings key、环境变量 key 名、child reload 状态、刷新或跳过的 session，以及存在时的 reload 错误。
Why it's needed
daemon reload 广播到活跃 session 后，UI 层之前没有识别 reload 事件，于是会把原始事件 payload 渲染进会话 transcript。这会让用户看到运维 reload 噪声，同时也没有给开发者提供一条清晰的日志来排查 reload 行为。
Reviewer Test Plan
How to verify
在 web shell session 打开时触发 daemon settings reload。transcript 中不应该再出现
settings_reloaded (unrecognized daemon event)system/debug 消息，依赖 settings 的 UI 状态应该通过 workspace settings signal 刷新，并且浏览器 DevTools 中应该出现[DaemonSessionProvider] settings reloaded:debug 记录，其中只包含诊断用的 key 名和 session 状态字段。运行
cd packages/sdk-typescript && npx vitest run test/unit/daemonUi.test.ts --testNamePattern "settings_reloaded"，预期一个 focused test 通过。运行cd packages/webui && npx vitest run src/daemon/session/DaemonSessionProvider.test.tsx --testNamePattern "logs settings reloads"，预期一个 focused test 通过。从仓库根目录运行npm run build和npm run typecheck，预期两个命令都成功退出。Evidence (Before & After)
Before：daemon settings reload 可能作为
settings_reloaded (unrecognized daemon event)连同 reload payload 一起出现在 web shell transcript 中。After：同一个 reload 会被归一化为 workspace settings refresh signal，不再进入 transcript，并且会作为简洁 debug log 供排障使用。Tested on
Environment (optional)
Node.js v22.22.3，npm 10.9.8，本地仓库 build 和 unit test 命令。
Risk & Scope
Linked Issues
N/A