fix(daemon): Avoid replaying truncated session diffs - #5108
Conversation
|
Thanks for the PR! Template looks good ✓ On direction: this is a well-targeted bug fix for #5106, a known follow-up to #3822. The recording side was fixed in #3822, but the replay/display paths still re-emit truncated session diffs as On approach: the three-layer fix (daemon emitter → skip rawOutput for truncated diffs, web-shell adapter → preserve normalized tool content, web-shell renderer → ignore truncated raw diff fallbacks) covers the full data path cleanly. Scope is tight at 6 files / +239/-16. I don't see a materially simpler path — each layer addresses a distinct gap the issue describes. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 方向:这个 PR 针对的是 #5106 已知问题(#3822 的后续修复)。#3822 修了录制侧,但回放/展示路径仍会把已截断的 session diff 重新放进 rawOutput,导致 web-shell 渲染不完整的 diff 并增大回放负载。与 方案:三层修复(daemon emitter 跳过截断 diff 的 rawOutput、web-shell adapter 保留规范化的 tool content、web-shell renderer 忽略已截断的 raw diff fallback)覆盖了完整的数据路径。6 文件 / +239/-16 的范围很紧凑,没有看到更简单的替代方案——每一层都在解决 issue 中描述的不同缺口。进入代码审查 🔍 — Qwen Code · qwen3.7-max |
|
Qwen Code review did not complete successfully: Qwen review aborted with an API error before posting comments. See workflow logs. |
Code ReviewClean implementation — no blockers found. The three-layer fix is well-structured: Daemon (ToolCallEmitter): Extracts Web-shell adapter (transcriptToMessages): Adds Web-shell renderer (ToolGroup): No correctness bugs, no security issues, no regressions spotted. Types are consistent — Unit TestsAll three focused test suites pass: New assertions cover the key scenarios: rawOutput omitted for truncated diffs, rawOutput present for normal diffs, content preserved in adapter, truncated raw diff fallback ignored in renderer, preview text rendered and expandable for both edit and write tools. Tmux Real-Scenario TestingNot applicable for this PR. The behavior change is internal to the daemon→web-shell session replay data pipeline — there's no CLI command that replays saved sessions or renders web-shell diff views. The original 503 ingress error from #5106 was also not locally reproducible (stated in the PR). CLI smoke test confirms the dev build works with no regression: 中文说明代码审查实现整洁,没有阻塞问题。三层修复结构良好: Daemon (ToolCallEmitter): 提取 Web-shell 适配器 (transcriptToMessages): 新增 Web-shell 渲染器 (ToolGroup): 无正确性 bug、无安全问题、无回归。类型一致—— 单元测试三个聚焦测试套件全部通过:47/47 + 87/87 + 16/16 = 150/150 ✓ Tmux 真实场景测试本 PR 不适用。 行为变更在 daemon→web-shell 会话回放数据管道内部,没有 CLI 命令可以回放已保存会话或渲染 web-shell diff 视图。#5106 的原始 503 错误也未在本地复现(PR 中已说明)。 CLI 冒烟测试确认 dev 构建无回归(见上方终端输出)。 — Qwen Code · qwen3.7-max |
|
Stepping back: this is a well-executed bug fix. The problem is concrete (truncated session diffs leaking through replay → web-shell renders incomplete diffs + payload bloat → eventual 503s on long sessions). The fix is surgical — three small guards at the exact points where the data crosses boundaries (emitter, adapter, renderer). No over-engineering, no speculative features, no unnecessary abstractions. The The My independent proposal would have been similar: guard rawOutput emission on the daemon side and add a truncated-session check in the renderer. The adapter All CI tests pass on macOS, Ubuntu, and Windows. All 150 focused unit tests pass. CLI smoke test is clean. The code is ready to ship. 中文说明回顾整体:这是一个执行良好的 bug 修复。问题具体(截断的 session diff 泄漏到回放管道 → web-shell 渲染不完整的 diff + 负载膨胀 → 长会话最终 503)。修复精准——在数据跨越边界的三个关键点(emitter、adapter、renderer)各加一个小守卫。没有过度工程、没有投机性功能、没有不必要的抽象。 adapter 中的
所有 CI 测试在 macOS、Ubuntu、Windows 上通过。150 个聚焦单元测试全部通过。CLI 冒烟测试无回归。代码可以合并。 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
| return null; | ||
| } | ||
|
|
||
| private isTruncatedSessionDiffDisplay(resultDisplay: unknown): boolean { |
There was a problem hiding this comment.
[Suggestion] The same three-condition check (truncatedForSession === true && 'fileName' in obj && 'newContent' in obj) is duplicated here as isTruncatedSessionDiffDisplay and in ToolGroup.tsx:113 as isTruncatedSessionDiff. They live in separate packages (packages/cli vs packages/web-shell) with different names and share no type or constant.
If the FileDiff shape ever changes (e.g., truncatedForSession is renamed, or a new truncation signal is added), both sites must be updated in lockstep. Silent divergence risk: one side strips rawOutput but the other still renders the diff — the exact bug this PR fixes.
Consider extracting a shared predicate into packages/core (alongside the FileDiff type in tools.ts) and importing it in both consumers.
— qwen3.7-max via Qwen Code /review
| return []; | ||
| }); | ||
|
|
||
| return content.length > 0 ? content : undefined; |
There was a problem hiding this comment.
[Suggestion] normalizeToolContent silently drops entries whose type doesn't match 'content', 'diff', or 'terminal', and also drops malformed entries (null body, missing type string). No console.warn, no log, no metric.
Since block.content arrives over the SSE wire with no compile-time shape guarantee, any daemon regression or MCP server producing non-standard content shapes will cause tool results to vanish from the web-shell with no observable signal — the hardest kind of failure to debug in production.
Consider adding a console.warn for the fallthrough and malformed-entry paths:
if (typeof type === 'string') {
console.warn(`normalizeToolContent: unknown content type '${type}' dropped`);
}
return [];— qwen3.7-max via Qwen Code /review
Local verification — fix is correct & well-tested, but the PR is CONFLICTING and must be rebased before mergeBuilt this PR locally on Linux 6.12 / Node v22.22.2 (isolated worktree at head Verdict: the three-layer fix is sound and the regression tests genuinely pin it — but the branch does not merge cleanly (it's ~2 weeks behind The bug still exists on current
|
| Layer | File | Focused suite | Revert-proof (new test on pre-fix source) |
|---|---|---|---|
daemon emitter — skip rawOutput for truncated diffs |
ToolCallEmitter.ts |
47/47 | should not replay truncated session previews as full diffs → FAILS (pre-fix emits the truncated rawOutput) |
| web-shell adapter — preserve normalized tool content | transcriptToMessages.ts |
86/86 | passes content but not locations or preview to DaemonMessageToolCall → FAILS (pre-fix drops the content) |
| web-shell renderer — ignore truncated raw diff, show preview | ToolGroup.tsx |
7/7 | 3 tests FAIL on pre-fix — e.g. ignores truncated session rawOutput diffs actually rendered the truncated diff (…1-old1+new), and the preview-text cases showed just ✓Edit instead of "Full diff omitted from saved session …" |
So each layer's regression test fails on the unfixed source and passes on the PR — the coverage is load-bearing, not decorative. prettier --check is clean on all six files.
⚠️ Blocker — merge conflict (DIRTY / CONFLICTING)
The branch's merge-base is 2ba4ca90a; every file it touches has since changed on main:
| PR file | Conflicts with | Severity |
|---|---|---|
ToolCallEmitter.ts |
#5085 (Kind.Agent) |
trivial — independent regions |
transcriptToMessages.ts |
#5098 (persist goal status) | likely mechanical — different functions |
ToolGroup.tsx |
#5088 (reveal full tool detail + auto-collapse finished tools) | substantive — #5088 refactored ~105 lines of the same tool-detail/expand logic this PR rewrites |
Good news for the rebase: the functions this PR modifies (hasDiffContent, extractDiff, ExpandedEditDiff, hasExpandableContent) all still exist on current main, so the fix re-applies conceptually — but #5088 changed the surrounding code and the PR also renames ExpandedEditDiff → ExpandedEditContent, so the ToolGroup.tsx portion needs careful manual re-application and a re-run of ToolGroup.test.tsx against #5088's version.
Recommendation
Rebase onto current main, resolve the three conflicts (the ToolGroup/#5088 one needs the most care), then re-run the three suites. With the fix being correct and fully revert-proved, the only open item is the rebase. After that I'd re-verify the renderer layer against #5088.
Notes
- I did not run a full browser E2E: with the renderer layer being reworked on the feat(web-shell): reveal full tool detail and auto-collapse finished tools #5088 rebase, a Playwright run on this stale branch would exercise code that won't ship. The unit revert-proof above is the deterministic before/after evidence for all three layers.
tsc --noEmitonpackages/web-shellreported only environmentalCannot find module '@qwen-code/sdk/daemon'-style errors (siblingwebui/sdkdists not built in my worktree) — none in the PR's changed files; full typecheck needs the built monorepo (CI does this).
🇨🇳 中文版(点击展开)
本地验证 —— 修复正确且测试到位,但本 PR 存在合并冲突,合并前需先 rebase
在 Linux 6.12 / Node v22.22.2 本地构建本 PR(head 5790776f 的隔离 worktree,npm ci),运行了测试计划列出的三个聚焦套件,对三层都做了 revert-proof,并针对当前 main(cf56874f4)分析了合并冲突。
结论:三层修复是正确的,回归测试确实锁定了它 —— 但分支无法干净合并(落后 main 约两周),需要先 rebase。 它修复的 bug 在 main 上仍然存在,所以这份工作并未过时。
bug 在当前 main 上仍存在(所以修复仍然需要)
ToolCallEmitter.ts:174(main)仍然无条件执行update.rawOutput = params.resultDisplay—— 已截断的 session diff 仍被作为rawOutput重新发出。ToolGroup.tsx(main)的hasDiffContent/extractDiff仍无截断守卫地读取raw.fileDiff—— 已截断的保存 diff 仍会被当成完整 diff 渲染。
修复(三层)—— 全绿且已 revert-proof
| 层 | 文件 | 聚焦套件 | revert-proof(新测试在修复前源码上) |
|---|---|---|---|
daemon emitter —— 对截断 diff 跳过 rawOutput |
ToolCallEmitter.ts |
47/47 | should not replay truncated session previews as full diffs → 失败(修复前发出了截断的 rawOutput) |
| web-shell adapter —— 保留规范化的 tool content | transcriptToMessages.ts |
86/86 | passes content but not locations or preview... → 失败(修复前丢弃了 content) |
| web-shell renderer —— 忽略截断 raw diff、显示预览 | ToolGroup.tsx |
7/7 | 3 个测试在修复前失败 —— 例如 ignores truncated session rawOutput diffs 实际渲染出了截断 diff(…1-old1+new),预览文本用例只显示 ✓Edit 而非 "Full diff omitted from saved session …" |
即每层的回归测试在未修复源码上失败、在 PR 上通过 —— 覆盖是有约束力的。六个文件 prettier --check 全部干净。
分支 merge-base 为 2ba4ca90a;它改动的每个文件此后都在 main 上变过:
| PR 文件 | 与之冲突 | 严重度 |
|---|---|---|
ToolCallEmitter.ts |
#5085(Kind.Agent) |
轻微 —— 区域独立 |
transcriptToMessages.ts |
#5098(持久化 goal 状态) | 大概率机械冲突 —— 函数不同 |
ToolGroup.tsx |
#5088(reveal full tool detail + auto-collapse) | 实质冲突 —— #5088 重构了本 PR 重写的同一段 tool 详情/展开逻辑(约 105 行) |
rebase 的好消息:本 PR 修改的函数(hasDiffContent、extractDiff、ExpandedEditDiff、hasExpandableContent)在当前 main 上都还在,所以修复在概念上可重新套用 —— 但 #5088 改动了周边代码,且本 PR 还把 ExpandedEditDiff 改名为 ExpandedEditContent,因此 ToolGroup.tsx 这部分需要仔细手工重套并对 #5088 版本重跑 ToolGroup.test.tsx。
建议
rebase 到当前 main,解决三处冲突(ToolGroup/#5088 那处最需小心),再重跑三个套件。鉴于修复本身正确且已完全 revert-proof,唯一未决项就是 rebase。之后我会针对 #5088 重新验证 renderer 层。
说明
- 我没有跑完整的浏览器 E2E:renderer 层会在 feat(web-shell): reveal full tool detail and auto-collapse finished tools #5088 rebase 时被重做,在这个陈旧分支上跑 Playwright 会验证不会上线的代码。上面的单测 revert-proof 已是三层确定性的前后证据。
packages/web-shell的tsc --noEmit只报了环境性的Cannot find module '@qwen-code/sdk/daemon'类错误(worktree 未构建webui/sdk兄弟包 dist)—— PR 改动的文件里没有错误;完整 typecheck 需要构建好的 monorepo(CI 会做)。
wenshao
left a comment
There was a problem hiding this comment.
No new review findings. Downgraded from Approve to Comment: CI failing: review-pr. — qwen3.7-max via Qwen Code /review
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
5790776 to
c1a1099
Compare
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
🔁 Re-verification after rebase — conflict resolved, fix correctly re-applied; LGTMFollow-up to my first review, where the only blocker was that the branch was CONFLICTING (stale merge-base, Verdict: ✅ LGTM — my prior blocker is resolved. Recommend merge. Conflicts resolved correctly
Key integration check (the #5088 risk): I confirmed the truncation guard covers every raw-diff read on the rebased file — the only Tests + revert-proof (rebased head)
CIThe rebase also turned CI green: Lint ✅, CodeQL ✅, Test macOS ✅, Test ubuntu ✅ (Windows still running at post time) — previously Lint + all three Test jobs were red from the stale-branch conflicts/formatting. Nothing else to flag — the fix targets a real bug (truncated session diffs replayed as 🇨🇳 中文版(点击展开)🔁 rebase 后复核 —— 冲突已解决,修复正确重套;LGTM承接我的首次评审:当时唯一的阻塞项是分支处于 CONFLICTING(merge-base 陈旧, 结论:✅ LGTM —— 我之前的阻塞项已解决。建议合并。 冲突解决正确
关键集成检查(#5088 风险): 我确认截断守卫覆盖了 rebase 后文件里每一处 raw-diff 读取 —— 唯一的 测试 + revert-proof(rebase 后)
CI 没有其他需要指出的 —— 修复针对的是真实 bug(已截断 session diff 被作为 |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
|
Qwen Code review did not complete successfully: Qwen review aborted with an API error before posting comments. See workflow logs. |
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
* fix(daemon): avoid replaying truncated session diffs Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#5108) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
What this PR does
This PR changes saved-session tool replay so truncated edit/write diff displays are treated as preview-only content instead of being replayed again through raw output. The web shell now preserves normalized tool content from daemon transcript blocks, ignores truncated raw diff fallbacks, and shows the omitted-diff preview text when no diff is available.
Why it's needed
Long session histories can contain truncated file diff result displays. Before this change, replay still copied that truncated display into raw output, which let web-shell consumers recover and render the raw diff payload as if it were a full diff and also kept unnecessary payload in replayed session events. Removing that raw replay exposed another gap: web-shell was dropping normal tool content from transcript blocks, so the human-readable preview could disappear. This PR fixes both sides of that path.
Reviewer Test Plan
How to verify
Run the focused tests for daemon replay, web-shell transcript adaptation, and web-shell tool rendering. The daemon test should show truncated session diffs no longer include raw output. The adapter test should show tool content is preserved without converting it into raw output. The renderer test should show truncated raw diff fallbacks are ignored while the preview text remains visible and expandable.
Commands used locally:
cd packages/cli && npx vitest run src/acp-integration/session/emitters/ToolCallEmitter.test.ts;cd packages/web-shell && npx vitest run client/adapters/transcriptToMessages.test.ts;cd packages/web-shell && npx vitest run client/components/messages/ToolGroup.test.tsx;git diff --check;npx prettier --check packages/cli/src/acp-integration/session/emitters/ToolCallEmitter.ts packages/cli/src/acp-integration/session/emitters/ToolCallEmitter.test.ts packages/web-shell/client/adapters/transcriptToMessages.ts packages/web-shell/client/adapters/transcriptToMessages.test.ts packages/web-shell/client/components/messages/ToolGroup.tsx packages/web-shell/client/components/messages/ToolGroup.test.tsx;npm run build;npm run typecheck.Evidence (Before & After)
Before: a saved truncated edit/write diff could still be replayed as raw output, so web-shell fallback rendering could treat
fileDiffas a displayable full diff; if raw output was omitted, web-shell transcript adaptation dropped the content preview. After: truncated session diffs are emitted only as preview content, web-shell keeps that content, ignores truncated raw diff fallbacks, and renders the preview text when no diff is available. The before/after behavior is covered by focused regression tests.Tested on
Environment (optional)
Local repository workspace with npm scripts; validation used package-level Vitest tests plus repository
npm run buildandnpm run typecheck.Risk & Scope
Linked Issues
Fixes #5106
中文说明
What this PR does
这个 PR 调整了已保存会话里的工具回放逻辑:对已截断的 edit/write diff 展示,只作为预览内容回放,不再再次通过 raw output 回放。web shell 现在会保留 daemon transcript 工具块里的规范化 content,忽略已截断的 raw diff fallback,并在没有 diff 可展示时显示“diff 已省略”的预览文本。
Why it's needed
长会话历史里可能包含已截断的文件 diff result display。变更前,回放仍会把这个截断 display 复制进 raw output,web-shell 的 fallback 渲染可能继续把
fileDiff当成完整 diff 展示,同时也让回放事件保留了不必要的负载。移除这部分 raw replay 后又暴露了另一个缺口:web-shell 会丢弃 transcript 工具块里的普通 tool content,导致人可读的预览文本可能消失。这个 PR 同时修复了这条路径的两端。Reviewer Test Plan
How to verify
运行 daemon 回放、web-shell transcript 适配和 web-shell 工具渲染的聚焦测试。daemon 测试应确认已截断 session diff 不再包含 raw output;adapter 测试应确认 tool content 被保留且不会被冒充成 raw output;renderer 测试应确认已截断 raw diff fallback 被忽略,同时预览文本仍可见并可展开。
本地使用的命令:
cd packages/cli && npx vitest run src/acp-integration/session/emitters/ToolCallEmitter.test.ts;cd packages/web-shell && npx vitest run client/adapters/transcriptToMessages.test.ts;cd packages/web-shell && npx vitest run client/components/messages/ToolGroup.test.tsx;git diff --check;npx prettier --check packages/cli/src/acp-integration/session/emitters/ToolCallEmitter.ts packages/cli/src/acp-integration/session/emitters/ToolCallEmitter.test.ts packages/web-shell/client/adapters/transcriptToMessages.ts packages/web-shell/client/adapters/transcriptToMessages.test.ts packages/web-shell/client/components/messages/ToolGroup.tsx packages/web-shell/client/components/messages/ToolGroup.test.tsx;npm run build;npm run typecheck。Evidence (Before & After)
Before:已保存且被截断的 edit/write diff 仍可能作为 raw output 回放,因此 web-shell fallback 渲染会把
fileDiff当成可展示的完整 diff;如果省略 raw output,web-shell transcript 适配又会丢掉 content 预览。After:已截断 session diff 只作为预览 content 发出,web-shell 保留这段 content,忽略已截断的 raw diff fallback,并在没有 diff 时渲染预览文本。这个前后行为已由聚焦回归测试覆盖。Tested on
Environment (optional)
本地仓库 workspace,验证使用 package 级 Vitest 测试以及仓库级
npm run build和npm run typecheck。Risk & Scope
Linked Issues
Fixes #5106