feat(web-shell): bind plan approval to its Todo revision - #8393
Conversation
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterℹ️ No screenshot changed against the PR base — but this PR edits 3 render-shaping files:
Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
|
Thanks for the PR! Template: complete ✓ Problem: real and documented. Open issue #8389 ( Direction: aligned. The Plan & Review experience is experimental, default-off, and scoped to the daemon WebShell; binding the approval to the submitted revision is exactly what makes the pre-execution DAG deterministic. The claude-code CHANGELOG has no direct reference to revision binding, but plan-approval dialog correctness is an actively maintained area there (fixes for stale-snapshot labeling and empty plan-approval modals) — a signal this UX deserves the care taken here. Size: small. Phase 2 (this PR's own commit): ~91 production-logic lines + ~178 test lines. Cumulative with stacked #8391: ~154 production / ~232 test / 5 schema / ~54 docs lines. The only Stage 0 core-path match is Approach: minimal. The revision identity ( Stacking: the branch carries #8391's commit, so until #8391 merges GitHub shows both phases in this diff. The review below focuses on the phase-2 commit ( Risk: Moving on to code review. 🔍 中文说明感谢贡献！ 模板：完整 ✓ 问题：真实且有据可查。开放 issue #8389（ 方向：对齐。Plan & Review 是实验性、默认关闭、仅限 daemon WebShell 的能力；把审批绑定到提交的版本正是让执行前 DAG 确定化的关键。claude-code 的 CHANGELOG 没有直接对应"版本绑定"的条目，但 plan 审批框的正确性在那里是活跃维护区域（修复过快照过期标注、空审批弹窗等问题）——说明这块 UX 值得细致打磨。 规模：小。阶段 2（本 PR 自有 commit）约 91 行生产逻辑 + 178 行测试；与叠加的 #8391 累计约 154 生产行 / 232 测试行 / 5 行 schema / 54 行文档。唯一命中 Stage 0 核心路径的是 方案：最小化。版本身份（ 叠加关系：分支包含 #8391 的 commit，在 #8391 合并前 GitHub 会显示两个阶段的累计 diff。下一阶段的代码审查聚焦阶段 2 的 commit（ 风险： 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Code review — no blockers found. I proposed the solution independently before reading the diff (carry a stable revision identity in the permission
One non-blocking observation: legacy text-parsed plan messages ( The binding handshake: sequenceDiagram
participant P1 as daemon Session
participant P2 as WebShell adapter
participant P3 as todos resolver
participant P4 as Approval dialog
P1->>P1: capture activeTodoPlanRevision from plan update meta
P1->>P2: exit_plan_mode permission request with qwenTodoApproval (planId, sourceCallId)
P2->>P3: getActiveTodosForPlanRevision(messages, revision)
P3-->>P4: bound snapshot todos, or empty for text-only fallback
Files changed (phase 2 — 12 of 22 shown)The cumulative diff also carries the 10 phase-1 files tracked in PR 8391 (experimental setting, entry points, i18n, schemas).
Testing evidence — this is an unattended CI run, so no PR code was built or executed here; the evidence below is the PR's own CI on the reviewed commit, fetched via API. The author's macOS manual verification in the PR body is a self-reported claim, not independently re-run. All three
The unit tests pin the binding logic on both sides, but the end-to-end path (a live daemon session producing the meta and the real WebShell dialog rendering it, including cold-resume replay) rests on the author's macOS run. Sandboxed verification would settle it: 中文说明代码审查——未发现阻塞问题。我在读 diff 前先独立提出了方案（在 permission
一个非阻塞观察：旧的文字解析 plan 消息（ 测试证据——这是无人值守 CI 运行，此处未构建或执行任何 PR 代码；以上证据是通过 API 获取的该 commit 自身 CI 结果。作者在 PR 正文中的 macOS 手工验证为自述声明，未独立复跑。三个 单元测试在两端钉住了绑定逻辑，但端到端路径（真实 daemon 会话产生 meta、真实 WebShell 对话框渲染、冷启动回放恢复）目前依据作者的 macOS 运行。沙箱验证可以补齐： — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 4/5 — clean, minimal, well-tested increment of a planned feature; the only gap is that the end-to-end daemon→dialog path is verified by the author's self-reported macOS run plus unit tests, not an independent live run. Stepping back: this is exactly the change the linked issue asked for, implemented the simplest way that works. My independent proposal before reading the diff was the same mechanism — carry the revision identity in the permission CI is fully settled and green on the reviewed commit (macOS/Windows/integration legs are merge-queue-only by design), the fork-refactor guardrail does not apply ( 中文说明置信度：4/5——干净、最小化、测试充分的计划内增量；唯一缺口是 daemon 到对话框的端到端路径只有作者自述的 macOS 运行加单元测试佐证，没有独立的真实运行。 整体来看：这正是关联 issue 要求的变更，且用了最简单可行的实现。我在读 diff 前独立提出的方案就是同一机制——版本身份放进 permission CI 在受审 commit 上已全部完成且为绿色（macOS/Windows/集成任务按设计仅在 merge queue 运行），fork-refactor 护栏不适用（ — Qwen Code · qwen3.8-max-preview Reviewed at |
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. ✅
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Phase 1 (blind) + Phase 2 (cross-check) complete.
The revision-binding flow is clean: the daemon captures the latest {planId, sourceCallId} from the plan update metadata, injects it into the exit_plan_mode permission request, and the web-shell resolves the exact Todo snapshot via getActiveTodosForPlanRevision instead of the latest active list. Gating the Workflow UI on experimental.sessionWorkflow keeps the feature opt-in and preserves existing behavior when disabled. Tests cover the metadata injection, adapter extraction, revision matching, and setting gating.
A couple of non-blocking observations:
getActiveTodosForPlanRevisionreturns the matched snapshot regardless of todo status (including fully completed ones). This is consistent with the updated unit tests, but differs slightly from the design doc phrase "nodes remain pending before approval." Consider whether the approval UI should filter or warn on terminal snapshots.- After a successful
exit_plan_mode,activeTodoPlanRevisionis not cleared in the post-execution path (the clear only runs when the new mode isPLAN, i.e., onenter_plan_mode). It is reset on the nextsetMode/enter_plan_mode/plan-update, so it's not a correctness issue, but clearing it on exit would make the lifecycle more explicit.
No Critical issues. Approving.
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 — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max-preview via Qwen Code /review (v0.21.4)
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 — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max via Qwen Code /review (v0.21.4)
|
Addressed the two stale-revision defects found in review: bulk replay no longer restores an old Todo plan revision when the loaded session is already in Plan mode, and rewind now clears any active Todo plan revision after truncating history. Verified with focused loadSession and rewind/exit_plan_mode tests, targeted ESLint, Prettier, and git diff checks. I intentionally did not add the suggested helper abstractions, duplicate parser extraction, extra logging, or additional mutation-only coverage in this round because they would expand this PR without fixing a current stale-approval bug. |
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 — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max via Qwen Code /review (v0.21.3)
yiliang114
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
History replay re-sends stale plan updates through Session.sendUpdate, re-stamping activeTodoPlanRevision from finished plan cycles. Clear the revision after every replay path (cold replayHistory and live non-bulk loadSession) so a replayed snapshot can never bind a later exit_plan_mode approval; reloaded sessions fall back to text-only approval until the next live todo_write re-establishes the binding. Also drop the bulk-load restore that could never be read before a plan-mode transition cleared it, and pin the workflow gates and mode-entry clears with negative tests.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #8393Feedback dispositionsCritical — fixed
Suggestions — implemented
Verified previously fixed (no change needed)
Declined (replied on their threads, left open)
Notes
VerificationCommands actually run (after the final state of the change set):
中文说明Autofix 审查轮次 — PR #8393反馈处理结论Critical — 已修复
Suggestion — 已实现
已核实此前修复（无需改动）
已拒绝（已在对应线程回复，线程保持打开）
说明
验证实际执行的命令（针对改动集的最终状态）：
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 建议见行内评论。 未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max via Qwen Code /review (v0.21.3)
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Address-review summary — PR #8393 (round 4)Feedback points and dispositions
Conflict notes
VerificationCommands actually run this round (all in the PR checkout):
Commit: 中文说明处理审查意见总结 — PR #8393（第 4 轮）反馈点及处理结果
冲突说明
验证本轮实际运行的命令（均在 PR 检出目录中）：
提交： Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 建议见行内评论。 1 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。 未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max via Qwen Code /review (v0.21.3)
…nLM#8393) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Autofix review round summary — PR #8393Feedback dispositions
No other actionable feedback this round. No conflict ( Changes
Verification
中文说明Autofix 审查轮次总结 — PR #8393反馈处理
本轮无其他可执行反馈。无冲突（ 改动
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind What I found before stopping: Blocked: failing
|
|
Review follow-up: I rechecked the four remaining threads against the current head. The two replay/bulk-restore findings no longer apply after |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round — no code changesThe only feedback this round was a failed check; there were no review comments or inline findings. After an evidence-based investigation, no code change is warranted for this PR. Details below. Failed check investigated
Using the GitHub Actions job-steps API, the failing step was identified precisely:
Why this is not caused by this PR
Why no fix was attempted
ConclusionThe failure is a flaky or environment-specific unit-test failure in the Java SDK suite, uncorrelated with this PR's content. The appropriate remedy is re-running the failed job (or relying on the repository's flaky-rerun automation); no source change is made by this round. 中文说明Autofix 审查轮次 —— 无代码改动本轮唯一的反馈是一个失败的 CI 检查；没有任何审查评论或行内意见。经过基于证据的排查，本 PR 无需任何代码改动。详情如下。 排查的失败检查
通过 GitHub Actions 的 job-steps API 精确定位了失败步骤：
为什么该失败与本 PR 无关
为什么没有尝试修复
结论该失败是 Java SDK 套件中一次 flaky 或环境相关的单元测试失败，与本 PR 的内容无关。合适的处理方式是重跑失败的 job（或依赖仓库已有的 flaky 自动重跑机制）；本轮不做任何源码改动。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: the linked-issue fidelity pass, the line-by-line correctness pass, the security pass, the reuse and duplication pass, the altitude and abstraction pass, the consistency and clarity pass, the performance pass, the test-coverage pass, the open-ended audit (attacker mindset), the open-ended audit (oncall mindset), the open-ended audit (maintainer mindset), the removed-behavior audit, the cross-file consistency pass, the build-and-test check — its prompt was built, but no agent on record was launched with it.
中文说明
已审查。 建议见行内评论。 未审查：关联 issue 一致性检查、逐行正确性检查、安全检查、复用与重复代码检查、修复层次与抽象合理性检查、一致性与可读性检查、性能检查、测试覆盖检查、开放式审计（攻击者视角）、开放式审计（值班排障视角）、开放式审计（后续维护者视角）、删除行为审计、跨文件一致性检查、构建与测试验证——它的 prompt 已构建，但没有任何 agent 有记录用它启动过。
— qwen3.7-max via Qwen Code /review (v0.21.4)
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Code Review Overview (AI Generated)
PR: #8393 feat(web-shell): bind plan approval to its Todo revision
Type: New feature / bug fix（实验性 Session Workflow）
Change size: +793/-142 across 18 files
HEAD reviewed: 8ecb774517a216c9d67a630de633a0deabe8e151
Findings Summary
- Critical: 0
- Major: 0
- Minor: 0
- Nit / Suggestion: 0 条新的独立发现；现有 reviewer 的 Suggestion 已在当前 HEAD 处理或明确 declined
Key Observations
本 PR 将 exit_plan_mode 审批绑定到 daemon 通过 _meta.qwenTodoApproval 携带的精确 {planId, sourceCallId} 快照，避免审批框误用最新的 active Todo。核心实现分布在 Session.ts、acpAgent.ts 与 web-shell/client/utils/todos.ts。我在 HEAD 独立核查了 revision 生命周期与匹配逻辑，确认之前的 Critical 泄漏路径均已得到清理。
Cross-Validation
| Finding | Other Reviewer | My Assessment |
|---|---|---|
| Plan mode 实际使用路径上的 stale plan-revision 泄漏（进入 plan mode 时未清理） | qwen-code-ci-bot (Critical) | 已确认修复 — Session.ts:6515 在切换至 plan mode 后清理 activeTodoPlanRevision |
| Session cold-load / history replay 会重新植入过期 revision | qwen-code-ci-bot (Critical) | 已确认修复 — acpAgent.ts:4546 在非 bulk loadSession replay 结束后清理；Session.ts:2216/2257 在 replayHistory 后清理 |
rewindToTurn 截断历史未使 revision 失效 |
qwen-code-ci-bot (Suggestion) | 已确认修复 — Session.ts:4464 在 rewind 后清理 |
requestPermission 对 exit_plan_mode 注入 _meta.qwenTodoApproval 缺少 revision 有效性校验 |
qwen-code-ci-bot (Suggestion) | 已确认修复 — Session.ts:8259 仅在 isExitPlanModeTool && this.activeTodoPlanRevision 时注入 |
getLatestActiveTodos 误用最新快照而非审批对应快照 |
qwen-code-ci-bot (Suggestion) | 已确认修复 — 已替换为 getActiveTodosForPlanRevision(messages, revision)，按 callId + planId 精确匹配 |
sessionApprovalMode ext-method 与 settings-reload 切换至 plan mode 未清理 revision |
qwen-code-ci-bot (Suggestion) | 已确认修复 — acpAgent.ts:9168/10648 在切换时清理 |
exit_plan_mode 字符串比较多处重复 |
qwen-code-ci-bot / doudouOUC (Suggestion) | 已确认 todos.ts 提供 isExitPlanModeToolName 共享辅助函数 |
getString 等辅助函数重复 |
doudouOUC (Suggestion) | 作者已在 inline reply 中说明超出本 PR 范围，建议后续统一提取 |
| Integration Tests (CLI, No Sandbox) 未在 CI 中运行 | qwen-code-ci-bot | 作者已说明该 suite 被 skip；非代码问题，建议后续在 CI matrix 中补齐 |
Additional Audit Coverage
- 在 HEAD 直接读取
Session.ts、acpAgent.ts、todos.ts，列出activeTodoPlanRevision所有读写位置，确认清理路径覆盖：plan mode 进入、history replay、rewind、非 bulk loadSession replay、sessionApprovalMode切换、settings-reload 切换、enter_plan_mode执行后。 - 核对
getActiveTodosForPlanRevision的匹配条件为tool.callId === revision.sourceCallId && getTodoPlanId(tool) === revision.planId，实现 PR 描述的精确绑定。 - 确认 text-only fallback：当 revision 缺失时返回
[]，与关闭实验性 Session Workflow 的行为一致。
Final Verdict
APPROVE。审批 revision 绑定逻辑在 HEAD 已覆盖主要生命周期清理路径，测试也针对 stale revision 场景进行了 negative 覆盖。剩余的 Suggestion 级别问题（如辅助函数提取、特定分支的测试覆盖）作者已处理或明确标记为后续跟进，不构成合并阻塞。
This review was generated by QoderWork AI
|
Local verification at
The one smoke failure ( UI evidence for the revision-bound approval. The CI visuals artifact only captures the generic prompt-stream / model-switch videos, so I added a throwaway Playwright capture on this branch using the repo's mock-daemon harness (real browser, real WebShell client and SSE pipeline; daemon mocked at the HTTP boundary):
(Full-page view of case 1: 2-full-view-workflow-on-bound.png) The binding behaves as described in the PR: the DAG shows only when both |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 建议见行内评论。 未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max via Qwen Code /review (v0.21.5)
restoreHistory was the one history-resetting path that kept activeTodoPlanRevision, so a restored snapshot could let a stale revision bind the next exit_plan_mode approval. Clear it like the sibling reset paths, pin the behavior with a test, and pin the live-load clear ordering after the replayed updates.
Review: bind plan approval to its Todo revisionReviewed at What works well
Findings1. [Suggestion] The this.config.setApprovalMode(approvalMode);
if (approvalMode === ApprovalMode.PLAN) {
if (previousApprovalMode !== ApprovalMode.PLAN) {
this.activeTodoPlanRevision = undefined;
}
this.clearTodoStopGuardTrust();
}If the stop-guard change is intended, it deserves its own sentence in the description — it is a separate user-visible behavior from plan-approval binding. 2. [Suggestion] The invalidation clears aren't failure-safe. for (const update of replay.updates) {
await liveSession.sendUpdate(update); // a transport rejection here...
}
liveSession.clearActiveTodoPlanRevision(); // ...skips thisIf 3. [Suggestion] 4. [Nit] Live-session re-attach loses a legitimately current binding, asymmetrically. 5. [Nit] The extracted predicate only de-duplicates half the check. 6. [Nit] The pre-clear in if (update.sessionUpdate === 'plan') {
this.activeTodoPlanRevision = undefined;
}
await this.client.sessionUpdate(params);
this.#captureTodoPlanRevision(update);
7. [Nit] Four new 8. [Suggestion] R6-1 (order-blind live-load test) also still stands — Risk / other
|
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 5/100 轮）。改动内容与我反驳保留之处如下： Address-review summary — PR #8393Commit: Feedback points and decisionsrc:3711145984 —
|
The previous-mode guard added for the revision binding also skipped the Todo Stop Guard trust clear on a redundant plan re-select; scope the guard to the revision reset so every transition into plan clears the stop guard as before. The replay-time revision clears now run in finally blocks so a transport failure part-way through a replay cannot leave a replayed binding on the live session, and the web-shell exit-plan approval rule is unified in one predicate. Revision tests assert through the observable qwenTodoApproval approval metadata instead of the private field.
|
🤖 Addressed the latest review feedback (round 6/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 6/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressed — PR #8393Commit: Findings and dispositions1. [Suggestion] The 2. [Suggestion] The invalidation clears aren't failure-safe — FIXED. 3. [Suggestion] 4. [Nit] Live-session re-attach asymmetry — FIXED. 5. [Nit] Predicate only de-duplicated half the check — FIXED. 6. [Nit] Pre-clear in 7. [Nit] Tests poked the private 8. [Suggestion] R6-1 order-blind live-load test — ALREADY FIXED in Notes
VerificationCommands actually run (after the final code state):
Integration tests were not run: the touched behavior (ACP session internals and web-shell client rendering) is exercised by the unit suites above, not only through the bundled CLI or integration harness. Settings sources were not changed, so no schema regeneration was needed. 中文说明已处理的评审反馈 — PR #8393提交： 各条反馈的处理结果1. [建议] 2. [建议] 失效清理不具备失败安全性 — 已修复。 3. [建议] 4. [Nit] 活跃会话重新挂接的不对称性 — 已修复。 5. [Nit] 谓词只去重了一半检查 — 已修复。 6. [Nit] 7. [Nit] 测试直接探测私有字段 8. [建议] R6-1 实时加载测试对顺序不敏感 — 已在 说明
验证实际执行的命令（针对最终代码状态）：
未运行集成测试：本次改动的行为（ACP 会话内部逻辑与 web-shell 客户端渲染）由上述单元测试套件覆盖，并非只能通过打包后的 CLI 或集成测试框架验证。未改动 settings 源文件，因此无需重新生成 schema。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
Follow-up review: bind plan approval to its Todo revisionRe-reviewed at Verified fixed
Nothing else moved, so the remaining findings below are re-stated only as far as needed; all were re-read against the blobs at this commit. Still open1. [Suggestion] if (approvalMode === ApprovalMode.PLAN) {
if (previousApprovalMode !== ApprovalMode.PLAN) {
this.activeTodoPlanRevision = undefined;
}
this.clearTodoStopGuardTrust();
}Impact is bounded — the prompt loop clears trust on every turn while in PLAN ( 2. [Suggestion] The invalidation clears still aren't failure-safe — for (const update of replay.updates) {
await liveSession.sendUpdate(update); // rejection here...
}
liveSession.clearActiveTodoPlanRevision(); // ...skips thisA transport rejection part-way through a replay (client disconnect, SSE fanout failure) leaves a replayed plan update's re-stamped revision bound on a live session — the exact case the PR guards against. 4. [Nit] Live re-attach asymmetry. Non-bulk 5. [Nit] The extracted predicate only de-duplicates half the check. 6. [Nit] The pre-clear in 7. [Nit, slightly regressed] Private-field assertions. The new VerdictThe mechanism is right and the identity round-trips cleanly ( |
|
Released in v0.21.6. |
7 participants
What this PR does
Binds each
exit_plan_modeapproval request to the exact structured Todo revision that was submitted for review. The daemon carries the Todo plan ID and source tool-call ID in permission metadata, and WebShell resolves the approval DAG only when both values match the transcript snapshot.Live plan updates establish the revision through the existing session-update path. History replay, rewind, and fresh transitions into Plan mode invalidate the binding so stale snapshots from completed cycles cannot bind a later approval. A resumed session without a new live
todo_writesafely falls back to the existing text-only approval; the next valid live Todo update establishes a new binding. Once an approval request is created, later Todo updates cannot replace its DAG, and an all-completed snapshot remains visible for review.The structured Todo DAG is shown only when the experimental Session Workflow setting from #8391 is enabled. With the setting disabled, Plan mode keeps the existing text-only approval presentation.
This PR is stacked on #8391. Until #8391 merges, GitHub will show both phases in this PR's diff.
Why it's needed
The approval dialog previously selected the latest active Todo list, which could differ from the plan the user was actually approving. Binding approval to a stable revision makes the pre-execution DAG deterministic without introducing another Workflow store or scheduler.
Reviewer Test Plan
How to verify
exit_plan_mode; confirm the approval dialog shows that Todo DAG before execution begins.exit_plan_modewithout a new livetodo_write; confirm the approval uses the existing text-only presentation. Then produce a new structured Todo update and submit again; confirm the exact new DAG is shown.exit_plan_mode; confirm Plan mode keeps the existing text-only approval presentation.Evidence (Before & After)
Before: exit-plan approval used whichever active Todo snapshot appeared latest in the WebShell transcript.
After: with Session Workflow enabled, exit-plan approval resolves only the exact
{planId, sourceCallId}pair carried by the daemon permission request. Replay, rewind, and Plan-mode re-entry invalidate stale bindings; sessions without a fresh live Todo revision fall back to text-only approval.Automated checks passed on macOS: repository lint, full typecheck, full build, 149 focused WebShell tests, and focused daemon approval tests.
Tested on
Environment (optional)
Node.js 22, local daemon WebShell build.
Risk & Scope
_meta; clients that ignore it keep the existing text approval behavior. Replayed and rewound sessions also use text-only approval until a new live Todo update establishes a binding.Linked Issues
Related to #8389. Depends on #8391.
中文说明
本 PR 做了什么
将每一次
exit_plan_mode审批请求绑定到本次实际提交审阅的结构化 Todo 版本。daemon 在权限元数据中携带 Todo 的计划 ID 与来源工具调用 ID，WebShell 只有在两者都与 transcript 快照精确匹配时才展示审批 DAG。实时 plan update 会通过现有 session update 路径建立 revision。历史回放、rewind 和重新进入 Plan 模式会使绑定失效，避免已完成周期中的过期快照绑定后续审批。恢复后的 session 如果没有新的 live
todo_write，会安全降级为现有纯文本审批；下一条有效的实时 Todo update 会重新建立绑定。审批请求创建后，后续 Todo 更新不能替换正在审批的 DAG；即使快照中的节点都已 completed，审批 DAG 仍会保留。结构化 Todo DAG 仅在开启 #8391 提供的实验性 Session Workflow 设置后展示。关闭该设置时，Plan 模式保持现有纯文本审批界面。
本 PR 叠加在 #8391 之上。在 #8391 合并前，GitHub 会暂时在本 PR 中显示两个阶段的累计 diff。
为什么需要
此前审批框会选择 WebShell transcript 中最新的 active Todo，它可能并不是用户本次实际审批的计划。绑定稳定版本后，执行前 DAG 可以保持确定，同时不需要新增 Workflow store 或调度器。
Reviewer Test Plan
如何验证
exit_plan_mode；确认执行开始前审批框展示该 Todo DAG。todo_write的情况下提交exit_plan_mode；确认使用现有纯文本审批。随后生成新的结构化 Todo update 并再次提交；确认展示精确绑定的新 DAG。exit_plan_mode；确认 Plan 模式保持现有纯文本审批界面。前后对比证据
之前：退出 Plan 的审批会使用 WebShell transcript 中最后出现的 active Todo 快照。
之后：开启 Session Workflow 时，退出 Plan 的审批只解析 daemon 权限请求携带的准确
{planId, sourceCallId}。历史回放、rewind 和重新进入 Plan 模式会使过期绑定失效；没有新的实时 Todo revision 时，session 会降级为纯文本审批。已在 macOS 通过仓库 lint、全量 typecheck、全量 build、149 项 WebShell 相关测试以及 daemon 审批定向测试。
测试环境
Node.js 22，本地 daemon WebShell 构建。
风险与范围
_meta传递；忽略该字段的客户端继续使用现有纯文本审批。历史回放或 rewind 后的 session 也会使用纯文本审批，直到新的实时 Todo update 建立绑定。关联 Issue
关联 #8389。依赖 #8391。