Skip to content

feat: visualize ordinary-session plan execution - #7580

Merged
wenshao merged 16 commits into
QwenLM:mainfrom
yiliang114:cx/feat-7525-plan-dag-pr
Aug 1, 2026
Merged

feat: visualize ordinary-session plan execution#7580
wenshao merged 16 commits into
QwenLM:mainfrom
yiliang114:cx/feat-7525-plan-dag-pr

Conversation

@yiliang114

@yiliang114 yiliang114 commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

This PR adds a Session Workflow view for ordinary daemon-backed sessions by projecting the existing Todo plan, Agent executions, and persisted transcript into one dependency graph. Todo snapshots retain a stable plan ID, stable node IDs, and optional blockedBy edges across live ACP updates and replay. Top-level Agent calls may identify the Todo node they implement through todo_id, while existing task call IDs provide the deterministic live-status join.

Plan Mode now provides the opt-in pre-execution gate: when exit_plan_mode requests approval, the existing approval surface shows the submitted plan together with the current Todo workflow. Approval keeps the existing permission lifecycle and starts execution; rejection leaves the session in Plan Mode. Sessions that do not use Plan Mode continue normally.

During execution, the Web Shell layers dependency nodes, overlays live task state, keeps nested subagents under their root execution, and lets users inspect each step and open the existing Agent detail view. Completed sessions keep a collapsed workflow entry beside each persisted Todo snapshot. Expanding it lazily rebuilds root Agent executions from the parent transcript and lightweight descendant lineage from existing Agent sidecars; clicking a root or nested Agent loads its exact persisted virtual session, including prompt, progress, tool calls, and final output.

The implementation keeps one source of truth per concern and adds no workflow scheduler, graph store, or graph endpoint. Todo remains the business-state source, the task registry remains the live execution source, transcripts and Agent sidecars remain the durable history source, and the Web Shell only joins those streams for presentation. Historical indexes are runtime-scoped, bounded, short-lived, and explicitly report partial lineage when a safety limit is reached.

Why it's needed

Before this change, ordinary-session Todo plans, live tasks, and completed Agent details were separate views. Users could not review a dependency workflow before execution, see which Agent implemented a plan node, understand fork/join relationships while work was running, or return to the same workflow and nested Agent output after the session completed or the daemon restarted.

This preserves the existing best-effort behavior of ordinary sessions while making the plan observable before, during, and after execution. Todo completion remains authoritative; Agent state is execution evidence and never schedules, completes, retries, or unlocks Todo nodes automatically.

Reviewer Test Plan

How to verify

  1. Start a daemon-backed session in Plan Mode, publish a Todo plan with stable IDs and blockedBy dependencies, and call exit_plan_mode before launching Agents or modifying files.
  2. Confirm the approval panel shows the complete dependency graph and that rejecting keeps the session in Plan Mode while approving allows execution to continue.
  3. After approval, launch independent top-level Agents with matching todo_id values and confirm concurrent running states appear on the corresponding nodes, nested Agents remain under their root, and incomplete dependencies display as blocked.
  4. Select a workflow node and open both a root and nested Agent. Confirm the detail panel streams live progress and retains prompt, tool activity, metrics, and final output after completion.
  5. Complete the Todo plan, restart the daemon, reload the session, expand a historical Todo snapshot, and confirm the same DAG, root/nested Agent hierarchy, and completed Agent details are still available.
  6. Confirm an empty Todo snapshot clears active state, legacy snapshots without dependency metadata remain list-compatible, and unlinked Agent calls remain valid as unassigned executions.

Evidence (Before & After)

Before: Todo, Tasks, and Agent history were independent surfaces with no dependency graph, pre-execution workflow review, durable node-to-execution relationship, or completed-session workflow entry.

After — a real completed session restored after a daemon restart, including fork/join edges and nested Agent hierarchy:

Restored Session Workflow DAG

After — clicking the completed nested Agent still opens its persisted prompt, metrics, progress, and final output:

Completed nested Agent detail

Tested on

OS Status
🍏 macOS ✅ tested
🪟 Windows N/A
🐧 Linux N/A

Environment (optional)

Node.js 22, local daemon and Web Shell development build, real persisted session 2c91c2b4-7896-4906-85b0-c409aa7934fe. Verified pre-execution approval, parallel and nested Agent linkage, terminal completion, daemon restart, cold transcript recovery, and completed Agent detail reopening. After merging the latest main, full workspace lint, build, and post-build typecheck pass. The affected Todo and Web Shell suites also pass (43 and 479 tests).

Risk & Scope

  • Main risk or tradeoff: Agent-to-Todo linkage is intentionally best effort when a model omits todo_id; durable lineage is loaded lazily and bounded, and the UI labels truncated or failed history hydration instead of presenting partial data as complete.
  • Not validated / out of scope: automatic dependency scheduling, retries, completion propagation, percentage progress, forced plan-first execution for every session, and the broader Data Agent cockpit.
  • Breaking changes / migration notes: none. New Todo dependency metadata, plan identity, Agent association, and lineage resolution are optional; older clients continue to receive standard ACP plan entries and older sessions retain their existing list presentation.

Linked Issues

Closes #7525

中文说明

这个 PR 做了什么

这个 PR 为普通 daemon Session 增加 Session Workflow 视图,把现有 Todo 计划、Agent 执行和持久化 transcript 投影为同一张依赖图。Todo 快照在实时 ACP 更新与历史回放中保留稳定的计划 ID、节点 ID 和可选 blockedBy 边。顶层 Agent 可以通过 todo_id 声明自己实现的 Todo 节点,现有 task call ID 则用于确定性关联实时状态。

Plan Mode 现在提供可选的执行前门禁:当 exit_plan_mode 请求审批时,现有审批区域会同时展示提交的计划正文和当前 Todo workflow。批准后沿用现有权限生命周期并开始执行;拒绝后仍停留在 Plan Mode。不使用 Plan Mode 的 Session 继续按原有方式执行。

执行过程中,Web Shell 会对依赖节点分层、叠加实时 task 状态、把 nested subagent 保留在根执行之下,并允许用户查看每个步骤以及打开现有 Agent 详情。Session 完成后,每个持久化 Todo 快照旁仍保留一个折叠的 workflow 入口。展开时会按需从父 transcript 重建根 Agent 执行,并从现有 Agent sidecar 读取轻量后代层级;点击根 Agent 或 nested Agent 时,再加载它对应的精确持久化虚拟 Session,包括 prompt、进展、工具调用和最终输出。

实现对每类数据只保留一个事实来源,不新增 workflow 调度器、图存储或图接口。Todo 仍是业务状态来源,task registry 仍是实时执行来源,transcript 与 Agent sidecar 仍是持久历史来源,Web Shell 只负责把这些数据流关联展示。历史索引按 runtime 隔离、有界、短时缓存;达到安全上限时会明确标记只展示部分层级。

为什么需要

改动前,普通 Session 的 Todo 计划、实时 Tasks 和完成后的 Agent 详情是彼此独立的视图。用户无法在执行前审阅依赖 workflow,也无法在执行过程中确认哪个 Agent 对应哪个计划节点、理解 fork/join 关系,或者在 Session 完成与 daemon 重启后重新查看同一张 workflow 和 nested Agent 输出。

这个实现保留普通 Session 原有的 best-effort 行为,同时让计划在执行前、执行中和执行后都可观察。Todo 完成状态仍然权威;Agent 状态只是执行证据,不会自动调度、完成、重试或解锁 Todo 节点。

Reviewer Test Plan

如何验证

  1. 在 Plan Mode 中启动 daemon Session,发布带稳定 ID 与 blockedBy 依赖的 Todo 计划,并在启动 Agent 或修改文件前调用 exit_plan_mode
  2. 确认审批面板展示完整依赖图;拒绝后仍停留在 Plan Mode,批准后才继续执行。
  3. 批准后使用匹配的 todo_id 启动互相独立的顶层 Agent,确认并发运行状态出现在对应节点,nested Agent 保留在根 Agent 下方,依赖未完成的节点显示为阻塞。
  4. 选择 workflow 节点并分别打开根 Agent 与 nested Agent,确认详情面板能展示实时进展,并在完成后仍保留 prompt、工具活动、指标和最终输出。
  5. 完成 Todo 计划、重启 daemon、重新加载 Session,再展开历史 Todo 快照,确认同一张 DAG、根/nested Agent 层级和已完成 Agent 详情仍可查看。
  6. 确认空 Todo 快照会清除当前状态;没有依赖元数据的旧快照仍兼容列表展示;没有关联 Todo 的 Agent 调用仍有效,并显示为未分配执行。

证据(改动前后)

改动前:Todo、Tasks 和 Agent 历史是三个独立区域,没有依赖图、执行前 workflow 审阅、持久的节点与执行关联,也没有完成后可回看的 workflow 入口。

改动后——真实 Session 在 daemon 重启后恢复,包含 fork/join 连线与 nested Agent 层级:

恢复后的 Session Workflow DAG

改动后——点击已完成的 nested Agent,仍可打开持久化的 prompt、指标、进展和最终输出:

已完成的 nested Agent 详情

测试环境

OS 状态
🍏 macOS ✅ 已测试
🪟 Windows N/A
🐧 Linux N/A

环境(可选)

Node.js 22,本地 daemon 与 Web Shell 开发构建,真实持久化 Session 2c91c2b4-7896-4906-85b0-c409aa7934fe。已验证执行前审批、并行与 nested Agent 关联、终态完成、daemon 重启、冷 transcript 恢复,以及完成后重新打开 Agent 详情。合并最新 main 后,完整 workspace lint、build 与 build 后 typecheck 均通过;受影响的 Todo 与 Web Shell 测试也分别通过 43 与 479 个用例。

风险与范围

  • 主要风险或取舍:模型省略 todo_id 时,Agent 与 Todo 的关联按设计保持 best effort;持久层级按需加载并设有上限,发生截断或历史加载失败时,UI 会明确提示,不会把部分数据伪装成完整结果。
  • 未验证或不在范围内:自动依赖调度、重试、完成传播、百分比进度、对所有 Session 强制 plan-first,以及更完整的 Data Agent cockpit。
  • 破坏性改动或迁移说明:无。新的 Todo 依赖元数据、计划身份、Agent 关联与层级解析均为可选;旧客户端仍会收到标准 ACP plan entries,旧 Session 继续使用原有列表展示。

关联 Issue

Closes #7525

@gwinthis gwinthis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Architecture Review: COMMENT (C=0)

Summary

Large feature (+3266/-192) connecting Todo plans to live subagent executions via layered dependency DAG visualization in Web Shell. The design keeps Todo as source of truth for business completion while treating Agent task state as execution evidence.

Architecture Assessment

Strengths:

  1. Todo as single source of truth — plan nodes own completion state; agent executions are evidence, not completion triggers. Failed/cancelled executions draw attention without completing a Todo.
  2. No new engine — uses existing Todo and task data paths, native SVG/CSS. No workflow engine, scheduler, or graph dependency added.
  3. Backward compatible — legacy Todo lists without dependency metadata render as lists; unlinked Agent calls appear as unassigned executions.
  4. Edge-count cap — dense plans fall back without materializing excessive SVG edges. Resize observation and scale normalization bound browser layout.
  5. Stable identity across replays — Todo snapshots retain stable plan/node identity + dependency metadata across live ACP updates and history replay.

Scope observations (non-blocking):

  • Graph edges depend on measured browser layout — this is a known tradeoff of SVG-based graph rendering vs. canvas/WebGL
  • Automatic dependency scheduling, retries, completion propagation are explicitly out of scope — correct for Phase 1

Pattern

Execution evidence vs. completion authority: Separate the "what should be done" (plan/Todo) from "what is being done" (agent execution). Plan nodes own completion; executions provide evidence. This prevents a failed execution from falsely completing a plan item, and allows independent retry without unlocking dependent work.

中文说明

架构评审:COMMENT (C=0)

概要

大型功能(+3266/-192):将 Todo 计划连接到实时子代理执行,在 Web Shell 中展示分层依赖 DAG 可视化。

架构评估

  • Todo 为唯一事实来源 — 计划节点拥有完成状态;代理执行是证据
  • 无新引擎 — 复用现有 Todo 和任务数据路径,原生 SVG/CSS
  • 向后兼容 — 旧 Todo 列表仍为列表,未关联 Agent 调用显示为未分配执行

模式

执行证据 vs. 完成权威: 将"应做什么"(计划/Todo)与"正在做什么"(代理执行)分离。计划节点拥有完成权;执行提供证据。

— qwen3.7-max via Qwen Code /review

gwinthis pushed a commit that referenced this pull request Jul 23, 2026
@yiliang114

Copy link
Copy Markdown
Collaborator Author

E2E Test Report

Result: PASS

Tested a real daemon-backed Plan Mode session with a four-node fork/join workflow: prepare → {alpha, beta} → verify.

  • Before approval, the complete Todo DAG was visible in the existing exit_plan_mode permission surface, and no Agent launch or file mutation occurred.
  • After approval, Alpha and Beta ran in parallel with exact todo_id linkage. Alpha launched one nested Agent, and live node/task status remained associated with the correct workflow nodes.
  • The plan reached terminal completion and produced the expected files and workflow-e2e-ok result.
  • After stopping and restarting the daemon, a cold reload recovered the persisted DAG, both root Agent executions, the nested Agent hierarchy, and each completed detail entry.
  • Clicking the recovered nested Agent reopened its persisted prompt, progress metrics, and exact final output nested-e2e-ok.
  • The final merged commit passed full workspace lint, build, and post-build typecheck.

Final cold-recovery evidence:

Restored Session Workflow DAG

Completed nested Agent detail

Environment: macOS, Node.js 22, local daemon/Web Shell development build, persisted session 2c91c2b4-7896-4906-85b0-c409aa7934fe.

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

🖼️ web-shell visual preview

Rendered against a mock daemon (no real backend): the PR base vs this PR head 2f0b685. Only screenshots that changed are shown (flows below, if any, are head-only) — refreshes on every push.

Screenshots · before / after

ℹ️ No screenshot changed against the PR base — but this PR edits 11 render-shaping files:

  • packages/web-shell/client/App.tsx
  • packages/web-shell/client/components/ChatPane.tsx
  • packages/web-shell/client/components/messages/PlanExecutionView.module.css
  • packages/web-shell/client/components/messages/PlanExecutionView.tsx
  • packages/web-shell/client/components/messages/TasksStatusMessage.tsx
  • packages/web-shell/client/components/messages/ToolApproval.module.css
  • packages/web-shell/client/components/messages/ToolApproval.tsx
  • packages/web-shell/client/components/panels/TodoPanel.module.css
  • …and 3 more.

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 packages/web-shell/client/e2e/visuals/screenshots.spec.ts that seeds whatever state the UI is gated on; it then appears here as a head-only (NEW) capture.

Full-resolution recordings (.webm) are attached to the workflow run.

Qwen Code · web-shell visuals

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

🩺 serve daemon A/B

Built the PR base vs this PR head d965596, drove a fixed endpoint set against each, and diffed the JSON responses. Only fields that changed are shown.

No response changes against the PR base across 4 scenario(s).

Qwen Code · serve A/B

@github-actions github-actions Bot added the review/self-reported The linked issue was opened by the PR author (self-reported) label Jul 26, 2026
SubAgentTracker: the emitter now guards subagent TodoWrite results
(tool-call-emitter emitResult early-returns on subagentMeta), so a
subagent todo no longer promotes into a session-level plan. Flip the
stale assertion to expect no plan emission, mirroring the dedicated
guard test in tool-call-emitter.test.ts.

HistoricalPlanExecution: the pagination fixture's onLoadOlderHistory
returned Promise<void>, but PlanExecutionHistoryProvider requires
Promise<boolean> and throws 'Unable to load earlier session history'
on a falsy resolution. Production wires loadOlderHistory (resolves
true after layout); return true in the fixture to model a successful
load.
# Conflicts:
#	packages/cli/src/acp-integration/session/Session.test.ts
#	packages/cli/src/acp-integration/session/Session.ts
#	packages/web-shell/client/App.test.tsx
#	packages/web-shell/client/App.tsx
#	packages/web-shell/client/components/ChatPane.tsx
#	packages/web-shell/client/components/MessageList.tsx
#	packages/web-shell/client/components/messages/TasksStatusMessage.test.tsx
#	packages/web-shell/client/components/messages/TasksStatusMessage.tsx
# Conflicts:
#	packages/core/src/tools/todoWrite.test.ts
#	packages/core/src/tools/todoWrite.ts
#	packages/sdk-typescript/scripts/build.js
#	packages/web-shell/client/App.test.tsx
#	packages/web-shell/client/components/ChatPane.tsx
#	packages/web-shell/client/components/messages/ToolApproval.test.tsx
@yiliang114
yiliang114 marked this pull request as ready for review August 1, 2026 06:12
@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Re-running triage on @wenshao's request. The head (2f0b685) is unchanged since the last full pass, so the gate findings below still hold — I'm refreshing the footer and folding in what the deeper review confirmed.

Template looks good ✓ — all sections present, with before/after screenshots and a filled-in test matrix.

Problem: a roadmap feature request, not theoretical hardening. The linked issue (#7525) is labeled type/feature-request, priority/P2, roadmap/subagents-tools, roadmap/multi-agent, and spells out concrete gaps (Todo nodes can't express dependencies, the ACP projection drops the original Todo ID, live tasks and completed Agent details are separate views). The gap is real and scoped.

Direction: aligned. Visualizing the plan→execution relationship is squarely within the multi-agent direction the roadmap labels point to, and the PR follows the issue's key constraint — it joins the existing Todo / task-registry / transcript streams for presentation and adds no workflow scheduler, graph store, or graph endpoint. That restraint is the right call.

Size: ~1,910 production-logic lines (≈1,537 TS/TSX + 373 CSS) across six packages, ~214 of them in packages/core/src; test/snapshot adds ~2,131 lines and docs ~126. The author (@yiliang114) is a /packages/core/ code owner with admin permission, so this is a maintainer-authored PR and the two-tier core gate is exempt — no block, no escalation. The 1,000+ advisory still stands as advice: it's large, and the Plan Mode approval gate is the natural split point if a future iteration wants one.

Approach: large but cohesive — one feature spanning the data contract (todoWrite/agent), the wire projection (acp-bridge, acp-integration), and the presentation layer (web-shell, webui). Splitting contract from UI would leave the contract PR untestable on its own, so I wouldn't insist on it. One thing the deeper review surfaced: the PR description reaches further than the diff (see the code-review comment) — worth aligning, but that's a documentation fix, not a scope problem with the code.

Risk: elevated-review-depth signal. The diff touches packages/cli/src/acp-integration/** (Session.ts, PlanEmitter.ts, tool-call-emitter.ts, types.ts), a path correlated with post-merge reverts in this repo. That doesn't block anything, but it's where a reviewer should focus — and it has now been reviewed in depth and exercised live (see the testing section).

Moving on to code review. 🔍

中文说明

@wenshao 的要求重新运行 triage。head(2f0b685)自上次完整审查以来未变,因此下方门禁结论依然成立——本次只刷新页脚,并纳入更深入审查所确认的内容。

模板完整 ✓——各节齐全,附 before/after 截图,测试矩阵也已填写。

问题:路线图上的 feature request,不是理论性加固。关联 issue(#7525)带 type/feature-requestpriority/P2roadmap/subagents-toolsroadmap/multi-agent 标签,并明确列出具体缺口(Todo 节点无法表达依赖、ACP 投影丢失原始 Todo ID、实时 task 与完成后的 Agent 详情彼此独立)。缺口真实且有界定。

方向:对齐。把"计划→执行"关系可视化正属于路线图标签所指的多智能体方向;PR 也遵循了 issue 的关键约束——只关联展示现有 Todo / task-registry / transcript 数据流,不新增 workflow 调度器、图存储或图接口。这种克制是对的。

规模:约 1,910 行生产逻辑(约 1,537 行 TS/TSX + 373 行 CSS),跨六个 package,其中约 214 行位于 packages/core/src;测试/快照约 2,131 行,文档约 126 行。作者(@yiliang114)是 /packages/core/ 的 code owner,在本仓库拥有 admin 权限,因此这是维护者自己提交的 PR,两层核心门禁豁免——不 block,也不升级。1,000+ 大 PR 建议仍然成立:体量不小,若后续迭代想拆,Plan Mode 审批门禁是自然的切分点。

方案:范围虽大但内聚——一个 feature 贯穿数据契约(todoWrite/agent)、线上投影(acp-bridgeacp-integration)与展示层(web-shellwebui)。把契约与 UI 拆开会让契约 PR 无法独立测试,故不强求。深入审查发现一点:PR 描述覆盖的范围超出了 diff(见代码审查评论)——值得对齐,但那是文档修正,不是代码的范围问题。

风险:review 深度升级信号。diff 触及 packages/cli/src/acp-integration/**Session.tsPlanEmitter.tstool-call-emitter.tstypes.ts),这是本仓库中与合并后回滚相关的路径。不阻塞任何事,但应是 reviewer 重点关注的地方——目前它已被深入审查并在真实环境演练过(见测试部分)。

进入代码审查 🔍

Qwen Code · qwen3.8-max-preview

Reviewed at 2f0b68535319ee062d634e335cf41ddbbea34709 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Code review

I re-read the production diff across all six packages at the current head. No correctness, security, or regression blockers — the design is coherent and the implementation is careful. The shape: todo_write becomes the single source of plan truth (stable node ids, optional blockedBy edges, a stable planId), the Agent tool grows an optional todo_id so a top-level execution can name the node it implements, and the existing task-registry toolUseId provides the deterministic live-status join. The Web Shell and daemon UI only join those streams for presentation — no scheduler, no graph store, exactly what the issue asked for.

Things done well:

  • todoWrite.ts validation is genuinely solid — unique-id, unknown-dependency, self-dependency, duplicate-edge, and cycle detection (iterative Kahn's, no recursion), all deduped into one validateTodos shared by validateToolParams and the invocation, and now applied to the modified_by_user path too. planId assignment (new plan when the list was empty or fully completed) is sensible and backward-compatible on read.
  • The whole chain is backward-compatible: blockedBy, planId, and todo_id are all optional, and legacy snapshots without metadata still render as a plain list.
  • Test coverage is heavy (~2,131 test/snapshot lines): todoWrite +195, PlanExecutionView +797, web-shell todos +261, App +146, plus emitter/replay/selector coverage.

I independently re-verified the findings @wenshao posted above against the diff — they hold up, and none of them block the merge:

  • Description vs. diff (F1). Confirmed: the body and Reviewer Test Plan step 5 promise a post-completion history story — a collapsed workflow entry beside persisted snapshots, lazy rebuild of root executions from the parent transcript, descendant lineage from Agent sidecars, "partial lineage" labeling. None of that is in this revision (grepping the diff for lineage/hydrat/sidecar-rebuild turns up nothing related). The durability that does ship is plain transcript replay, which wenshao confirmed survives a full daemon restart. So the code is fine; the description/test-plan should be aligned to it.
  • transcriptAdapter.ts permission content is not scoped to exit_plan_mode. Confirmed: getPermissionContent replaces the previous unconditional [{ text: perm.title }] for every permission request, and ToolApproval.tsx renders a <pre> whenever contentText !== request.title. Any daemon approval whose tool call carries a text content block now shows that content instead of the title — a UI change across all approvals, with only the exit-plan case tested. Worth gating on isExitPlanApproval (the consumer already computes it) or an explicit "yes, intended for all tools".
  • webui planId / sourceCallId / blockedBy are populated but unread in-repo. Confirmed — no read site anywhere; Web Shell uses its own parallel implementation in client/utils/todos.ts. They are exported through the public daemon-react-sdk surface, so if they're deliberate SDK surface for out-of-repo consumers, say so; otherwise the webui selector change set is unconsumed.
  • App.tsx computes approvalPlanTodos unconditionally. Confirmed: useMemo(() => getLatestActiveTodos(messages), [messages]) runs a full transcript scan on every streaming tick, while ChatPane gates the identical call behind isExitPlanApproval. Gate it the same way.
  • todos.ts getAgentToolsForPlan with a null planId. Confirmed: the collection loop returns at the first subsequent todo snapshot when plan.planId === null, where a non-null plan continues until a different plan — so agents launched after a plan revision drop out on that path. If plan-role messages are reachable in the daemon Web Shell, plumb the id through; if legacy-only, a comment would save the next reader the trace.
  • selectors.ts empty-list retention. Confirmed: if (!items || items.length === 0) became if (!items), so a cleared plan emits a DaemonTodoList with items: [] (needed for the active-list selector to clear). Worth confirming no history consumer renders an empty snapshot card.

My own earlier notes still stand and are likewise non-blocking: the setCallId generalization (now every tool invocation, not just MONITOR — the mechanism that feeds AgentTask.toolUseId, intentional and tested) and the narrowed replay args-fallback in the high-risk acp-integration path; the redundant } else if (!isTodoWriteTool) { in Session.ts (the else already implies it — cosmetic); and the plan-extraction logic duplicated across web-shell and webui (defensible across the package boundary). Two minor todoWrite edge cases wenshao flagged also check out: a brand-new plan written entirely as completed right after a completed plan reuses the previous planId, and there's no maxItems bound on blockedBy/todos (ids are capped at 500 chars, but the array isn't, and it's echoed into _meta for every client).

sequenceDiagram
    participant U as User
    participant M as Model
    participant T as todo_write tool
    participant A as ACP plan stream
    participant R as Task registry
    participant W as Web Shell
    M->>T: write todos (id, blockedBy)
    T->>T: validate DAG, assign planId
    T->>A: plan update with qwenTodo metadata
    A->>W: plan entries with stable ids
    M->>R: launch Agent with todo_id
    R->>W: live status by toolUseId
    W->>W: join plan nodes to executions
    W-->>U: render workflow DAG
Loading
Files changed (22 of 53 shown — production logic; tests and snapshots folded)
File What changed
packages/core/src/tools/todoWrite.ts Heart of the contract: blockedBy edges, stable planId, and full DAG validation (cycle, self, unknown, duplicate) deduped into one shared validator
packages/core/src/tools/agent/agent.ts Adds the optional todo_id param with length/emptiness validation and prompt guidance
packages/core/src/agents/runtime/agent-core.ts Excludes todo_write from subagents and teammates since they share the parent Todo sidecar
packages/core/src/hooks/types.ts Adds optional blockedBy to the shared TodoItem type
packages/core/src/core/prompts.ts One new prompt line telling the model to pass todo_id (snapshot updated to match)
packages/core/src/tools/tools.ts Extends TodoResultDisplay with planId and blockedBy
packages/acp-bridge/src/transcript-replay.ts Projects planId and blockedBy into ACP plan updates and replay; narrows the args fallback
packages/cli/src/acp-integration/session/Session.ts Propagates callId to all tool invocations; emits a plan snapshot for todo_write
packages/cli/src/acp-integration/session/emitters/PlanEmitter.ts Emits the plan with planId and the source call id
packages/cli/src/acp-integration/session/emitters/tool-call-emitter.ts Skips plan emission for subagent metadata and failed writes
packages/cli/src/acp-integration/session/types.ts Adds blockedBy and the TodoPlanSnapshot type
packages/sdk-typescript/src/daemon/ui/normalizer.ts Carries plan metadata through the daemon UI normalizer
packages/web-shell/client/utils/todos.ts Plan-id tracking plus the agent-to-plan join (getAgentToolsForPlan)
packages/web-shell/client/components/messages/PlanExecutionView.tsx The new DAG component: topological layering, bounded SVG edges, execution join
packages/web-shell/client/components/messages/PlanExecutionView.module.css Styles for the workflow DAG view
packages/web-shell/client/components/messages/ToolApproval.tsx Plan Mode pre-execution gate: shows the workflow on exit_plan_mode approval
packages/web-shell/client/components/panels/TodoPanel.tsx Renders blockedBy dependencies in the todo panel
packages/web-shell/client/App.tsx Wires the plan view and approval gate into the app shell
packages/web-shell/client/adapters/transcriptAdapter.ts Adapts plan metadata from the transcript; broadens permission content (see findings)
packages/webui/src/daemon/session/selectors.ts Daemon-side plan and blockedBy selectors (mirrors the web-shell logic; fields currently unread in-repo)
packages/webui/src/daemon/session/DaemonSessionProvider.tsx Provider wiring for the new selectors
…and 31 more files (docs, tests, snapshots, i18n, styles)

Test evidence

Unattended CI run — I did not build or drive the product myself; the signal below is the PR's own CI on the reviewed commit, fetched via the API. CI is fully green on 2f0b685 — the decisive primary JS unit suite (Test (ubuntu-latest, Node 22.x)) that was still running during my last pass has now completed success, and there are no failures and nothing pending.

Check Conclusion
Test (ubuntu-latest, Node 22.x) ✅ success
Test (macos-latest, Node 22.x) skipped
Test (windows-latest, Node 22.x) skipped
Integration Tests (CLI, No Sandbox) skipped
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success
Capture web-shell visuals (ubuntu-latest, Node 22.x) ✅ success
precheck-pr / precheck ✅ success
Real daemon E2E / Java 11 ✅ success
OS/Java analysis matrix (ubuntu/macos/windows, Java 11/17/21) ✅ success
verify skipped (not triggered)
tmux-testing skipped (not triggered)

The macOS/Windows unit legs and the CLI integration tests are skipped on this trigger, so the unit suite's only running leg was ubuntu/Node 22 — now green. The web-shell visuals capture and the web-shell E2E smoke both passed, the most relevant completed signals for a UI-heavy PR.

The verification gap my last pass flagged is now closed by independent evidence. After my previous review, @wenshao verified this end-to-end on Linux — a fresh npm ci + npm run build, a real qwen serve daemon serving the production web-shell/dist, a real Chromium driven by Playwright, and a mock model endpoint, with an A/B "before" built from the same worktree reverted to the merge base (report above). His findings: 1,581 unit tests green across the six packages; the contract reaches the model (blockedBy and agent.todo_id present in the real request body, absent at base); the live DAG renders the diamond topology with correct fork/join edges and blocked derivation, and live task status correctly wins over Todo status; the exit_plan_mode approval shows the plan + graph without leaking into the approval's keyboard handling; and the workflow survives both a browser reload and a full daemon restart (cold replay from disk). That is exactly the integrated behaviour the unit suite can't pin in isolation, so the earlier /verify ask is satisfied — no further sandboxed run is needed.

中文说明

代码审查

我在当前 head 上重新通读了全部六个 package 的生产代码 diff。没有正确性、安全性或回归层面的阻塞问题——设计自洽,实现细致。整体形态:todo_write 成为计划状态的唯一事实来源(稳定节点 id、可选 blockedBy 边、稳定 planId),Agent 工具新增可选 todo_id 让顶层执行声明自己实现的节点,现有 task-registry 的 toolUseId 提供确定性的实时状态关联。Web Shell 与 daemon UI 只负责把这些数据流关联展示——没有调度器、没有图存储,正是 issue 所要求的。

做得好的地方:

  • todoWrite.ts 的校验相当扎实——唯一 id、未知依赖、自依赖、重复边、环检测(迭代 Kahn,无递归),全部去重到一个由 validateToolParams 与 invocation 共享的 validateTodos,且现在也应用到 modified_by_user 路径。planId 分配(列表为空或全部完成时开启新计划)合理,读取侧向后兼容。
  • 整条链路向后兼容:blockedByplanIdtodo_id 均为可选,没有元数据的旧快照仍按普通列表渲染。
  • 测试覆盖很重(约 2,131 行测试/快照):todoWrite +195、PlanExecutionView +797、web-shell todos +261、App +146,外加 emitter/replay/selector 覆盖。

我独立对照 diff 复核了上方 @wenshao 提出的发现——全部成立,且都不阻塞合并:

  • 描述与 diff 不符(F1)。 确认:正文与 Reviewer Test Plan 第 5 步承诺了完成后的历史能力——持久化快照旁的折叠 workflow 入口、从父 transcript 懒重建根执行、从 Agent sidecar 读取后代层级、"部分层级"标记。这些在本版本里都没有(在 diff 中 grep lineage/hydrat/sidecar 重建均无相关结果)。真正落地的持久化能力是普通 transcript 回放,wenshao 已确认它能扛住完整 daemon 重启。所以代码没问题;需要把描述/测试计划与之对齐。
  • transcriptAdapter.ts 的权限内容未限定于 exit_plan_mode 确认:getPermissionContent 把此前无条件的 [{ text: perm.title }] 替换为对每个权限请求都生效的逻辑,且 ToolApproval.tsxcontentText !== request.title 时渲染 <pre>。任何带有 text 内容块的工具调用,其审批面板现在都会显示该内容而非标题——这是跨所有审批的 UI 变更,而只测了 exit-plan 一种情形。值得用 isExitPlanApproval(消费方已经算出它)加以限定,或明确"是的,希望对所有工具生效"。
  • webuiplanId / sourceCallId / blockedBy 被填充但仓库内无人读取。 确认——全仓库无任何读取点;Web Shell 用的是自己在 client/utils/todos.ts 的并行实现。它们经由公共 daemon-react-sdk 面导出,因此若这是给仓库外消费方的有意 SDK 面,请说明;否则 webui 的 selector 改动集是未被消费的。
  • App.tsx 无条件计算 approvalPlanTodos 确认:useMemo(() => getLatestActiveTodos(messages), [messages]) 会在每个流式 tick 做一次完整 transcript 扫描,而 ChatPane 把同样的调用放在 isExitPlanApproval 之后。应按同样方式加门禁。
  • todos.tsgetAgentToolsForPlanplanId 为 null 时。 确认:当 plan.planId === null 时,收集循环会在第一个后续 todo 快照处返回,而非 null 的计划会持续到遇到不同的计划为止——因此计划修订之后启动的 agent 会在该路径上掉出图。若 plan 角色消息在 daemon Web Shell 中可达,应把 id 也透传过去;若仅遗留路径,加一行注释能省下下一位读者的追踪。
  • selectors.ts 保留空列表。 确认:if (!items || items.length === 0) 变成了 if (!items),因此被清空的计划会发出一个 items: []DaemonTodoList(active-list selector 需要它来清空)。值得确认没有历史消费方会渲染空快照卡片。

我此前的备注同样成立,也都非阻塞:setCallId 泛化(现在是每个工具调用,而不只是 MONITOR——这是喂给 AgentTask.toolUseId 的机制,有意且有测试覆盖)与高风险 acp-integration 路径里收窄后的回放 args 回退;Session.ts 中冗余的 } else if (!isTodoWriteTool) {else 本就蕴含它——纯观感);以及计划提取逻辑在 web-shellwebui 之间重复(跨 package 边界,可以接受)。wenshao 指出的两个 todoWrite 小边界也核实成立:紧跟一个已完成计划之后、整份以 completed 写出的全新计划会复用上一个 planId;以及 blockedBy/todos 没有 maxItems 上限(id 限制 500 字符,但数组不限,且会逐字回显进每个客户端的 _meta)。

(时序图见英文正文,描述计划发布与实时关联的关键路径。)

测试证据

无人值守 CI 运行——我没有在本地构建或驱动产品;下方信号是该 PR 自身在受审 commit 上的 CI,通过 API 获取。CI 在 2f0b685 上全绿——我上次审查时仍在运行的决定性主 JS 单测套件(Test (ubuntu-latest, Node 22.x))现已完成且为 success,没有失败,也没有任何待处理项。

(CI 表格见英文正文区域标记内。)

macOS/Windows 单测分支与 CLI 集成测试在此触发下被跳过,因此单测套件唯一在跑的分支是 ubuntu/Node 22——现已绿。web-shell 视觉截图捕获与 web-shell E2E smoke 均通过,对这个 UI 占比高的 PR 是最相关的已完成信号。

我上次标注的验证缺口现已由独立证据弥合。在我上一轮审查之后,@wenshao 在 Linux 上做了端到端验证——全新 npm ci + npm run build、真实的 qwen serve daemon(服务生产 web-shell/dist)、Playwright 驱动的真实 Chromium,以及 mock 模型端点,并用同一 worktree 回退到 merge base 构建了 A/B 的"before"(报告见上)。他的结论:六个 package 共 1,581 个单测全绿;契约确实抵达模型(真实请求体中存在 blockedByagent.todo_id,base 中没有);实时 DAG 正确渲染菱形拓扑、fork/join 连线与 blocked 推导,且实时 task 状态正确优先于 Todo 状态;exit_plan_mode 审批同时展示计划与依赖图,且不会泄漏进审批的键盘处理;workflow 在浏览器刷新与完整 daemon 重启(从磁盘冷回放)后均存活。这正是单测套件无法在隔离状态下钉住的集成行为,因此此前的 /verify 请求已被满足——无需再做沙箱运行。

Qwen Code · qwen3.8-max-preview

Reviewed at 2f0b68535319ee062d634e335cf41ddbbea34709 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — a solid, well-tested feature that does exactly what the roadmap issue asked; the verification gap from my last pass is now closed by a maintainer's end-to-end A/B run, and what remains is non-blocking nits (named in the code-review comment) plus a description that should be aligned with the diff.

Stepping back: this is genuinely good work, and it's now genuinely verified. The independent read I formed before digging into the diff — give todo_write stable IDs and dependency edges, give the Agent tool an association key, and join the two in the presentation layer off the existing task registry — is precisely the path the PR takes, and it executes it carefully. It honors the issue's central constraint (no scheduler, no graph store, no new endpoint) and stays backward-compatible throughout. The validation in todoWrite.ts (iterative Kahn's, full edge-case matrix, shared by both entry points) and the test coverage (~2,131 lines) are both stronger than typical. If I had to maintain this in six months I'd be fine with it.

What changed since my last pass: the one thing holding this at 4/5 then was the verification gap — the integrated behaviour (a real session restored after a daemon restart, fork/join edges, nested-Agent detail) rested on the author's macOS-only testing and wasn't observable from the diff. @wenshao has since closed that with a real Linux A/B run (report above): green unit suites, the contract confirmed in the actual request body, the live DAG topology and blocked derivation correct, the plan-mode gate working without leaking into approval keyboard handling, and the workflow surviving a full daemon restart. That's the evidence I was missing, and it matches what the code promised.

What keeps it at 4/5 rather than an unreserved 5/5 is no longer doubt about whether it works — it's the loose ends a maintainer should track before/after merge, all non-blocking:

  • F1 — the description outruns the diff. The body and Reviewer Test Plan step 5 describe a post-completion history story (collapsed snapshot entry, lazy sidecar lineage rebuild, "partial lineage" labeling) that isn't in this revision. The code that ships is correct; the description should be brought back in line so reviewers don't chase absent behavior.
  • The transcriptAdapter permission-content change applies to all daemon approvals, not just exit_plan_mode — likely fine, possibly even nicer, but it's a broadening that deserves an explicit "intended" or a one-line gate.
  • The webui planId/sourceCallId/blockedBy fields are populated and exported but unread in-repo — confirm they're deliberate public-SDK surface.
  • The App.tsx unconditional transcript scan and the todos.ts null-planId early-stop are small correctness/perf edges worth a follow-up.

None of these change the verdict; they're the kind of thing to fold into a fast-follow or address in a quick description edit.

On authorship and the gate: the author (@yiliang114) is a /packages/core/ code owner with admin permission, so this is a maintainer-authored PR — the two-tier core gate is exempt and the fork-refactor approval guardrail does not apply (it's a feat, not a refactor). The size advisory stands as advice only.

On the approval itself: my prior run already posted an APPROVED review pinned to exactly this commit (2f0b685), and CI is fully green on it — the decisive Test (ubuntu-latest, Node 22.x) unit suite has completed success with nothing pending. So there is nothing to defer and no fresh approval to add: stacking a second approval on the same commit would be noise, not signal. @wenshao has also approved. This re-run confirms the verdict against the current head and refreshes the staged comments; the approval stands.

中文说明

置信度:4/5——一个扎实、测试充分的 feature,完全做到了路线图 issue 的要求;我上次留下的验证缺口已由一位维护者的端到端 A/B 运行弥合,剩下的只有非阻塞的小问题(已在代码审查评论中点名),以及一份应与 diff 对齐的描述。

退一步看:这确实是好活,而且现在确实被验证过了。我在深入 diff 之前形成的独立判断——给 todo_write 稳定 ID 与依赖边、给 Agent 工具一个关联键、再在展示层基于现有 task registry 把两者关联起来——正是这个 PR 走的路,而且执行得很细致。它符合 issue 的核心约束(无调度器、无图存储、无新接口),并全程保持向后兼容。todoWrite.ts 的校验(迭代 Kahn、完整边界矩阵、两个入口共享)与测试覆盖(约 2,131 行)都比一般水平更强。六个月后让我维护它,我不会有意见。

自上次审查以来的变化:上次把本 PR 压在 4/5 的唯一原因,是验证缺口——集成层面的行为(daemon 重启后恢复真实 session、fork/join 连线、嵌套 Agent 详情)依赖作者在 macOS 上的本地测试,无法从 diff 观察到。@wenshao 此后用一次真实的 Linux A/B 运行弥合了它(报告见上):单测套件全绿、契约在真实请求体中得到确认、实时 DAG 拓扑与 blocked 推导正确、plan-mode 门禁正常且不泄漏进审批键盘处理、workflow 在完整 daemon 重启后存活。这正是我之前缺的证据,且与代码所承诺的一致。

之所以仍是 4/5 而非毫无保留的 5/5,已不再是怀疑它能不能用——而是维护者在合并前后应当跟进的几个收尾项,全部非阻塞:

  • F1——描述超出了 diff。 正文与 Reviewer Test Plan 第 5 步描述了一套完成后的历史能力(折叠快照入口、懒加载 sidecar 层级重建、"部分层级"标记),这些不在本版本里。落地的代码是正确的;应把描述拉回一致,免得 reviewer 去追不存在的行为。
  • transcriptAdapter 的权限内容变更对所有 daemon 审批生效,而不只是 exit_plan_mode——大概率没问题,甚至可能更好,但这是一处范围扩大,值得一句明确的"有意为之"或一行门禁。
  • webuiplanId/sourceCallId/blockedBy 字段被填充并导出,但仓库内无人读取——请确认它们是有意的公共 SDK 面。
  • App.tsx 的无条件 transcript 扫描与 todos.ts 的 null-planId 提前停止,是值得跟进的小正确性/性能边界。

这些都不改变结论;属于可以放进快速跟进、或用一次描述编辑解决的范畴。

关于作者与门禁:作者(@yiliang114)是 /packages/core/ 的 code owner,拥有 admin 权限,因此这是维护者自己提交的 PR——两层核心门禁豁免,fork-refactor 批准护栏也不适用(它是 feat,不是 refactor)。规模建议仅作参考。

关于批准本身:我上一轮运行已经针对恰好这个 commit(2f0b685)发布了一条 APPROVED 审查,且 CI 在其上全绿——决定性的 Test (ubuntu-latest, Node 22.x) 单测套件已完成且为 success,没有任何待处理项。因此既无需推迟,也无需新增批准:在同一 commit 上再叠一条批准只会是噪声,而非信号。@wenshao 也已批准。本次重新运行针对当前 head 确认了结论,并刷新了各阶段评论;批准保持有效。

Qwen Code · qwen3.8-max-preview

Reviewed at 2f0b68535319ee062d634e335cf41ddbbea34709 · re-run with @qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship — CI landed green after the review. ✅

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-preview via Qwen Code /review

// Skip tool_call_update event for TodoWriteTool
// Still log and return function response for LLM
} else {
} else if (!isTodoWriteTool) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Tautological guard: else if (!isTodoWriteTool) is the direct alternative to if (isTodoWriteTool), and isTodoWriteTool is a const (line 7420), so on entry to this branch it is provably false — exactly equivalent to a plain else. — Concrete cost: the redundant negation implies a code path where isTodoWriteTool could be true here, which cannot exist; a maintainer may waste time hunting the imagined third case or "correct" surrounding logic under a false assumption about the control flow.

Suggested change
} else if (!isTodoWriteTool) {
} else {
中文说明

冗余判断:else if (!isTodoWriteTool)if (isTodoWriteTool) 的直接分支,而 isTodoWriteToolconst(第 7420 行),进入此分支时它必然为 false,完全等价于普通的 else。具体代价:这个多余的否定暗示此处可能存在 isTodoWriteTool 为 true 的代码路径,而该路径并不存在;维护者可能浪费时间寻找这个想象中的第三种情况,或在错误理解控制流的前提下“修正”周边逻辑。

— qwen3.8-max-preview via Qwen Code /review

Comment on lines +230 to +232
const blocked = (todo.blockedBy ?? []).some(
(id) => todosById.get(id)?.status !== 'completed',
);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] A blockedBy reference to a non-existent todo makes a node permanently "blocked". todosById.get(id)?.status !== 'completed' evaluates undefined !== 'completed'true for an unknown id, so the node is badged blocked forever. But layerPlanTodos (~line 56, byId.has(dependencyId)) and the topology/edge-drawing (~line 331, knownIds.has(dependencyId)) both filter out unknown ids — so the node is laid out as unblocked (correct layer, no incoming edge) yet labelled blocked, a contradiction the user cannot resolve. — Failure scenario: a plan is revised and todo "3" removed while todo "5" still carries blockedBy: ["3"]; the layout ignores the stale ref but the status stays "blocked" permanently. Filter to known ids first, consistent with the other two sites:

Suggested change
const blocked = (todo.blockedBy ?? []).some(
(id) => todosById.get(id)?.status !== 'completed',
);
const blocked = (todo.blockedBy ?? []).some(
(id) => todosById.has(id) && todosById.get(id)!.status !== 'completed',
);
中文说明

指向不存在 todo 的 blockedBy 引用会让节点永久处于 “blocked” 状态。对未知 id,todosById.get(id)?.status !== 'completed' 会得到 undefined !== 'completed'true,因此该节点会一直被标记为阻塞。但 layerPlanTodos(约第 56 行,byId.has(dependencyId))与拓扑/连线绘制(约第 331 行,knownIds.has(dependencyId))都会过滤掉未知 id——所以该节点在布局上显示为未阻塞(正确的层级、无入边),却被标记为阻塞,形成用户无法解决的矛盾。失败场景:计划被修订、todo “3” 被删除,而 todo “5” 仍带有 blockedBy: ["3"];布局忽略了这个过期引用,但状态会永久停留在 “blocked”。建议与其它两处保持一致,先过滤到已知 id。

— qwen3.8-max-preview via Qwen Code /review

'planExecution.stepDetails': 'Step details',
'planExecution.subagents': 'Subagents',
'planExecution.openDetails': 'Open subagent details',
'planExecution.view': 'View plan execution',

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Dead i18n key: planExecution.view is declared in both EN (here) and ZH (~line 4646) but referenced by no t(...) call site anywhere — a repo-wide grep finds only the two definitions, and no dynamic key construction reaches it. The only new element that opens the plan view (the TodoPanel progress button) labels itself via summaryAriaLabel ("Step X / Y"), not this string. — Concrete cost: Messages is an untyped Record<string, MessageValue>, so nothing flags the dead key; it must be translated and kept in sync across locales indefinitely and misleads readers into thinking a "View plan execution" affordance exists. Remove it from both locales (or wire it into the TodoPanel button if an action label was intended):

// remove from both EN (~2165) and ZH (~4646):
'planExecution.view': 'View plan execution',
'planExecution.view': '查看计划执行',
中文说明

死 i18n key:planExecution.view 在英文(此处)和中文(约第 4646 行)都有声明,但整个仓库没有任何 t(...) 调用引用它——全仓搜索只找到这两处定义,也没有动态拼接 key 能到达它。唯一会打开计划视图的新元素(TodoPanel 进度按钮)用 summaryAriaLabel(“Step X / Y”)作为标签,并非这个字符串。具体代价:Messages 是无类型的 Record<string, MessageValue>,因此没有任何机制能标记这个死 key;它必须被翻译并在各语言间长期保持同步,还会误导读者以为存在一个 “View plan execution” 入口。建议从两种语言中删除(或如果本想用一个操作标签,则将其接到 TodoPanel 按钮上)。

— qwen3.8-max-preview via Qwen Code /review

@wenshao

wenshao commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Review — feat: visualize ordinary-session plan execution

Overview. The PR threads an optional dependency graph through the existing Todo → ACP plan → Web Shell pipeline: todo_write gains validated blockedBy edges and a persisted planId; the Agent tool gains an optional todo_id; plan metadata rides in _meta.qwenTodoPlan / _meta.qwenTodo so _meta-ignoring clients are unaffected; and Web Shell adds PlanExecutionView, mounted in the Tasks dialog and in the exit_plan_mode approval panel. The layering is sound — one source of truth per concern, no new endpoint, no graph store, no graph library — and the validation/replay work is genuinely careful (iterative Kahn's algorithm on both sides, no recursion, id+content todo identity preserved). Test coverage is strong: a full todoWrite validation matrix (duplicate / self / unknown / cycle / oversized ids), plan-identity lifecycle tests, and 22 PlanExecutionView cases including CSS-scaled measurement, cross-layer edge routing, remeasure avoidance on task polling, and the dense-plan cap.

Findings below, roughly by severity.

Description vs. diff

  • The PR body describes features I cannot find in this diff. PlanExecutionView has exactly two mount points — the Tasks dialog and the exit-plan approval. There is no collapsed workflow entry beside persisted Todo snapshots, no lazy rebuild of root executions from the parent transcript, no descendant lineage read from Agent sidecars, and no "partial lineage" labeling; grepping for lineage/hydrat in web-shell/webui turns up nothing related. Reviewer Test Plan step 5 (restart the daemon, expand a historical snapshot) therefore can't be exercised against this branch. Either that work landed earlier or it's out of scope here — please align the description and test plan with what this diff actually ships, otherwise reviewers will chase absent behavior.

Major

  • packages/webui/src/daemon/session/* — the new fields are written but never read. DaemonTodoList.planId, DaemonTodoList.sourceCallId, and DaemonTodoItem.blockedBy are populated in selectors.ts and exported through daemon-react-sdk.ts, but no read site exists anywhere in the repo (Web Shell uses its own parallel implementation in client/utils/todos.ts). If these are deliberate public-SDK surface for out-of-repo consumers, say so; otherwise they're dead additions and the whole webui selector change set is unconsumed.

  • transcriptAdapter.ts — the permission-content change is not scoped to exit_plan_mode. getPermissionContent replaces the previous unconditional [{ text: perm.title }] for every permission request. Previously contentText === request.title always held, so showsCommandBlock was false and no <pre> rendered; now any permission whose tool call carries a text content block gets a new content block in the approval panel. That's a UI change across all daemon approvals, and the only test covers the exit-plan case. Recommend gating on exit_plan_mode (the consumer already computes isExitPlanApproval), or explicitly confirming the change is wanted for other tools — the non-plan path also renders into .content, whose -webkit-line-clamp is the only length bound.

  • selectors.ts — empty todo lists are now retained. if (!items || items.length === 0) became if (!items), so a cleared plan is emitted as a DaemonTodoList with items: []. That's needed for selectDaemonActiveTodoList to clear correctly, but selectDaemonTodoLists is also the history list. Please confirm no consumer renders an empty snapshot card now.

  • App.tsxapprovalPlanTodos is computed unconditionally. ChatPane gates getLatestActiveTodos(messages) behind isExitPlanApproval; App does not, so a second full transcript scan (every message × every tool, with extractTodosFromToolCall per tool) runs on every messages identity change, i.e. every streaming tick, on top of the existing getFloatingTodos scan. Gate it the same way ChatPane does.

  • todos.tsgetAgentToolsForPlan degrades when planId is null. With plan.planId === null (which getFloatingTodos hardcodes for role === 'plan' messages), the loop returns at the first subsequent todo snapshot, so any Agent launched after a plan revision drops out of the graph on that path. If plan-role messages are reachable in the daemon Web Shell, plumb the plan id through them too; if they're a legacy-only path, a comment saying so would save the next reader the trace.

Minor

  • Session.ts:8500} else if (!isTodoWriteTool) { is redundant; the else already implies it. Leftover from the edit.

  • Session.ts:7454 — widening setCallId from MONITOR-only to every invocation is the right fix (it's what feeds AgentTask.toolUseId for the live join, and coreToolScheduler.ts:1887 already duck-types it identically), but the diff deletes the only signal of intent. One line on why every invocation now gets its call id would help.

  • transcript-replay.tsextractTranscriptTodoPlan changes the exported extractTranscriptTodos contract. The new if (resultDisplay !== null && resultDisplay !== undefined) return null; means a non-todo result display (e.g. an error string) no longer falls back to args.todos. That's the desired behavior for a failed write, and the doc comment was updated on extractPlan — but extractTranscriptTodos is exported from @qwen-code/acp-bridge and its own contract silently narrowed. Worth a note at the function.

  • todoWrite.ts — plan-identity edge case. startsNewPlan requires finalTodos.some(status !== 'completed'), so a brand-new plan written entirely as completed immediately after a completed plan reuses the previous planId and joins two unrelated plans. Rare, but cheap to close by also comparing the id sets.

  • todoWrite.ts — no maxItems bound. Ids are capped at 500 chars, but neither blockedBy nor todos has a length cap, and blockedBy is echoed verbatim into _meta.qwenTodo for every ACP client. A maxItems on blockedBy would bound both the sidecar and the wire payload.

  • todoWrite.ts — the modified_by_user path now validates. Previously externally-edited content was persisted verbatim; now an invalid edit throws. Good change, but untested — a case for an invalid external edit would lock it in.

  • PlanExecutionView.tsxMAX_RENDERED_PLAN_EDGES truncates silently. Above 500 edges the SVG is skipped entirely with no user-visible notice, which reads against the PR's "explicitly report partial lineage when a safety limit is reached."

  • PlanExecutionView.tsxlayerPlanTodos can produce a sparse array. If every node sits in a cycle, maxDepth stays 0 and all nodes land at depth 1, leaving a hole at layers[0]. React tolerates it, but client-side blockedBy comes from replayed _meta and is never re-validated, so the input isn't guaranteed acyclic — building the layer array densely would be more robust.

Nits

  • planExecution.view is added to both EN and ZH but never referenced — remove it.
  • key={JSON.stringify([edge.from, edge.to])} reads better as a template literal.
  • taskIndex.nestedByRootId is mutated during render. It's an idempotent cache keyed to a memoized object so it's safe, but a one-line comment saying that would preempt the question.
  • useStableArray's key now allocates a JSON string per todo per render; `${id}:${status}:${content}:${(blockedBy ?? []).join(',')}` does the same job.

Security & conventions

No new exec, network, or filesystem surface. Input bounds are mostly right (500-char todo_id and todo/dependency ids, validated before any write, and the write still happens only after validation). _meta additions are purely additive and older clients keep receiving standard ACP plan entries. House style is followed — ESM, no any, collocated tests, PascalCase.tsx for the new component, both locales updated.

Excluding TODO_WRITE from EXCLUDED_TOOLS_FOR_SUBAGENTS / EXCLUDED_TOOLS_FOR_TEAMMATES is justified — subagents share the session's Todo sidecar and would clobber the parent plan, and no built-in agent definition references the tool, so there's no prompt/tool mismatch. But it does remove a tool from every subagent and teammate, which is a user-visible behavior change; "Breaking changes: none" should probably mention it.

@wenshao

wenshao commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Local verification report — real build, real daemon, real browser

I verified this PR end-to-end on Linux with a fresh npm ci + npm run build, a real qwen serve daemon (serving the production packages/web-shell/dist), a real Chromium driven by Playwright, and a mock OpenAI-compatible endpoint so the tool sequence is deterministic. Every claim below is backed by an assertion or a screenshot from that run; the A/B "before" column comes from rebuilding the same worktree with only this PR's files reverted to the merge base.

Verdict: the shipped behaviour works and is solid. The PR description, however, still describes an earlier revision — see F1. Nothing here blocks merging the code; F1 asks for a description/test-plan correction (or the missing feature back).

Environment & method
PR head 2f0b685353
Merge base 07c832ce37 (true scope: 51 files under packages/)
Node v22.22.2, Linux
Build npm ci + npm run build in a dedicated worktree (no symlinked node_modules)
Daemon node packages/cli/dist/index.js serve --port … --workspace … --no-open
UI production packages/web-shell/dist, driven by Playwright 1.58.2 / Chromium
Model local mock OpenAI SSE endpoint (agent, todo_write, exit_plan_mode emitted on demand)

A/B method. In one worktree: git checkout <merge-base> -- packages docs + delete the three new PlanExecutionView.* files → npm run build → same daemon, same mock, same script. Then git checkout HEAD -- packages docs → rebuild. The "before" and "after" numbers come from identical driver runs. The live build was confirmed each time by grepping the built bundle for feature strings (Plan execution, data-plan-workflow, …).

Scenario. One prompt produces a diamond plan — t1 → (t2, t3) → t4t5 — with t1 completed, t2/t3 in_progress, t4/t5 pending. Then three top-level background agent calls: two carrying todo_id: t2 / todo_id: t3, one deliberately without todo_id.

1. Unit suites at PR head — all green

Package Files Tests
coretodoWrite, agent, agent-core, config.workflow-registration, prompts 5 422
cliPlanEmitter, tool-call-emitter, history-replayer, Session, SubAgentTracker 5 649
web-shellPlanExecutionView, todos, ToolApproval, TodoPanel, TasksStatusMessage, transcriptAdapter, ChatPane 7 203
sdk-typescriptdaemonUi 1 279
acp-bridgetranscript-replay 1 17
webuiDaemonSessionProvider.subagent, selectors 2 11
Total 21 1581 passed, 0 failed

2. The contract actually reaches the model

Captured from the real request body the daemon sent (not from source):

This PR Merge base
todo_write item properties content, status, id, blockedBy content, status, id
blockedBy guidance in tool description present absent
agent.todo_id declaration {type: string, maxLength: 500} not declared
todo_id guidance in agent description 1 line 0 lines

Persisted transcript (durable side) carries the identity too:

"resultDisplay":{"type":"todo_list","planId":"6ada526b-…","todos":[
  {"id":"t1","content":"Design the release data schema","status":"completed"},
  {"id":"t2","content":"Implement the API layer","status":"in_progress","blockedBy":["t1"]}, …
"functionCall":{"name":"agent","args":{…,"run_in_background":true,"todo_id":"t2"}}

3. Live workflow — before vs after

Assertion (same script, same mock) Merge base This PR
section[aria-label="Plan execution"] 0 1
[data-plan-node-id] 0 5 (t1…t5)
path[data-plan-edge] 0 5 — t1→t2, t1→t3, t2→t4, t3→t4, t4→t5
Node statuses t1 completed, t2 running, t3 running, t4 blocked, t5 blocked
Todo panel is a button no (plain <span>) yes (Step 2 / 5)
Executions nested under their node t2, t3 each show their live agent
agent without todo_id lands in Unassigned executions

The fork/join topology, the layering, and the blocked derivation (a node is blocked while any blockedBy target is not completed) all came out exactly right, and live task status correctly wins over the Todo status (t2/t3 show Running, not In progress, because their agents are live).

After — live DAG with per-node executions and the unassigned bucket:

Live plan-execution DAG

Before (merge base) — same session, same plan: a flat list, no dependencies, no linkage, not clickable:

Base: flat todo list

Scrolled right — the join edges t2→t4, t3→t4, t4→t5:

Join edges

4. Plan-mode gate (Reviewer Test Plan steps 1–2)

Switching the composer to Plan and letting the model call todo_write then exit_plan_mode produces exactly the intended surface: the submitted plan markdown and the dependency graph inside one approval, with the normal Reject / allow options untouched. Clicking a node inside the approval selects it and shows Step details — it does not leak into the approval's keyboard/confirm handling (the data-plan-interactive guard works): the approval stayed open (approvalStillOpen: true) and no option was confirmed.

exit_plan_mode approval with the workflow

5. Durability (Reviewer Test Plan steps 4–5, live half)

Step Result
Browser reload + transcript replay same 5 nodes, same 5 edges, executions still attached
Full daemon restart (cold replay from disk) same 5 nodes, same 5 edges; the three agents come back as Paused, and the nodes correctly downgrade Running → Paused
Click an execution inside a node opens the real Agent detail panel (prompt, running badge, Stop)

After a full daemon restart

Agent detail opened from a plan node


Findings

F1 — The PR description (and Reviewer Test Plan step 5) describes code that is not in this revision

The body promises a post-completion history story:

Completed sessions keep a collapsed workflow entry beside each persisted Todo snapshot. Expanding it lazily rebuilds root Agent executions from the parent transcript and lightweight descendant lineage from existing Agent sidecars […] Historical indexes are runtime-scoped, bounded, short-lived, and explicitly report partial lineage when a safety limit is reached […] the UI labels truncated or failed history hydration instead of presenting partial data as complete.

None of that exists in git diff 07c832ce37 2f0b685353 -- packages/. There is no sidecar lineage loader, no bounded/short-lived historical index, and no truncation label. (The single sidecar hit in the diff is a comment in agent-core.ts about the Todo sidecar file.) This looks like text left over from before f251be909 refactor: simplify session plan execution workflow.

Behaviourally, the entry point this PR adds is gated on the plan being active:

// App.tsx
const nextTodoPanelMode =
  connection.catchingUp || floatingTodos.length === 0 || floatingTodosAllCompleted
    ? 'hidden' : 'active';

onOpen={showFloatingTodos ? openTasksPanel : undefined}

Measured on a real session that runs the plan to completion:

Step N / M button while the plan is running present
Step N / M button after every todo is completed gone

getFloatingTodos() additionally returns empty once any user message follows the last snapshot, so planTodos becomes [] and PlanExecutionView renders null from then on. The tasks dialog can still be opened through pre-existing routes and would show the graph while floatingTodos is non-empty, but there is no per-snapshot historical entry anywhere in the diff.

Consequence: Reviewer Test Plan step 5 is not reproducible at this head, and neither are the two "After" screenshots in the description (both show a restored completed session).

Suggested resolution — either is fine, but please pick one before merging:

  • trim the description + test plan to the scope that actually shipped (pre-execution gate + live execution + reload/restart-while-active), replacing the two stale screenshots; or
  • restore the historical snapshot entry point.

F2 — todo_id is the only unbounded field in the subagent projection (nit)

packages/webui/src/daemon/session/DaemonSessionProvider.tsx:

const subagentType = boundedString(rawInput?.['subagent_type'], 120);
const prompt      = boundedString(rawInput?.['prompt'], 240);
const description = boundedString(rawInput?.['description'], 240);
const todoId = typeof rawInput?.['todo_id'] === 'string' ? rawInput['todo_id'] : undefined;

AgentTool.validateToolParams caps todo_id at 500 chars, but this projection also runs over tool-call events that never went through that validation. Suggest boundedString(rawInput?.['todo_id'], 500) for consistency with its siblings.

F3 — The DAG is clipped on open in the fixed-width dialog (UX)

.dagViewport is overflow-x: auto, so nothing is lost — but the dialog does not widen with the window. Measured at a 2400 px viewport: clientWidth: 688, scrollWidth: 1148. A 5-layer plan therefore opens with t4/t5 off-screen, and the join edges — arguably the most informative part of the graph — are the first thing hidden. Worth considering a wider dialog when hasDependencies, or a fit-to-width transform on the canvas.

F4 — Duplicate "Yes, allow once" in the exit_plan_mode approval (pre-existing, not from this PR)

Visible in the screenshot above. ToolApproval's option list is untouched by this PR, so this comes from the permission options the daemon sends for exit_plan_mode; flagging it only because this PR puts that panel front and centre.


Things I specifically tried to break, and couldn't

  • Cycle / self-dependency / unknown-dependency / duplicate IDs — the new validateTodos is a proper Kahn topological sort and rejects all of them with clear messages; layerPlanTodos also degrades safely on cyclic legacy data (cycle members are pushed into a trailing layer, no infinite loop).
  • Subagent plan pollutionTODO_WRITE is now excluded for subagents and teammates, and emitResult returns early on params.subagentMeta, so a child can no longer overwrite the parent's visible plan.
  • Failed todo_write — no longer emits a phantom plan update (if (!params.success) return, plus extractPlan(returnDisplay, succeeded ? args : undefined)).
  • agent without todo_id, and todo_id pointing at an unknown node — both degrade to Unassigned executions rather than silently vanishing.

Nice work — the live and pre-execution halves are genuinely solid, and the "one source of truth per concern, no new graph store" design holds up under a cold daemon restart.

中文版

本地验证报告 —— 真实构建、真实 daemon、真实浏览器

我在 Linux 上做了完整的端到端验证:全新 npm ci + npm run build,真实 qwen serve(对外提供生产版 packages/web-shell/dist),Playwright 驱动的真实 Chromium,以及一个 mock 的 OpenAI 兼容端点来保证工具调用序列确定。下面每一条结论都有断言或截图支撑;A/B 的「改动前」一列,是把同一个 worktree 里本 PR 涉及的文件回退到 merge base 后重新构建得到的。

结论:已实现的行为是可靠的,可以合入。但 PR 描述仍停留在更早的版本 —— 见 F1。 F1 不阻塞代码合入,只是希望修正描述/测试计划(或把缺失的功能补回来)。

环境与方法
PR head 2f0b685353
Merge base 07c832ce37(真实范围:packages/ 下 51 个文件)
Node v22.22.2,Linux
构建 独立 worktree 中 npm ci + npm run build(不使用软链 node_modules
Daemon node packages/cli/dist/index.js serve --port … --workspace … --no-open
UI 生产版 packages/web-shell/dist,Playwright 1.58.2 / Chromium 驱动
模型 本地 mock OpenAI SSE 端点(按需产出 agenttodo_writeexit_plan_mode

A/B 方法。 同一个 worktree 内:git checkout <merge-base> -- packages docs 并删除新增的三个 PlanExecutionView.* 文件 → npm run build → 同一个 daemon、同一个 mock、同一份脚本跑一遍;之后 git checkout HEAD -- packages docs 再重建。两侧数据来自完全相同的驱动脚本。每次都通过 grep 构建产物里的特征串(Plan executiondata-plan-workflow 等)确认当前生效的是哪一版。

场景。 一次 prompt 产出菱形计划 —— t1 → (t2, t3) → t4t5,其中 t1 已完成、t2/t3 进行中、t4/t5 待办。随后发起三个顶层后台 agent:两个分别带 todo_id: t2 / todo_id: t3,另一个刻意不带 todo_id

1. PR head 上的单测 —— 全绿

文件 用例
coretodoWriteagentagent-coreconfig.workflow-registrationprompts 5 422
cliPlanEmittertool-call-emitterhistory-replayerSessionSubAgentTracker 5 649
web-shellPlanExecutionViewtodosToolApprovalTodoPanelTasksStatusMessagetranscriptAdapterChatPane 7 203
sdk-typescriptdaemonUi 1 279
acp-bridgetranscript-replay 1 17
webuiDaemonSessionProvider.subagentselectors 2 11
合计 21 1581 通过,0 失败

2. 新契约确实到达了模型

以下取自 daemon 真实发出的请求体(不是从源码推断):

本 PR Merge base
todo_write 条目字段 content, status, id, blockedBy content, status, id
工具描述里的 blockedBy 指引
agent.todo_id 声明 {type: string, maxLength: 500} 未声明
agent 描述里的 todo_id 指引 1 行 0 行

持久化 transcript(durable 侧)同样带上了计划身份:

"resultDisplay":{"type":"todo_list","planId":"6ada526b-…","todos":[
  {"id":"t1","content":"Design the release data schema","status":"completed"},
  {"id":"t2","content":"Implement the API layer","status":"in_progress","blockedBy":["t1"]}, …
"functionCall":{"name":"agent","args":{…,"run_in_background":true,"todo_id":"t2"}}

3. 运行期 workflow —— 改动前后对比

断言(同脚本、同 mock) Merge base 本 PR
section[aria-label="Plan execution"] 0 1
[data-plan-node-id] 0 5(t1…t5
path[data-plan-edge] 0 5 —— t1→t2, t1→t3, t2→t4, t3→t4, t4→t5
节点状态 t1 completedt2 runningt3 runningt4 blockedt5 blocked
Todo 面板可点击 否(普通 <span> 是(Step 2 / 5
执行挂在对应节点下 t2t3 各自显示其实时 agent
没有 todo_idagent 落到 Unassigned executions

fork/join 拓扑、分层、以及 blocked 的推导(只要有任一 blockedBy 目标未 completed 即为阻塞)全部正确;实时 task 状态也正确覆盖了 Todo 状态(t2/t3 显示 Running 而不是「进行中」,因为它们的 agent 在跑)。

改动后 —— 带节点内执行与未关联执行的实时 DAG:

Live plan-execution DAG

改动前(merge base)—— 同一会话、同一计划:一个扁平列表,没有依赖、没有关联、不可点击:

Base: flat todo list

右滚后 —— 汇聚边 t2→t4t3→t4t4→t5

Join edges

4. Plan Mode 门禁(Reviewer Test Plan 第 1–2 步)

把 composer 切到 Plan,让模型先 todo_writeexit_plan_mode,得到的正是设计中的形态:同一个审批面板里同时呈现提交的计划正文与依赖图,原有的 Reject / 放行选项不受影响。在审批面板内点击节点会选中它并展示 Step details,且不会被审批的键盘/确认逻辑吞掉(data-plan-interactive 守卫生效):审批面板保持打开(approvalStillOpen: true),没有任何选项被确认。

exit_plan_mode approval with the workflow

5. 持久性(Reviewer Test Plan 第 4–5 步的运行期部分)

步骤 结果
浏览器刷新 + transcript 回放 5 个节点、5 条边不变,执行仍挂在节点下
daemon 完全重启(从磁盘冷回放) 5 个节点、5 条边不变;三个 agent 恢复为 Paused,节点也正确从 Running 降级为 Paused
点击节点内的执行 打开真实的 Agent 详情面板(prompt、running 标记、Stop

After a full daemon restart

Agent detail opened from a plan node


问题清单

F1 —— PR 描述(以及 Reviewer Test Plan 第 5 步)描述的是本次未包含的代码

描述里承诺了「完成后可回看」的能力:

Completed sessions keep a collapsed workflow entry beside each persisted Todo snapshot. Expanding it lazily rebuilds root Agent executions from the parent transcript and lightweight descendant lineage from existing Agent sidecars […] Historical indexes are runtime-scoped, bounded, short-lived, and explicitly report partial lineage when a safety limit is reached […]

git diff 07c832ce37 2f0b685353 -- packages/ 里没有这些代码:没有 sidecar 层级加载器、没有有界/短时的历史索引、也没有截断提示。(diff 中唯一的 sidecar 命中,是 agent-core.ts 里关于 Todo sidecar 文件 的一句注释。)这看起来是 f251be909 refactor: simplify session plan execution workflow 之前留下的文案。

从行为上看,本 PR 新增的入口是以计划处于活跃状态为前提的:

// App.tsx
const nextTodoPanelMode =
  connection.catchingUp || floatingTodos.length === 0 || floatingTodosAllCompleted
    ? 'hidden' : 'active';

onOpen={showFloatingTodos ? openTasksPanel : undefined}

在一个真实跑到计划全部完成的会话上实测:

计划运行中的 Step N / M 按钮 存在
所有 todo 变为 completed 之后 消失

此外,只要最后一次快照之后出现了新的用户消息,getFloatingTodos() 就返回空,planTodos 变成 []PlanExecutionView 从此渲染为 null。通过既有的其它入口仍可打开 tasks 弹窗,并在 floatingTodos 非空期间看到图,但 diff 里没有任何「按快照回看」的历史入口。

结论:Reviewer Test Plan 第 5 步在当前 head 上无法复现,描述里那两张「After」截图(都展示的是恢复后的已完成会话)同样无法复现。

建议二选一,合入前确定一个:

  • 把描述与测试计划收敛到实际交付的范围(执行前门禁 + 运行期 + 活跃状态下的刷新/重启恢复),并替换那两张过期截图;
  • 把历史快照入口补回来。

F2 —— todo_id 是子智能体投影里唯一没有长度上限的字段(nit)

packages/webui/src/daemon/session/DaemonSessionProvider.tsx

const subagentType = boundedString(rawInput?.['subagent_type'], 120);
const prompt      = boundedString(rawInput?.['prompt'], 240);
const description = boundedString(rawInput?.['description'], 240);
const todoId = typeof rawInput?.['todo_id'] === 'string' ? rawInput['todo_id'] : undefined;

AgentTool.validateToolParams 会把 todo_id 限制在 500 字符,但这个投影同样会处理未经该校验的 tool-call 事件。建议改成 boundedString(rawInput?.['todo_id'], 500),与相邻字段保持一致。

F3 —— 固定宽度弹窗里 DAG 打开即被裁切(UX)

.dagViewportoverflow-x: auto,所以内容不会丢失;但弹窗不会随窗口变宽。在 2400px 视口下实测:clientWidth: 688scrollWidth: 1148。因此 5 层的计划打开时 t4/t5 在屏幕外,而汇聚边 —— 图里信息量最大的部分 —— 恰恰最先被藏起来。可以考虑在 hasDependencies 时放宽弹窗,或对画布做自适应缩放。

F4 —— exit_plan_mode 审批里出现两个「Yes, allow once」(既有问题,非本 PR 引入)

上面的截图可见。本 PR 没有改动 ToolApproval 的选项列表,所以这来自 daemon 为 exit_plan_mode 下发的权限选项;提出来只是因为本 PR 把这个面板放到了很显眼的位置。


我特意尝试攻击但没能攻破的点

  • 环 / 自依赖 / 未知依赖 / 重复 ID —— 新的 validateTodos 是标准的 Kahn 拓扑排序,全部会被拒绝且报错清晰;layerPlanTodos 对遗留的成环数据也能安全降级(环上的节点被推到末层,不会死循环)。
  • 子智能体污染父计划 —— TODO_WRITE 现已对 subagent 与 teammate 屏蔽,emitResult 也会在 params.subagentMeta 时提前返回,子任务无法再覆盖父会话的可见计划。
  • 失败的 todo_write —— 不再产生幽灵计划更新(if (!params.success) return,以及 extractPlan(returnDisplay, succeeded ? args : undefined))。
  • 没有 todo_idagent,以及指向不存在节点的 todo_id —— 都会降级到 Unassigned executions,而不是悄悄消失。

整体做得很好 —— 执行前与运行期这两块确实扎实,「每类数据一个事实来源、不新增图存储」的设计在 daemon 冷重启下也站得住。

@wenshao

wenshao commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run

Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

Scripted assertions: 40 passed · 0 failed · 40 total

中文 — 判定:✅ 通过 · 可合入(agent 判定)

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查

脚本断言:40 通过 · 0 失败 · 40 总计

Verification report

PR #7580 Deep Verification — feat: visualize ordinary-session plan execution

Verdict: merge-ready — 40/40 scripted assertions passed, 0 failed. Verified head 2f0b68535319ee062d634e335cf41ddbbea34709 (base tip bc382c3ff91636ffdaab2758f2cacbb2ff1a30e0). No blocking findings. The central data-layer claim is proven load-bearing by two independent A/B proofs against the base build, the new tests are proven non-vacuous, and every affected workspace gate is green.

中文摘要
  • 结论merge-ready。40/40 脚本化断言通过,0 失败;未发现阻塞性问题。
  • A/B 结论(均与 base 构建 HEAD^1 对照,驱动真实编译产物 dist/):
    • A/B pre-release: fix ci #1(核心 todo_write:依赖校验(环 / 自依赖 / 未知依赖 / id 超长 / blockedBy 超长)在 head 全部拒绝、base 全部接受;duplicate-id 两侧都拒绝(对照组,证明 harness 有判别力);planId 仅在 head 生成、跨同一计划更新保持稳定、计划全部完成后重新生成新 id。见 01-ab-todowrite-validation-planid.png
    • A/B Where is the config saved? #2(ACP wire 预言机):head 的 plan SessionUpdate_meta 中携带 qwenTodoPlan.id、每个 entry 携带 qwenTodo.idqwenTodo.blockedBy;base 的 entry 完全不带这些字段。即依赖边与计划身份确实"上线"到了 Web Shell。见 02-ab-wire-payload-blockedby-planid.png
  • 测试非空泛:仅禁用环检测守卫(最细突变)使恰好 1 个测试(should reject a 'cycle')失败,其余 42 个仍通过;还原后 43/43 绿。见 03-vacuity-cycle-mutation.png
  • 纵深防御:UI 渲染器 layerPlanTodos 对环 / 自环 / 悬空引用均能终止,5000 节点环 9ms 完成分层(线性)。即便恶意/遗留回放数据绕过核心校验器也不会卡死。见 04-cycle-safety-layerplantodos.png
  • 门禁:core todoWrite 43、cli emitters 82、acp-bridge replay 17、web-shell todos 77、cli session 层 567、webui selectors 11、web-shell 全量 2690 —— 全绿。
  • 未覆盖:逐 commit 归因(depth-2 浅克隆不可达)、完整 typecheck/lint(CI 已覆盖)、真实 daemon 端到端与 Plan Mode 审批面板的交互级验证(仅 jsdom 单测层面)、Windows/Linux。

Scope selection

This is a large feature PR (53 files, +3947/−225) whose visible surface is a Web Shell
"Session Workflow" DAG view. The DAG is only renderable if the data layer underneath it
carries node identity and dependency edges end to end. I therefore scoped the round to that
load-bearing foundation rather than the presentation:

  • Central claim: Todo plans carry a stable planId and per-node blockedBy dependency
    edges, validated in the core todo_write tool and delivered intact into the ACP
    SessionUpdate the Web Shell consumes.
  • Secondary claim 1: subagent todo_write results are isolated — they do not promote
    into a session-level plan (tool-call-emitter.ts guard).
  • Secondary claim 2: the renderer is safe against malformed graph data (cycles) that the
    core validator never saw (legacy / replayed transcripts).

Out of scope by design (listed under Not covered): the React rendering itself beyond its
own test suite, live-daemon E2E, and the Plan Mode approval interaction at the UI level.

Central claim — A/B load-bearing proof

Both arms drive the real compiled dist/ output (not source, not stubs). Base side is a
scratch worktree at HEAD^1 with only packages/core / packages/acp-bridge rebuilt; the
root node_modules is reused (the PR leaves package.json / package-lock.json untouched,
verified empty diff). Per the workspace-link hazard, each harness imports each tree's dist by
absolute path so relative deps resolve inside that tree — readlink -f node_modules/@&#8203;qwen-code/qwen-code-core resolves into the head tree, so importing by
package name would have silently loaded head code on the base arm; this was checked and
avoided.

A/B #1 — core todo_write: validation + plan identity

node ab-todowrite.mjs <dist> <head|base> instantiates the real TodoWriteTool, drives
tool.build() (runs validateToolParams) and invocation.execute() against a hermetic
QWEN_RUNTIME_DIR, and reads back the persisted file. Expectations are encoded per arm so
each line can fail; base's passes assert the absence of the behavior.

behavior head (PR) base (HEAD^1) flip?
cycle a→b→a rejected "must not contain a cycle" accepted
self-dependency a→a rejected "must not depend on itself" accepted
unknown dependency a→ghost rejected "references unknown dependency" accepted
id > 500 chars rejected "at most 500 characters" accepted
blockedBy item > 500 chars rejected "at most 500 characters" accepted
duplicate id (control) rejected rejected — (both; proves the harness discriminates)
valid independent plan accepted accepted — (no over-rejection)
empty todos accepted accepted
planId in result display f93d0936-… (UUID) absent
planId persisted to file same UUID absent
planId stable across same-plan update f93d… == f93d… n/a
planId renewed after all-completed plan a75b… != f93d… n/a

Head 12/12, base 12/12. Witness: evidence/01-ab-todowrite-validation-planid.png.

A/B #2 — wire oracle: the ACP SessionUpdate the Web Shell receives

node ab-wire.mjs <dist> <head|base> drives the real compiled transcript-replay module —
the code that produces the actual plan SessionUpdate sent over the wire.

Head payload (captured live):

{"sessionUpdate":"plan","entries":[
  {"content":"Task A","status":"completed","_meta":{"qwenTodo":{"id":"a"}}},
  {"content":"Task B","status":"pending","_meta":{"qwenTodo":{"id":"b","blockedBy":["a"]}}}],
 "_meta":{"qwenTodoPlan":{"id":"plan-XYZ"},"qwenTranscript":{"planToolCallId":"call-1"}}}

Base payload (captured live):

{"sessionUpdate":"plan","entries":[
  {"content":"Task A","status":"completed"},
  {"content":"Task B","status":"pending"}],
 "_meta":{"qwenTranscript":{"planToolCallId":"call-1"}}}
check head base
wire carries qwenTodoPlan.id ✅ present absent
wire carries per-entry qwenTodo.blockedBy ["a"] absent
extract round-trips planId / blockedBy drops both (base returns a bare array)
non-todo resultDisplay precedence ✅ returns null (no args fallback) falls back to args

Head 6/6, base 6/6. This is the smoking gun for the whole feature: on head the dependency
edge, node ids, and plan identity ride the wire; on base the entries are bare, so no DAG
could be drawn. Witness: evidence/02-ab-wire-payload-blockedby-planid.png.

Findings

No blocking findings. The central and secondary claims hold under adversarial input, and
every gate is green. Non-blocking observations a reviewer may note:

  1. (informational) Failed todo_write no longer emits a plan update. tool-call-emitter.ts
    adds if (!params.success) return;. Base emitted an empty plan when a failed write's
    args still held todos; head emits nothing, which preserves the last-good plan in the UI
    rather than wiping it. The failure itself still surfaces through the tool result
    (Todo list modification failed with error: …), so no information is lost — only an
    arguably-incorrect empty-plan emission is suppressed. Covered by the 62/62 emitter suite,
    which includes does not promote a subagent TodoWrite as the session plan (the
    if (params.subagentMeta) return; guard) and should not emit anything for TodoWriteTool with empty/no extractable todos.
  2. (informational) The PR body cites "43 and 479 tests" for the Todo and Web Shell suites.
    The 43 matches packages/core/src/tools/todoWrite.test.ts exactly; the full
    packages/web-shell suite is actually 2690 tests / 163 files, all green — a stronger
    result than the number quoted, not a discrepancy worth correcting in code.

Vacuity check — the new tests are load-bearing

Finest-grained mutation: in validateTodos, change the cycle guard
if (queue.length !== todos.length)if (queue.length !== todos.length && false),
disabling only cycle detection while leaving every earlier guard (self-dep, unknown-dep,
duplicate-dep, oversize) intact. Result against the real source via vitest:

  • mutated: 1 failed | 42 passed — exactly should reject a 'cycle' fails, with
    validateToolParams returning null where the test expects toContain('must not contain a cycle'). The intended assertion fails on the behavioral mismatch (not an import/compile
    break).
  • restored: 43/43 passed (positive control — the suite is green unmutated, so the
    harness can both pass and fail).

The single-test kill with 42 siblings surviving is the signature of a correctly-pinned guard:
the cycle test asserts exactly the clause this PR added. Witness:
evidence/03-vacuity-cycle-mutation.png.

Defense-in-depth — renderer cycle-safety

The core validator rejects cycles, but the Web Shell also renders replayed / legacy
transcripts the validator never saw
. I probed the real exported layerPlanTodos
(PlanExecutionView.tsx) with hostile graph shapes via a scratch vitest probe (added, run,
removed; tree confirmed clean afterward):

  • 2-cycle a↔b → terminates, both nodes bucketed into the fallback layer.
  • self-loop a→a → ignored (filtered by dependencyId !== todo.id).
  • dangling ref a→ghost → ignored (filtered by byId.has(...)).
  • 5000-node ring → layered in 9 ms (iterative Kahn's algorithm; cyclic nodes land at
    maxDepth + 1, never an infinite loop).

4/4 probe assertions passed. Witness: evidence/04-cycle-safety-layerplantodos.png.

Targeted gates (all green)

suite result
packages/core todoWrite.test.ts 43/43
packages/cli PlanEmitter.test.ts + tool-call-emitter.test.ts 82/82 (20 + 62)
packages/acp-bridge transcript-replay.test.ts 17/17
packages/web-shell utils/todos.test.ts 77/77
packages/cli Session + SubAgentTracker + history-replayer 567/567
packages/webui selectors + DaemonSessionProvider.subagent 11/11
packages/web-shell full package (incl. 797-line PlanExecutionView.test.tsx) 2690/2690, 163 files

Not covered

  • Per-commit attribution. The checkout is depth-2 (git rev-parse --is-shallow-repository
    = true); git rev-list --count HEAD^1..HEAD^2 returns the shallow-boundary artifact 1
    while the metadata snapshot lists 16 commits, so the intermediate commits are unreachable.
    I verified the aggregate HEAD^1..HEAD diff and did not exercise each commit's claim
    individually.
  • Full typecheck / lint. Covered by the PR's own CI; not re-run here. My base-side
    rebuilds emitted JS despite unrelated type-declaration resolution errors from the worktree
    context (@lydell/node-pty, mime/lite, fdir, ignore, ajv) — an environment artifact
    of building inside a nested worktree, not a defect in the PR; the emitted base dist was
    confirmed genuine base (0 blockedBy/planId/qwenTodoPlan references vs head's 10/7/1)
    before use.
  • Live-daemon end-to-end and the Plan Mode approval panel at the interaction level
    (Reviewer Test Plan steps 1–5: approval/rejection keeping the session in Plan Mode, live
    concurrent node states across a running daemon, daemon-restart recovery of a real session).
    These are exercised here only at the jsdom/unit level via the web-shell suite; the reviewer
    test plan's GUI flow was not driven against a real daemon.
  • The base-side wire replay calibration for a full historical transcript (the A/B proved
    the per-update wire shape, not a byte-for-byte replay of a persisted session).
  • Windows / Linux rendering (PR reports macOS only).

Methodology

Environment: the CI verify sandbox (node:22-bookworm, Node v22.23.2), merge-ref checkout
at depth 2 (HEAD = merge, HEAD^1 = base tip, HEAD^2 = verified PR head). npm ci and
npm run build had completed at head before the round. A/B controls were built in a scratch
worktree at tmp/base-tree (git worktree add tmp/base-tree HEAD^1), rebuilding only
packages/core and packages/acp-bridge; the root node_modules was reused (lockfile
unchanged) with the nested packages/core/node_modules and a base-local
@qwen-code/qwen-code-core symlink added so the base dist resolved base code and the correct
ajv@8 (root ajv is v6 and lacks dist/2020.js). Harnesses (ab-todowrite.mjs,
ab-wire.mjs) import each tree's compiled dist/ by absolute path and assert both the
head-presence and base-absence of each behavior; the cycle probe and vacuity mutation ran the
real source through vitest. Raw per-cell stdout/stderr and build logs are in logs/;
harness scripts are in this directory; image witnesses are in evidence/.

Evidence images

01-ab-todowrite-validation-planid

02-ab-wire-payload-blockedby-planid

03-vacuity-cycle-mutation

04-cycle-safety-layerplantodos

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

Qwen Code · sandboxed verification

@wenshao
wenshao added this pull request to the merge queue Aug 1, 2026
Merged via the queue into QwenLM:main with commit bd85599 Aug 1, 2026
86 checks passed
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Triage re-run completed without a new review.

The bot already has a review of its own on 2f0b68535319ee062d634e335cf41ddbbea34709, which still stands.

机器人在 2f0b68535319ee062d634e335cf41ddbbea34709 上已有自己的评审,且仍然有效。

The stage comments above were updated with the latest result. View workflow run.

上方各阶段评论已更新为最新结果。查看工作流运行

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.21.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review/self-reported The linked issue was opened by the PR author (self-reported)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: visualize ordinary-session plan DAG and link Todo nodes to subagent executions

4 participants