Skip to content

feat(web-shell): visualize and manage dynamic workflow runs - #10412

Closed
qqqys wants to merge 40 commits into
QwenLM:mainfrom
qqqys:feat/workflow-web-shell
Closed

feat(web-shell): visualize and manage dynamic workflow runs#10412
qqqys wants to merge 40 commits into
QwenLM:mainfrom
qqqys:feat/workflow-web-shell

Conversation

@qqqys

@qqqys qqqys commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Adds a capability-gated Workflow experience to Web Shell. Users can open a dedicated Runs page, browse saved workflows and live or historical runs, inspect phase and dispatch progress, view approvals and token usage, and pause, resume, cancel, retry, rerun, or delete history when the daemon exposes those controls. Workflow activity also appears in the existing task status surfaces and transcript tool details without changing the default task contract for older clients. A saved workflow can also be opened in place: its parsed meta (description, when to use, phases), script path, recent runs, and highlighted source are read through a new GET /session/:id/saved-workflows/:name route that fails closed to workflow: null for unknown names and untrusted workspaces.

This supersedes the closed drafts #8950 and #9807 (same branch history, reopened on a fresh branch after the daemon PR was reopened as #10411) and is stacked on #10411. The current diff therefore includes the daemon/API layer until #10411 merges; after that lands, this branch will merge main normally so the PR contains only the Web Shell layer. No force-push is planned. The branch is merged with main at d6533785bd, which includes the collapsible tool summaries of #10231: every transcript tool row is expandable there, so the earlier workflow-specific hasExpandableContent rule is gone and only the detailsVisible control for the inline execution view remains.

Why it's needed

Dynamic workflows currently run without a first-class Web Shell surface, leaving users unable to see orchestration progress or safely manage a live or historical run. This completes the visualization and control layer tracked by #8941 while preserving the opt-in compatibility boundary introduced by #10411.

Reviewer Test Plan

How to verify

  • With Workflows enabled in a trusted non-bare workspace, open Workflows from the sidebar or submit /workflows; expect Saved, Running, and History tabs with accurate counts.
  • Click a saved workflow's name; expect its description, phases, path, recent runs, and a folded source view. Rename or delete the file and refresh; expect the detail to close. In an untrusted workspace, expect the read to report the definition as unavailable rather than leak the script.
  • Start a saved workflow and inspect a run; expect phase visits, dispatch dependencies and status, approvals, logs, and token usage to update without stale responses crossing session changes.
  • Exercise pause/resume/cancel on a live run and retry/rerun/delete-history on eligible terminal runs; expect unavailable or stale actions to fail visibly without mutating another session.
  • Disable Workflows or use a session whose capability snapshot does not enable them; expect the sidebar entry and local /workflows page to remain unavailable.
  • Confirm ordinary agent, shell, and monitor task surfaces continue to behave as before.

Evidence (Before & After)

Before: Web Shell has no dedicated Workflow navigation, live execution graph, saved-run browser, or Workflow controls.

After: The new capability-gated Runs page and task details expose Workflow progress and controls. Live macOS browser validation against a bearer-authenticated local daemon covered the Saved entry, launch action, live phase and dispatch timing, Pause/Stop controls, completed History, and token usage. The execution graph uses a flat visual language (one surface, hairline dividers, status carried by a single accent per node/edge; no gradients or shadows) and renders in both themes; the captures below are 1280×800 Web Shell pages from the visuals harness (mock daemon, fixture runs), so they also show the sidebar entry and the New workflow action.

Saved workflows

Workflows page, Saved tab: three saved workflows with scope badges, Run actions, and the New button

Saved workflow definition

Saved workflow expanded in place: description, when to use, phases, recent runs, script path, and highlighted source

Running workflow

Workflows page, Running tab: expanded run with phase lanes, dispatch nodes and dependency edges, a pending approval, Pause/Stop controls

Completed workflow history

Workflows page, History tab: saved read-only run with three completed phases, run-history bar, and Delete saved run

Running workflow (dark theme)

Workflows page, Running tab in the dark theme

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux

Environment (optional)

macOS: Node.js 22.17.0; focused Web Shell Vitest suites and the production/library TypeScript build; live Web Shell at localhost:5174 against an isolated daemon at localhost:4270. Linux (this reopen): the 11 Web Shell test files this branch touches plus ToolGroup.test.tsx (829 tests), tsc --noEmit for webui and cli, and builds of core, webui, sdk-typescript after merging main; the web-shell typecheck error at client/App.tsx:8359 is present on main d6533785bd itself.

Risk & Scope

  • Main risk or tradeoff: The Workflow execution graph is a substantial UI surface. Live macOS validation covers a normal wide viewport and real daemon update timing; narrow layouts and unusually long labels remain to be checked.
  • Not validated / out of scope: Windows manual testing, Linux live-browser testing, narrow-layout coverage, and daemon/API behavior reviewed on feat(serve): expose Workflow tasks and controls #10411.
  • Breaking changes / migration notes: None. Workflow snapshots remain explicit opt-in; legacy task callers are unchanged.

Linked Issues

Closes #8941

Depends on #10411

Supersedes #9807

中文说明

本 PR 做了什么

为 Web Shell 增加受 capability 控制的 Workflow 体验。用户可以打开独立的 Runs 页面,浏览已保存的 Workflow、运行中记录和历史记录,查看阶段与 dispatch 进度、审批和 token 使用情况,并在 daemon 提供对应能力时执行暂停、恢复、取消、重试、重新运行和删除历史。Workflow 活动也会进入现有任务状态区和 transcript 工具详情,同时不改变旧客户端依赖的默认 task 合约。已保存的 Workflow 也可以就地展开查看:通过新增的 GET /session/:id/saved-workflows/:name 读取解析后的 meta(描述、适用场景、阶段)、脚本路径、最近运行和高亮源码;名字未知或 workspace 未受信任时统一返回 workflow: null,不暴露脚本。

本 PR 取代已关闭的 Draft #8950#9807(分支历史相同;daemon PR 重开为 #10411 后,本 PR 也在新分支上重开),并叠加在 #10411 之上。因此在 #10411 合入前,当前 diff 会包含 daemon/API 层;其合入后,本分支会通过普通方式合并 main,使 PR 只保留 Web Shell 层。不会使用 force-push。分支已合并到 maind6533785bd,其中包含 #10231 的可折叠工具摘要:那里每一行 transcript 工具都可展开,因此此前针对 workflow 的 hasExpandableContent 规则已不存在,只保留控制内联执行视图的 detailsVisible

为什么需要

动态 Workflow 目前在 Web Shell 中没有一等展示入口,用户无法查看编排进度,也无法安全管理运行中或历史 run。本 PR 完成 #8941 跟踪的可视化与控制层,同时保留 #10411 引入的显式 opt-in 兼容边界。

Reviewer 测试计划

如何验证

  • 在可信、非 bare 且开启 Workflows 的 workspace 中,从侧栏打开 Workflows 或提交 /workflows;应看到 Saved、Running 和 History 标签及准确计数。
  • 点击已保存 Workflow 的名字;应展开描述、阶段、路径、最近运行和折叠的源码。重命名或删除文件后刷新,详情应自动收起;未受信任的 workspace 中应提示定义不可用,而不是返回脚本。
  • 启动一个已保存 Workflow 并打开 run 详情;阶段访问、dispatch 依赖与状态、审批、日志和 token 使用应更新,切换 session 后旧响应不得污染新 session。
  • 对运行中 run 执行 pause/resume/cancel,对符合条件的终态 run 执行 retry/rerun/delete-history;不可用或过期操作应明确失败,且不得修改其他 session。
  • 关闭 Workflows,或使用 capability snapshot 未开启 Workflows 的 session;侧栏入口与本地 /workflows 页面都应不可用。
  • 普通 agent、shell 和 monitor 的任务展示与行为应保持不变。

证据(Before & After)

Before:Web Shell 没有独立的 Workflow 导航、实时执行图、已保存 run 浏览器或 Workflow 控制能力。

After:新增受 capability 控制的 Runs 页面和任务详情,展示 Workflow 进度与控制。已在 macOS 上连接带 bearer 认证的本地 daemon 完成浏览器验证,覆盖 Saved 入口、启动操作、实时 phase/dispatch 时序、Pause/Stop 控制、完成态 History 和 token 使用。执行图采用扁平视觉(单一底色、hairline 分隔、每个节点/边只用一种状态色,无渐变与投影),明暗主题均可用;上方五张截图为 visuals harness(mock daemon + fixture 运行)下 1280×800 的 Web Shell 整页,可同时看到侧栏入口和“New”新建按钮;更早的实机截图见验证评论

测试平台

OS 状态
🍏 macOS
🪟 Windows ⚠️
🐧 Linux

环境(可选)

Node.js 22.17.0;运行了 Web Shell 聚焦 Vitest 套件以及生产/library TypeScript build;Web Shell 使用 localhost:5174,连接隔离的 localhost:4270 daemon。 Linux(本次重开):本分支涉及的 11 个 Web Shell 测试文件加 ToolGroup.test.tsx(829 个用例)、对 webuicli 执行 tsc --noEmit、合并 main 后构建 corewebuisdk-typescriptweb-shellclient/App.tsx:8359 的类型错误在 main d6533785bd 上本身就存在。

风险与范围

  • 主要风险或取舍:Workflow 执行图是较大的 UI 表面。macOS 实机已覆盖普通宽屏布局和真实 daemon 更新时序;窄屏与异常长标签仍需验证。
  • 未验证/范围外:Windows 手工验证、Linux 真实浏览器验证、窄屏覆盖,以及在 feat(serve): expose Workflow tasks and controls #10411 上审查的 daemon/API 行为。
  • 破坏性变更/迁移说明:无。Workflow snapshot 仍需显式 opt-in;旧 task 调用方不变。

关联 Issue

Closes #8941

Depends on #10411

替代 #9807

qqqys and others added 30 commits August 20, 2026 14:03
…-api' into codex/issue-8941-workflow-web-shell
…-api' into codex/issue-8941-workflow-web-shell
Rework the Workflow run card as a flat surface: one background, one
hairline, status carried by a single accent per node and edge. Drops the
radial/dot-grid canvas, gradient node fills, drop shadows and glow
filters, and raises the 9–10px type to 11–12px.

- Lanes share the viewport width (196–336px) so a three-phase run fills
  the card instead of ending in a blank strip; the last lane absorbs any
  remaining width. Nodes grow with their lane up to 300px.
- Node: 3px status bar on the left, lucide status icon, label on its own
  line with status and elapsed time beneath it. Queued nodes are dashed.
- Run controls (pause / stop / retry / rerun) move from above the card
  into the graph's metrics strip so they sit next to what they act on.
- Inspector drops its tinted panel and boxed key/value cells for a
  hairline-separated key/value list; callouts keep only a 2px bar.
- Saved workflows list becomes one bordered list with row dividers
  instead of individually bordered, striped cards.

Claude-Session: https://claude.ai/code/session_01Umbt74AQ1QDT8bGwFhRYks
A saved workflow row was name + scope + Run; the definition itself was
unreachable from the browser because the daemon only forwarded `{ name,
source }` and the `/file` route stops at the workspace boundary, which
excludes user-scope `~/.qwen/workflows`.

Add a read-only definition surface end to end:

- daemon: `GET /session/:id/saved-workflows/:name` →
  `_qwen/session/saved_workflow` → `qwen/status/session/saved_workflow`.
  The ACP child resolves the name through `resolveSavedWorkflowScript`
  (so the existing saved-workflow-directory guard applies), parses the
  `export const meta` block with `extractAndStripMeta`, and returns
  `{ source, scriptPath, script, meta, metaError? }`. Every miss —
  unknown name, illegal name, unreadable file, Workflow controls
  unavailable, untrusted workspace — fails closed to `workflow: null`
  with the same envelope on both transports, so the route cannot be used
  to probe the filesystem.
- sdk / webui: `DaemonSessionSavedWorkflowStatus`, route-table entry,
  `DaemonSessionClient.savedWorkflow`, and a `readSavedWorkflow` action.
- web-shell: the Saved row's identity block is now a disclosure button.
  Expanding it shows the description, "when to use", the phase list with
  optional model badges, the script path, the definition's recent runs in
  this session (with a jump to History), and a folded, highlighted copy of
  the source. Stale reads across session switches are dropped, and a
  definition that disappears from the list closes its detail.
- e2e: the mock daemon learns `/session/:id/tasks`, a configurable
  `supported-commands`, and `/session/:id/saved-workflows/:name`, and a
  `workflow-page` visuals spec captures Saved, the expanded definition,
  Running, and History in both themes for the CI preview.

Claude-Session: https://claude.ai/code/session_01Umbt74AQ1QDT8bGwFhRYks
… task reads (QwenLM#9546)

- Session history merge now treats a persisted snapshot as authoritative
  over a stale callback cache, and retires the cache once the runner
  confirms the snapshot write (new registry snapshot-persisted hook), so
  a sibling's deletion is not resurrected on refresh.
- History deletion consults every sibling session's run registry (live
  entries and settling handles) before deleting from the shared store,
  and a successful deletion purges sibling terminal entries so retries
  cannot re-persist a deleted run.
- Workflow holds mirror the registry's hasRunningEntries: paused runs no
  longer pin the session indefinitely.
- The includeWorkflows opt-in is gated on workspace trust at the daemon
  boundary in both the ACP-HTTP dispatch and the REST tasks route,
  matching the fail-closed shape of the other workflow surfaces.
…oles

Addresses R5-9, R7-4, R7-5 and R7-10 — all four Criticals open on this PR.
They are one family: a run's history can be deleted, retried, or listed
from any session, and each gate had a different idea of which runs exist.

R5-9 — the mutation claim is task-global, not session-scoped. Keyed
`sessionId\0taskId`, it serialized nothing that mattered: every session
shares one snapshot store. A sibling's retry passed canStart (`failed`,
no handle), took its own per-session claim, then awaited journal
load/compile before `register()`; a delete-history landing in that
structural window found the run terminal and handle-less in every
registry, removed the journal directory and snapshot, and answered
`{changed: true}` — after which the retry re-registered and its
settlement re-persisted the history the user was told was deleted. The
claim is now keyed by taskId alone and taken by delete-history, retry,
rerun and run-saved. `run-saved` keys off a saved workflow's NAME, so it
claims in its own `saved\0` namespace rather than colliding with runIds.

R7-4 — deletion tests membership against the uncapped merged set.
`buildSessionTasksStatus` serializes every registry entry
unconditionally while `refreshWorkflowHistory` truncates to
MAX_RETAINED_SNAPSHOTS by startTime, so a long run that settled after
~30 newer ones started stayed listed via the registry but fell out of
the window — terminal, handle-free, live in no sibling, and permanently
undeletable. `refreshWorkflowHistory` now records the merged id set
before the cap, and deletion gates on that, the registry, or the
unpersisted cache. `deleteWorkflowSnapshot` already tolerates an absent
target, so the wider gate cannot delete what is not there.

R7-5 — snapshot retirement is a latch. The registry's dispatch-drain
callbacks emit status changes on TERMINAL entries with no status gate,
and in-flight dispatches keep draining across the snapshot write, so a
terminal emission routinely landed after `notifySnapshotPersisted` had
retired the cache entry — re-inserting the run as "never persisted".
A sibling's deletion was then undone by the next refresh, which reads
"absent on disk, present in cache" as a pending write and republishes.
Persistence is now remembered per runId and `#rememberWorkflowHistory`
returns early for members; the latch releases when the runId goes active
again, so a genuine re-run is still cached.

R7-10 — the liveness gate sees runs whose session is gone. It iterated
`this.sessions` only, but close/kill/shutdown use force semantics and a
background run owns a detached controller, so after
`removeStoredSessionEntry` a still-settling run was invisible to the
gate and unreachable by the delete handler's sibling `removeTerminal`
loop: a sibling delete-history removed the LIVE run's journal and
snapshot, and the orphan's settlement write recreated it. Two halves —
`Session.dispose()` now aborts its workflow registry the way it already
aborts the agent registry (before the callbacks are torn down), and the
registry of a removed session is retained here until its runs drain, so
the gate still answers across the settlement window an abort cannot
compress to zero. Retention is bookkeeping: a Config that cannot answer
is logged, never turned into a shutdown failure.

Regressions, each mutation-checked against its own repair:
- acpAgent: a sibling's parked retry makes delete-history answer
  `{changed: false}` without reaching the store, and the deletion goes
  through once the claim releases
- acpAgent: a live run's registry stays visible to the gate across its
  session's close, and is dropped once the handle is released
- Session: a status emission after `snapshotPersisted` no longer
  resurrects a sibling-deleted run (reverting the latch reproduces the
  reviewer's probe verbatim), and a re-registered runId is remembered
  again
- Session: a run with the oldest startTime behind 30 newer snapshots is
  deletable
- Session: dispose aborts the workflow registry before clearing its
  callbacks

Verification: Session.test.ts 733 passed, acpAgent.test.ts 514 passed,
`tsc --noEmit -p packages/cli` clean, eslint and prettier clean. The 14
`packages/cli/src/serve` failures (fast-path import boundary,
capabilities-docs contract, workspace fs/agents/memory, conversation
runtime ownership) reproduce identically on the unmodified head — base
skew, untouched by this change.

Claude-Session: https://claude.ai/code/session_01M7z4PccYfDPyyfg3oGr8V1
Merging upstream/main (053f17b) clears the 9 `client.telemetrySwap`
failures this branch had from predating QwenLM#10220, but main carries its own
TS1117 on the same file: two commits added `getToolRegistry` to the same
object literal independently and neither saw the other.

  032b907 feat(serve): backfill session PR bindings ... (QwenLM#9729)
  8241905 test(core): give the telemetry-swap client mock a
             getToolRegistry (QwenLM#10220)

Checking that file out from upstream/main here and running
`tsc --noEmit -p packages/core` reproduces
`client.telemetrySwap.test.ts(103,5): error TS1117` verbatim, so taking
the merge unmodified would have traded 9 test failures for a build that
never reaches the tests at all.

Removed as part of the merge rather than left for a follow-up: keeps
QwenLM#10220's copy, which was added for this purpose and carries the
explanation, and drops QwenLM#9729's incidental one. main still needs the same
removal — this only keeps it out of the branch.

Verified after the merge: `tsc --noEmit` clean for both packages/core
and packages/cli; client.telemetrySwap 10 passed, Session.test.ts and
acpAgent.test.ts 1260 passed together.

Claude-Session: https://claude.ai/code/session_01M7z4PccYfDPyyfg3oGr8V1
qqqys and others added 10 commits August 27, 2026 20:56
Four of the behavioural items the review deferred across rounds 8-9; the
test-coverage-only entries stay deferred.

- Cancel during the start window (R9 acpAgent.ts:10884 + workflow-runner.ts:181).
  Between `reserveStart` and `register` the runner loads the script and
  replays the journal — seconds, for a resume of a large one — and the
  registry has no entry yet. `sessionTaskCancel` answered `not_found` for a
  run the client could see starting, and `registry.cancel` could not reach
  the reserved controller either. New `cancelStarting` aborts it (the
  reservation stays the runner's to release, as after `abortAll`), and the
  cancel handler routes there when the liveness gate would say "starting".
  Doing that exposed the second half: the runner threw a bare `Error` for
  an abort during start, and the tool's catch only recognised the CALLER's
  signal — a registry-side abort surfaced as an unexplained failure. It is
  now a typed `WorkflowStartCancelledError`, mapped to the same
  "cancelled before it could start" result.
- `detachedWorkflowRegistries` (R8 acpAgent.ts:3386) was pruned only inside
  the delete-history liveness check. A daemon that closes sessions mid-run
  and never deletes history retained every registry for its lifetime. Prune
  on session close as well.
- Refresh/delete race (R9 Session.ts:3428). `refreshWorkflowHistory` reads
  the directory and merges without a claim; a delete landing between the
  read and the merge was overwritten by the stale listing and the run
  reappeared until the next refresh. Deletions are now sequenced, and a
  refresh drops any run deleted after its read began — keyed by runId and
  compared against the refresh's own mark, so a later retry that reuses
  the id is not suppressed.
- The "Register a new run" JSDoc sat on `reserveStart` (R9 registry:594).

Mutation-verified, all four at once against the full suites: disabling the
starting-window branch, the prune-on-close, the deletion filter, and the
typed-error mapping reddens exactly the four new tests and nothing else.

Claude-Session: https://claude.ai/code/session_01VXsC4f71S6U6YkW82NRw7m
- Key the starting-window cancel on a live reservation rather than on the
  absence of an entry: a retry reuses its runId, so its terminal entry
  shadowed the reservation and cancel answered `not_running` about a run
  that was actively starting.
- Answer `changed: false` from retry when `execute()` reports a start that
  never registered (no `workflowRunId`), mirroring rerun and run-saved.
- Classify a registry-side abort of the reserved controller as a cancel in
  foreground starts too, not only background ones; the tool maps
  `WorkflowStartCancelledError` in either mode.
- Report reserved-but-unregistered runs as workflow active-work holds
  (`WorkflowRunRegistry.listStartingRunIds`), so a daemon conditional close
  cannot dispose the session under a start it just accepted.
- Propagate a successful history deletion into every sibling session's
  deletion marker and cached history, symmetric to the `removeTerminal`
  sweep, so a sibling refresh that had already read the directory cannot
  republish the deleted run.

Claude-Session: https://claude.ai/code/session_018dYE4LwSMeMPFchXk5UBdM
…ent across sessions

Two cross-session gaps in the workflow control surface:

- A retry consulted only the requesting session's registry. Every
  session shares one journal/snapshot store and the task-global claim is
  released as soon as the background start returns, so a sibling whose
  registry still showed the run `failed` started a second runner under
  the same runId. Retry now refuses while the runId is live in any
  session (or in its own starting window), checked synchronously beside
  canStart so the answer cannot go stale before the claim is taken.

- Workspace reload updated `tools.workflowsEnabled` for `/capabilities`
  but never told existing sessions; `Config.workflowsEnabled` was set
  once at construction. The reload's `tools` branch now propagates the
  flag and pushes an available-commands update when it flips.

Claude-Session: https://claude.ai/code/session_01NkW1J2aBKcsKS62dkPcWbT
…ry entry survived

`deleteWorkflowHistory` ignored `removeTerminal()`'s answer. The registry
refuses to remove a live or handle-held entry — its own last word on
whether the run is still active in this session — so a `false` for an
entry that exists meant the run re-registered under the deletion and
would re-persist the history the client was just told was gone. The
entry is now retired before the store is touched, and a refusal fails
the deletion; a persisted-only run has no entry and is unaffected.

Claude-Session: https://claude.ai/code/session_01NkW1J2aBKcsKS62dkPcWbT
Resolves ToolGroup conflicts against QwenLM#10231: hasExpandableContent and
forceExpandable are gone (every row is expandable now); the workflow
detailsVisible prop is kept.
@github-actions github-actions Bot added the review/self-reported The linked issue was opened by the PR author (self-reported) label Aug 28, 2026
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

🖼️ web-shell visual preview

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

Screenshots · before / after

⚠️ One or more scenarios failed to render on this head, so this preview may be missing views — see the workflow run. The composites below are the scenarios that did render.

workflow-page-history-dark before/after

workflow-page-history-light before/after

workflow-page-running-dark before/after

workflow-page-running-light before/after

workflow-page-saved-dark before/after

workflow-page-saved-detail-dark before/after

workflow-page-saved-detail-light before/after

workflow-page-saved-light before/after

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

Qwen Code · web-shell visuals

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

🩺 serve daemon A/B

Built the PR base vs this PR head 8a7a63f, 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 12 scenario(s).

Qwen Code · serve A/B

@wenshao wenshao 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.

Partially reviewed — gaps disclosed.

Not reviewed: reverse audit — stopped before round 4 of the 5-round cap: rounds 1-3 each reported new findings and the loop was halted with rounds 4-5 unspent so the verified findings could be composed and posted; no convergence is claimed.

Test Plan (not a blocker): client/App.tsx:8359no such file or directory.

[Critical] R1-8: [certifies-falsely] refreshWorkflowHistory's merge branch treats ANY on-disk snapshot for a runId as newer than the cached unpersisted settlement, without comparing incarnations — wrong exactly when the runId was reused by a retry/resume and the disk holds the PREVIOUS incarnation: wf_a fails (snapshot on disk), the retry settles 'completed' cached unpersisted, and before the new write lands a poll merges the old failed copy unconditionally and deletes the cache entry — the run shows failed with the old times, and if the new write fails the newest settlement is gone permanently. Fix: keep the cached projection (skip the delete) when stored.startTime < snapshot.startTime. Witness: not run — the merge branch has no recency comparison; the persisted-newer direction is pinned at Session.test.ts:3125-3133, leaving the cached-newer direction untested. Acceptance: a mirrored test with times flipped (cache completed@5_000, disk failed@1_000) must show completed/5_000 after refresh — red at HEAD; the persisted-wins direction at >= must stay green and the R7-5 retirement must stay unconditional.

[Critical] R1-14: [certifies-falsely] After a reload flips workflowsEnabled off, the already-registered Workflow tool remains callable — no execute-time gate exists (zero references to isWorkflowsEnabled/disabled-tools in the tool; registration is evaluated once at construction), so the model keeps spawning live runs the user just disabled, invisible in tasks and uncancelable short of a daemon restart. Probe after setWorkflowsEnabled(false): stillRegistered=true and the session-owned start path still exposed; the env kill-switch likewise leaves it registered. Fix: refuse in WorkflowTool.execute when !config.isWorkflowsEnabled() (inherits the env kill switch for already-registered tools), or add workflow to the live-applied setDisabledTools on flip-off; consider letting cancel still settle runs live at flip time. Acceptance: a test registering the tool with the flag true, flipping false, and asserting an invocation is refused — no test today covers post-reload tool calls; a gate keyed on tool absence would break the retry/run-saved branches, so key on the flag.

中文说明

仅完成部分审查,审查缺口已披露。

未审查:reverse audit — stopped before round 4 of the 5-round cap: rounds 1-3 each reported new findings and the loop was halted with rounds 4-5 unspent so the verified findings could be composed and posted; no convergence is claimed。

Test Plan(非阻断):client/App.tsx:8359no such file or directory

[Critical] R1-8: [certifies-falsely] refreshWorkflowHistory's merge branch treats ANY on-disk snapshot for a runId as newer than the cached unpersisted settlement, without comparing incarnations — wrong exactly when the runId was reused by a retry/resume and the disk holds the PREVIOUS incarnation: wf_a fails (snapshot on disk), the retry settles 'completed' cached unpersisted, and before the new write lands a poll merges the old failed copy unconditionally and deletes the cache entry — the run shows failed with the old times, and if the new write fails the newest settlement is gone permanently. Fix: keep the cached projection (skip the delete) when stored.startTime < snapshot.startTime. Witness: not run — the merge branch has no recency comparison; the persisted-newer direction is pinned at Session.test.ts:3125-3133, leaving the cached-newer direction untested. Acceptance: a mirrored test with times flipped (cache completed@5_000, disk failed@1_000) must show completed/5_000 after refresh — red at HEAD; the persisted-wins direction at >= must stay green and the R7-5 retirement must stay unconditional.

[Critical] R1-14: [certifies-falsely] After a reload flips workflowsEnabled off, the already-registered Workflow tool remains callable — no execute-time gate exists (zero references to isWorkflowsEnabled/disabled-tools in the tool; registration is evaluated once at construction), so the model keeps spawning live runs the user just disabled, invisible in tasks and uncancelable short of a daemon restart. Probe after setWorkflowsEnabled(false): stillRegistered=true and the session-owned start path still exposed; the env kill-switch likewise leaves it registered. Fix: refuse in WorkflowTool.execute when !config.isWorkflowsEnabled() (inherits the env kill switch for already-registered tools), or add workflow to the live-applied setDisabledTools on flip-off; consider letting cancel still settle runs live at flip time. Acceptance: a test registering the tool with the flag true, flipping false, and asserting an invocation is refused — no test today covers post-reload tool calls; a gate keyed on tool absence would break the retry/run-saved branches, so key on the flag.

— glm-5.3 via Qwen Code /review (v0.22.2)

Comment on lines +7571 to +7573
private canUseWorkflowControls(config: Config): boolean {
return (
config.isWorkflowsEnabled() &&

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.

[Critical] R1-6: [fails-closed] [new-surface] Every daemon workflow control fail-closes on canUseWorkflowControls (bare mode / untrusted folder refused), but the Workflow tool that creates those runs is registered on isWorkflowsEnabled() alone — core config.ts:9539 has no trust clause and packages/core/src/tools/workflow contains no gate. In an untrusted folder with the flag on, the model can start a run that is then invisible in every tasks poll (tasksSnapshot.ts:217-229 drops registry runs) and refused by cancel ({cancelled:false,reason:'disabled'} while the agent/shell/monitor branches in the same switch cancel unconditionally) — a live run that keeps dispatching until the session closes. Make one predicate authoritative: fold the trust/bare clauses into the registration condition, or let cancel fall through for already-live runs.

Witness:

witness: not run — controls-side links quoted line-by-line (cancel branch vs the
unconditional sibling branches; registration condition; zero trust references
under core/src/tools/workflow).

Extend config.workflow-registration.test.ts with untrusted-folder (and bare-mode) cases asserting the Workflow tool is NOT registered — deleting the added gate turns them red (or server-side: a bare-mode live-entry case asserting cancel resolves {cancelled:true}). Keep flag-off ⇒ unregistered (pinned at :128) and env-enabled-in-trusted-folder ⇒ registered (config.ts:7729-7730).

中文说明 daemon 侧所有 workflow 控制都按 canUseWorkflowControls 失败关闭,但 Workflow 工具本身只按 isWorkflowsEnabled 注册(无信任/裸模式检查):在未受信任目录中模型可以启动运行,但该运行在任务列表中不可见、也无法取消,直到会话关闭。请让注册条件与控制谓词一致,或允许取消已在运行中的任务。

— glm-5.3 via Qwen Code /review (v0.22.2)

Comment on lines +12471 to +12474
const workflowsWereEnabled = config.isWorkflowsEnabled();
config.setWorkflowsEnabled(
newMerged.tools?.workflowsEnabled === true,
);

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.

[Critical] R1-13: [certifies-falsely] [new-surface] The settings-reload propagation only reaches idle sessions: the per-session loop returns before the tools block, so setWorkflowsEnabled — whose only production call site this is — never runs for a mid-turn session, with no queue and no catch-up (these ACP sessions never restart). After an operator flips the flag off, /capabilities (which reads the reloaded setting live) advertises off while that session still accepts run-saved/pause/resume/retry and advertises workflowsEnabled: true. Hoist the synchronous flag setters past the idle gate, deferring only sendAvailableCommandsUpdate.

Witness:

[probe] busy arm (isIdle mocked false): {"sessionsSkipped":["s-wf-reload"],
"setWorkflowsEnabledCalls":0,"cancelAfterReload":{"cancelled":true,"status":
"running"}} — the session cancelled a LIVE run after the operator's flip-off;
idle arm (the PR's tested path): setWorkflowsEnabledCalls=1,
{cancelled:false,reason:"disabled"}.

Extend the reload test with a second session whose isIdle returns false: after reload, assert its config received setWorkflowsEnabled(false) and cancel answers {cancelled:false,reason:'disabled'} — red while the sync stays inside the idle branch. The idle gate protects stateful refresh (switchModel/refreshAuth/sendAvailableCommandsUpdate) — hoist only the synchronous setters, and keep sessionsSkipped in the response contract.

中文说明 设置重载只作用于空闲会话:正在运行回合的会话永远保持旧标志和全部可用控制——探测显示操作者关闭开关后,忙会话仍取消了正在运行的 workflow。请把同步的标志设置提到空闲判断之外(仅延迟 sendAvailableCommandsUpdate)。

— glm-5.3 via Qwen Code /review (v0.22.2)

Comment on lines +12471 to +12474
const workflowsWereEnabled = config.isWorkflowsEnabled();
config.setWorkflowsEnabled(
newMerged.tools?.workflowsEnabled === true,
);

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.

[Critical] R1-7: [fails-closed] [new-surface] The reload flips workflowsEnabled on for live idle sessions and opens canUseWorkflowControls and the advertisement — but the Workflow tool is registered only at tool-registry construction (built once, config.ts:3480/9539; nothing on the reload path registers it later). A session created with the flag off then advertises workflowsEnabled: true + the saved-workflow list while every start action hits getTool(ToolNames.WORKFLOW) → undefined → "The workflow tool is unavailable; cannot run this saved workflow." Refresh cannot help — only a new session recovers (settings.md:377 documents the key as Requires restart: Yes). Gate the flipped-on state on the session actually owning the tool, or register WORKFLOW into the live registry on flip-on.

Witness:

witness: not run — construction-vs-reload asymmetry quoted (single registration
site; nothing on the reload path registers the tool; docs mark the key
restart-required).

A reload test mirroring acpAgent.test.ts:24827 in the false→true direction — flag-off session, reload flips on, assert buildSessionSupportedCommandsStatus still reports workflowsEnabled: false — is red when propagation alone opens the gate. The existing flip-off pin ({cancelled:false,reason:'disabled'} at :24924-24928) must stay green under a tool-presence gate.

中文说明 设置重载把 workflowsEnabled 置真时会打开广告与控制,但 Workflow 工具只在构建时注册一次:以关闭状态创建的会话重载后广告已开、每次启动却报“工具不可用”,刷新也无法恢复。请在翻转时检查会话是否真的持有该工具(扩展 canUseWorkflowControls),或在翻转时补注册。

— glm-5.3 via Qwen Code /review (v0.22.2)

Comment on lines +3112 to +3114
const workflows = this.config.getWorkflowRunRegistry?.()?.list?.() ?? [];
if (
workflows.some(

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.

[Critical] R1-9: [fails-closed] [new-surface] The todo-stop-guard relevance gate counts a paused background workflow as live background input — but a paused run emits only status changes, never the completion notification the guard defers to, so the forced todo continuation is silently dropped and the turn ends with the todo list unfinished. The PR's own sibling gates exclude paused twice with the no-backstop rationale (hasRunningEntries' R12 comment; collectActiveWorkHolds). Count only running/pausing here, mirroring collectActiveWorkHolds — do NOT use isActiveWorkflowStatus (it includes paused and would not fix the bug).

Witness:

witness: not run — status-set mechanics quoted (terminal = completed/failed/
cancelled only; emitCompletion fires only for those; both sibling gates' paused
exclusions quoted with their rationale comments).

Beside 'classifies workflow notifications from the captured baseline': seed the registry with a post-baseline {status:'paused'} entry, rebuild the session, invoke #hasRelevantTodoStopGuardBackgroundInput via the internals cast the file already uses, and assert false — reverting to !isTerminalWorkflowStatus turns it red.

中文说明 todo 停止守卫把 paused 状态的 workflow 当作活跃后台输入,但 paused 的运行永远不会发出完成通知,守卫被无限推迟,强制续跑被静默丢弃。请与 collectActiveWorkHolds 一致只统计 running/pausing(不要用 isActiveWorkflowStatus,它包含 paused)。

— glm-5.3 via Qwen Code /review (v0.22.2)

);
expect(keys).toHaveLength(61);
expect(new Set(keys).size).toBe(61);
expect(keys).toHaveLength(62);

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.

[Critical] R1-1: The route-count assertions were bumped to 62 total / 60 handler_resolved, but this PR adds two handler_resolved entries to legacySessionTelemetryRoutes — the catalog now holds 63 routes / 61 handler_resolved + 2 pre_resolved, so this test is red at the reviewed commit (and it is the failing ubuntu CI check). Fix: toHaveLength(63), set size 63, handler_resolved 61, retitle to "63 unique routes with the audited 61/2 attribution split". telemetry-catalog.test.ts:101 already pins 63 and passes.

Witness:

Reproduced red twice on the PR tree (in-suite and standalone); the same file
passes clean on merge base d6533785b (65 passed / 0 failed).

The test itself is its own acceptance criterion — it is red at HEAD and green at 63/61. Note telemetry-catalog.test.ts pins registered==catalog==63, so raise the expected counts rather than removing an entry.

中文说明 路由计数断言更新为 62/60,但本 PR 新增了两个 handler_resolved 条目,实际为 63/61,该测试在当前提交是红的(也是 ubuntu CI 失败的原因)。请改为 63/61/2 并更新标题;telemetry-catalog.test.ts 已固定 63,不能通过删除条目来凑数。

— glm-5.3 via Qwen Code /review (v0.22.2)

Comment on lines +111 to +114
agentsDispatched: 0,
agentsCompleted: 0,
tokensSpent: 0,
recentLogs: [],

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] R1-63: PATTERN (3 locations in this diff): new workflow task fixtures omit required DaemonSessionWorkflowTaskStatus fields — WorkflowRunsPage.test.tsx:113 (missing tokenBudgetTotal), TasksStatusMessage.test.tsx:130 (missing tokenBudgetTotal), composerTasks.test.ts:59 (missing currentPhase + tokenBudgetTotal). All pass only because web-shell excludes *.test.ts from typecheck; every sibling fixture sets the fields. Any branch under test reading the field observes undefined where the type and the daemon guarantee number | null — budget display is never exercised through these suites.

Witness:

[probe] tsc with the exclusion lifted: TS2322 on composerTasks.test.ts(56,9)
'missing the following properties … currentPhase, tokenBudgetTotal'; adding
the fields → exit 0 (flip confirmed on the same class).

The compiler is the red gate: include the files in a tsc --noEmit run — errors without the fields, clean with them (measured flip). Fields are number | null / string | null (types.ts:2746/2766); null = 'no budget set' per sibling usage.

中文说明 模式问题(3 处):新的 workflow 测试夹具缺少必填字段(tokenBudgetTotal / currentPhase),只因测试被排除在类型检查外才通过。请补齐 null 值;用 tsc 验证(现已实测可翻转)。

— glm-5.3 via Qwen Code /review (v0.22.2)

Comment on lines +85 to +88
const [nextSnapshot] = await Promise.all([
actions.getWorkflowTasks(),
actions.refreshCommands(),
]);

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] R1-34: reload's Promise.all fail-fasts on refreshCommands: a single rejection lands in the catch, sets loadError, and setSnapshot never runs — on first load the page renders only the "Failed to load workflow runs." banner with no tab content, though the runs data arrived and was thrown away (recovery requires clicking Refresh until both succeed). The codebase treats the same call as best-effort at App.tsx:5805. Settle independently (Promise.allSettled), apply the snapshot when the tasks arm fulfils, and log/ignore a refreshCommands rejection.

Witness:

witness: not run — Promise.all/catch/render-gating and the App.tsx contrast are
deterministic JS semantics over quoted lines.

A test where getWorkflowTasks resolves one task and refreshCommands rejects once must show runs content and no loadFailed banner — red without the fix. Note refreshCommands is the only writer of supportedCommands here (savedWorkflows derives from it): demote its failure, do not skip the call.

中文说明 reload 的 Promise.all 会因 refreshCommands 单独失败而整体失败:已取到的运行数据被丢弃,首屏只显示失败横幅。请用 allSettled 独立结算(仍需调用 refreshCommands,只是降级其失败)。

— glm-5.3 via Qwen Code /review (v0.22.2)

Comment on lines +148 to +150
if (
selectedName !== null &&
!savedWorkflows.some((workflow) => workflow.name === selectedName)

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] R1-62: The saved-list staleness invariant — collapse the expanded detail when its definition leaves supportedCommands.savedWorkflows — has no witness: deleting the entire effect keeps the suite green (measured), and no test ever changes the savedWorkflows list while a detail is expanded. Without the effect, a definition deleted and re-saved under the same name renders expanded with the stale detailState (old description, phases, script source) instead of reloading — exactly the stale content the comment promises to prevent.

Witness:

[probe] effect deleted → 11/11 passed; restored → byte-identical.

A test that expands deep-review, then sets savedWorkflows = [] with sessionId fixed and re-renders must assert no [data-workflow-detail] and aria-expanded='false' — run with the effect deleted it goes red; today nothing does. Keep sessionId unchanged in the test — the session-switch effect also resets selection and would mask the membership effect.

中文说明 “定义从列表消失时收起详情”这一不变量没有见证(删除整个 effect 全绿,已实测):同名重存的定义会以陈旧详情展开。请补充列表变化后断言收起的用例(保持 sessionId 不变)。

— glm-5.3 via Qwen Code /review (v0.22.2)

Comment on lines +1984 to +1985
const session = sessionRef.current;
if (!session) throw new Error('Daemon session is not connected');

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] R1-35: getWorkflowTasks' silent-poll error contract (transient rethrow under {silent:true} + once-key notice dispatch) is pinned by no test — only its happy path is covered, while its twin getTasks has failure variants in the same file. A regression dropping the silent-transient branch or the once-key ships green; a transient 503 during the routine 3-second poll would then surface a user-facing "Get tasks failed" notice — without the once-key, one per poll for the rest of the session.

Witness:

witness: not run — absence sweep over the harness that already pins the twin
(getTasks failure/silent tests exist in the same file).

Mirror the getTasks failure tests: mock a transient rejection and assert addNotice is not called under {silent:true}; add a hard-failure variant asserting exactly one notice across two calls.

中文说明 getWorkflowTasks 的静默轮询错误契约(瞬态重抛 + 一次性通知)没有测试:瞬态 503 会在每次轮询弹出用户可见错误。请照 getTasks 的失败用例补齐。

— glm-5.3 via Qwen Code /review (v0.22.2)

);
try {
return await withActionTimeout(
session.controlWorkflowTask(taskId, action),

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] R1-36: controlWorkflowTask's (taskId, action) forwarding is pinned by no test anywhere — the only test reaching this line mocks the session method with a promise that ignores arguments, and the UI tests assert against a mock that replaces this very function. A mutation swapping the action or the id keeps the entire suite green: every workflow control button would issue a wrong action with no failing test.

Witness:

witness: not run — grep: no toHaveBeenCalledWith on controlWorkflowTask exists
repo-wide; the stale-suppression test's mock ignores args.

Add a happy-path pin beside the runSavedWorkflow test: mockResolvedValueOnce({changed:true,status:'paused'}); await actions.controlWorkflowTask('wf-1','pause'); expect(...).toHaveBeenCalledWith('wf-1','pause') plus one second action.

中文说明 controlWorkflowTask 的参数转发全仓库无测试:参数换错的回归全绿,所有控制按钮都会发出错误动作。请补充参数精确断言。

— glm-5.3 via Qwen Code /review (v0.22.2)

@qqqys

qqqys commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator Author

Superseded by #10594: #10411 merged, so this reopens on a fresh branch (feat/workflow-web-shell-v2) merged with main at 4908f859d5 — the diff there now contains only the Web Shell layer plus the saved-workflow read surface (62 files instead of ~90). No force-push; this branch's history is preserved in the new one. The daemon-layer findings from the round-1 review here are settled by the merged #10411 (main's version wins in the merge); the Web Shell-side findings remain relevant and will be addressed on #10594.

#10594 取代:#10411 已合并,故在新分支上重开并合并 main,diff 只保留 Web Shell 层与 saved-workflow 读取面。未使用 force-push,历史在新分支中保留。本 PR review 中 daemon 层的发现已由合入的 #10411 解决,Web Shell 侧的发现将在 #10594 上处理。

@qqqys qqqys closed this Aug 31, 2026
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(web-shell): visualize and manage dynamic workflow runs

4 participants