fix(core): preserve rewind parents after resume - #5923
Conversation
|
@qwen-code /triage |
|
Thanks for the PR! Template looks good ✓ On direction: this is a clear bug fix for #5920 — session history disappearing after resume + rewind due to On approach: the fix is minimal and correct. Using each record's persisted Claude Code CHANGELOG has Moving on to code review. 🔍 中文说明感谢贡献!模板完整 ✓ 方向:这是 #5920 的明确 bug 修复——resume 后 rewind 导致 方案:修复最小化且正确。使用每条 record 持久化的 Claude Code CHANGELOG 有 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewMy independent proposal before reading the diff: fix No critical blockers. No AGENTS.md violations. The change is minimal and well-scoped. One note on the old code: the The ACP path addition mirrors TestingBefore (old source, new assertions)(ACP test: After (this PR)All 191 tests pass, zero regressions. 中文说明代码审查独立方案:使用 无关键阻塞项,无 AGENTS.md 违规。旧代码中 测试修复前:两个新测试均失败(parentUuid 为 null 而非预期值,ACP 路径未调用 rebuildTurnBoundaries)。修复后:191 个测试全部通过,无回归。 — Qwen Code · qwen3.7-max |
|
This is a clean, well-reasoned bug fix. The old The ACP path extension is a natural follow-through — Before/after testing confirms: new assertions fail against the old code (parentUuid is Ships the fix cleanly. ✅ 中文说明这是一个干净、合理的 bug 修复。旧的 ACP 路径扩展是自然的补全——CLI resume 已经在调用 前后测试确认:新断言在旧代码下失败,修复后通过,191 个现有测试无回归。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
✅ Local real-runtime verification (maintainer)Built the real What the fix does: Environment: macOS (Darwin), Node/npm workspace, Vitest 3.2.4. Worktree on 1. PR test plan — green
2. Mutation — both halves of the fix are non-vacuous
So the core test pins the persisted-parent behavior, and the two ACP tests pin that both resume entry points rebuild boundaries — exactly the two things the PR adds. 3. Real-service A/B — the actual rewind marker written by
|
| Source | rewind marker written |
|---|---|
| PR (fixed) | {subtype:"rewind", parentUuid:"pre-resume-parent"} ✅ keeps the persisted edge |
| base | {subtype:"rewind", parentUuid:null} ❌ parent edge dropped → chain breaks on the next resume |
That null is the #5920 root cause: the rewind marker detaches from the record that was the real parent of the resumed turn, so a later resume walking the parent chain stops there and loses everything before it. The PR re-anchors the marker to pre-resume-parent, keeping earlier history reachable.
Notes / scope honesty
- This is a recording-tree data-integrity fix. The user-visible symptom (history disappears after a resume + auto rewind/edit) is a downstream consequence of the
nullboundary breaking the next resume's parent-chain walk; the authoritative fix point — the rewind marker'sparentUuid— is verified directly with the real service. A full multi-resume TUI repro isn't deterministically forceable, so verification sits at the real-service level where the bug and fix live. - The fix correctly uses
record.parentUuid ?? null, so a genuinely root record (no parent) still getsnull— only the spuriousnullfrom list-position inference is removed. - Verified on macOS only (matches the PR's Tested on table).
- Merge is gated on
REVIEW_REQUIRED(needs a maintainer approval) — orthogonal to the code.
🇨🇳 中文版(完整对应)
✅ 本地真实运行时验证(维护者)
在本 PR head(6b2e628)构建了真实 core + cli 包,从三个角度验证 rewind 父边修复:PR 完整测试计划、对核心逻辑和 ACP 接线两半都做的非空过变异,以及捕获 ChatRecordingService 真正写出的 rewind marker 的真实服务 A/B。resume 之后 rewind marker 现在会保留持久化的 parent edge 而不是 null,且两个 ACP resume 入口都会重建 boundaries。从验证角度 LGTM。
这个修复做了什么: rebuildTurnBoundaries() 原来从重建列表里的前一项(prevUuid,resume 后从 null 开始)推断每个 user turn 的 boundary —— 所以第一个 resumed user turn 拿到 null 父,rewind marker 被写成 parentUuid: null,下一次 resume 沿 parent chain 回溯时过早停止、丢失更早历史(#5920)。本 PR 改成 push 每条记录持久化的 record.parentUuid,并(在 acpAgent.ts 里)在 ACP resume 路径(loadSession + unstable_resumeSession,VS Code / daemon 客户端用)上调用 rebuildTurnBoundaries() —— 这一步之前只有 CLI 路径做了。
环境: macOS,Node/npm workspace,Vitest 3.2.4。worktree 在 6b2e628,npm ci + 构建。
1. PR 测试计划 — 全绿
| 检查 | 结果 |
|---|---|
core · chatRecordingService.test.ts |
35 / 35 通过 |
cli · acpAgent.test.ts |
156 / 156 通过 |
typecheck(core + cli) |
各 0 错误 |
eslint(4 文件) |
✅ 干净 |
prettier --check(4 文件) |
✅ 干净 |
git diff --check |
✅ 干净 |
2. 变异测试 —— 修复的两半都非空过
| # | 变异 | 结果 |
|---|---|---|
| M1 — core | 把 rebuildTurnBoundaries() 还原成 base(按列表位置推断) |
preserves a resumed user turn parent… 失败:expected null to be 'pre-resume-parent' ✓ |
| M2 — cli | 去掉 acpAgent.ts 的 rebuildTurnBoundaries() 调用 |
两个 ACP resume 测试都失败 —— loadSession 和 unstable_resumeSession,各自 rebuildTurnBoundaries Number of calls: 0 ✓ |
也就是说:核心测试钉住"用持久化父"的行为,两个 ACP 测试钉住"两个 resume 入口都重建 boundaries" —— 正是 PR 新增的两件事。
3. 真实服务 A/B —— ChatRecordingService 真正写出的 rewind marker
用真实 ChatRecordingService 跑 getResumedSessionData() → rebuildTurnBoundaries([user-1{parentUuid:'pre-resume-parent'}, assistant-1]) → rewindRecording(0) → flush(),并 dump 真正写出的 marker:
| 源 | 写出的 rewind marker |
|---|---|
| PR(已修复) | {subtype:"rewind", parentUuid:"pre-resume-parent"} ✅ 保留持久化边 |
| base | {subtype:"rewind", parentUuid:null} ❌ 父边丢失 → 下一次 resume 时链断 |
那个 null 就是 #5920 的根因:rewind marker 与 resumed turn 真正的父记录脱钩,于是后续 resume 沿 parent chain 回溯就停在这里、丢掉之前的一切。本 PR 把 marker 重新锚到 pre-resume-parent,让更早的历史仍可达。
备注 / 范围诚实说明
- 这是 recording-tree 的数据完整性修复。用户可见症状(resume + 自动 rewind/edit 后历史消失)是
nullboundary 破坏下一次 resume 父链回溯的下游后果;而权威修复点 —— rewind marker 的parentUuid—— 已用真实服务直接验证。完整的多次-resume TUI 复现无法确定性触发,所以验证落在 bug/修复所在的真实服务层。 - 修复用的是
record.parentUuid ?? null,所以真正的根记录(无父)仍得null—— 只去掉了按列表位置推断出来的伪null。 - 仅在 macOS 验证(与 PR Tested on 表一致)。
- 合并被
REVIEW_REQUIRED卡住(需维护者 approve)—— 与代码正交。
What this PR does
Preserves persisted parent UUID edges when rebuilding rewind turn boundaries after a session resume. The CLI resume path already rebuilt those boundaries; this PR makes that rebuild use each resumed record's stored
parentUuidinstead of inferring from the adjacent item in the reconstructed list, and applies the same rebuild step to ACP session resume paths used by VS Code and daemon clients.Why it's needed
Issue #5920 reports resumed session history disappearing after an automatic rewind/edit flow. The recorder stores a tree of records, and the rewind marker must be attached to the record that was the parent of the target user turn. If the boundary is rebuilt as
null, the next resume can stop walking the parent chain too early and drop earlier conversation history.Reviewer Test Plan
How to verify
Run the focused tests and confirm that resumed rewind boundaries preserve the stored parent edge, and that both ACP resume entry points rebuild recorder boundaries from resumed conversation records.
Commands run locally:
Evidence (Before & After)
Before:
rebuildTurnBoundaries()inferred the first resumed user turn boundary from local list traversal, so a sliced reconstructed history could lose the persisted parent edge and a later rewind marker could be written with the wrongparentUuid.After:
chatRecordingService.test.tsverifies that a resumed user turn with a stored parent writes the rewind marker under that same parent.acpAgent.test.tsverifies that bothloadSessionandunstable_resumeSessionrebuild recorder boundaries from resumed conversation records.Tested on
Environment (optional)
Local Node/npm workspace on macOS.
Risk & Scope
ChatRecordinstead of deriving a new edge from list position./rewindUI/API model handled by fix(cli): map rewind turns after compression #4242.Linked Issues
Fixes #5920
AI Assistance Disclosure
I used Codex to review the changes, sanity-check the implementation against existing patterns, and help spot potential edge cases.
中文说明
What this PR does
这个 PR 在 session resume 后重建 rewind turn boundaries 时保留持久化记录里的 parent UUID 边。CLI resume 路径之前已经会重建这些 boundaries;这个 PR 让重建逻辑使用每条 resumed record 自己保存的
parentUuid,而不是从 reconstructed list 中相邻元素推断,并且把同样的重建步骤应用到 VS Code 和 daemon clients 使用的 ACP session resume 路径。Why it's needed
#5920 报告了恢复会话后,自动 rewind/edit 流程导致历史消失的问题。recorder 存的是一棵 record tree,rewind marker 必须挂到目标 user turn 原本的 parent record 下。如果 boundary 被重建成
null,下一次 resume 沿 parent chain 回溯时可能过早停止,并丢失更早的对话历史。Reviewer Test Plan
How to verify
运行聚焦测试,确认 resumed rewind boundaries 会保留存储的 parent edge,并确认两个 ACP resume 入口都会从 resumed conversation records 重建 recorder boundaries。
本地运行过的命令:
Evidence (Before & After)
Before:
rebuildTurnBoundaries()会根据本地列表遍历推断第一个 resumed user turn 的 boundary;如果 reconstructed history 是一个被裁剪过的链,就可能丢失持久化的 parent edge,导致后续 rewind marker 写入错误的parentUuid。After:
chatRecordingService.test.ts验证带有已存储 parent 的 resumed user turn 会把 rewind marker 写到同一个 parent 下。acpAgent.test.ts验证loadSession和unstable_resumeSession都会从 resumed conversation records 重建 recorder boundaries。Tested on
Environment (optional)
macOS 上的本地 Node/npm workspace。
Risk & Scope
ChatRecord已经持久化保存的 tree edge,而不是根据列表位置重新推导 edge。/rewindUI/API 模型。Linked Issues
Fixes #5920
AI Assistance Disclosure
我使用 Codex 审查变更、对照现有模式做 sanity check,并帮助发现潜在边界情况。