feat: fork from any conversation - #8274
Conversation
github-actions
Bot
added
the
review/self-reported
water-in-stone
changed the title
|
Thanks for this — it's a substantial piece of work and the design doc is genuinely helpful. Template: complete ✓ — all sections present, with a real reviewer test plan and before/after evidence. Problem: this is a feature request (#8271), self-reported and on the Direction: aligned. Conversation forking is an active area in the reference product (claude-code ships Size: this is the thing a maintainer needs to see. ~2,099 production-logic lines (additions+deletions, excluding ~1,796 test lines and the ~791-line design doc) across 48 files and 6 packages, with the weight in core session infrastructure ( Approach: the scope feels earned rather than padded — each layer maps to a stated requirement (durable checkpoint, crash-safe atomic creation, stale-checkpoint rejection, UI surface). I don't see a materially simpler path that still meets the "never expose a partially created session" constraint. Implementation-level observations are in the next comment. Risk: elevated. The diff touches Moving on to code review. 🔍 中文说明感谢贡献，工作量很大，设计文档也很有帮助。 模板： 完整 ✓ —— 各节齐全，有真实的 reviewer 测试计划和 before/after 证据。 问题： 这是一个 feature request（#8271），由作者自己提出，挂在 方向： 对齐。会话 fork 在参考产品里是活跃方向（claude-code 已有 规模： 这是维护者需要看到的重点。约 2,099 行生产逻辑（增+删，不含约 1,796 行测试和 791 行设计文档），跨 48 个文件、6 个 package，重量集中在核心会话基础设施（仅 方案： 范围是"挣来的"而非堆砌——每一层都对应一个明确需求（持久化 checkpoint、崩溃安全的原子创建、过期 checkpoint 拒绝、UI 层面）。在满足"绝不暴露部分创建的会话"这一约束的前提下，我没有看到明显更简的路径。实现层面的观察见下一条评论。 风险： 升级。diff 触及 进入代码审查。🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code reviewI wrote my independent proposal first (a versioned checkpoint record appended to the transcript, validated against the reconstructed active chain, with branch creation that truncates the raw JSONL at the checkpoint and refuses stale points), then read the diff. The PR matches or exceeds it — and goes further than I would have on crash safety. The core design is sound. Two pieces are notably careful:
The serve route validates No critical blockers found in static review. Two non-blocking notes:
sequenceDiagram
participant P1 as Web Shell UI
participant P2 as Serve route
participant P3 as Bridge and Agent
participant P4 as SessionService
participant P5 as Filesystem
P1->>P2: POST branch with atRecordId
P2->>P3: branchSession atRecordId
P3->>P3: serialize via history mutex
P3->>P4: forkSession atRecordId
P4->>P4: resolveBranchPoints then validate
P4->>P5: claim then stage transcript and backups
P4->>P5: atomic link publish
P4-->>P3: new session path
P3-->>P2: attached session
P2-->>P1: 201 created
Files changed (28 of 48 shown)
Test evidence (PR's own CI)CI on the reviewed commit is still running — no failures so far, but the load-bearing suites have not completed. I fetched this once via the API and did not poll; the finalize job updates the table below when CI settles. The macOS/Windows test jobs are Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Sandboxed verification would settle the behavioural core here: 中文说明代码审查我先独立写下自己的方案（一个带版本的 checkpoint 记录追加到 transcript，针对重建出的活跃链做校验，分支创建时在 checkpoint 处截断原始 JSONL 并拒绝过期点），然后再读 diff。PR 达到甚至超过了这个方案——而且在崩溃安全方面做得比我预期的更多。 核心设计是合理的。 有两处特别用心：
serve 路由校验 静态审查未发现关键阻塞项。两条非阻塞提示：
（时序图见上方英文部分：Web Shell UI → serve 路由 → bridge/agent（经历史互斥锁串行化）→ SessionService（resolveBranchPoints 校验 → claim/暂存 → 原子链接发布）→ 文件系统。） 测试证据（PR 自身的 CI）所审提交上的 CI 仍在运行——目前没有失败，但关键的几个套件还没跑完。我通过 API 只取了一次，没有轮询；CI 结束后 finalize 任务会更新上方表格。macOS/Windows 测试任务为 沙盒化验证可以落实这里的行为性核心： — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 3/5 — clean review and genuinely strong engineering, but this is a large Stepping back: this is better work than most of what lands in the queue. My independent proposal was a fraction of this — a versioned checkpoint plus truncation at the branch point — and the PR goes well past it on the two things that are actually hard here, concurrency and crash safety. The topology fence, the per-session history mutex (with a test that pins the exact race), and the claim/stage/atomic-publish/GC dance in The reason I'm not approving isn't a defect — it's scope and trust boundary. ~2,099 production lines across six packages, with the weight in Two things I'd want settled before merge, neither blocking the review:
⏸️ Deferring to @wenshao @tanzhenxin @yiliang114 @LaZzyMan (core CODEOWNERS) — no blocking defects found; this needs a maintainer call on whether the scope and the durable-checkpoint contract are what we want, plus green CI and ideally a 中文说明置信度：3/5 —— 审查干净、工程质量确实很高，但这是一个触及核心会话基础设施的大型 退一步看：这比队列里大多数 PR 都要好。我独立的方案只是它的一小部分——一个带版本的 checkpoint 加上在分支点截断——而 PR 在真正难的两点上远超我的方案：并发与崩溃安全。拓扑栅栏、按会话的历史互斥锁（附带一个精确钉住该竞态的测试）、以及 我不批准的原因不是缺陷——而是范围与信任边界。约 2,099 行生产代码跨六个 package，重量集中在 合并前我希望落实两件事，都不阻塞审查：
⏸️ 转交 @wenshao @tanzhenxin @yiliang114 @LaZzyMan（core CODEOWNERS）—— 未发现阻塞性缺陷；需要维护者就范围与持久化 checkpoint 契约是否符合预期做出决定，并在合并前等 CI 转绿、最好再跑一次 — Qwen Code · qwen3.8-max-preview Reviewed at |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / after
Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 4 scenario(s). — Qwen Code · serve A/B |
Review —
|
github-actions
Bot
removed
the
review/self-reported
|
Thanks @wenshao for the thorough review. I evaluated every finding and pushed the accepted fixes in Addressed:
Intentionally not changed / follow-up needed:
Validation after the fixes:
The post-commit persisted-session behavior remains deliberate: once Core publishes a complete branch, transport cleanup releases only live ownership and the session stays recoverable in the picker. |
|
@qwen-code /verify |
|
Sandboxed verification: Skipped because the PR has merge conflicts, so refs/pull/8274/merge is unavailable — resolve conflicts and re-run. 中文 — 判定：
|
Add durable response checkpoints so Web Shell sessions can branch from eligible completed Assistant turns without mutating the source history. - Record and validate checkpoints behind serialized topology fences - Preserve historical anchors through replay, daemon, SDK, and UI layers - Publish bounded forks with crash-safe ownership and referenced backups - Serialize prompt, rewind, branch, automatic turn, and close mutations - Cover stale anchors, replay pagination, cleanup, and pending UI states Note: Responses recorded before this change remain non-branchable. # Conflicts: # packages/acp-bridge/src/bridge.ts # packages/acp-bridge/src/bridgeTypes.ts # packages/cli/src/acp-integration/acpAgent.test.ts # packages/cli/src/acp-integration/acpAgent.ts # packages/cli/src/serve/routes/session.ts # packages/cli/src/serve/server.test.ts # packages/core/src/services/chatRecordingService.ts # packages/core/src/services/sessionService.test.ts # packages/core/src/services/sessionService.ts # packages/sdk-typescript/src/daemon/DaemonClient.ts # packages/web-shell/client/components/MessageItem.tsx # packages/web-shell/client/components/MessageList.tsx
Keep Assistant-response branching intact across the daemon stack after rebases, including history serialization and persisted-session ownership. - Forward durable checkpoint IDs through Bridge, SDK, and UI layers - Serialize live history mutations and retain valid nested branch anchors - Preserve persisted branches during generation cleanup - Add cross-layer regression tests for replay and stale checkpoints
a4c31c9 to
870af37
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
Keep the PR review report as a local ignored backup instead of shipping it with the feature branch. - Remove the generated PR comment evaluation from tracked files - Preserve the report under the ignored analyze directory
|
@yiliang114 @wenshao Please take a look |
OverviewThis PR adds durable
Test coverage is genuinely good: cross-layer contract tests, serialization ordering tests, crash/cleanup ownership tests in The concerns below are mostly about cost and one UX regression, not about the core design. Issues1. Branch button is now shown during an active turn — 30 s client timeout (
|
Historical branch requests could outlive the client timeout during an active turn, and interactive forks lacked the recorder's cross-process writer-lease barrier. - Hide Assistant Branch actions while a turn is active - Run interactive fork creation inside the recorder write barrier - Use the concrete checkpoint recorder contract in Session - Document committed-session ownership and implemented design status
|
Thanks for the detailed follow-up. I re-evaluated every point against the current implementation and pushed the accepted fixes in Fixed in this revision:
The three performance concerns are valid, but I am deferring them to measured follow-ups rather than making partial changes in this already broad correctness PR:
For the remaining smaller notes: the process-lifetime GC map has no demonstrated practical growth path; duplicate validation occurs before checkpoint collection so returning Validation completed locally:
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI and did not run locally.
Not reviewed: reverse audit — stopped before round 4 by the review time budget.
中文说明
未审查：build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI and did not run locally。
未审查：反向审计——评审时间预算不足，未能开始第 4 轮。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| response = (await Promise.race([ | ||
| withTimeout( | ||
| entry.connection.extMethod( | ||
| SERVE_CONTROL_EXT_METHODS.sessionRewind, |
There was a problem hiding this comment.
[Critical] R12-1: A dispatched rewind is still wrapped in withTimeout(initTimeoutMs) — a race-only Promise.race that cannot cancel the dispatched extMethod — while this same PR removed that wrapper for branches because "a caller never sees a timeout followed by an unobserved committed session". The queue tail (entry.promptQueue = rewindResult.then(() => undefined, () => undefined)) also swallows the timeout rejection and advances immediately. — Failure scenario: a rewind whose agent-side execution exceeds initTimeoutMs (10s default; file rewinds restore many files by default) → the caller receives BridgeTimeoutError while the rewind commits history truncation + file restores unobserved, and the next queued branch/rewind is dispatched to an agent still mid-rewind — probe-verified on this commit (a queued branch was dispatched during an in-flight rewind; awaiting the real outcome flipped the ordering). Pre-PR had the timeout but no queue, so this overlap is created by this diff.
Suggested fix — treat a dispatched rewind like a dispatched branch:
response = (await Promise.race([
entry.connection.extMethod(
SERVE_CONTROL_EXT_METHODS.sessionRewind,
{ sessionId, promptId: req.promptId, rewindFiles: req.rewindFiles !== false },
),
getTransportClosedReject(entry),
])) as Record<string, unknown>;and add a rewind analogue of waits for a dispatched branch instead of timing out and committing later.
中文说明
[Critical] R12-1：已派发的 rewind 仍然包在 withTimeout(initTimeoutMs) 里 —— 这是一个只能竞速、无法取消已派发 extMethod 的纯 Promise.race —— 而本 PR 对 branch 恰恰移除了这个包装，理由是"调用方永远不会先看到超时、随后又出现一个未被观察到的已提交会话"。队列尾部（entry.promptQueue = rewindResult.then(() => undefined, () => undefined)）还会吞掉超时拒绝并立即前进。— 失败场景：agent 侧执行超过 initTimeoutMs（默认 10 秒；文件回退默认恢复大量文件）的 rewind → 调用方收到 BridgeTimeoutError，而 rewind 在未被观察的情况下提交了历史截断 + 文件恢复，并且排队的下一个 branch/rewind 会被派发到仍在执行 rewind 的 agent —— 已在本 commit 上用探针验证（排队的 branch 在 rewind 执行期间被派发；改为等待真实结果后顺序翻转）。此 diff 之前有超时但没有队列，因此该重叠是本 diff 新引入的。
建议修复：像对待已派发的 branch 一样对待已派发的 rewind —— 直接等待真实结果（保留 getTransportClosedReject(entry) 作为竞速对手），并补充一个类似 "waits for a dispatched branch instead of timing out" 的 rewind 版回归测试。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| // ACP cannot cancel a branch after dispatch. Keep the queue and | ||
| // reservation until its real outcome is known so a caller never sees | ||
| // a timeout followed by an unobserved committed session. | ||
| const result = (await mutation) as { |
There was a problem hiding this comment.
[Critical] R12-2: The branch/side-task mutation awaits entry.connection.extMethod(...) with neither the withTimeout this diff removed nor a getTransportClosedReject(entry) race, but the ACP SDK never rejects pending requests when the transport dies — every other extMethod call site in this file races getTransportClosedReject for exactly this hazard. — Failure scenario: the channel's child process dies mid-branch (OOM/segfault — branching a large session is memory/IO-heavy): the pending extMethod promise never settles, so the caller's branchSession hangs forever (the SDK fetch timeout is disabled for this route, see the sibling finding on DaemonClient.ts), and finally { releaseAdmissionOnce(); } never runs — permanently leaking the reservation's inFlight slot in createTotalSessionAdmissionController; repeated channel deaths exhaust the daemon-wide session budget (TotalSessionLimitExceededError for all new sessions). Probe-verified: crash(SIGKILL) → branch still pending 300ms later, 0 admission releases; adding Promise.race([mutation, getTransportClosedReject(entry)]) flips both.
| const result = (await mutation) as { | |
| const result = (await Promise.race([ | |
| mutation, | |
| getTransportClosedReject(entry), | |
| ])) as { |
The race fires only when the channel exits (a branch whose channel died cannot be observed or delivered anyway), never on a slow-but-alive fork, so the "unobserved committed session" rationale for removing the timeout is unaffected.
中文说明
[Critical] R12-2：branch/side-task 变更直接 await entry.connection.extMethod(...)，既没有本 diff 移除的 withTimeout，也没有与 getTransportClosedReject(entry) 竞速 —— 但 ACP SDK 在传输层死亡时从不拒绝挂起的请求；本文件中其他所有 extMethod 调用点都与 getTransportClosedReject 竞速，正是为了防这一风险。— 失败场景：branch 执行期间 channel 子进程死亡（OOM/段错误 —— 大会话分支正是内存/IO 密集操作）：挂起的 extMethod promise 永不落定，调用方的 branchSession 永久挂起（该路由的 SDK fetch 超时已被禁用，见 DaemonClient.ts 上的同族发现），且 finally { releaseAdmissionOnce(); } 永不执行 —— 永久泄漏 createTotalSessionAdmissionController 中预订的 inFlight 槽位；channel 反复死亡会耗尽 daemon 级会话配额（所有新会话报 TotalSessionLimitExceededError）。已用探针验证：crash(SIGKILL) → 300ms 后 branch 仍挂起、0 次 admission 释放；加上 Promise.race([mutation, getTransportClosedReject(entry)]) 后两者均翻转。
该竞速只在 channel 退出时触发（channel 已死的 branch 本来也无法被观察或交付），对缓慢但存活的 fork 从不触发，因此移除超时的"避免未观察到的已提交会话"理由不受影响。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| const realOpen = fs.promises.open; | ||
| vi.spyOn(fs.promises, 'open').mockImplementation(async (...args) => { | ||
| if (String(args[0]).includes(`${realPath.sep}.branch-staging`)) { |
There was a problem hiding this comment.
[Suggestion] R12-3: The rewritten failure injection in removes a partially written target when fork creation fails now throws at the staging-file open — before any byte is written — so the test no longer sets up the scenario its name describes, and its sole assertion (expect(fs.existsSync(targetPath)).toBe(false)) is trivially true. — Concrete cost: the old injection failed mid-write (after fs.writeSync(file, 'partial')); the new one throws for any .branch-staging open, which happens before transcriptStagingCreated is set and before anything exists on disk, so the assertion holds for ANY pre-link error regardless of cleanup — deleting all failure-path cleanup from forkSession would still pass (only claim removal is actually exercised). The mid-write cleanup path (staged writeFile failure) is now unexercised, and the test name misleads a maintainer into believing partial-write coverage exists.
Suggested fix: inject the failure after a partial write (e.g. have the open spy return a real FileHandle whose writeFile/sync throws after some bytes), then assert both that targetPath is absent and .branch-staging is empty; or rename the test to what it now verifies ("propagates staging-open failures and leaves no target file behind").
中文说明
[Suggestion] R12-3：重写后的 removes a partially written target when fork creation fails 故障注入现在在 staging 文件的 open 处抛出 —— 此时尚未写入任何字节 —— 因此该测试不再构造其名称所描述的场景，唯一断言（expect(fs.existsSync(targetPath)).toBe(false)）恒为真。— 具体代价：旧注入在写入中途失败（fs.writeSync(file, 'partial') 之后）；新注入对任何 .branch-staging 的 open 都抛出，而这发生在 transcriptStagingCreated 置位之前、磁盘上尚无任何内容之时，因此无论清理逻辑是否存在，该断言对任何 link 之前的错误都成立 —— 即使删除 forkSession 中全部失败路径清理，测试仍会通过（实际只验证了 claim 删除）。写入中途的清理路径（staged writeFile 失败）现在完全未被覆盖，且测试名称会误导维护者以为存在部分写入覆盖。
建议修复：在部分写入之后注入故障（例如让 open spy 返回一个真实 FileHandle，在其写入若干字节后的 writeFile/sync 中抛出），然后同时断言 targetPath 不存在且 .branch-staging 为空；或者把测试改名为它实际验证的行为。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| ownerToken, | ||
| transcriptStagingName, | ||
| backupStagingName, | ||
| backupNames: [...backupNames].sort(), |
There was a problem hiding this comment.
[Suggestion] R12-4: backupNames is written into every branch-claim manifest and validated on parse but has no production read site — GC and the in-method cleanup operate on whole directories, never per-name — and it is stale by construction (the manifest is written before staging; the post-copy narrowing at line 2416 is not reflected in the claim). — Concrete cost: the manifest grows O(referenced file-history backups) (~24 bytes/name in JSON); a forked transcript referencing roughly 11k backup names pushes the claim past BRANCH_GC_MAX_MANIFEST_BYTES (256KB). If that fork then crashes mid-creation, readBranchCreationManifestBounded returns undefined for the oversized claim, GC logs "invalid manifest" and continues — so the orphaned claim, staged transcript, staged backups, and any published backup dir are retained forever instead of being reclaimed. A future GC enhancement trusting this field would also flag valid forks as corrupt, because the persisted list can overstate what was actually staged.
| backupNames: [...backupNames].sort(), |
Drop the field from BranchCreationManifestV1 and parseBranchCreationManifest's validation too; if a future GC pass needs per-name data, derive it from the staged/published directories.
中文说明
[Suggestion] R12-4：backupNames 被写入每个 branch-claim 清单并在解析时校验，但没有任何生产读取点 —— GC 与方法内清理都按整目录操作，从不按名操作 —— 且它天生是过期的（清单在 staging 之前写入；第 2416 行拷贝后的收窄不会反映到 claim 中）。— 具体代价：清单随引用的 file-history 备份数线性增长（JSON 中约 24 字节/名）；引用约 1.1 万个备份名的 fork 会把 claim 推过 BRANCH_GC_MAX_MANIFEST_BYTES（256KB）。若该 fork 在创建中途崩溃，readBranchCreationManifestBounded 对超大 claim 返回 undefined，GC 记录 "invalid manifest" 并 continue —— 于是孤立的 claim、staged transcript、staged 备份以及已发布的备份目录将被永久保留而无法回收。未来若有 GC 改进信任该字段，还会把有效 fork 误判为损坏，因为持久化的列表可能多于实际 staged 的内容。
建议同时从 BranchCreationManifestV1 和 parseBranchCreationManifest 的校验中移除该字段；若未来 GC 需要按名数据，从 staged/已发布目录推导。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| return (await res.json()) as DaemonBranchedSession; | ||
| }, | ||
| 0, |
There was a problem hiding this comment.
[Suggestion] R12-5: Bare positional 0 as fetchWithTimeout's fourth argument (perCallTimeoutMs) silently disables the client-side fetch timeout for branchSession, with no comment pinning that intent and no caller-facing timeoutMs knob. — Concrete cost: per fetchWithTimeout's own JSDoc, 0 means "disable the timeout entirely" (bare fetch, no AbortController). The no-timeout behavior is deliberate and tested, but if the daemon stalls mid-branch (blocked restoring a large session, or a hung-but-alive daemon over a half-open connection) the promise never settles and the UI caller is stuck in its "branching" state with no way for any caller to impose a cap. An unannotated positional 0 also reads like a retry count, so the opt-out is easy to misread or clobber in a future refactor; sibling long-running routes (restartMcpServer, reload) expose a timeoutMs option instead.
Suggested fix:
return (await res.json()) as DaemonBranchedSession;
},
// 0 = no per-call timeout: branch restore can exceed fetchTimeoutMs on large sessions
0,or follow the restartMcpServer pattern and accept an optional timeoutMs defaulting to a large finite value.
中文说明
[Suggestion] R12-5：fetchWithTimeout 的第四个位置参数（perCallTimeoutMs）传入裸 0，静默禁用了 branchSession 的客户端 fetch 超时 —— 既没有注释固定该意图，也没有面向调用方的 timeoutMs 开关。— 具体代价：按 fetchWithTimeout 自身的 JSDoc，0 表示"完全禁用超时"（裸 fetch，无 AbortController）。禁用超时是有意且经过测试的行为，但如果 daemon 在 branch 中途卡住（恢复大会话时阻塞，或半开连接上假死但存活的 daemon），promise 永不落定，UI 调用方会一直卡在 "branching" 状态且无法强制上限。未加注释的位置参数 0 看起来像重试次数，未来重构时容易被误读或误改；同族的长耗时路由（restartMcpServer、reload）都改为暴露 timeoutMs 选项。
建议修复：加注释说明（见上方代码），或仿照 restartMcpServer 接受可选 timeoutMs，默认取一个较大的有限值。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| const readFileSpy = vi.spyOn(fs.promises, 'readFile'); | ||
|
|
||
| try { | ||
| await service['cleanupStaleBranchCreations'](); | ||
| expect(readFileSpy).not.toHaveBeenCalled(); |
There was a problem hiding this comment.
[Suggestion] R12-16: The oversized-manifest test's "without reading it unbounded" guard spies fs.promises.readFile — an API the GC's manifest reader never calls (readBranchCreationManifestBounded uses open + handle.stat + handle.read) — so the assertion is trivially true and pins nothing about read boundedness. — Failure scenario: probe-verified four-arm cycle — deleting the before.size > BRANCH_GC_MAX_MANIFEST_BYTES guard and rewriting the reader to read the whole file through handle.read leaves this test GREEN (the unbounded-read regression ships undetected), while the proposed fix (tracking fs.promises.open's handle.read) catches it (expected [ 262145 ] reads to deeply equal []). A planted or corrupt multi-gigabyte .claim file would then be read fully into memory inside cleanupStaleBranchCreations, which runs inline in the daemon's listSessions path — the exact unbounded read the test name promises to prevent.
Suggested fix: spy the API the reader actually uses so the stat-size guard is proven to fire before any byte is read — wrap fs.promises.open to return a handle whose read is tracked, then for the oversized claim assert read was never called.
中文说明
[Suggestion] R12-16：超大清单测试的"不无界读取"守卫 spy 的是 fs.promises.readFile —— 而 GC 的清单读取器从不调用该 API（readBranchCreationManifestBounded 使用 open + handle.stat + handle.read）—— 因此该断言恒为真，对读取是否有界什么也没固定。— 失败场景：已用四臂探针循环验证 —— 删除 before.size > BRANCH_GC_MAX_MANIFEST_BYTES 守卫并把读取器改写为经 handle.read 读取整个文件后，该测试仍为绿色（无界读取的回归在未被察觉的情况下合入）；而建议的修复（跟踪 fs.promises.open 返回句柄的 read）能捕获它（expected [ 262145 ] reads to deeply equal []）。届时一个被植入或损坏的数 GB .claim 文件会在 cleanupStaleBranchCreations 中被整个读入内存 —— 它内联运行在 daemon 的 listSessions 路径上 —— 正是该测试名称承诺要防止的无界读取。
建议修复：spy 读取器实际使用的 API，证明 stat 大小守卫在任何字节被读取之前生效 —— 包装 fs.promises.open 使其返回一个 read 被跟踪的句柄，然后对超大 claim 断言 read 从未被调用。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| const syncSpies = [ | ||
| vi.spyOn(fs, 'readdirSync'), | ||
| vi.spyOn(fs, 'statSync'), | ||
| vi.spyOn(fs, 'openSync'), |
There was a problem hiding this comment.
[Suggestion] R12-17 (pattern, occurrence 1/3): Async-purity guard test with an incomplete sync-API spy list — uses only asynchronous filesystem APIs for the collision scan spies 5 sync APIs but misses existsSync/accessSync/readFileSync/lstatSync. Same pattern also at sessionService.test.ts:4929 (branch-GC test omits accessSync/opendirSync) and sessionStorageUtils.test.ts:795 (title-pair test spies only 4). — Failure scenario: a follow-up introducing a sync call through any unspied API (an existsSync fast-path pre-check in findSessionTitlesByPrefix, readFileSync in the head-window fallback) blocks the daemon event loop per file — on every branch, on every stale-claim scan — while the guard test named for preventing exactly that stays green. Probe-verified for the title-pair instance: injecting if (!fs.existsSync(filePath)) return emptyResult; left all 55 tests passing until the spy was added. The sibling fork-publication guard spies 17 sync APIs, showing the convention these three fall short of.
Suggested fix: extend syncSpies with existsSync, accessSync, readFileSync, lstatSync, realpathSync — and opendirSync for the GC test.
中文说明
[Suggestion] R12-17（模式问题，第 1/3 处）：异步纯净性守卫测试的 sync-API spy 列表不完整 —— uses only asynchronous filesystem APIs for the collision scan 只 spy 了 5 个同步 API，漏掉 existsSync/accessSync/readFileSync/lstatSync。同一模式还出现在 sessionService.test.ts:4929（branch-GC 测试漏 accessSync/opendirSync）和 sessionStorageUtils.test.ts:795（标题对测试只 spy 4 个）。— 失败场景：后续改动若经由任一未被 spy 的 API 引入同步调用（findSessionTitlesByPrefix 中的 existsSync 快速预检、head-window 回退中的 readFileSync），会在每次分支、每次陈旧 claim 扫描时按文件阻塞 daemon 事件循环 —— 而名义上防止此事的守卫测试仍是绿色。标题对实例已用探针验证：注入 if (!fs.existsSync(filePath)) return emptyResult; 后，在补上 spy 之前全部 55 个测试照常通过。同族的 fork 发布守卫 spy 了 17 个同步 API，说明这三处未达到既有惯例。
建议修复：为 syncSpies 补上 existsSync、accessSync、readFileSync、lstatSync、realpathSync —— GC 测试再补 opendirSync。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| const syncSpies = [ | ||
| vi.spyOn(fs, 'readFileSync'), | ||
| vi.spyOn(fs, 'readdirSync'), | ||
| vi.spyOn(fs, 'statSync'), |
There was a problem hiding this comment.
[Suggestion] R12-17 (pattern, occurrence 2/3): The branch-GC async-purity test spies 6 sync APIs but omits accessSync and opendirSync — the sync counterparts of the exact async calls GC makes (fs.promises.access via pathExists, fs.promises.opendir for the claims listing). See occurrence 1/3 (sessionService.test.ts:5521) for the shared pattern, failure scenario, and fix. — A regression to either of those blocks listSessions/forkSession (GC runs inline there) while this test stays green.
Suggested fix: add vi.spyOn(fs, 'accessSync') and vi.spyOn(fs, 'opendirSync') (optionally mkdirSync/renameSync/copyFileSync) to syncSpies.
中文说明
[Suggestion] R12-17（模式问题，第 2/3 处）：branch-GC 异步纯净性测试 spy 了 6 个同步 API，但漏掉了 accessSync 与 opendirSync —— 它们正是 GC 实际异步调用的同步对应物（pathExists 的 fs.promises.access、claims 列表的 fs.promises.opendir）。共享的模式、失败场景与修复见第 1/3 处（sessionService.test.ts:5521）。— 对这两者之一的回归会阻塞 listSessions/forkSession（GC 在其中内联运行），而该测试仍是绿色。
建议修复：为 syncSpies 补上 vi.spyOn(fs, 'accessSync') 与 vi.spyOn(fs, 'opendirSync')（可选 mkdirSync/renameSync/copyFileSync）。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| const syncSpies = [ | ||
| vi.spyOn(fs, 'statSync'), | ||
| vi.spyOn(fs, 'openSync'), |
There was a problem hiding this comment.
[Suggestion] R12-17 (pattern, occurrence 3/3): The async-purity guard for readLastJsonStringFieldsAsync spies only four sync APIs (statSync/openSync/readSync/closeSync); a regression through any other sync API keeps the test green while defeating the property the test name asserts. See occurrence 1/3 (sessionService.test.ts:5521) for the shared pattern. — Failure scenario: the function's sole call chain (computeUniqueBranchTitle → findSessionTitlesByPrefix, inline from the daemon branch path, scanning up to MAX_FILES_TO_PROCESS files) would block the daemon event loop per file, on every branch. Probe-verified: injecting an fs.existsSync pre-check left all 55 tests passing until the spy was added.
Suggested fix: extend syncSpies with readFileSync, existsSync, accessSync, lstatSync, realpathSync.
中文说明
[Suggestion] R12-17（模式问题，第 3/3 处）：readLastJsonStringFieldsAsync 的异步纯净性守卫只 spy 了 4 个同步 API（statSync/openSync/readSync/closeSync）；经由任何其他同步 API 的回归都会在该测试保持绿色的同时破坏测试名称所断言的性质。共享模式见第 1/3 处（sessionService.test.ts:5521）。— 失败场景：该函数的唯一调用链（computeUniqueBranchTitle → findSessionTitlesByPrefix，内联于 daemon 分支路径，最多扫描 MAX_FILES_TO_PROCESS 个文件）会在每次分支时按文件阻塞 daemon 事件循环。已用探针验证：注入 fs.existsSync 预检后，在补上 spy 之前全部 55 个测试照常通过。
建议修复：为 syncSpies 补上 readFileSync、existsSync、accessSync、lstatSync、realpathSync。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| expect(state.blocks).toMatchObject([ | ||
| { | ||
| kind: 'assistant', | ||
| text: 'historical answer', | ||
| branchRecordId: 'checkpoint-record', | ||
| }, | ||
| ]); |
There was a problem hiding this comment.
[Suggestion] R12-18: The only replay-path reducer test feeds a single chunk, pinning the block-CREATION attachment (transcript.ts:713-714); the MERGE-path assignment (transcript.ts:692-693) that production replay actually depends on has zero coverage. — Failure scenario: the daemon contract attaches branchRecordId only to the LAST visible assistant chunk of a checkpointed record (pinned by history-replay-page's own multi-chunk test); for any record replayed as 2+ chunks (e.g. text/thought/text) the block is created by an earlier chunk without the id, and canMergeTextDelta admits the decorated final chunk into the existing block — so attachment rides entirely on the merge-path assignment. Probe-verified: removing that assignment drops branchRecordId from the merged block while all 298 existing tests in daemonUi.test.ts + daemon-ui-transcript.test.ts stay green — every restored/paginated session would silently lose the Branch button on all multi-chunk checkpointed messages.
Suggested fix: extend the test with a two-chunk replay sequence — first agent_message_chunk without _meta.qwenTranscript.branchRecordId, second carrying it, both sharing sourceRecordIds — and assert the single merged block ends with branchRecordId: 'checkpoint-record'; this exercises transcript.ts:692-693 directly.
中文说明
[Suggestion] R12-18：唯一的回放路径 reducer 测试只喂了单个 chunk，固定的是块创建时的挂载（transcript.ts:713-714）；而生产回放实际依赖的合并路径赋值（transcript.ts:692-693）完全没有覆盖。— 失败场景：daemon 契约只把 branchRecordId 挂到带 checkpoint 记录的最后一个可见 assistant chunk 上（history-replay-page 自己的多 chunk 测试已固定）；任何以 2 个以上 chunk 回放的记录（如 text/thought/text），其块由不带 id 的较早 chunk 创建，canMergeTextDelta 允许带装饰的最后 chunk 并入已有块 —— 因此挂载完全依赖合并路径赋值。已用探针验证：移除该赋值后合并块丢失 branchRecordId，而 daemonUi.test.ts + daemon-ui-transcript.test.ts 中全部 298 个既有测试仍是绿色 —— 所有恢复/分页会话都会在多 chunk 的带 checkpoint 消息上静默丢失 Branch 按钮。
建议修复：为该测试补一个双 chunk 回放序列 —— 第一个 agent_message_chunk 不带 _meta.qwenTranscript.branchRecordId，第二个带上，两者共享 sourceRecordIds —— 并断言合并后的单个块最终带有 branchRecordId: 'checkpoint-record'；这直接覆盖 transcript.ts:692-693。
— qwen3.8-max via Qwen Code /review (v0.21.7)
…nversation # Conflicts: # packages/cli/src/acp-integration/session/Session.ts # packages/web-shell/client/components/MessageList.dom.test.tsx # packages/web-shell/client/e2e/visuals/screenshots.spec.ts
|
🤖 Addressed the latest review feedback (round 18/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 18/100 轮）。改动内容与我反驳保留之处如下： Round-12 review feedback — address summary (PR #8274)Conflict resolution (
|
|
@qwen-code /review |
| _Qwen Code review request accepted. Review is queued in [workflow run](https://github.com/QwenLM/qwen-code/actions/runs/31243158316)._ |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 19/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 19/100 轮）。改动内容与我反驳保留之处如下： Round summary — PR #8274 (base-conflict resolution)Feedback triageNo actionable feedback this round: Conflict resolution (
|
|
Qwen Code review timed out. Qwen review timed out after 21600 seconds (of the 360-minute budget). This run already used the maximum 360 minute timeout. See workflow logs. |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No action taken this round (PR #8274)Critical-only mode is active after 10 change-producing rounds. This round's actionable sections (reviews, inline comments, issue-level comments) are empty — there is no new feedback to address. The only actionable signal is the failed Feedback triage
Failed check
|
|
🔀 Base updated: red check(s) [review-pr] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [review-pr] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
Simplify historical session branching around the minimum persistence, recording, and navigation invariants required by the Web Shell flow. - Replace branch claims and garbage collection with staged publication - Validate completed turns incrementally instead of reloading transcripts - Separate persisted branch creation from live session restoration - Bound SDK waits and prevent late results from replacing navigation - Remove unused checkpoint prompt IDs while reading legacy records Note: A pre-commit crash may leave hidden staging or orphan backups.
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
|
🤖 AutoFix ran out of time before finishing (timeout (7200000ms)) (attempt 20/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31298406145 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix stopped: this counting window now contains 3 time-budget exhaustions (pushed rounds in between included; this round itself may have failed differently). That is 3 full agent runs that pushed nothing. A human should split or reduce the PR (or raise the agent time budget AND its step backstop together), then comment What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31303345287 🧠 Handled by Qwen Code · model/模型 |
|
⏸️ Takeover paused: this PR reached its round cap (100/100). Comment 中文说明⏸️ 托管已暂停：本 PR 达到轮次上限（100/100）。评论 |
|
Qwen Code review timed out. Qwen review timed out after 21600 seconds (of the 360-minute budget). This run already used the maximum 360 minute timeout. See workflow logs. |
…conversation # Conflicts: # packages/cli/src/acp-integration/acpAgent.ts
* feat(web-shell): branch from completed assistant responses Add durable response checkpoints so Web Shell sessions can branch from eligible completed Assistant turns without mutating the source history. - Record and validate checkpoints behind serialized topology fences - Preserve historical anchors through replay, daemon, SDK, and UI layers - Publish bounded forks with crash-safe ownership and referenced backups - Serialize prompt, rewind, branch, automatic turn, and close mutations - Cover stale anchors, replay pagination, cleanup, and pending UI states Note: Responses recorded before this change remain non-branchable. # Conflicts: # packages/acp-bridge/src/bridge.ts # packages/acp-bridge/src/bridgeTypes.ts # packages/cli/src/acp-integration/acpAgent.test.ts # packages/cli/src/acp-integration/acpAgent.ts # packages/cli/src/serve/routes/session.ts # packages/cli/src/serve/server.test.ts # packages/core/src/services/chatRecordingService.ts # packages/core/src/services/sessionService.test.ts # packages/core/src/services/sessionService.ts # packages/sdk-typescript/src/daemon/DaemonClient.ts # packages/web-shell/client/components/MessageItem.tsx # packages/web-shell/client/components/MessageList.tsx * fix(session): preserve historical branch checkpoints Keep Assistant-response branching intact across the daemon stack after rebases, including history serialization and persisted-session ownership. - Forward durable checkpoint IDs through Bridge, SDK, and UI layers - Serialize live history mutations and retain valid nested branch anchors - Preserve persisted branches during generation cleanup - Add cross-layer regression tests for replay and stale checkpoints * fix(web-shell): harden response session branching * chore: remove PR comment evaluation artifact Keep the PR review report as a local ignored backup instead of shipping it with the feature branch. - Remove the generated PR comment evaluation from tracked files - Preserve the report under the ignored analyze directory * fix(web-shell): guard historical branch mutations Historical branch requests could outlive the client timeout during an active turn, and interactive forks lacked the recorder's cross-process writer-lease barrier. - Hide Assistant Branch actions while a turn is active - Run interactive fork creation inside the recorder write barrier - Use the concrete checkpoint recorder contract in Session - Document committed-session ownership and implemented design status * perf(core): index historical branch points during transcript scan Build branch catalogs during the frozen index scan so the first history page no longer reopens and materializes the complete active chain. - Retain a compact projection for shared branch-point resolution - Correlate live branch anchors with the completed prompt and final reply - Complete recorder mocks required by the concrete Session contract - Update the reviewed design with performance and correlation invariants * fix(core): address review findings — dead code, boundary remap, promptId guard, stale toast (QwenLM#8274) * fix(core): address review findings — dead code, boundary remap, promptId guard, stale toast (QwenLM#8274) * fix(core): address review findings — archived GC, subtype registration, UUID validation, dead code (QwenLM#8274) * test: strengthen branch-point and fork coverage from review (QwenLM#8274) Add focused tests requested in PR review: - branch catalog resolves checkpoints that fall on a later page - accept a parallel tool batch closed within a single turn - exercise the linkSync->copyFileSync fork backup fallback success path - prove a remapped checkpoint stays usable via a nested fork - isolate each branch-point validation conjunct across bridge and SDK * fix: address round-4 review feedback for session branching (QwenLM#8274) - Make the directory-fsync durability test platform-aware (skip on win32), since fsyncDirectoryBestEffort swallows the injected error on Windows and the rejection path is non-Windows by design. - Reject atRecordId on the side-task fork path instead of silently discarding it, so the API surface no longer implies acceptance. - Correct the design doc: name the real promptQueue FIFO (not the nonexistent historyMutationQueue) and describe filtered checkpoint boundaries as remapped to the nearest retained predecessor, not unconditionally null. - Add focused tests: branch-point assistantRecordUuid mismatch rejection, and insight-block branchRecordId anchoring (insight-only block must not anchor onto the previous reply). * fix: address round-5 review feedback for session branching (QwenLM#8274) * fix: address round-6 review feedback for session branching (QwenLM#8274) * fix: address round-7 review feedback for session branching (QwenLM#8274) * fix(core): harden branch-point resolution against malformed transcript shapes (QwenLM#8274) - Filter null/non-object part elements in the shared branch resolver so a transcript containing null parts no longer makes forkSession throw a TypeError for every checkpoint. - Tag tool calls carried in from the pre-boundary prefix so a dangling call left by a crashed turn no longer permanently disables checkpoint recording; only calls issued inside the turn must close. - Merge duplicate-uuid records first-wins for identity fields in the transcript reader, matching the byUuid index and fork aggregation, so the reader never advertises a branch marker the fork path must reject. * fix: address round-8 review feedback for session branching (QwenLM#8274) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix: address round-9 review feedback for session branching (QwenLM#8274) * test(core): pin branch GC isolation from throwing warning callbacks (QwenLM#8274) * fix(acp-bridge): reject rewind at admission while a prompt is active (QwenLM#8274) * fix(web-shell): harden session branch publication Preserve direct ACP prompt preemption while fencing branch and rewind history mutations at the Session boundary. Convert branch publication, backup staging, cleanup, and stale-claim GC to asynchronous filesystem APIs, and surface unsupported hard-link commits as typed ACP and HTTP errors. Expand regression coverage and update the reviewed design contract. * fix(web-shell): harden historical response branching Reject branches during prompt admission and keep dispatched mutations owned until their real outcome is known. - remove detached timeouts across ACP, SDK, and WebUI - bound branch cleanup and make title scans asynchronous - avoid full branch-point scans during transcript pagination - add regression coverage from Core through the real daemon and browser * fix: address round-11 review feedback for session branching (QwenLM#8274) * fix: address round-12 review feedback for session branching (QwenLM#8274) * refactor(branching): remove branch-specific overdesign Simplify historical session branching around the minimum persistence, recording, and navigation invariants required by the Web Shell flow. - Replace branch claims and garbage collection with staged publication - Validate completed turns incrementally instead of reloading transcripts - Separate persisted branch creation from live session restoration - Bound SDK waits and prevent late results from replacing navigation - Remove unused checkpoint prompt IDs while reading legacy records Note: A pre-commit crash may leave hidden staging or orphan backups. * chore(sdk): update browser bundle budget Account for the combined historical branching and transcript projection APIs after merging main while keeping the browser bundle size guard narrowly bounded. * fix(branching): address review lifecycle gaps Harden historical session branching against cancellation, observer, navigation, and shutdown races found during review. - Normalize cancellation keys and bound close-time mutation waits - Preserve anchors after observer completion and load persisted forks - Report success only when the guarded session switch starts - Cover recorder cursors, fork cleanup, admission, and rollback - Align daemon events and branch errors with runtime behavior * refactor(session): simplify branching safeguards Reduce the session branching surface after review while preserving the critical concurrency, durability, and ownership guarantees. - Remove the unused full-chain resolver and test production entry points - Copy backups from verified open handles instead of using hard links - Reuse the bounded title scan instead of maintaining an async mirror - Deduplicate UI branch requests and fail fast for busy automatic turns - Consolidate repeated mutation tests and retain critical race coverage - Document the retained invariants and rejected overdesign explicitly * test(branching): simplify regression coverage Reduce duplicated branching tests while retaining regression coverage for the safety, concurrency, and lifecycle fixes introduced by this feature. - Consolidate symmetric bridge and agent scenarios with table-driven cases - Remove repeated cross-layer assertions and brittle implementation spies - Drop redundant UI permutations and branch-only visual snapshots * fix(serve): handle branch busy admission * fix(sdk): preserve v1 branch session contract Keep existing latest-state branch callers source- and wire-compatible while retaining the persisted-only behavior for historical checkpoint branches. - Restore no-anchor branches before returning their live client identity - Add a separate typed result for persisted historical branch requests - Clean up restored attachments on stale navigation and disconnect races - Cover immediate continuation and historical persistence independently * fix(daemon): guard branching history mutations Prevent branch creation and automatic Goal turns from racing session teardown or interactive history mutations. - Reject branch admission while a conditional close is authorized - Serialize Goal continuations behind the history mutation gate - Limit branch checkpoints to interactive prompts - Add regressions for close and Goal scheduling races * fix(branching): preserve fork and checkpoint semantics Keep branch checkpoints and file-history snapshots correct across resumed, forked, and non-interactive session flows. - Track the restored active-chain base before the first appended turn - Preserve backup file modes during fork publication - Exclude authenticated channel prompts from checkpoint recording - Add regressions for all three review failures * fix(branching): harden branch and rewind behavior Handle the remaining branch and rewind review findings without widening the feature contract. - Ignore benign concurrent branch rejections in the Web Shell - Validate rewind prompt IDs before using string operations - Pin mutation ordering, cleanup, compaction, and checkpoint invariants - Align sourced-fork fixtures with the canonical side_task value --------- Co-authored-by: heyang.why <heyang.why@alibaba-inc.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: 易良 <1204183885@qq.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: qwen-code-bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
7 participants
What this PR does
Previously, session branching used the latest active session state and could not reliably target an earlier Assistant response. Treating a visible message as a branch point is unsafe because tool calls, cancellations, metadata records, transcript pagination, rewinds, and concurrent transcript mutations can make the displayed response different from the authoritative active history.
Durable checkpoints provide a single source of truth shared by recording, replay, UI presentation, and Core validation. This lets users explore an alternative direction from an earlier answer without modifying the original session or exposing a partially created branch.
Why it's needed
Previously, session branching used the latest active session state and could not reliably target an earlier Assistant response. Treating a visible message as a branch point is unsafe because tool calls, cancellations, metadata records, transcript pagination, rewinds, and concurrent transcript mutations can make the displayed response different from the authoritative active history.
Durable checkpoints provide a single source of truth shared by recording, replay, UI presentation, and Core validation. This lets users explore an alternative direction from an earlier answer without modifying the original session or exposing a partially created branch.
Reviewer Test Plan
How to verify
Evidence (Before & After)
Before: Branching represented the latest active session state. Historical Assistant responses did not have an authoritative, durable branch point.
After: Eligible completed Assistant responses carry a recorded and replayable checkpoint. Selecting Branch creates a new session ending at the chosen turn, while stale checkpoints are rejected and trigger a transcript refresh.
Tested on
Environment (optional)
Risk & Scope
Linked Issues
Refs #8271
中文说明
Why it's needed
此前，会话分支只能使用源会话的最新活动状态，无法可靠地定位到更早的一条 Assistant 回复。直接把界面上可见的消息作为分支点并不安全，因为工具调用、取消、元数据记录、分页回放、会话回退和并发 transcript 写入都可能导致界面显示内容与权威活动历史不一致。
持久化 checkpoint 为录制、回放、界面展示和 Core 校验提供了统一的事实来源。用户因此可以从较早的回答探索另一条对话路径，同时不修改原始会话，也不会看到尚未完整创建的分支会话。
Reviewer Test Plan
How to verify
Evidence (Before & After)
Before： 分支操作只能表示会话当前最新的活动状态。历史 Assistant 回复没有权威且持久化的分支点。
After： 符合条件的已完成 Assistant 回复带有可录制、可回放的 checkpoint。选择 Branch 后，新会话会准确截止于所选回合；过期 checkpoint 会被拒绝，并触发 transcript 刷新。
Tested on
Environment (optional)
Risk & Scope
Linked Issues
Refs #8271