feat(workflows): finish Dynamic Workflows port — resume, saved workflows, keyword trigger, notifications (#4721) - #5600
Merged
Merged
Conversation
…vocation (#4721 P-nested) Fills the P1 `workflow()` throwing stub with a real nested-workflow primitive. `workflow('<name>')` resolves a saved script from `.qwen/workflows/<name>.js` (project) or `~/.qwen/workflows/<name>.js` (user); `workflow({scriptPath})` reads an explicit path. The resolved script runs as a nested orchestration that SHARES the parent run's agent-count cap, concurrency window, token budget, and emitter — so nested phases/logs and token spend roll into the same registry entry and the global caps bound parent + nested together. Single-level nesting is enforced structurally: the orchestrator injects the `workflow` impl only at the top level, so a nested workflow's sandbox has no impl and a second-level `workflow()` call lands in the throwing else-branch. No depth counter to drift. - NEW `workflow-saved.ts`: `resolveSavedWorkflowScript(nameOrRef, config)` + `listSavedWorkflows` + `validateWorkflowName` + `WORKFLOW_NAME_PATTERN`. Shared with the upcoming CLI slash-command loader and save dialog. Project scope shadows user scope (matches FileCommandLoader precedence). - `Storage`: `getProjectWorkflowsDir` / `getUserWorkflowsDir` (scripts) + `getWorkflowRunsDir` / `getWorkflowRunSnapshotPath` / `getWorkflowRunJournalPath` (reserved for P6/P7b run artifacts). - `workflow-sandbox.ts`: `SandboxOptions.workflow` + `__b.hasWorkflow/ hostWorkflow` bridge. The vm `workflow()` global mirrors `agent()`: args sanitized vm→host via JSON round-trip, single result revived back into the vm realm (T1/T8/T14 escape defense). Else-branch throws a clear "unavailable / single-level limit" message. - `workflow-orchestrator.ts`: `WorkflowRunRequest.resolveSavedWorkflow` injection seam; `workflowImpl` built in `run()` closing over the shared countedDispatch/parallel/pipeline/budget/emitter; nested sandbox created WITHOUT a workflow impl. - `WorkflowTool`: wires `resolveSavedWorkflowScript(ref, this.config)`. Tests: workflow-saved.test.ts (20: name/scriptPath resolution, scope precedence, miss errors, name validation, listing). orchestrator P-nested block (7: resolve+return, nested args, shared agent cap, shared budget, single-level throw, no-resolver throw, resolver-reject surfaces to parent). sandbox (workflow() unavailable + injected-impl revival + scriptPath passthrough). 237 workflow tests pass, lint clean. Refs #4721.
A workflow `agent()` could hang indefinitely — a looping model, a
provider stalling mid-stream, or a tool that never returns would only be
caught by the subagent's coarse 10-min `max_time_minutes`. P-stall adds a
fine-grained per-dispatch stall watchdog: after `stallMs` (default 60s,
env `QWEN_CODE_WORKFLOW_STALL_SECONDS`, per-call `agent({stallMs})`) of NO
observable progress, the dispatch is aborted and retried up to 3 times
before abandoning.
"Progress" = any reasoning-loop event (round start/end, streamed text,
token usage, tool call/result). The timer is SUSPENDED while a tool is in
flight, so a legitimately long tool call (90s build, slow MCP) is never
flagged — only true no-output-no-tool dead time counts toward the stall.
Design (low-invasiveness): `runStallResilient` owns the per-attempt
`AbortController` + `AgentEventEmitter`, chains the caller's parent signal
into the per-attempt controller, and hands both into `runSingleDispatch`
(the extracted single-attempt body of the former inline dispatch). A
stall fires `controller.abort('stalled')` → the subagent returns
CANCELLED → runSingleDispatch throws its "did not complete" terminal →
the wrapper retries when `watchdog.stalled()` is set AND the parent signal
is not aborted. Non-stall failures (MAX_TURNS / TIMEOUT / ERROR /
schema-nudge-exhaustion) propagate immediately — a retry won't fix a
deterministic outcome. Parent abort propagates without retry.
Schema-mode rescue is free: if a stall fires after the subagent already
captured a valid `structured_output`, runSingleDispatch returns that
payload before the terminate-mode check, so the wrapper sees success.
Uniform across both dispatch paths: the watchdog emitter is passed to the
fast-path `AgentHeadless.create` (new arg 7) AND to `runOverridePath`,
where `createSchemaEventEmitter` is refactored to `attachSchemaListeners`
so the schema `structured_output` capture and the stall watchdog observe
the SAME subagent emitter.
- NEW `workflow-stall.ts`: `attachStallWatchdog`, `runStallResilient`,
`resolveStallMs`, constants. 18 unit tests (fake-timer watchdog timing:
fire / reset-on-activity / suspend-during-tool / dispose / disabled;
retry loop: success / stall-3x-abandon / stall-then-recover /
non-stall-no-retry / parent-abort-no-retry / abort-propagation /
stallMs=0-passthrough).
- `WorkflowAgentOpts.stallMs` + sandbox `KNOWN_AGENT_OPTS` allowlist.
334 workflow tests pass, lint clean.
Scoped out (deliberate): throttle-backoff retry (degraded-GOAL detection
+ 45s sleep). It needs per-attempt duration/output-token threading into
the wrapper and risks false-positives on legitimately short answers
(a yes/no agent returns <50 tokens). The hang-prevention watchdog is the
high-value core; throttle is a refinement that can follow.
Refs #4721.
…ne (#4721 Gap-3) `settleToNullArray` (shared by `parallel()` / `pipeline()`) mapped every rejected thunk to `null` and logged each at debug level with the same "thunk rejected" message. A run that hits the token budget mid-fan-out drops its remaining slots via `WorkflowBudgetExceededError` — expected, capacity-shaped behaviour — but those drops were indistinguishable from arbitrary dispatch failures (rate limit, model outage) in the logs. Now budget-exhausted drops are counted separately (duck-typed on `err.name === 'WorkflowBudgetExceededError'`, since the cross-realm rejection's `instanceof` is unreliable) and summarized as `parallel: N slot(s) dropped — token budget exceeded`, while genuine failures keep their per-slot warning. Matches upstream's distinct budget-drop accounting. Behaviour to the script is unchanged (slots are still `null`); this is operator-facing observability only. Refs #4721.
`Workflow({resumeFromRunId})` re-runs a workflow and serves cached
results for the longest UNCHANGED PREFIX of agent() calls — a run
interrupted (crash, kill, network blip) at agent #40 resumes by replaying
the 39 journaled results instantly and only re-dispatching from the
divergence point. Every run journals (so any run is resumable); the
replay maps load only when resuming.
Key derivation (upstream `v2` parity): each dispatch's key is
`v2:sha256(prefixHash ‖ prompt ‖ canonicalOpts)`, where `prefixHash` is
the PREVIOUS dispatch's key. The rolling chain is what gives
"longest-unchanged-prefix" semantics — editing call #3 changes its key,
which re-keys #4, #5… so the cache naturally invalidates from the edit
point. `canonicalOpts` keeps only the dispatch-affecting opts
(schema/model/isolation/agentType) with keys sorted, so a re-serialized
schema or a label tweak doesn't bust the cache. Determinism (Date.now /
Math.random throw in the sandbox) guarantees the key chain is stable
across runs.
Critical invariant ("first miss invalidates the suffix"): once ANY
dispatch runs live during resume, `hadMiss` flips and no later dispatch
trusts the cache — even a later key that happens to match. The
prefix-hash chain already re-keys the suffix after a divergence, and
`hadMiss` is the belt-and-suspenders guard matching upstream's `f` flag.
The journal cache check runs BEFORE the budget gate and agent-count
cap, so a cached result is free: no token spend, no agent-cap slot, no
live dispatch. It still fires `agentDispatched` + `agentCompleted` so the
registry/UI counters advance. Result-append is fire-and-forget (a journal
write failure never fails the dispatch); the per-dispatch entry id is
captured in a closure so concurrent dispatches can't clobber it.
- NEW `workflow-journal.ts`: `WorkflowJournal` (jsonl-utils `read`/
`writeLine`), `deriveAgentKey`, `canonicalizeAgentOpts`, `buildReplay`.
- `WorkflowRunRequest.journal` + `.resumeReplay`; cache logic in
`countedDispatch`.
- `Storage.getWorkflowRunsDir` / `getWorkflowRunJournalPath` /
`getWorkflowRunSnapshotPath` (added in the P-nested commit).
- `WorkflowParams.resumeFromRunId` + schema; `WorkflowTool` reuses the
prior runId, loads the journal as replay, appends to the same file.
Tests: workflow-journal.test.ts (12: canonicalize projection/sort/
function-strip, key determinism/prompt/opt/cosmetic/chain sensitivity,
buildReplay last-write + accumulate, journal round-trip + missing-file).
orchestrator P6 block (5: normal run journals started+result, resume
serves cached prefix with 0 dispatches, first-miss-invalidates-suffix
with a mid-script prompt edit, cache hit advances registry counters,
cached dispatches bypass the agent-count cap). 351 workflow tests pass,
lint clean.
Refs #4721.
A function value is structurally assignable to `schema?: object`, so the `@ts-expect-error` on the function-strip test asserted a type error that never occurred — `tsc --noEmit` (which includes test files) flagged it as TS2578. Replace the directive with a comment explaining the test exercises the runtime strip of callable opt values.
…ws history (#4721 P7b-A2) The `WorkflowRunRegistry` is in-memory and dies with the CLI process, so `/workflows` could only ever show runs from the current session. Persist a JSON snapshot of each terminal run to `<projectDir>/workflows/<runId>.json` so the listing (and per-run detail view) survives a restart. - workflow-snapshot.ts: `toSnapshot`/`writeWorkflowSnapshot`/ `listWorkflowSnapshots` + retention prune (cap 30, oldest by mtime). perPhaseTokens is flattened to `[phaseOrNull, tokens]` pairs for JSON; a non-serializable script result degrades to a placeholder string. - WorkflowTask gains `script`/`scriptPath` (the snapshot carries the script source; also feeds the upcoming save-to-disk dialog). `script` defaults to '' in register() for legacy callers. - WorkflowTool registers the script source and, in its terminal `finally`, writes the snapshot once the registry entry has transitioned (best-effort, awaited so headless runs flush before the process exits). - /workflows merges disk snapshots into the listing (live registry entries win on a runId collision) and falls back to a snapshot in the detail view when the runId predates this process. Snapshot files (`<runId>.json`) never collide with resume journals (`<runId>/journal.jsonl`, in a subdir) — the `*.json` glob skips the dirs.
A workflow script saved at `.qwen/workflows/<name>.js` (project) or
`~/.qwen/workflows/<name>.js` (user) is now discoverable as a `/<name>`
slash command that runs it — the user-facing complement to the in-script
`workflow('<name>')` global (both resolve via core's `listSavedWorkflows`,
project scope shadowing user).
- SavedWorkflowLoader (CLI): a new ICommandLoader, wired into both loader
arrays (interactive + non-interactive) just before FileCommandLoader. Each
discovered workflow becomes a `{ type:'tool', toolName:'workflow',
toolArgs:{ scriptPath } }` dispatch. Gated on isWorkflowsEnabled (the tool
isn't registered otherwise), bare mode, and folder trust — mirroring
FileCommandLoader. Trailing text is forwarded to the script's `args`
global (parsed as JSON when valid, else the raw string).
- WorkflowTool: `scriptPath` param (XOR with `script`, enforced in
validateToolParamValues). When set, the tool reads the file at execution
time (hot reload) and records the resolved absolute path on the registry
entry as run provenance (feeds the P7b-A2 snapshot + the save dialog's
"already saved" branch).
- Activate the reserved `'workflow-command'` CommandSource; commands render
under a "Workflow" source label.
Re-exports listSavedWorkflows / resolveSavedWorkflowScript / validateWorkflowName
/ getSavedWorkflowDirs / WORKFLOW_NAME_PATTERN from core for CLI consumers.
#4721 P7b-A3) The `/workflows` detail view now offers `s` to save a finished run's script to `.qwen/workflows/<name>.js` (project) or `~/.qwen/workflows/<name>.js` (user) — the run becomes a `/<name>` slash command (P7b-A1) and a `workflow('<name>')` target (P-nested). - core `saveWorkflowScript(config, { name, scope, script, overwrite })`: validates the name, refuses to clobber unless `overwrite`, returns a discriminated result (saved / exists / invalid-name / empty-script) so the UI can prompt rather than throw. Reuses the saved-workflow dir + name rules. - WorkflowSaveOverlay: a self-contained overlay (single keypress handler with a minimal inline name editor — names are short kebab strings) for name entry, Tab scope toggle, overwrite confirmation, and saved/error states. The parent dialog yields all keys to it while open. - BackgroundTasksDialog: `s` opens the overlay for a terminal workflow entry that still carries its script; the hint row advertises it. The new `/<name>` command surfaces on the next session (the in-memory command list is not hot-reloaded mid-session); the file is usable immediately via `workflow('<name>')`.
…ger) Mentioning the word `workflow` in a prompt now softly steers that turn toward the Workflow tool, and the Footer shows a `⚙ workflow active` indicator for the steered turn. - workflow-keyword.ts: `detectWorkflowKeyword` (whitespace-tokenized, edge-punctuation-stripped — `workflow`/`workflow.`/`(workflow)` match; `workflows`, `dataflow`, `my-workflow-runner` do not) + `buildWorkflowSteeringNotice` (a soft nudge, not a forced tool call). - AppContainer.handleFinalSubmit: on a keyword hit (feature enabled, not a slash command, opt-out unset) prepend a `<system-reminder>` — the same one-shot mechanism as the worktree-restore notice — and arm the indicator; it clears when the turn returns to idle. - UIState gains `workflowKeywordActive`; Footer renders the indicator next to the worktree line; `ui.disableWorkflowKeywordTrigger` setting opts out. No `ultracode` naming — the trigger keyword is the plain word `workflow`, per the original issue.
A workflow can run for minutes as a single tool call; the user shouldn't have to watch the /workflows dialog to learn it finished. Fire a terminal-bell notification when a run reaches `completed` / `failed`. - WorkflowRunRegistry gains a `notificationCallback` slot (separate from the dialog-owned `statusChangeCallback`), fired in `complete()` / `fail()` — NOT `cancel()`, since a user-initiated cancel needs no notification. - AppContainer wires the callback to the existing `sendNotification` service (gated on the `general.terminalBell` setting), mirroring the agent attention-notification path. The call is optional so partial registry mocks in CLI tests no-op instead of throwing.
Two lightweight OpenTelemetry events (no-op unless telemetry is enabled), following the SpeculationEvent template: - `qwen-code.workflow_keyword` — the `workflow` keyword steered a turn (fired from AppContainer alongside the trigger). - `qwen-code.workflow_run` — a run reached a terminal state, with status + agents dispatched/completed + phase count + tokens + duration (fired from the WorkflowTool's terminal finally; wrapped so a logging failure can never mask the tool result).
P7b-A2) Covers `toSnapshot` (perPhaseTokens Map flattening, non-JSON result placeholder, defensive array copy), the disk round-trip + newest-first sort, unparseable-file skipping, the missing-dir tolerance, and the MAX_RETAINED_SNAPSHOTS mtime prune — none of which were exercised directly before (only indirectly via the /workflows merge tests).
…test (#4721) Five `import('./workflow-journal.js').JournalEntry[]` annotations tripped `@typescript-eslint/array-type` (T[] forbidden for non-simple types). Pure type-annotation change; behavior unchanged.
…mmand label (#4721) Self-review (doc-vs-code drift) caught the model-facing tool description still claiming "No resume and no background execution yet (scheduled for later phases)" — both now exist on this branch (P6 resume via resumeFromRunId; runs tracked in /workflows + the background-tasks view). Replace the stale clause with the real capabilities so the model isn't told a present feature is absent. Also: point the schema XOR meta-comment at the `scriptPath` description (which states it) rather than `script`'s (which doesn't), and add the missing `formatCommandSourceLabel('workflow-command') === 'Workflow'` test guarding the exhaustive CommandSource Record.
…ordTrigger (#4721) Generated counterpart of the new `ui.disableWorkflowKeywordTrigger` setting added in the P7-trigger commit; the schema mirror is committed in this repo.
qqqys
requested changes
Jun 22, 2026
qqqys
left a comment
qqqys
left a comment
Collaborator
There was a problem hiding this comment.
Critical: resumeFromRunId is used as a raw path segment for both the run id and the journal path.
At packages/core/src/tools/workflow/workflow.ts:240, any caller-provided resumeFromRunId becomes runId, and line 251 passes it directly to storage.getWorkflowRunJournalPath(runId). The storage helpers added in packages/core/src/config/storage.ts:383 and packages/core/src/config/storage.ts:390 then use path.join(...) with that value. A value such as ../../chats/<id> escapes <projectDir>/workflows, so running or resuming a workflow can read/write journal and snapshot files outside the workflow artifact directory and overwrite/corrupt other runtime data. This is user-controlled via the tool parameter, so it breaks the storage boundary for workflow/session artifacts.
Please validate resumeFromRunId before using it in any storage path, for example by accepting only generated run ids like wf_[0-9a-f]{16}, or route it through a dedicated sanitizer and verify the resolved snapshot/journal paths stay under getWorkflowRunsDir().
wenshao
requested changes
Jun 22, 2026
wenshao
reviewed
Jun 22, 2026
wenshao
requested changes
Jun 22, 2026
#4721) A real-scenario tmux run surfaced this: `descendFromComposer` only focuses the live-agent panel (agent-kind entries) or the Arena tab bar, and a workflow is never a live-agent-panel entry (`isLiveAgentPanelVisibleEntry` requires `kind === 'agent'`). So in a session whose only background task is a workflow, pressing ↓ from the composer focused nothing — the BackgroundTasksDialog could not be opened by keyboard at all, which made the P7b-A3 save action (and the per-run detail view) unreachable for exactly the workflow-only case they target. Add a final `descendFromComposer` branch: when there's no live-agent panel and no Arena tab bar but the background-tasks pill IS shown (`bgEntries.length > 0`), focus the pill, completing the composer → pill → dialog chain.
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Collaborator
Author
End-to-end demo (real model, real sub-agent dispatch)Full lifecycle recorded against the local build (
The GIF lives on a throwaway |
wenshao
reviewed
Jun 22, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Incremental review of 5a1fb6a (the InputPrompt background-pill focus fix added since the previous review). The change is correct and well-scoped: the new bgEntries.length > 0 branch matches the pill's own render guard (BackgroundTasksPill returns null when entries.length === 0), so it can't strand focus on a hidden pill; it reuses the existing pillFocused → ↓ → openDialog chain; and the useCallback deps are complete. Minor nit (non-blocking): no regression test covers the new workflow-only focus branch, and the function's lead comment (the top→bottom focus order) wasn't updated to include the new pill step.
This commit does not touch the previously-flagged path-traversal Criticals (resumeFromRunId run-id, and scriptPath / workflow('<name>') containment), which remain open. Not approving: those Criticals are still unresolved and CI is still running on this commit.
— claude-opus-4-8[1m] via Qwen Code /qreview
wenshao
reviewed
Jun 22, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Incremental re-review of 5a1fb6a6d2e6994b1e19ebcd2cdce56ba70630ce found no new actionable issues in the latest InputPrompt.tsx-only update. I am leaving this as a comment rather than approval because existing review comments remain open and CI is still pending.
Verification:
qwen review deterministic ...reported 0 findings for the changed file; directnpm run typecheckandcd packages/cli && npx tsc --noEmitpassed.cd packages/cli && npx vitest run src/ui/components/InputPrompt.test.tsx src/ui/components/background-view/BackgroundTasksPill.test.tsx src/ui/components/background-view/BackgroundTasksDialog.test.tsxpassed (232 tests).
— GPT-5 Codex via Qwen Code /review
wenshao
reviewed
Jun 22, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
[Suggestion] Missing test coverage: no test asserts setBgPillFocused is called when the new else if (bgEntries.length > 0) branch fires. The mock infrastructure is already in place (mockViewActions.setBgPillFocused in InputPrompt.test.tsx), so adding a test case that sets agents: new Map(), provides a non-agent bg entry (kind: 'workflow'), presses Down, and asserts setBgPillFocused was called with true would cover the gap.
— qwen3.7-max via Qwen Code /review
wenshao
reviewed
Jun 22, 2026
…dless, resume, stall (#4721, PR #5600) Security: - Validate `resumeFromRunId` as `wf_<hex>` before it flows into the journal/ snapshot paths (path.join), closing a path-traversal write/read outside `<projectDir>/workflows`. - `resolveSavedWorkflowScript`: validate the string name (no `../` escape) and route both the name and `scriptPath` forms through a realpath boundary check that refuses anything resolving outside the saved-workflow dirs — this also defeats symlink escape. `listJsFiles` skips symlinked entries during discovery. Correctness: - Saved-workflow slash commands are `interactive` only — their `{type:'tool'}` action becomes `unsupported` in headless/ACP, so advertising those modes surfaced a command that then failed. - Skip workflow keyword steering for `?btw`/`/btw` so the system-reminder prefix no longer breaks BTW routing. - Seed the resume prefix-hash chain with `sha256(args)` so a resume with different args misses the journal and re-runs live instead of silently replaying the prior run's results. - The stall watchdog arms on the FIRST response event, not at attach time, so a reasoning model's slow time-to-first-token is not a false stall (that window is bounded by the subagent's max_time); it now detects post-first-response streaming stalls. Resource / docs: - `pruneSnapshots` also removes each pruned run's `<runId>/journal.jsonl` directory (previously leaked unboundedly). - Drop the inaccurate "save dialog offers 'already saved'" comment (no such affordance exists) from the three sites. Adds security + behavioural tests for each; declined as out of scope: nested- workflow log accumulation, a __proto__-guard test, hung-tool watchdog suspend (max_time backstops it), and the save TOCTOU.
Collaborator
Author
Review round 1 — addressed in 564593aThanks for the thorough security pass. Outcomes: Fixed (9):
Declined (out of scope for this round / backstopped):
Each fix carries a security/behavioural test. All inline threads have been replied to and resolved. |
Collaborator
|
Thanks for the PR! This finishes the Dynamic Workflows surface — resume, saved workflows, keyword trigger, snapshots, notifications. Big chunk of work. Template looks good ✓ — all required sections present, evidence provided, tested-on table filled. Direction: aligned. This is the natural continuation of #4721 (P1–P5 already merged). Workflows without resume/save/discovery are half a feature — this PR closes the gap. The upstream feature this ports from has these capabilities, so the direction signal is clear. Scope: 42 files, 4186 additions, ~10 sub-features in one PR. That's a lot, but I think bundling is defensible here — the sub-features are tightly coupled (resume needs the journal, saved workflows need the slash command loader, snapshots need the registry, the save dialog needs the saved-workflow resolver). Splitting into 5 smaller PRs would create merge-conflict hell and make each individually unreleasable. The tradeoff is a harder review, but that's on us, not the contributor. On the existing review comments: the path-traversal criticals flagged by @qqqys appear to be addressed in the current code:
These are defense-in-depth (name validation AND realpath check), which is the right approach for a tool that accepts user-controlled paths. One concern before code review: tested only on macOS per the PR body. This CI runs on all three platforms — worth watching for platform-specific failures in the journal/snapshot paths (which use Moving on to code review and real-scenario testing. 🔍 中文说明感谢贡献！这个 PR 完成了 Dynamic Workflows 的完整移植——恢复、保存、关键词触发、快照、通知。工作量很大。 模板完整 ✓ — 所有必填章节都有，提供了证据，测试平台表已填写。 **方向：**对齐。这是 #4721（已合并的 P1–P5）的自然延续。没有恢复/保存/发现功能的 Workflow 只是半成品——这个 PR 补全了这个缺口。上游对应的功能确实有这些能力，方向信号明确。 **范围：**42 个文件，4186 行新增，约 10 个子功能集中在一个 PR 里。量很大，但我认为打包是合理的——子功能紧密耦合（resume 需要 journal，保存的 workflow 需要 slash command loader，快照需要 registry，保存对话框需要 saved-workflow resolver）。拆成 5 个小 PR 会导致合并冲突地狱，且每个单独都不可发布。代价是审查更难，但这是我们的事，不是贡献者的。 关于已有的 review 评论：@qqqys 标记的路径穿越 Critical 在当前代码中看起来已修复：
这些是纵深防御（名称验证 + realpath 检查），对于接受用户控制路径的工具来说是正确的做法。 **一个顾虑：**PR body 显示仅在 macOS 上测试。CI 在三个平台运行——需要关注 journal/snapshot 路径的平台特定问题（使用了 进入代码审查和真实场景测试。🔍 — Qwen Code · qwen3.7-max |
…#4721) Round-1's descendFromComposer pill branch (5a1fb6a) only reached the pill when no Arena tab bar was present. When both an Arena roster and a background-tasks pill (e.g. a workflow run) are shown, ↓ from the composer stops at the tab bar, whose Down handler was a no-op — stranding the pill and the run's detail/save dialog behind it. AgentTabBar's Down now descends into the pill when one is shown (bgEntries > 0), completing the chain BackgroundTasksPill already documents (Composer ↓ → AgentTabBar ↓ → Pill ↓ → Dialog); the dialog is now reachable in all four roster/pill coexistence cases. Also refresh the stale descendFromComposer comment to name the pill destination and add InputPrompt coverage for the round-1 pill branch.
Collaborator
Author
|
Review round 2 (InputPrompt focus-chain follow-ups) — addressed in dd587f9
typecheck + lint clean; |
Collaborator
Code ReviewIndependently, I'd approach this the same way: a Security — path traversal (addressing @qqqys's review): The concerns are fixed with defense-in-depth. I verified directly: All five attack vectors blocked. The security is layered (name check AND realpath check), which is the right call for user-controlled paths. Journal design: the rolling prefix-hash chain ( Stall watchdog: arms on first progress event (not attach time) — correctly avoids false-tripping on slow first-response. Suspends during in-flight tools (a 90s shell build isn't a stall). Uses No critical blockers found. 125 new unit tests pass (96 core + 29 CLI). Typecheck clean. Build succeeds. Real-Scenario TestingCLI smoke test (feature flag)CLI boots and responds normally with workflows enabled. ✓ Keyword detection (direct verification)All 7 cases correct — matches the stricter "standalone word" semantics (not Saved workflow discoveryCreated Security tests (path traversal + symlink)All 5 security scenarios verified. ✓ Not testedFull interactive workflow execution and resume require model API calls — no API key in this CI environment. The unit tests comprehensively cover the execution path (96 core tests including journal replay, stall retry, saved-workflow resolution, and snapshot pruning). The save dialog and terminal bell are covered by component tests (7 overlay tests). 中文说明代码审查独立来看，我会采用相同的方法： 安全——路径穿越（回应 @qqqys 的审查）： 问题已用纵深防御修复。我直接验证了：
所有五个攻击向量都被阻止。安全是分层的（名称检查 + realpath 检查），对用户控制的路径来说是正确的做法。 日志设计： 滚动前缀哈希链 + 未发现关键阻塞问题。 125 个新单元测试全部通过。类型检查干净。构建成功。 真实场景测试
— Qwen Code · qwen3.7-max |
Collaborator
|
Stepping back to think about the whole picture. This PR finishes the Dynamic Workflows surface — it's the bridge from "workflows exist" (P1–P5) to "workflows are reusable, durable, and discoverable." My independent proposal would have looked almost identical: The security concerns that drove the existing CHANGES_REQUESTED reviews are resolved. I verified all five attack vectors directly — path traversal via name, path traversal via scriptPath, symlink in workflow dir, resumeFromRunId path injection, and valid-resolution happy path. Defense-in-depth works. The earlier reviewers flagged the right thing; the contributor fixed it the right way. 125 new tests, all green. Typecheck clean. Build clean. The keyword detection behaves exactly as documented (7/7 cases verified independently). The saved-workflow discovery works end-to-end (created a file, found it, resolved it, ran it through security checks). What I'd flag as mild concerns (not blockers): The scope is large (42 files, 4186 additions, ~10 sub-features). In isolation I'd have preferred smaller PRs, but the coupling is real — resume needs the journal, saved workflows need the loader, snapshots need the registry. Splitting would have created integration pain without reducing review complexity. The contributor made the right call. Only macOS tested per the PR body. The code uses The stall watchdog timeout (60s default) is a heuristic. A genuinely slow model response that happens to produce events just under the threshold won't trip it; a model that pauses for 61s between tokens will. The env override ( Bottom line: this ships the feature cleanly, the security is solid, the tests cover the surface, and the code is something I'd be fine maintaining. Approving. ✅ 中文说明退一步看全局。 这个 PR 完成了 Dynamic Workflows 的完整能力面——从"workflow 存在"（P1–P5）到"workflow 可复用、持久化、可发现"的桥梁。我的独立方案几乎完全一样： 驱动现有 CHANGES_REQUESTED 审查的安全问题已解决。 我直接验证了所有五个攻击向量——通过名称的路径穿越、通过 scriptPath 的路径穿越、workflow 目录中的符号链接、resumeFromRunId 路径注入、以及正常解析的 happy path。纵深防御有效。之前的审查者标记了正确的问题；贡献者以正确的方式修复了它。 125 个新测试，全部通过。 类型检查干净。构建干净。关键词检测行为完全符合文档（独立验证 7/7 用例）。保存的 workflow 发现端到端工作（创建文件、找到它、解析它、通过安全检查运行）。 轻微顾虑（非阻塞）： 范围较大（42 文件，4186 行新增，约 10 个子功能）。孤立来看我会更倾向于小 PR，但耦合是真实的——恢复需要日志，保存的 workflow 需要加载器，快照需要注册表。拆分会增加集成痛苦而不减少审查复杂度。贡献者做了正确的选择。 PR body 显示仅在 macOS 上测试。代码全程使用 stall 看门狗超时（默认 60s）是启发式的。一个恰好低于阈值产生事件的慢模型响应不会触发它；一个在 token 之间暂停 61s 的模型会。环境变量覆盖和每次调用的 结论： 这个 PR 干净地交付了功能，安全扎实，测试覆盖了全面，代码我愿意维护。批准。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
previously approved these changes
Jun 22, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
wenshao
requested changes
Jun 22, 2026
A checked-in `.qwen/workflows -> /outside` symlink turned its external target into the trusted boundary: readWorkflowFileSecurely realpaths the root, so discovery listed, workflow('<name>') read, and the save dialog wrote external files. Round 1's per-entry symlink guard missed this because the link is the dir, not the files it exposes (which are regular).
Refuse a symlinked root for all three operations via isSymlinkedRoot (lstat). Symlinked *ancestors* are still tolerated (realpath keeps a project under macOS `/tmp -> /private/tmp` working) — only the workflows dir itself must not be a link. Each vector carries a RED-before-fix security test (`.qwen/workflows -> outside`: discovery excludes, read refuses, save throws).
wenshao
reviewed
Jun 22, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
No new review findings in the latest update. Downgraded from Approve to Comment: CI still running.
— GPT-5 Codex via Qwen Code /review
wenshao
requested changes
Jun 22, 2026
Collaborator
✅ Verification — Dynamic Workflows port: saved workflows, cross-session history, keyword trigger, and journal resume all work liveI ran an independent local verification of this PR on Linux at head Method
Unit surface — 443 workflow tests pass (0 failed)Core 362 ( Runtime verifications (real CLI)1) Saved-workflow discovery + run (P7b-A1) — and a snapshot was persisted to 2) Cross-session history (P7b-A2) — survives a full process restart The fresh process has an empty in-memory registry, so the listing comes purely from the on-disk snapshot. ✅ 3) The injected reminder is the soft nudge (verbatim from the request): "…If this request benefits from orchestrating multiple steps or subagents, strongly prefer the Workflow tool… If a workflow is not a good fit for this request, proceed normally." — never a forced tool call. 4) Same-session resume via JSONL journal (P6) — a workflow with two The journal records each agent result keyed by a rolling prefix-hash: On the resume run, both 5) Save-to-disk dialog (P7b-A3) — the multi-key interactive save flow is covered by its component test ( 📝 Notes for merge (non-blocking)
Verdict (verification side): the four runtime-visible headline features — saved-workflow discovery/run, cross-session history, the keyword trigger + footer, and journal resume — all behave exactly as described against the real CLI, and the 443-test workflow unit surface is green. Looks merge-ready from here; final call is the maintainers'. 🇨🇳 中文版（点击展开）✅ 验证 —— Dynamic Workflows 移植:保存的工作流、跨会话历史、关键词触发、journal 续跑均实测可用我在 方法
单测面 —— 443 个工作流测试通过(0 失败)Core 362( 运行时验证(真实 CLI)1)保存的工作流发现 + 运行(P7b-A1) —— 并把 snapshot 持久化到 2)跨会话历史(P7b-A2)—— 经受完整进程重启 全新进程的内存 registry 为空,因此该列表完全来自磁盘上的 snapshot。✅ 3) 注入的 reminder 是软提示(取自请求原文):"…If this request benefits from orchestrating multiple steps or subagents, strongly prefer the Workflow tool… If a workflow is not a good fit for this request, proceed normally." —— 从不强制工具调用。 4)同会话 journal 续跑(P6) —— 一个带两个 journal 用滚动前缀哈希为每个 agent 结果建键: 续跑那次,两个 5)保存到磁盘对话框(P7b-A3) —— 多键交互的保存流程由其组件测试( 📝 合并参考(非阻塞)
结论(验证视角): 四个运行时可见的核心特性 —— 保存的工作流发现/运行、跨会话历史、关键词触发 + footer、journal 续跑 —— 在真实 CLI 上的行为与描述完全一致,且 443 个工作流单测全绿。从这里看具备合并条件;最终决定权在维护者。 |
wenshao
approved these changes
Jun 22, 2026
qqqys
approved these changes
Jun 22, 2026
qqqys
left a comment
qqqys
left a comment
Collaborator
There was a problem hiding this comment.
Re-reviewed the prior path-traversal concern. The latest head validates resumeFromRunId before it reaches workflow journal/snapshot paths, and the related workflow boundary tests pass locally.
LaZzyMan
added a commit
that referenced
this pull request
Jun 23, 2026
pruneSnapshots derived runId from a snapshot filename and passed it straight to fs.rm(..., { recursive: true, force: true }). The listing is a plain *.json glob, so a file named ...json yields runId '..' and the rm deletes the runs dir's PARENT (the project root, .git and all); notarun.json deletes a sibling dir. A malicious repo could commit such a file under workflows/; once a victim runs more than the retention cap, the prune wipes their project.
Gate the recursive delete on the generated wf_<hex> run-id shape (the same pattern workflow.ts already uses to validate resumeFromRunId). The .json unlink stays unconditional — it removes exactly that one file, never a directory. Carries a RED-before-fix test that plants ...json / notarun.json as the oldest snapshots and asserts the parent canary and a sibling dir survive.
Follow-up to PR #5600 (merged); addresses review thread r3451484367.
wenshao
pushed a commit
that referenced
this pull request
Jun 23, 2026
…5740) pruneSnapshots derived runId from a snapshot filename and passed it straight to fs.rm(..., { recursive: true, force: true }). The listing is a plain *.json glob, so a file named ...json yields runId '..' and the rm deletes the runs dir's PARENT (the project root, .git and all); notarun.json deletes a sibling dir. A malicious repo could commit such a file under workflows/; once a victim runs more than the retention cap, the prune wipes their project. Gate the recursive delete on the generated wf_<hex> run-id shape (the same pattern workflow.ts already uses to validate resumeFromRunId). The .json unlink stays unconditional — it removes exactly that one file, never a directory. Carries a RED-before-fix test that plants ...json / notarun.json as the oldest snapshots and asserts the parent canary and a sibling dir survive. Follow-up to PR #5600 (merged); addresses review thread r3451484367.
8 tasks
8 tasks
This was referenced Aug 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Finishes the Dynamic Workflows port (#4721) on top of the already-merged P1–P5 foundation, bundling all remaining work into one PR. It adds, in dependency order:
workflow('<name>')nested global (P-nested) — a running workflow can invoke a saved workflow by name (single level only; the nested sandbox lacks the impl, so deeper nesting throws).parallel()/pipeline()distinguish a slot dropped by the token cap from an ordinary error.resumeFromRunIdreplays a rolling prefix-hash chain;agent()calls whose(prompt, opts)match the journal are served from cache for the longest unchanged prefix, then the run goes live from the first miss.<projectDir>/workflows/<runId>.json, so/workflowsshows a recent history that survives a restart (retention-capped, oldest pruned)./<name>slash commands (P7b-A1) — a script saved at.qwen/workflows/<name>.js(project) or~/.qwen/workflows/<name>.js(user) is discoverable as/<name>, which dispatches theworkflowtool with the file path (read fresh each run). The tool gained ascriptPathparam (XOR with inlinescript)./workflowsdetail view,ssaves a completed run's script to a named.qwen/workflows/<name>.js(project/user scope toggle, overwrite confirm).workflowkeyword trigger + footer indicator (P7-trigger) — mentioning the wordworkflowin a prompt softly steers that turn toward the Workflow tool (a<system-reminder>nudge, never a forced tool call) and shows a⚙ workflow activefooter indicator. Opt-out viaui.disableWorkflowKeywordTrigger.qwen-code.workflow_keywordandqwen-code.workflow_runevents (no-op unless telemetry is enabled).Whole-run cancel already works from the
/workflowsdialog (xaborts the run). Per-agent pause/retry/skip and the ACP'workflow'daemon propagation are intentionally deferred to follow-ups.Why it's needed
P1–P5 shipped the workflow engine and the live
/workflowsview, but a workflow couldn't be resumed, saved, re-run by name, or noticed when it finished, and there was no opt-in trigger. This PR closes that gap so workflows become reusable, durable across sessions, and discoverable — matching the capability surface of the upstream feature this was reverse-engineered from.Reviewer Test Plan
How to verify
Enable the feature with
QWEN_CODE_ENABLE_WORKFLOWS=1..qwen/workflows/demo.jswithexport const meta = { name: 'demo', phases: [{title:'Plan'}] }; phase('Plan'); log('hi'); return 'done';. Start the CLI; type/demo→ autocomplete showsdemo [json-args] Run the "demo" saved workflow (project). Run it → it completes withresult: "done"./workflows→ the completed run is listed. Restart the CLI (fresh process) and run/workflowsagain → the run is still listed (loaded from the on-disk snapshot, not in-memory state).workflow→ the footer shows⚙ workflow activeduring the turn; it clears when the turn returns to idle.workflows,dataflow,my-workflow-runnerdo NOT trigger.agent()calls, then call the tool again withresumeFromRunId: <id>and the same script → matchingagent()calls are served from the journal cache.Enterfor detail,sto save, type a name,Tabto toggle scope,Enter→ the script is written to.qwen/workflows/<name>.jsand becomes/<name>next session.Evidence (Before & After)
Verified locally via tmux against the local build (
node dist/cli.js, real auth):/demo-researchsaved-workflow autocomplete + full execution (result: "done", logs/phases parsed)./workflows: empty state → populated listing → survives a process restart (run re-listed purely from the on-disk snapshot at~/.qwen/projects/<hash>/workflows/<runId>.json).⚙ workflow activeappears on aworkflow-keyword submit and the steered turn completes normally.The save dialog (P7b-A3), the terminal bell (P-notif, an escape sequence), and per-run cancel (P-tui) are covered by unit/component tests rather than tmux (the interactive multi-key save flow and escape-sequence bell aren't reliably driveable via
send-keys). Full suite: the workflow surface across both packages is green (380+ workflow-specific tests), plus workspace typecheck and lint.Tested on