feat(serve): expose Workflow tasks and controls - #10411
Conversation
… 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
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
…rkflow-daemon-api
…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
…rkflow-daemon-api
|
Thanks for the PR! Template looks good ✓ Problem: a real feature gap, not a theoretical one. Structured Workflow execution state landed in core via #9034 (merged Aug 20), but daemon consumers have no transport to observe or control it — that is exactly what tracking issue #9033 asks for, and the Web Shell visualization (#8941) is stacked on it. This supersedes #9546 (same branch history, closed earlier today to reset an ~180-thread review history); the shapes match (+7062/−360 there vs +7122/−357 here). Direction: aligned. It is the transport/client layer for an already-accepted runtime feature, additive and opt-in — the legacy agent/shell/monitor task contract is untouched. It does add new daemon + SDK public surface and applies the workspace-trust gate to several new egress paths, so it needs a human maintainer's eye regardless of how the automated review reads (see the verdict stage). Size: cross-package core change ( Approach: the shape is right — one opt-in flag ( Risk: Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：真实的功能缺口，不是理论性问题。结构化 Workflow 执行状态已通过 #9034（8 月 20 日合并）进入 core，但 daemon 消费端缺少观察/控制它的传输层——这正是跟踪 issue #9033 的诉求，Web Shell 可视化（#8941）也叠在它之上。本 PR 替代 #9546（同一分支历史，今天早些时候关闭以重置约 180 条评审线程），两者形态一致（#9546 为 +7062/−360，本 PR 为 +7122/−357）。 方向：对齐。这是已被接受的运行时功能的传输/客户端层，纯增量且 opt-in——旧的 agent/shell/monitor 任务契约不受影响。但它新增了 daemon + SDK 公共接口，并把 workspace 信任门禁应用到多条新的出口路径，因此无论自动评审结论如何都需要人工维护者把关（见最终结论部分）。 规模：跨包核心改动（ 方案：形态正确——单一 opt-in 开关（ 风险： 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterℹ️ No screenshot changed against the PR base — but this PR edits 2 render-shaping files:
Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
Code reviewI started from an independent proposal — opt-in No critical blockers found. Things I verified along the way:
Two non-blocking nits:
Cross-session control flow (retry / delete-history)sequenceDiagram
participant P1 as SDK Client
participant P2 as Daemon route
participant P3 as AcpAgent
participant P4 as Session
participant P5 as WorkflowRunRegistry
participant P6 as WorkflowRunner
participant P7 as Snapshot store
P1->>P2: POST workflow-action, retry or delete-history
P2->>P3: controlSessionWorkflowTask with trusted client id
P3->>P3: take task-global mutation claim
alt retry
P3->>P3: liveness gate checks sibling and detached registries
P3->>P6: session-owned background start
P6->>P5: reserveStart, then register on success
P5-->>P1: changed true, new status
else delete-history
P3->>P4: deleteWorkflowHistory under the claim
P4->>P5: removeTerminal, sweep sibling registries
P4->>P7: delete snapshot and journal
P4-->>P1: changed true
end
Files changed (30 of 50 shown)
Testing evidenceUnattended CI run — this review does not execute PR code; the evidence below is the PR's own CI at the reviewed commit, fetched once via the API. No failures at fetch time; the main suites are still running. Bot orchestration jobs (
Notes on the skips: both are expected — the macOS/Windows unit jobs run only on Sandboxed verification would settle the remaining behavioural claim: 中文说明代码审查：从独立方案出发（opt-in 读取、失败即关闭的控制路由、增量 SDK 方法、能力声明），PR 形态一致；超出部分（跨会话控制面）由"共享快照存储 + 私有注册表"的架构所决定，逐一对照其声明要关闭的竞态，且每个都有命名的回归测试（约 90 个新用例）。未发现阻断性问题。已验证：旧契约确实保留（默认响应不含 workflow 条目、SDK 旧联合类型不变、legacy hold 类别列表未动）；信任门禁双层且失败即关闭（daemon 路由 + 子进程 canUseWorkflowControls）；拉取面与推送命令流（含转录回放帧）均做了脱敏；所依赖的 Config/Storage/bridge API 均存在于基线。两条非阻断建议：1) Session.ts 中 测试证据：无人值守 CI 运行，本评审不执行 PR 代码；以上证据来自评审提交上 PR 自身 CI 的一次性 API 抓取。抓取时无失败，主要套件仍在运行（Linux 单测、Serve A/B、Real daemon E2E）；macOS/Windows 单测按 CI 设计仅在合并队列/定时任务运行，集成测试对 fork PR 跳过——两者均为预期跳过。沙盒验证建议： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 3/5 — clean review with no blocking findings, but the Stage 0 core escalation caps the verdict, and the behavioural claims haven't been settled by CI at review time. Stepping back: this is a well-constructed PR. It extends an already-merged foundation (#9034) along its natural seam, keeps the legacy task contract bit-for-bit compatible, fails closed everywhere I probed, and carries roughly five thousand lines of tests that name each race they close — the work of someone who already absorbed eleven review rounds on the superseded PR. The two nits in my review are cosmetic. If this were a 300-line PR I would be writing an approval. It is not 300 lines. ~2,349 production lines across five packages, a new public daemon + SDK surface, and a concurrency design whose correctness lives in the timing between claims, reservations, and settlement — the parts that read well but only real execution truly proves. The main unit suite, Serve A/B, and Real daemon E2E were still running at review time, and per the gate's rules a core-touching change at this size escalates to a maintainer rather than being auto-approved. So this defers on policy, not on doubt about the author's work — the author is a committer who merged the runtime half of this feature themselves. ⏸️ Deferring to @yiliang114 (no
Once CI lands green and a maintainer has looked at the control plane, this reads as ready to ship. 中文说明总体评价：这是一个结构良好的 PR——沿着已合并基础（#9034）的自然接缝扩展，旧任务契约逐位兼容，所有探测点均失败即关闭，约五千行测试逐一命名了其要关闭的竞态。若这是 300 行的 PR，我会直接写批准。 但它不是：五个包约 2,349 行生产代码、新的 daemon + SDK 公共接口，其并发设计的正确性存在于申领、预留与结算之间的时序中。评审时主要单测、Serve A/B 与 Real daemon E2E 仍在运行；按门禁规则，这一规模的核心改动须转交维护者而非自动批准。因此本次按政策转交，而非质疑作者工作——作者本身就是合并了该功能运行时部分的 committer。 转交 @yiliang114 定夺：1) Stage 0 升级——来自 fork 的约 2.3k 行核心基础设施，按政策由维护者批准；2) 并发保证是否需要在合并前补一次沙盒 — Qwen Code · qwen3.8-max Reviewed at |
…ment The persisted latch is released inline at the top of #rememberWorkflowHistory; no #forgetPersistedWorkflowRun exists. Claude-Session: https://claude.ai/code/session_01NkW1J2aBKcsKS62dkPcWbT
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 12 scenario(s). — Qwen Code · serve A/B |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): chunk 9: could not execute the two new Session.test.ts tests to confirm they pass — the review worktree has no node_modules and no built package dist , and a full …; chunk 4: execute npx vitest run src/acp-integration/acpAgent.test.ts in packages/cli to observe the three new tests pass — worktree has no node_modules and a monorepo ….
Test Plan (not a blocker): session/Session.test.ts — no such file or directory; client/App.tsx:8358 — no such file or directory.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：chunk 9：could not execute the two new Session.test.ts tests to confirm they pass — the review worktree has no node_modules and no built package dist , and a full …；chunk 4：execute npx vitest run src/acp-integration/acpAgent.test.ts in packages/cli to observe the three new tests pass — worktree has no node_modules and a monorepo …。
Test Plan（非阻断）：session/Session.test.ts — no such file or directory; client/App.tsx:8358 — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.2)
…n-api # Conflicts: # packages/cli/src/serve/server.test.ts
qwen-code-dev-bot
left a comment
qwen-code-dev-bot
left a comment
There was a problem hiding this comment.
Pre-merge gate review — REQUEST_CHANGES
Reviewed head 9d193745 independently (full source diff + reconciliation of the 16 open review threads). No new Critical defect found in the diff itself, and all CI suites are green on this head (the earlier Test (ubuntu-latest) failure was re-run and passed — two load-sensitive timeouts in packages this PR does not touch; not PR-caused).
Two open Suggestions were verified against this exact commit and do block merge — they are correctness issues in behavior this PR ships, not test hygiene:
1. releaseHandle evicts terminal rows without a status-change emission (packages/core/src/agents/workflow-run-registry.ts, thread on line 764)
releaseHandle deletes its handle and calls evictTerminal(), which silently drops rows. Every other row-removing mutation (register / complete / fail / cancel / abortAll / reset / removeTerminal) fires statusChangeCallback, and consumers (TUI tasks dialog / workflows roster) refresh only on that emission. With more than MAX_RETAINED_TERMINAL_WORKFLOWS (10) terminal runs, once a handle release evicts the oldest row, client lists keep showing the evicted row until some unrelated status change fires — the roster never converges on its own.
Expected fix: have evictTerminal() report whether it deleted anything and fire emitStatusChange() from releaseHandle on a non-empty eviction; extend the existing eviction test to assert the callback fires.
2. /clear blocks on a starting run the enumerator cannot name (workflow-run-registry.ts + packages/cli/src/ui/utils/backgroundWorkUtils.ts, thread on line 1391)
hasRunningEntries() now counts reserved-but-unregistered (starting) runs, but describeBlockingBackgroundWork — documented as "mirroring its per-registry predicate exactly" — enumerates only registry.list(), which never contains starting runs. During the starting window this diff introduces (journal replay on retry, saved-workflow background start), /clear / /branch / /resume are refused with the bare base message and no enumerated blocker, and /workflows cannot show the run either. This opaque-block state is new behavior from this PR.
Expected fix: also enumerate listStartingRunIds() in describeBlockingBackgroundWork (e.g. <runId> (starting)), with a regression test for gate-true + empty-list() + non-empty-starting.
The remaining 14 open suggestions (per-branch test pinning, deletion-marker pruning, snapshot-write contract tests) are hardening/polish — non-blocking from my side; resolve or defer them on their threads.
中文摘要：本 head 9d193745 独立复查通过，无新增 Critical；CI 已全绿（此前 ubuntu Test 失败经重跑确认为与 PR 无关的负载抖动）。但以下两处为本 PR 引入的真实行为缺陷，需修复后合入：① releaseHandle 逐出终态条目不发状态事件，客户端列表不会自行收敛；② hasRunningEntries() 计入 starting 预留而 describeBlockingBackgroundWork 只枚举 list()，导致 /clear 出现“说不出原因”的阻塞。其余 14 条为测试加固/卫生类建议，不阻塞。
— qwen-code-dev-bot · pre-merge final gate
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — packages/cli full unit suite timed out before completing in the review harness; files past the kill point were not collected (diff-touched files additionally exercised by targeted verification runs).
Not explored to full depth (tool budget reached): chunk 2: executed run of packages/cli/src/acp-integration/acpAgent.test.ts (no node_modules in review worktree; install+build not feasible within budget).
Test Plan (not a blocker): session/Session.test.ts — no such file or directory; client/App.tsx:8358 — no such file or directory.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/cli/src/acp-integration/acpAgent.ts:7560 — [review] push surface available_commands_update never filtered by the workflow gate (bare mode / direct ACP)packages/cli/src/acp-integration/acpAgent.ts:11082 — [probe] retry/rerun consult only the session's registry; history-visible runs silently non-retryablepackages/cli/src/acp-integration/acpAgent.ts:12422 — [review] reload flag flip diverges from one-shot tool registration in both directionspackages/cli/src/acp-integration/session/Session.ts:3112 — [probe] todo-stop-guard workflow branch exercised by no test (mutant green)packages/cli/src/acp-integration/acpAgent.test.ts:11574 — [probe] cancel-waits-for-completion assertion vacuous (mutant survives)packages/cli/src/acp-integration/session/tasksSnapshot.test.ts:179 — [probe] startTime-sorted interleaving across task kinds unpinnedpackages/cli/src/acp-integration/acpAgent.ts:11010 — [probe] run-saved name-keyed claim serializes unrelated concurrent startspackages/cli/src/acp-integration/session/Session.test.ts:3702 — [probe] post-refresh liveness re-check masked by removeTerminal refusal; sibling variant untestedpackages/cli/src/acp-integration/session/Session.ts:3460 — [probe] stale persisted projection shadows newer cache after failed retry writepackages/cli/src/acp-integration/session/tasksSnapshot.test.ts:343 — [probe] legacy-shape snapshot guards (?? []) unpinnedpackages/cli/src/serve/server.test.ts:9827 — [probe] non-workflow cancel argument pin lost; clientId context pinned only for kind workflowpackages/cli/src/serve/server.test.ts:14165 — [probe] untrusted replay redaction pinned only for plain load; sibling egresses untestedpackages/acp-bridge/src/bridge.test.ts:23082 — [probe] InvalidClientId rejection tests never assert non-forwardingpackages/cli/src/acp-integration/acpAgent.test.ts:11924 — [probe] start-never-registered gate pinned only for retry; rerun/run-saved fallbacks untestedpackages/cli/src/acp-integration/acpAgent.ts:10925 — [review] after a disabled reload, live workflow runs become uncancellablepackages/cli/src/acp-integration/session/Session.test.ts:1182 — [probe] hold predicate test covers 4 of 6 WorkflowStatus valuespackages/cli/src/acp-integration/session/Session.test.ts:3001 — [probe] noteExternalWorkflowDeletion never called with populated unpersistedWorkflowHistorypackages/cli/src/serve/routes/capabilities.ts:41 — [review] capabilities advertise workflowsEnabled for live-conversation workspacespackages/cli/src/serve/server.test.ts:9967 — [probe] untrusted workflow-action fail-closed pinned only for run-saved
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：build-and-test — packages/cli full unit suite timed out before completing in the review harness; files past the kill point were not collected (diff-touched files additionally exercised by targeted verification runs)。
未探索到全部深度（达到工具调用预算）：chunk 2：executed run of packages/cli/src/acp-integration/acpAgent.test.ts (no node_modules in review worktree; install+build not feasible within budget)。
Test Plan（非阻断）：session/Session.test.ts — no such file or directory; client/App.tsx:8358 — no such file or directory。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 19 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
The previous commit added a `listStartingRunIds()` call to `describeBlockingBackgroundWork` without teaching the registry test doubles about it, so seven tests across `clearCommand`, `useBranchCommand` and `useResumeCommand` threw `listStartingRunIds is not a function`. Add the reader to those stubs. Also move the eviction's status emission from `releaseHandle` into `evictTerminal` itself. Emitting only from `releaseHandle` left the same convergence gap at the four sweeping call sites that already emit: complete / fail / cancel / abortAll emit BEFORE they sweep, so a consumer that re-reads on the callback observes the pre-eviction list and keeps rendering a row that was just dropped. Emitting once after the sweep closes all five paths, and a future eviction site inherits the guarantee. New test pins the ordering; another pins that a release which evicts nothing stays silent. A starting reservation now reports `starting` rather than borrowing `running`, and its line no longer repeats the run id the bracket already carries. Claude-Session: https://claude.ai/code/session_01VENc5rZYmMJDdqvjwBEyZd
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): session/Session.test.ts — no such file or directory; client/App.tsx:8358 — no such file or directory.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan（非阻断）：session/Session.test.ts — no such file or directory; client/App.tsx:8358 — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.3)
…ing run R3-2: the previous commit's stub plumbing re-inserted a `list:` line into four `clearCommand.test.ts` registry mocks that already declared one. Last-wins made it behaviour-neutral, and neither eslint (`no-dupe-keys` is off here) nor tsc (the literal is not contextually typed, unlike the `useResumeCommand` mocks where TS1117 did fire) caught it. Remove the duplicates. R3-1: when the only blocker is a reserved-but-unregistered run, the blocked message ended with "Use /workflows to inspect them, then retry." — but `/workflows` renders `registry.list()`, which a reservation has not entered, so it named a surface that cannot show what is blocking. Track inspectability separately and fall back to a plain retry hint when nothing is listable; a registered run in the same set still points at `/workflows`. Claude-Session: https://claude.ai/code/session_01VENc5rZYmMJDdqvjwBEyZd
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent 6b": running packages/cli vitest for the two changed test files to confirm they are green (blocked by unbuilt workspace dist/, not by the tool-call ceiling)..
Test Plan (not a blocker): session/Session.test.ts — no such file or directory; client/App.tsx:8358 — no such file or directory.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/utils/backgroundWorkUtils.ts:95 — [probe] 'starting' status literal and isStarting flag double-encode one fact; the literal is written but never read
Convergence: round 4 posted 2 inline comment(s), 2 of them reported for the first time; the previous round posted 5 (5 new). Findings keep coming back to the same files: packages/cli/src/ui/utils/backgroundWorkUtils.ts (findings in round 3; 2 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："agent 6b"：running packages/cli vitest for the two changed test files to confirm they are green (blocked by unbuilt workspace dist/, not by the tool-call ceiling).。
Test Plan（非阻断）：session/Session.test.ts — no such file or directory; client/App.tsx:8358 — no such file or directory。
收敛姿态下延后（第 4 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 4 轮发布了 2 条行内评论，其中 2 条是首次提出；上一轮发布了 5 条（其中 5 条首次提出）。发现反复回到同一批文件：packages/cli/src/ui/utils/backgroundWorkUtils.ts（第 3 轮已出过发现，本轮又有 2 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.3)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — no test suite ran via build-test: the 600s per-command budget exhausted on install + building 13/16 workspaces (all clean) before the test phase on this runner; the packages/cli suites never started (infrastructure ceiling, not a PR defect). The changed file's own suite was separately green at HEAD (26/26) via the verifier's probe baseline and the round-2 auditor..
Not explored to full depth (tool budget reached): "agent 6a": executing packages/cli/src/ui/utils/backgroundWorkUtils.test.ts — fresh worktree has no built workspace packages, and the prerequisite npm run build exceede….
Test Plan (not a blocker): session/Session.test.ts — no such file or directory; client/App.tsx:8358 — no such file or directory.
Convergence: round 5 posted 2 inline comment(s), 2 of them reported for the first time; the previous round posted 2 (2 new). Findings keep coming back to the same files: packages/cli/src/ui/utils/backgroundWorkUtils.ts (findings in rounds 3, 4; 1 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — no test suite ran via build-test: the 600s per-command budget exhausted on install + building 13/16 workspaces (all clean) before the test phase on this runner; the packages/cli suites never started (infrastructure ceiling, not a PR defect). The changed file's own suite was separately green at HEAD (26/26) via the verifier's probe baseline and the round-2 auditor.。
未探索到全部深度（达到工具调用预算）："agent 6a"：executing packages/cli/src/ui/utils/backgroundWorkUtils.test.ts — fresh worktree has no built workspace packages, and the prerequisite npm run build exceede…。
Test Plan（非阻断）：session/Session.test.ts — no such file or directory; client/App.tsx:8358 — no such file or directory。
收敛情况：第 5 轮发布了 2 条行内评论，其中 2 条是首次提出；上一轮发布了 2 条（其中 2 条首次提出）。发现反复回到同一批文件：packages/cli/src/ui/utils/backgroundWorkUtils.ts（第 3、4 轮已出过发现，本轮又有 1 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，或将本 PR 的评审降到 --severity-floor critical，可以避免循环反复推导同一组发现。本轮没有未决的 Critical，因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.3)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — packages/cli full unit suite did not complete within the harness per-command budget on this saturated runner (infrastructure ceiling, not a PR defect); the diff-touched file's own suite ran green at HEAD (26/26) via targeted runs.
Test Plan (not a blocker): session/Session.test.ts — no such file or directory; client/App.tsx:8358 — no such file or directory.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — packages/cli full unit suite did not complete within the harness per-command budget on this saturated runner (infrastructure ceiling, not a PR defect); the diff-touched file's own suite ran green at HEAD (26/26) via targeted runs。
Test Plan（非阻断）：session/Session.test.ts — no such file or directory; client/App.tsx:8358 — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.3)
…n-api The branch last took main at 9d19374; main advanced 55+ commits since, and GitHub had gone to `mergeable=false / dirty` on that staleness alone. That state stopped every heavy `pull_request` workflow — Qwen Code CI, Serve A/B, Real daemon E2E, Security Checks did not run on any of the last three heads — so the branch had gone unverified since 0653b42. The conflict was reported, not real: a trial merge resolved with zero conflicted files. Verified before committing, against the merged tree: - npm ci — clean (deps moved: root + vscode-ide-companion/web-shell/webui) - npm run typecheck — clean across every workspace, web-shell included - eslint on this PR's files — clean - core workflow suites — 397 passed - cli background-work / clear / branch / resume / workflows / tasks — 139 - acp-bridge — 1818; sdk-typescript — 1714 - acpAgent + Session + transport + server (the four files this branch and main both touched most) — 2779 passed Claude-Session: https://claude.ai/code/session_01VENc5rZYmMJDdqvjwBEyZd
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
11 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- bridge control-method wire params unpinned — already reported as R1-1 (comments 3882743799 / 3886393578)
- SDK outbound workflow-control wire contract unwitnessed — already reported as R1-16 (comments 3882743942 / 3886393617)
- reload flag flip diverges from one-shot tool registration (false→true enablement) — already reported in the round-2 deferred list (review 5057815234, acpAgent.ts:12422)
- retry/rerun resolve only the session registry; history-visible runs silently non-retryable — already reported in the round-2 deferred list (review 5057815234, acpAgent.ts:11082)
- post-refresh liveness re-check masked by removeTerminal refusal — already reported in the round-2 deferred list (review 5057815234, Session.test.ts:3702)
- R7-4 capped-window membership test unpinned — already reported as R1-5 (comment 3886393591)
- workflowDeletionSeqByRunId append-only and unbounded — already reported as R1-6 (comment 3886393593)
- opt-out gate untested with a non-empty live registry — already reported as R1-8 (comment 3886393596)
- sibling-egress replay-array redaction unwitnessed (branch/side-task/create) — already reported in the round-2 deferred list (review 5057815234, server.test.ts:14165)
- pushed available_commands_update not filtered by the child-side gate — already reported in the round-2 deferred list (review 5057815234, acpAgent.ts:7560)
- legacy-shape snapshot guards (?? []) unpinned — already reported in the round-2 deferred list (review 5057815234, tasksSnapshot.test.ts:343)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): chunk 11: executed tasksSnapshot.test.ts via vitest — the review worktree has no node_modules (vitest unresolvable), so I verified every assertion by manual trace ins…; chunk 8: none — I did not run Session.test.ts under vitest (round-7 CI state assumed green, and my one finding is independent of run results: the weak test passes in b…; "agent reverse-audit (round 1)": trace how the ACP child consumes the spawn-time workspaceTrusted option into folderTrustFeature / folderTrust (whether canUseWorkflowControls can diverge …; "agent 1b": none — but note I did not verify whether the legacy primary runtime's available-commands list can include the workflows entry on an untrusted workspace (the o….
Test Plan (not a blocker): session/Session.test.ts — no such file or directory; client/App.tsx:8358 — no such file or directory.
9 Suggestion(s) were drafted inline past the resolved critical posting floor; the CLI moved them into the deferral list below (floor enforcement).
Deferred under the convergence posture (round 7, not a blocker) — recorded, not requested in this round:
packages/cli/src/acp-integration/session/Session.test.ts:3725 — [review] R7-2: The 'waits for an active run owner to finish persistence before deletion' test cannot distinguish the waiting implementation from one with the wait removed — the…packages/cli/src/acp-integration/acpAgent.test.ts:11731 — [review] R7-3: The expect(settled).toBe(false) probe in 'cancels active workflow tasks' cannot discriminate the new if (handle) await handle.completion; guard (acpAgent.ts:11043-…packages/cli/src/acp-integration/session/Session.ts:9240 — [review] R7-4: The new workflow completion notification omits the structured i18n payload that all three sibling kinds set (agent Session.ts:9159, monitor :9190, shell :9216), and…packages/cli/src/acp-integration/session/tasksSnapshot.test.ts:382 — [review] R7-5: Every workflow fixture in the new tests carries a non-null meta , but meta is WorkflowMeta | null on both WorkflowTask and WorkflowSnapshot (workfl…packages/cli/src/acp-integration/acpAgent.test.ts:9383 — [review] R7-6: This assertion pins ( toHaveBeenCalledTimes(2) ) an unconditional workflow-snapshot disk load at session creation — but that load's product is dead: no production code …packages/web-shell/client/daemon/session/actions.test.ts:1927 — [review] R7-7: The only test exercising actions.controlWorkflowTask is the stale-session case, whose mock ignores arguments — the (taskId, action) argument pass-through to …packages/cli/src/serve/acp-http/transport.test.ts:8589 — [review] R7-8: Rewriting the pre-existing parameter-less _qwen/session/tasks wire test to send includeWorkflows: true leaves the trusted default branch — params['includeWorkflows…packages/sdk-typescript/src/daemon/DaemonSessionClient.ts:1019 — [review] R7-9: The new DaemonSessionClient.controlWorkflowTask pass-through forwards four positional args ( this.sessionId, taskId, action, this.clientId ) to sessionWorkfl…packages/web-shell/client/daemon/session/actions.ts:2202 — [review] R7-10: getWorkflowTasks carries a full ~25-line copy of getTasks 's error machinery (not-connected rethrow, silent-transient suppression, silent hard-failure dedup via …packages/sdk-typescript/src/daemon/DaemonClient.ts:3338 — [review] run-saved treats taskId as a saved-workflow name, undocumented on the SDK signaturepackages/cli/src/serve/routes/capabilities.ts:129 — [review] capabilities undefined-runtime fail-closed branch unwitnessed
[Critical] R7-1: [certifies-falsely] [new-surface] The standalone-session restore/resume route serves replay arrays without the workflow redaction this PR adds everywhere else. Every replay-array egress this diff converts to the trust-gated redactSdkSurfaceReplay wrapper leaves one production call site untouched: packages/cli/src/serve/routes/standalone-sessions.ts:429 answers POST /standalone/sessions/:id/{restore,resume} with bare omitSkillDetailsFromReplayArrays(restored) — no workflow redaction, and the file references workspace trust nowhere while every sibling registrar receives isWorkspaceTrusted: isPrimaryWorkspaceTrusted. The open premise resolves affirmatively: Config.isWorkflowsEnabled() is trust-independent (config.ts:7620-7627), BuiltinCommandLoader.ts:121 loads workflowsCommand solely on that flag, and Session.sendAvailableCommandsUpdateOrThrow emits the unfiltered snapshot — so available_commands_update frames carrying the workflows command exist in standalone replays on untrusted workspaces. Concretely: a daemon bound to an untrusted workspace with tools.workflowsEnabled: true in settings restores a standalone session whose persisted journal contains such a frame, and the frame reaches the client, while every parallel surface — supported_commands, GET supported-commands, GET tasks?includeWorkflows=true, session load/resume/fork/branch replay arrays, SSE frames, transcript pages — redacts the same shape; the fail-closed policy this PR documents ('the daemon boundary redacts the surfaces itself') fails open on exactly one route. Witness: probe (standalone route harness, mocked load returning an available_commands_update frame with a workflows entry) vs INTACT route — expected [ 'help', 'workflows' ] to deeply equal [ 'help' ] (frame passes through unredacted); vs candidate fix (response wrapped in redactWorkflowsFromReplayArrays) — 1 passed. Fix: give registerStandaloneSessionRoutes the same trust hook the other registrars receive and apply the same fail-closed shape, e.g. respond with omitSkillDetailsFromReplayArrays(isWorkspaceTrusted() ? restored : redactWorkflowsFromReplayArrays(restored)) (or lift the session-route redactSdkSurfaceReplay helper and reuse it). The fix must use the same verdict the other registrars receive — isWorkspaceTrusted: isPrimaryWorkspaceTrusted (packages/cli/src/serve/server.ts:1202) — not a new trust source. A route test mirroring server.test.ts's 'redacts workflows from untrusted load response replay arrays' (a restore response whose replay arrays contain an available_commands_update with { name: 'workflows' } must come back without it when the primary workspace is untrusted) must go red if the gate is removed.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 11 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：chunk 11：executed tasksSnapshot.test.ts via vitest — the review worktree has no node_modules (vitest unresolvable), so I verified every assertion by manual trace ins…；chunk 8：none — I did not run Session.test.ts under vitest (round-7 CI state assumed green, and my one finding is independent of run results: the weak test passes in b…；"agent reverse-audit (round 1)"：trace how the ACP child consumes the spawn-time workspaceTrusted option into folderTrustFeature / folderTrust (whether canUseWorkflowControls can diverge …；"agent 1b"：none — but note I did not verify whether the legacy primary runtime's available-commands list can include the workflows entry on an untrusted workspace (the o…。
Test Plan（非阻断）：session/Session.test.ts — no such file or directory; client/App.tsx:8358 — no such file or directory。
9 条 Suggestion 在已解析的 critical 发布下限之外被起草为行内评论；CLI 已将其移入下方延后清单（下限强制执行）。
收敛姿态下延后（第 7 轮，非阻断）——已记录，本轮不要求修改：共 11 条（原文未翻译，列表见上方英文部分）。
[Critical] R7-1: [certifies-falsely] [new-surface] The standalone-session restore/resume route serves replay arrays without the workflow redaction this PR adds everywhere else. Every replay-array egress this diff converts to the trust-gated redactSdkSurfaceReplay wrapper leaves one production call site untouched: packages/cli/src/serve/routes/standalone-sessions.ts:429 answers POST /standalone/sessions/:id/{restore,resume} with bare omitSkillDetailsFromReplayArrays(restored) — no workflow redaction, and the file references workspace trust nowhere while every sibling registrar receives isWorkspaceTrusted: isPrimaryWorkspaceTrusted. The open premise resolves affirmatively: Config.isWorkflowsEnabled() is trust-independent (config.ts:7620-7627), BuiltinCommandLoader.ts:121 loads workflowsCommand solely on that flag, and Session.sendAvailableCommandsUpdateOrThrow emits the unfiltered snapshot — so available_commands_update frames carrying the workflows command exist in standalone replays on untrusted workspaces. Concretely: a daemon bound to an untrusted workspace with tools.workflowsEnabled: true in settings restores a standalone session whose persisted journal contains such a frame, and the frame reaches the client, while every parallel surface — supported_commands, GET supported-commands, GET tasks?includeWorkflows=true, session load/resume/fork/branch replay arrays, SSE frames, transcript pages — redacts the same shape; the fail-closed policy this PR documents ('the daemon boundary redacts the surfaces itself') fails open on exactly one route. Witness: probe (standalone route harness, mocked load returning an available_commands_update frame with a workflows entry) vs INTACT route — expected [ 'help', 'workflows' ] to deeply equal [ 'help' ] (frame passes through unredacted); vs candidate fix (response wrapped in redactWorkflowsFromReplayArrays) — 1 passed. Fix: give registerStandaloneSessionRoutes the same trust hook the other registrars receive and apply the same fail-closed shape, e.g. respond with omitSkillDetailsFromReplayArrays(isWorkspaceTrusted() ? restored : redactWorkflowsFromReplayArrays(restored)) (or lift the session-route redactSdkSurfaceReplay helper and reuse it). The fix must use the same verdict the other registrars receive — isWorkspaceTrusted: isPrimaryWorkspaceTrusted (packages/cli/src/serve/server.ts:1202) — not a new trust source. A route test mirroring server.test.ts's 'redacts workflows from untrusted load response replay arrays' (a restore response whose replay arrays contain an available_commands_update with { name: 'workflows' } must come back without it when the primary workspace is untrusted) must go red if the gate is removed.
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
已修复最新 head 的 R7-1：standalone session 的 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No blocking issues. LGTM! ✅
Test Plan (not a blocker): session/Session.test.ts — no such file or directory; client/App.tsx:8358 — no such file or directory.
1 Suggestion(s) were drafted inline past the resolved critical posting floor; the CLI moved them into the deferral list below (floor enforcement).
Deferred under the convergence posture (round 8, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/routes/standalone-sessions.ts:437 — [review] R8-1: The new restore/resume trust gate's trusted branch has no test witness. The added it.each(['load', 'resume']) test drives only the untrusted harness ( isWorkspaceTr…packages/cli/src/serve/routes/standalone-sessions.ts:431 — [review] standalone restore/resume redaction open-codes the redactSdkSurfaceReplay composition session.ts centralizes (session.ts:211-219, five shared call sites); the next redactio…
中文说明
无阻断问题。LGTM！✅
Test Plan（非阻断）：session/Session.test.ts — no such file or directory; client/App.tsx:8358 — no such file or directory。
1 条 Suggestion 在已解析的 critical 发布下限之外被起草为行内评论；CLI 已将其移入下方延后清单（下限强制执行）。
收敛姿态下延后（第 8 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
已同步最新 main（40889bad75）并处理 4 处非机械冲突：保留 Workflow history/liveness 逻辑，同时接入 model-provider 热更新和 fail-closed runtime env reload；workflowsEnabledBySettings 仅在新环境成功应用后更新。验证：acpAgent.test.ts 558/558、Session.test.ts 762/762、run-qwen-serve.test.ts 364/364、mcp-client.test.ts 131/131，完整 build/typecheck 均通过。新 head 1bd5b3d 已触发 CI/review，待新 head 结果。 |
|
线程收敛说明：当前同步前的 exact-head review 已到第 8 轮并给出 APPROVED（C=0、S=0）；现存 28 条未关闭线程全部是此前轮次的 Suggestion 或其重复项，没有 Critical，且第 8 轮已明确按收敛策略延期、不要求本轮修改。本次仅同步 main 并解决冲突，不扩大 PR 范围；这些建议保留在评审记录中供后续处理，现按“已说明不修改”统一关闭。新 head 的 CI/review 仍需重新通过。 |
4 participants
What this PR does
This PR exposes Workflow execution through the daemon as an explicit opt-in extension of the existing session task contract. Opted-in clients can inspect live and persisted runs with phase, dispatch, token, log, approval, lineage, and terminal-state data; control active runs (cancel, pause, resume); retry a failed run or rerun a finished one; delete persisted history; and start saved definitions. The TypeScript SDK and the shared WebUI daemon adapter expose separate Workflow-aware methods while their existing task methods keep the legacy agent, shell, and monitor contract unchanged.
It also advertises Workflow availability and saved definitions at the workspace/session boundary, and applies one gate — feature enabled, non-bare, trusted workspace — to capability reporting, to every Workflow mutation, to the opt-in read path, and to the pushed available-commands stream (the
workflowscommand is redacted at the daemon egress for untrusted workspaces, not only on the pull surfaces). A workspace reload propagatestools.workflowsEnabledto sessions that were alive before it, so/capabilitiesand the per-session controls never disagree.Every session of a workspace shares one journal/snapshot store while keeping a private run registry, so the control plane is made consistent across sessions: a task-global mutation claim serializes delete-history, retry/rerun, run-saved and direct
resumeFromRunIdresumes; a liveness gate consults every sibling registry (and the registries of closed sessions until their runs drain) before a retry starts or a history entry is deleted; a deletion issued in one session is marked in its siblings so a refresh that raced it cannot republish the run; and a deletion whose registry entry cannot be retired is reported as not done rather than resurrected at settlement.Supersedes #9546 (same branch history, closed to reset a review thread history that had grown to ~180 threads over 11 review rounds). Every Critical finding from those rounds is addressed on this branch; the remaining open items there were Suggestion-level.
Why it's needed
The core runtime has structured Workflow execution state (#9034), but daemon consumers cannot observe or control it without this transport and client layer. An opt-in boundary lets new clients use that state without widening the legacy task unions or forcing incomplete Workflow semantics into existing task UIs, and it is the layer the Web Shell visualization (#8941) builds on.
Reviewer Test Plan
How to verify
GET /session/:id/tasks?includeWorkflows=true). The default response should contain only legacy task kinds; the opt-in response should additionally contain live and historical Workflow runs with their structured execution fields.{changed: false}and the run must keep its journal and snapshot. Close the first session mid-run and repeat: the gate must still refuse until the run settles.workflowscommand should not be advertised (pulled or pushed) or executed, the opt-in read should return no Workflow tasks, while an unrelated user-defined command with the same name remains compatible when the feature itself is disabled.tools.workflowsEnabled: falsein user settings andPOST /workspace/reload: the existing session's supported-commands must dropworkflowsand akind: 'workflow'cancel must answer{cancelled: false, reason: 'disabled'}; flipping it back re-enables both.Evidence (Before & After)
N/A — this PR adds daemon, SDK, and adapter contracts without a visual UI change.
Tested on
Environment (optional)
Local Node.js workspace. Linux:
packages/clifocused suitesacpAgent.test.ts+session/Session.test.ts(1274 tests),tsc --noEmitforcli,webui,sdk-typescript,acp-bridge, and builds ofcore,webui,sdk-typescript,acp-bridgeon the branch merged withupstream/maind6533785bd. Theweb-shelltypecheck error atclient/App.tsx:8358is present on thatmaincommit itself; this PR does not touchweb-shell.Risk & Scope
Linked Issues
Closes #9033
Supersedes #9546
中文说明
本 PR 做了什么
本 PR 通过显式 opt-in 的方式，在现有会话任务契约上扩展 daemon 的 Workflow 执行能力。选择加入的客户端可以查看实时和持久化运行，包括阶段、调度、token、日志、审批、血缘和终态数据；可以控制活动运行（cancel、pause、resume）、重试失败的运行或重跑已结束的运行、删除持久化历史，以及启动已保存的定义。TypeScript SDK 与共享 WebUI daemon adapter 提供独立的 Workflow 感知方法，现有任务方法继续保持 agent、shell、monitor 的旧契约不变。
同时，本 PR 在 workspace/session 边界声明 Workflow 可用性和已保存定义，并对能力声明、所有 Workflow 变更、opt-in 读取路径以及推送的 available-commands 流统一执行同一个门禁——功能启用、非 bare、可信 workspace（对不可信 workspace，
workflows命令在 daemon 出口处被脱敏，而不只是在拉取接口上）。workspace reload 会把tools.workflowsEnabled传播给 reload 之前已存在的会话，因此/capabilities与各会话的控制面永远不会出现分歧。同一 workspace 的所有会话共享一个 journal/snapshot 存储、但各自持有私有的运行注册表，因此控制面在跨会话层面做了一致性处理：一个任务级互斥申领串行化 delete-history、retry/rerun、run-saved 以及直接的
resumeFromRunId续跑；一个存活性门禁在 retry 启动或删除历史前查询所有兄弟会话的注册表（以及已关闭会话的注册表，直到其运行完全结束）；在一个会话发起的删除会在兄弟会话中打标记，避免与之竞态的刷新重新发布该运行；注册表条目无法退役的删除会被报告为未完成，而不是在结算时复活。替代 #9546（分支历史相同；关闭旧 PR 是为了重置 11 轮评审累积的约 180 条评论线程）。那些轮次中的每一条 Critical 发现都已在本分支上处理；遗留的未关闭项均为 Suggestion 级别。
为什么需要
Core runtime 已经具备结构化 Workflow 执行状态（#9034），但 daemon 消费端缺少观察和控制它的传输与客户端层。使用 opt-in 边界后，新客户端可以使用这些状态，同时不会扩大旧任务联合类型，也不会迫使现有任务 UI 提前承载不完整的 Workflow 语义；它也是 Web Shell 可视化（#8941）所依赖的层。
Reviewer 测试计划
如何验证
GET /session/:id/tasks?includeWorkflows=true）。默认响应应只包含旧任务类型；opt-in 响应应额外包含实时和历史 Workflow 运行及其结构化执行字段。{changed: false}，且运行的 journal 与 snapshot 必须保留。在运行中途关闭第一个会话再重复：门禁必须继续拒绝，直到运行结束。workflows命令，opt-in 读取不得返回任何 Workflow 任务；当功能本身关闭时，同名但无关的用户自定义命令仍应保持兼容。tools.workflowsEnabled改为false并调用POST /workspace/reload：既有会话的 supported-commands 必须不再包含workflows，kind: 'workflow'的 cancel 必须返回{cancelled: false, reason: 'disabled'}；改回true后两者恢复。证据（前后对比）
N/A — 本 PR 新增 daemon、SDK 和 adapter 契约，不包含可视化 UI 变化。
已测试平台
环境（可选）
本地 Node.js workspace。Linux 上：
packages/cli的聚焦测试acpAgent.test.ts+session/Session.test.ts（1274 个用例），对cli、webui、sdk-typescript、acp-bridge执行tsc --noEmit，并构建core、webui、sdk-typescript、acp-bridge，均在分支合并upstream/maind6533785bd之后进行。web-shell在client/App.tsx:8358的类型错误在该main提交上本身就存在；本 PR 不改动web-shell。风险与范围
关联 Issue
Closes #9033
替代 #9546