diff --git a/docs/dag-system-review-2026-08-09.md b/docs/dag-system-review-2026-08-09.md new file mode 100644 index 0000000000..318df8814d --- /dev/null +++ b/docs/dag-system-review-2026-08-09.md @@ -0,0 +1,127 @@ +# DAG system review — 2026-08-09 + +## Outcome + +The review found one reproducible correctness defect, fixed it before any +architecture work, and found no second defect that could be reproduced through +a supported runtime path. The scheduling core is modular and well covered. The +remaining work is change-locality and recovery hardening, not a rewrite. + +## Scope and evidence + +Reviewed surfaces: + +- graph validation, admission, scheduling, transitions, projection, and store + code under `packages/core/src/dag`; +- workflow commands, runtime scheduling, recovery, spawning, wake delivery, and + summary publication under `packages/opencode/src/dag` and + `packages/opencode/src/tool/workflow.ts`; +- DAG inspector state, reducers, layout helpers, and rendering under + `packages/tui/src`; +- durable events and generated boundary types used by those packages. + +Verification baseline: + +- `packages/core`: 90 targeted DAG tests passed; +- `packages/opencode`: 389 targeted DAG tests passed before the fix, 392 after + adding three deterministic regressions; +- `packages/tui`: 50 targeted DAG tests passed; +- `packages/opencode`: `bun typecheck` passed; +- the `dev` push CI for the fix passed Linux E2E; Linux unit and Windows E2E + were still running when this report was written. + +Coverage is strongest at the core state-machine seams: graph validation, +scheduling, transitions, admission, evaluation, and projection are effectively +fully covered. Runtime execution is also high but less complete: +`src/dag/runtime/loop.ts` was 93.73% line / 92% function, spawn 94.58% line, +recovery 99.30% line, and the workflow tool 89.81% line / 79.49% function. +Coverage alone did not expose the confirmed timing defect. + +## Confirmed bug — fixed + +### Summary updates could be lost during an in-flight read + +`packages/opencode/src/dag/runtime/summary-publisher.ts` used a `Set` as an +early-return coalescer. An event arriving while a workflow or Session summary +read was already running observed the key in the set and returned. The active +read could have captured the old state, and no dirty rerun was scheduled, so +the TUI could remain stale until an unrelated later event. + +Three `Deferred`-gated regression tests reproduced the lost update for: + +1. two events for one workflow; +2. two workflows sharing one parent Session; +3. a newer event arriving while the first read fails. + +The fix replaces the boolean in-flight set with keyed dirty state. Events +during debounce are absorbed; events during an active read mark the key dirty; +completion or failure reruns once with the latest durable state. Interruptions +still propagate. The fix is merged to `dev` in PR #202. + +## Architecture findings + +### A1 — Runtime loop has poor change locality (high, no behavior change yet) + +`packages/opencode/src/dag/runtime/loop.ts` constructs most runtime behavior +inside one roughly 1,200-line `layer` closure. It owns adoption, subscriptions, +child-session ownership, spawn planning, wake batching, delivery, recovery, +and terminal decisions. The public module is deep, but the internal +collaborators are invisible to code navigation and can only be tested through +the whole layer. + +Recommended boundary: keep one public runtime layer, but extract cohesive +private constructors for child ownership, wake delivery, and recovery. Each +constructor should receive the minimum services it uses and expose only the +operation needed by the coordinator. Do this in behavior-preserving commits +after adding tests for the uncovered failure branches. + +### A2 — Workflow command dispatch mixes transport and domain preparation (medium, scheduled) + +`packages/opencode/src/tool/workflow.ts` has one 200+ line `execute` switch +(cyclomatic complexity 21, cognitive complexity 64). It combines parameter +validation, file/YAML transport, admission normalization, model readiness, +domain commands, and user-facing receipts. This coupling is also why a one-off +graph must be written to YAML before it can start. + +The next product change will introduce one spec-source boundary: inline +structured specs are the default for one-off start/extend/replan operations; +`spec_path` remains for saved workflows. Action handlers may be extracted only +where this names a real boundary and lowers the main dispatch complexity. + +### A3 — Core and TUI seams are appropriately deep (retain) + +The core splits graph rules, scheduling, transitions, projection, and storage +into independently testable modules. The TUI consumes server summaries and +keeps non-trivial layout logic in pure utilities. Recombining these modules or +moving aggregation into the TUI would make the system harder to verify. + +## Unconfirmed robustness risks + +These are review observations, not bugs. No supported-path red test was found. + +- `readWakeBatch` catches typed failures, while store database failures are + defects (`orDie`). A defect can postpone delivery until another event or + restart. Add an explicit retry policy only after a fault-injection test + proves the desired semantics. +- Startup recovery intentionally admits that a store defect can defer + redelivery until the next restart. This is operationally weak, but changing + it requires a retry/backoff and shutdown contract. +- The final review-acceptance guard does not re-check an output fingerprint. + Normal spawn and recovery paths validate fingerprints before settlement, so + no supported writer currently reaches the stale state. Keep this as + defense-in-depth backlog unless a reachable sequence is demonstrated. + +## Batch C decision + +The `spawnReady` O(ready × nodes) candidate remains closed without code. Current +workflow limits and observed test/runtime scale do not show material cost, and +the scheduling implementation is easy to reason about. Reopen only with a +profile showing scheduling latency or CPU cost at realistic node counts. + +## Ordered follow-up + +1. Ship the inline workflow-spec and parent-orchestrator policy change. +2. Refactor `runtime/loop.ts` behind regression tests, without changing the + public layer contract. +3. Add store fault injection, then decide retry/backoff semantics from evidence. + diff --git a/packages/core/src/plugin/command/dag-flow.txt b/packages/core/src/plugin/command/dag-flow.txt index 4f69a2203b..033b646d63 100644 --- a/packages/core/src/plugin/command/dag-flow.txt +++ b/packages/core/src/plugin/command/dag-flow.txt @@ -17,14 +17,14 @@ For a non-empty task: - deep review of an already-built module, subsystem, or codebase → saved workflow `deep-review-dag-module` - a small bounded working-tree change review → saved workflow `change-review` - none of the above names resolves (bare dev checkout without the config repo) → compose the smallest fresh graph; do not force an unrelated reference -2. Treat the selected YAML as a reviewed topology reference, not as a script to replay blindly. Start a saved workflow by name only when its embedded target and inputs already match the request. Otherwise read the reference, derive a one-off YAML, inject the complete `/dag-flow` task into its root planning/exploration prompt, retarget its lanes, and pass that file to `workflow(action=start)`. +2. Treat the selected saved spec as a reviewed topology reference, not as a script to replay blindly. Start a saved workflow by name only when its embedded target and inputs already match the request. Otherwise read the reference, derive one inline `spec`, inject the complete `/dag-flow` task into its root planning/exploration prompt, retarget its lanes, and pass it directly to `workflow(action=start)`. Do not create a transient YAML file. 3. The derived graph may expand or prune non-protected lanes. Record the selected `reference_template`, every added node, and every prune as `{node, prune_reason, replacement_coverage}` in the first planning/exploration artifact; require the next fresh review gate to audit that manifest. Missing prune evidence is fail-closed. 4. Preserve the selected reference's protected spine: fresh-context local review, deterministic/evidence verification where applicable, one final arbiter, and PASS-only finalization. Gates return `PASS | LOOP | BLOCKED` with reason, evidence, minimal `loop_scope`, and `stop_reason`. `LOOP` means pause → replan new local correction/review nodes → resume; never create a cycle or restart terminal nodes. 5. During compilation, preserve every user constraint in the graph, including named `@agent` roles, exact model selections, read-only or "Do not modify files" scope, required checks, forbidden actions, and requested deliverables. 6. Resolve capability slots against the eligible configured worker types shown in the `workflow` tool description. Do not invent a missing role or model; if a required capability cannot be resolved, do not start and report the gap. -7. Scale the graph to the task's blast radius. A small, well-bounded target gets the smallest useful dependency graph. A large or system-level target (an entire module, subsystem, or codebase) is never satisfied by a single wave of parallel opinions: stage exploration, independent analysis, evidence verification, and synthesis as separate dependent waves. Keep independent viewpoints or work packages parallel and use real fan-in nodes for synthesis, arbitration, integration, and final reporting. +7. Scale one consolidated graph to the task's blast radius. Related flows for this user objective become nodes and edges under the same workflow ID. A small, well-bounded target gets the smallest useful dependency graph. A large or system-level target (an entire module, subsystem, or codebase) is never satisfied by a single wave of parallel opinions: stage exploration, independent analysis, evidence verification, and synthesis as separate dependent waves. Keep independent viewpoints or work packages parallel and use real fan-in nodes for synthesis, arbitration, integration, and final reporting. 8. For a large-target review or audit, require every reviewer to cite file:line evidence and to mark claims it could not verify. Insert a verification wave between the reviewers and the arbiter that checks disputed, unverified, and uncovered scope against the actual code, so the arbiter rules on verified findings only. -9. Call the `workflow` tool with `action=start` in this response. Merely printing a plan, graph, JSON, or YAML does not mean a workflow was started. +9. Call the `workflow` tool with `action=start` and inline `spec` in this response. Use `spec_path` only when the selected saved workflow already matches or persistence was explicitly requested. Merely printing a plan, graph, JSON, or YAML does not mean a workflow was started. 10. Do not claim the workflow is running unless the tool call succeeds. On success, report the exact Workflow ID and initial state returned by the tool, then tell the user to run `/dag` for live inspection. 11. The workflow runs asynchronously and wakes this parent session when attention or a terminal result is ready. Do not poll it with `action=status`, sleep, retry, or loop merely to wait. End the current response after the brief success report. 12. On failure, state that the workflow was not started and report the actual error. Never invent a Workflow ID or start a replacement workflow unless the user explicitly asked for automatic retries. diff --git a/packages/core/src/plugin/command/orchestration-domains.md b/packages/core/src/plugin/command/orchestration-domains.md index ec97ea93fb..ca78d4f916 100644 --- a/packages/core/src/plugin/command/orchestration-domains.md +++ b/packages/core/src/plugin/command/orchestration-domains.md @@ -133,9 +133,8 @@ against the code, with fix waves through the audit loop). ## Choosing and Combining -Playbooks compose: Large Engineering embeds Deep Review at its gate; Deep -Speculation can front-load any of them. Selection still obeys Execution Mode -Selection and the Depth Ladder — a playbook is justified only when the task -shows both a scenario and a structural signal, its wave count meets the +Playbooks compose inside one live DAG: Large Engineering embeds Deep Review at +its gate; Deep Speculation can front-load any of them. Selection still obeys +Execution Mode Selection and the Depth Ladder — its wave count meets the ladder's minimum for the target size, and explicit user constraints always override the playbook shape. diff --git a/packages/core/src/plugin/command/orchestration-policy.md b/packages/core/src/plugin/command/orchestration-policy.md index 58ba076a55..f591c5e762 100644 --- a/packages/core/src/plugin/command/orchestration-policy.md +++ b/packages/core/src/plugin/command/orchestration-policy.md @@ -52,17 +52,31 @@ like "review X" never does. ## Execution Mode Selection -Choose the smallest execution mode that can safely complete the request: - -1. Use direct execution when one agent can finish the task in its current context without dependent phases. -2. Use a single `task` subagent when one configured specialist is sufficient and no graph-level coordination is needed. -3. Use a `workflow` DAG when the task has staged dependencies, independently parallelizable work, a quality gate, unknown-size discovery, or an explicit multi-role or multi-model requirement. +The parent conversation owns user interaction, requirement and admission +decisions, the macro plan, workflow controls, checkpoint interpretation, and +the final user-facing synthesis. Once work is classified for delegation, the +parent MUST NOT perform executable leaf work itself. + +Choose the smallest child execution mode that can safely complete the request: + +1. Use direct execution only for conversation, trivial state inspection, + workflow control, final synthesis, or an explicit user opt-out. +2. Use one `task` subagent for one independent non-trivial leaf assignment when + no graph-level coordination is needed. The parent launches it once, consumes + its result, and does not duplicate the leaf work. +3. Use one live `workflow` DAG when one user objective contains staged + dependencies, two or more related workstreams, a quality gate, unknown-size + discovery, adaptive repair, or an explicit multi-role or multi-model + requirement. "Smallest" is measured against the Depth Ladder: a mode or graph that cannot deliver the ladder's hard minimum for the target size is not safe, merely small. -Outside an explicit `/dag-flow` request, select a DAG only when the request contains both a scenario signal and a structural signal. Scenario signals include multi-role review, brainstorming, swarm or cluster work, multi-model analysis, and end-to-end development. Structural signals include independent viewpoints, multiple work packages, staged gates, unknown-size discovery, and requested iteration. A lone keyword such as "review" is not sufficient. +Related flows for one user objective belong to one live DAG. Represent them as +nodes and dependency edges; use `extend` or `control(replan)` when discovery or +a verdict adds work. Start another DAG only after a terminal boundary prevents +live adaptation, and carry the prior outputs into the continuation explicitly. Explicit user constraints override profile defaults: @@ -133,8 +147,8 @@ continue QA, reduce scope, use `standard`, or explicitly waive. A `WAIVED` start is informed only when both `waiver_reason` and `acknowledged_risks` are non-empty; preserve them for audit. -Do not supply `protocol_version`, `state`, or `fingerprint` in the YAML -admission input. Those are durable audit fields owned by the workflow boundary: +Do not supply `protocol_version`, `state`, or `fingerprint` in the admission +input. Those are durable audit fields owned by the workflow boundary: it sets protocol version 1, initializes state from the verdict, normalizes the Brief for fingerprint computation, and computes the lowercase hexadecimal SHA-256 hash. A successful deep start alone transitions the durable record to diff --git a/packages/core/src/plugin/command/workflow.md b/packages/core/src/plugin/command/workflow.md index f8ed48f018..937a1b9f11 100644 --- a/packages/core/src/plugin/command/workflow.md +++ b/packages/core/src/plugin/command/workflow.md @@ -11,20 +11,20 @@ Compile every graph under the Tiered Orchestration Doctrine and Depth Ladder in ## When to start a workflow -A task is an implicit workflow candidate only when it has both a scenario -signal—such as multi-role review, brainstorming, swarm/cluster work, -multi-model analysis, or end-to-end development—and one of these structural +Use one live workflow when a user objective has any of these structural signals: - **Staged**: clear phase boundaries where later phases depend on earlier outputs (explore → plan → implement → verify). -- **Parallelizable**: ≥3 independent sub-units can execute concurrently (same fix across 5 packages). +- **Parallelizable**: ≥2 related sub-units can execute concurrently (same fix across 5 packages). - **Quality gate**: intermediate output must pass review before downstream work begins (architecture review before implementation). - **Adaptive scope**: discovery may reveal an unknown number of work packages or require a bounded repair wave. -A lone keyword such as "review" is not enough. An explicit `/dag-flow` request -does not require this implicit-trigger test. If a task fits in one context -window and has no inter-step dependencies, use the `task` tool instead. For -trivial work, use direct tools. +Use one `task` subagent for one independent non-trivial leaf assignment. Keep +related staged, parallel, gated, or adaptive flows under one workflow ID; use +`extend` or `control(replan)` instead of starting disconnected DAGs. An explicit +`/dag-flow` request always selects a workflow. Direct tools in the parent are +reserved for conversation, trivial state inspection, workflow control, and +final synthesis. ## Standard and deep workflow entry @@ -34,14 +34,14 @@ as independent workstreams, cross-domain uncertainty, high blast radius, conflicting constraints, evidence gathering, or multiple verification perspectives. -Before any graph-carrying action (`start`, `extend`, or `control(replan)`), -write its configuration to a `.yaml` or `.yml` file, then pass only -`spec_path` beside the shallow action fields. Never inline graph nodes, -admission, or replan fragments in the tool call. Keep the file after a -validation failure, edit only the reported problem, and retry the same path. +For a one-off graph, pass `spec` inline on `start`, `extend`, or +`control(replan)`. This is the default: do not create a transient YAML file. +Use `spec_path` only for a saved workflow name, a reusable workflow file, or an +explicitly requested file-backed spec. Exactly one of `spec` and `spec_path` is +valid. After a validation failure, correct the same source and retry the call. Before a deep start, qualify the request interactively in the parent session. -The start YAML places `mode: deep`, a versioned `READY` or informed `WAIVED` +The start spec places `mode: deep`, a versioned `READY` or informed `WAIVED` admission input, and `config` at the same level. The admission input contains `brief_revision`, `qa_mode`, `verdict`, `brief`, and waiver audit fields when applicable; the workflow boundary owns `protocol_version`, `state`, and @@ -93,22 +93,21 @@ code review workflow") and the saved target/inputs already match: starting it is one call, and its graph has already been reviewed. `/dag-flow` may also read a saved workflow as a topology reference, then derive a one-off spec that injects the current task, retargets module lanes, and records additions/prunes. -Compose a fresh spec file when the task is one-off or no reference fits — a -path-shaped `spec_path` keeps the original session-relative behavior. To turn a -working one-off spec into a saved workflow, move the file into one of the two -directories under a descriptive name. +Compose a fresh inline `spec` when the task is one-off or no reference fits. +To turn a working one-off spec into a saved workflow, persist it as YAML in one +of the two workflow-library directories under a descriptive name. ## Orchestration Lifecycle -Heavy tasks follow a meta-workflow: multiple workflows chained together, each producing a decision that shapes the next. The lifecycle is the two accuracy axes applied in sequence — breadth to cover the surface, depth to earn the verdict: +Heavy tasks follow one adaptive workflow whose decisions shape later waves. The lifecycle is the two accuracy axes applied in sequence — breadth to cover the surface, depth to earn the verdict: 1. **Explore + brainstorm** — exploration nodes fan out over the codebase while independent generators propose approaches; a required synthesizer converges them into a design plus architecture inventory. -2. **Design review gate** — an advanced-tier gate node (`report_to_parent: true`, normalized verdict `output_schema`) rules on the design. `required: true` fails the workflow only when the gate node fails to execute or satisfy its output contract; a successful `REVISE` or `REJECT` is a business verdict, not an execution failure. Route the static ACCEPT path through a downstream `condition`, and dispose of a reported non-ACCEPT verdict per the Verdict Disposal Contract. Dependencies cannot cross workflow boundaries — a gate in a separate workflow receives the prior result as static input. +2. **Design review gate** — an advanced-tier gate node (`report_to_parent: true`, normalized verdict `output_schema`) rules on the design. `required: true` fails the workflow only when the gate node fails to execute or satisfy its output contract; a successful `REVISE` or `REJECT` is a business verdict, not an execution failure. Route the static ACCEPT path through a downstream `condition`, and dispose of a reported non-ACCEPT verdict per the Verdict Disposal Contract. 3. **Parallel execution** — the accepted design decomposes into module-level worker nodes with disjoint write sets, fanning into a required assembler. 4. **Verify + diff review + audit** — production assurance follows `implementation → verification(PASS) → diff review → final gate/audit` with fingerprint echo; `REJECT` routes through corrected implementation and verification before a new diff review. Progress tracking is updated to reflect what shipped. -5. **Expansion decision** — iterate (bounded `control(replan)` of affected nodes), extend (additional parallel nodes), separate phase (a new workflow once the previous is terminal), or complete (`control(complete)`). +5. **Expansion decision** — iterate (bounded `control(replan)` of affected nodes), extend (additional parallel nodes in the same workflow), or complete (`control(complete)`). Start a continuation workflow only after the original is terminal and cannot be adapted. -Not every task needs all five phases: a well-specified task may enter at phase 3, a clear design with uncertain scope at phase 2. The lifecycle is a decision tree, not a pipeline. Concrete graph YAML for each shape is under Collaboration Patterns below. +Not every task needs all five phases: a well-specified task may enter at phase 3, a clear design with uncertain scope at phase 2. The lifecycle is a decision tree, not a pipeline. Concrete graph shapes are under Collaboration Patterns below. ## Node inputs and model selection @@ -150,9 +149,9 @@ the workflow uncreated so the user can configure a model and retry. ## Collaboration Patterns Four structural patterns cover the common cases. Real workflows often combine -them. Every YAML block below is workflow spec file content. Save the selected -shape to a `.yaml` file, then call the tool with -`{ action: "start", spec_path: ".yaml" }`. +them. Every block below shows the object shape for inline `spec`; pass the +selected shape with `{ action: "start", spec: { ... } }`. Persist it as YAML +and use `spec_path` only when the workflow itself should be saved. ### 1. Staged Pipeline with Gate @@ -459,8 +458,9 @@ is believed to be running. Two disciplines close the gap: `pending` (a `child_session_id` or `running` status). An acceptance receipt alone is never evidence of execution. - After any rejected graph-carrying call (SchemaError, validation error), - fix the spec AND re-issue the call in the same turn — a fixed file is not - a fixed operation, and the re-issue needs the same `status` verification. + fix the spec source AND re-issue the call in the same turn — corrected input + is not a corrected operation, and the re-issue needs the same `status` + verification. ## Model Assignment Strategy @@ -537,10 +537,11 @@ All nodes share the same workspace. Write conflicts are an orchestration concern ### Actions -**start** — Create a workflow from a YAML spec with `config` and optional -`title`, `mode`, and admission input at the file root. Write the file first, -then call `{ action: "start", spec_path: ".opencode/workflows/name.yaml" }`, or -pass a saved workflow name (`{ action: "start", spec_path: "code-review" }`). +**start** — Create a workflow from `config` and optional `title`, `mode`, and +admission input. For a one-off graph call +`{ action: "start", spec: { config: { ... } } }`. Use `spec_path` for a saved +workflow name (`{ action: "start", spec_path: "code-review" }`) or an explicit +YAML path. Returns the workflow ID. Nodes declare `depends_on` (node IDs); layers and execution order are computed automatically. @@ -552,9 +553,8 @@ not running workflows; use `status` for a workflow's live state. new nodes are immediately eligible for scheduling if their dependencies are met. It also accepts a genuinely additive wave after a reporting leaf checkpoint naturally completed the current graph; an early -`control(complete)` workflow remains terminal. Put the new nodes under a -file-root `nodes` array, then call -`{ action: "extend", workflow_id: "dag_...", spec_path: "extend.yaml" }`. +`control(complete)` workflow remains terminal. Put the new nodes under `spec.nodes`, +then call `{ action: "extend", workflow_id: "dag_...", spec: { nodes: [...] } }`. **status** — Read the durable state of one workflow and all of its nodes. Pass `workflow_id`. Use it when the user explicitly asks for current state or once before a decision that requires fresh state, such as replan/control. Do not poll a running workflow merely to wait: node reports and terminal outcomes wake the parent session automatically. @@ -562,7 +562,7 @@ file-root `nodes` array, then call - `pause` — let running nodes finish, don't spawn new ones (pause does NOT stop nodes that are already running). On a cancel/replan intent, always pause FIRST: it needs no fragment and freezes scheduling while you compose the replan, so the graph cannot terminalize under you. - `resume` — resume scheduling - `cancel` — cancel the entire workflow -- `replan` — write a YAML file with a file-root `fragment` object containing the graph fields and node definitions, then pass its `spec_path`; running nodes can be `restart: true` or `cancel: true`; pending nodes absent from the fragment are cancelled. Valid while paused — the pause → compose file → replan → resume sequence is the safe path. +- `replan` — pass `spec: { fragment: { ... } }` with the graph fields and node definitions; running nodes can be `restart: true` or `cancel: true`; pending nodes absent from the fragment are cancelled. Valid while paused — the pause → compose spec → replan → resume sequence is the safe path. Use `spec_path` only for a saved or explicitly file-backed fragment. - `complete` — early-complete: remaining pending nodes are skipped (non-violation) - `step` — advance exactly one ready node (the first by node ID lexicographic order), then wait. Use for controlled debugging or staged verification of a critical path. Unlike `pause`, which freezes all scheduling, `step` advances one node and re-waits. A second `step` while the stepped node is still running is rejected. Use `resume` to return to full-speed scheduling. Nodes are selected in lexicographic ID order for determinism. @@ -588,4 +588,4 @@ file-root `nodes` array, then call - No `node_complete` action — completion is automatic - No `history` action — inspect a known workflow with `status`; browsing running workflows remains TUI-only (`list` shows saved specs, not running workflows) -- No runtime-side magical topology selection — `/dag-flow` selects and adapts saved reference graphs in the parent agent; the workflow runtime executes only the resulting YAML +- No runtime-side magical topology selection — `/dag-flow` selects and adapts saved reference graphs in the parent agent; the workflow runtime executes the resulting validated spec diff --git a/packages/core/test/plugin/command.test.ts b/packages/core/test/plugin/command.test.ts index 85c344ae4e..bff6223f04 100644 --- a/packages/core/test/plugin/command.test.ts +++ b/packages/core/test/plugin/command.test.ts @@ -55,19 +55,39 @@ describe("CommandPlugin.Plugin", () => { }), ) - it.effect("documents the smallest execution mode and conservative implicit DAG trigger", () => + it.effect("documents the smallest child execution mode", () => Effect.sync(() => { expect(CommandPlugin.WorkflowContent).toContain("## Execution Mode Selection") - expect(CommandPlugin.WorkflowContent).toContain("direct execution") - expect(CommandPlugin.WorkflowContent).toContain("single `task` subagent") - expect(CommandPlugin.WorkflowContent).toContain("both a scenario signal and a structural signal") - expect(CommandPlugin.WorkflowFactsContent).toContain("both a scenario") + expect(CommandPlugin.WorkflowContent).toContain("Use direct execution only") + expect(CommandPlugin.WorkflowContent).toContain("one `task` subagent") + expect(CommandPlugin.WorkflowContent).toContain("Related flows for one user objective") expect(CommandPlugin.WorkflowFactsContent).not.toContain("when ANY") expect(CommandPlugin.WorkflowFactsContent).not.toContain("- **Multi-model**:") expect(CommandPlugin.DagFlowContent).toContain("workflow` tool with `action=start") }), ) + it.effect("keeps the parent at macro level and consolidates related work", () => + Effect.sync(() => { + expect(CommandPlugin.OrchestrationPolicyContent).toContain("The parent conversation owns") + expect(CommandPlugin.OrchestrationPolicyContent).toContain("MUST NOT perform executable leaf work") + expect(CommandPlugin.OrchestrationPolicyContent).toContain("one `task` subagent") + expect(CommandPlugin.OrchestrationPolicyContent).toContain("one live `workflow` DAG") + expect(CommandPlugin.OrchestrationPolicyContent).toContain("one user objective") + expect(CommandPlugin.DagFlowContent).toContain("one consolidated graph") + }), + ) + + it.effect("uses inline specs for one-off graphs without removing saved workflows", () => + Effect.sync(() => { + expect(CommandPlugin.WorkflowFactsContent).toContain("For a one-off graph, pass `spec` inline") + expect(CommandPlugin.WorkflowFactsContent).toContain("Use `spec_path` only") + expect(CommandPlugin.WorkflowFactsContent).not.toContain("Never inline graph nodes") + expect(CommandPlugin.WorkflowFactsContent).not.toContain("Before any graph-carrying action") + expect(CommandPlugin.DagFlowContent).toContain("inline `spec`") + }), + ) + it.effect("preserves opt-outs read-only scope and explicit role assignments", () => Effect.sync(() => { expect(CommandPlugin.OrchestrationPolicyContent).toContain("single agent") @@ -281,12 +301,12 @@ describe("CommandPlugin.Plugin", () => { expect(CommandPlugin.OrchestrationPolicyContent).toContain("invalidate the prior fingerprint") expect(CommandPlugin.OrchestrationPolicyContent).toContain("SHA-256 hash") expect(CommandPlugin.WorkflowFactsContent).toContain( - "The start YAML places `mode: deep`, a versioned `READY` or informed `WAIVED`", + "The start spec places `mode: deep`, a versioned `READY` or informed `WAIVED`", ) expect(CommandPlugin.WorkflowFactsContent).toContain( "the workflow boundary owns `protocol_version`, `state`, and\n`fingerprint`", ) - expect(CommandPlugin.WorkflowFactsContent).toContain("pass only\n`spec_path` beside the shallow action fields") + expect(CommandPlugin.WorkflowFactsContent).toContain("For a one-off graph, pass `spec` inline") expect(CommandPlugin.WorkflowFactsContent).not.toContain("`config.mode`") }), ) diff --git a/packages/opencode/src/dag/runtime/summary-publisher.ts b/packages/opencode/src/dag/runtime/summary-publisher.ts index e3487172c7..091d3d7672 100644 --- a/packages/opencode/src/dag/runtime/summary-publisher.ts +++ b/packages/opencode/src/dag/runtime/summary-publisher.ts @@ -1,6 +1,6 @@ export * as DagSummaryPublisher from "./summary-publisher" -import { Effect, Layer, Scope, Context } from "effect" +import { Cause, Effect, Exit, Layer, Scope, Context } from "effect" import { LayerNode } from "@opencode-ai/core/effect/layer-node" import { InstanceState } from "@/effect/instance-state" import { EventV2Bridge } from "@/event-v2-bridge" @@ -75,13 +75,46 @@ export const layer = Layer.effect( // removed entirely, correctness is unchanged — only more DagStore // reads would occur. This satisfies the "stateless derived view" // contract: no cached summary is ever served from this map. - const pending = new Set() + const pending = new Map() // Second coalescing tier keyed by workspace/dagID: node events don't // carry a sessionID, and resolving it eagerly meant one getWorkflow // query PER EVENT before the debounce window could absorb the burst // (P1-4). Resolve the sessionID once after the window, then hand off // to the workspace/session debounce. - const pendingByDag = new Set() + const pendingByDag = new Map() + + const coalesceLatest = ( + active: Map, + key: string, + body: () => Effect.Effect, + ) => + Effect.gen(function* () { + if (active.has(key)) { + active.set(key, true) + return + } + active.set(key, false) + yield* Effect.gen(function* () { + for (;;) { + yield* Effect.sleep("50 millis") + // Events during the debounce window are absorbed by the read + // that follows; only events racing the read require a rerun. + active.set(key, false) + const outcome = yield* body().pipe(Effect.exit) + const repeat = yield* Effect.sync(() => { + if (active.get(key)) return true + active.delete(key) + return false + }) + if (Exit.isFailure(outcome) && Cause.hasInterrupts(outcome.cause)) { + return yield* Effect.failCause(outcome.cause) + } + if (repeat) continue + if (Exit.isFailure(outcome)) return yield* Effect.failCause(outcome.cause) + return + } + }).pipe(Effect.ensuring(Effect.sync(() => active.delete(key)))) + }) const publishForSession = (sessionID: string, workspace: string | undefined) => Effect.gen(function* () { @@ -98,36 +131,19 @@ export const layer = Layer.effect( }) const schedulePublish = (sessionID: string, workspace: string | undefined) => - Effect.gen(function* () { - const key = `${workspace ?? ""}\0${sessionID}` - // Coalesce: if a recompute is already scheduled for this route, - // let it absorb this trigger rather than queueing a second read. - // The coalesced early return MUST NOT touch `pending` — only the - // owning fiber clears its own slot, otherwise a coalesced caller - // would delete the owner's entry and reopen the window. - if (pending.has(key)) return - pending.add(key) - yield* Effect.gen(function* () { - yield* Effect.sleep("50 millis") - yield* publishForSession(sessionID, workspace) - }).pipe(Effect.ensuring(Effect.sync(() => pending.delete(key)))) - }) + coalesceLatest(pending, `${workspace ?? ""}\0${sessionID}`, () => publishForSession(sessionID, workspace)) const schedulePublishByDag = (dagID: string, workspace: string | undefined) => - Effect.gen(function* () { - const key = `${workspace ?? ""}\0${dagID}` - if (pendingByDag.has(key)) return - pendingByDag.add(key) - yield* Effect.gen(function* () { - yield* Effect.sleep("50 millis") + coalesceLatest(pendingByDag, `${workspace ?? ""}\0${dagID}`, () => + Effect.gen(function* () { const wf = yield* store.getWorkflow(dagID) if (wf?.projectId !== ctx.project.id) return // Hand off to the session-level debounce (not publishForSession // directly) so a concurrent session-keyed window absorbs this // trigger instead of producing a duplicate read. yield* schedulePublish(wf.sessionId, workspace) - }).pipe(Effect.ensuring(Effect.sync(() => pendingByDag.delete(key)))) - }) + }), + ) const unsubscribe = yield* events.listen((evt) => { if (!SUMMARY_TRIGGER_EVENTS.some((def) => def.type === evt.type)) return Effect.void diff --git a/packages/opencode/src/tool/workflow.ts b/packages/opencode/src/tool/workflow.ts index 121285c1e0..470f72fb83 100644 --- a/packages/opencode/src/tool/workflow.ts +++ b/packages/opencode/src/tool/workflow.ts @@ -20,7 +20,7 @@ const id = "workflow" const MAX_WORKFLOW_SPEC_BYTES = 1_000_000 // ============================================================================ -// File schemas stay rich; tool-call parameters below stay shallow. +// Action schemas remain the single validation authority for file and inline input. // ============================================================================ const NodeSchema = Schema.Struct({ @@ -105,6 +105,7 @@ const decodeReplanSpec = Schema.decodeUnknownEffect(ReplanSpec) export const Parameters = Schema.Struct({ action: Schema.Literals(["start", "extend", "control", "status", "list"]).annotate({ description: "start: create workflow; extend: add nodes; control: pause/resume/cancel/replan/step/complete; status: inspect durable workflow and node state; list: show saved workflow specs in the library (not running workflows)" }), + spec: Schema.optional(Schema.Record(Schema.String, Schema.Unknown)).annotate({ description: "(start/extend/control replan) Inline structured spec for a one-off graph. Use this or spec_path, never both" }), spec_path: Schema.optional(Schema.String).annotate({ description: '(start/extend/control replan) A saved workflow name from the library (e.g. "code-review"), or a path to a YAML workflow spec. Relative paths resolve from the session directory' }), session_id: Schema.optional(Schema.String).annotate({ description: "(start) Parent session ID" }), project_id: Schema.optional(Schema.String).annotate({ description: "(start) Optional Project ID; must match the parent session project" }), @@ -216,7 +217,7 @@ export const WorkflowTool = Tool.define< if (params.project_id && params.project_id !== session.projectID) { return yield* Effect.die(new Error("project_id must match the parent session project")) } - const specFile = yield* readWorkflowSpec(params.spec_path, session.directory, ctx).pipe(Effect.orDie) + const specFile = yield* readWorkflowSpec(params.spec, params.spec_path, session.directory, ctx).pipe(Effect.orDie) const spec = yield* decodeStartSpec(specFile.value).pipe( Effect.mapError((error) => new Error(`Invalid workflow spec ${specFile.path}: ${String(error)}`)), Effect.orDie, @@ -274,7 +275,7 @@ export const WorkflowTool = Tool.define< case "extend": { if (!params.workflow_id) return yield* Effect.die(new Error("extend requires 'workflow_id'")) const session = yield* sessions.get(SessionID.make(ctx.sessionID)).pipe(Effect.orDie) - const specFile = yield* readWorkflowSpec(params.spec_path, session.directory, ctx).pipe(Effect.orDie) + const specFile = yield* readWorkflowSpec(params.spec, params.spec_path, session.directory, ctx).pipe(Effect.orDie) const spec = yield* decodeExtendSpec(specFile.value).pipe( Effect.mapError((error) => new Error(`Invalid workflow spec ${specFile.path}: ${String(error)}`)), Effect.orDie, @@ -292,7 +293,7 @@ export const WorkflowTool = Tool.define< case "control": { if (!params.workflow_id || !params.operation) { return yield* Effect.die(new Error( - `control requires 'workflow_id' and 'operation' (got workflow_id=${params.workflow_id ?? ""}, operation=${params.operation ?? ""}). Example: { action: "control", workflow_id: "dag_...", operation: "pause" }. On a cancel/replan intent, issue pause FIRST — it needs no spec file and freezes scheduling instantly while you compose the replan.`, + `control requires 'workflow_id' and 'operation' (got workflow_id=${params.workflow_id ?? ""}, operation=${params.operation ?? ""}). Example: { action: "control", workflow_id: "dag_...", operation: "pause" }. On a cancel/replan intent, issue pause FIRST — it needs no spec and freezes scheduling instantly while you compose the replan.`, )) } const wfId = params.workflow_id @@ -311,7 +312,7 @@ export const WorkflowTool = Tool.define< return { title: "Workflow completed (early)", output: ``, metadata: { workflowId: wfId } as Metadata } case "replan": { const session = yield* sessions.get(SessionID.make(ctx.sessionID)).pipe(Effect.orDie) - const specFile = yield* readWorkflowSpec(params.spec_path, session.directory, ctx).pipe(Effect.orDie) + const specFile = yield* readWorkflowSpec(params.spec, params.spec_path, session.directory, ctx).pipe(Effect.orDie) const spec = yield* decodeReplanSpec(specFile.value).pipe( Effect.mapError((error) => new Error(`Invalid workflow spec ${specFile.path}: ${String(error)}`)), Effect.orDie, @@ -321,7 +322,7 @@ export const WorkflowTool = Tool.define< // the recovery options instead of a bare iron-law rejection. const r = yield* withTerminalRecovery( dag.replan(wfId, { nodes: spec.fragment.nodes as NodeConfig[] }), - "The workflow reached a terminal status before the replan arrived — terminal workflows are immutable. Recover by writing a new start spec with the updated node definitions and passing its spec_path, or extend if a reporting leaf checkpoint naturally completed the graph. Next time issue control(pause) BEFORE composing the spec file.", + "The workflow reached a terminal status before the replan arrived — terminal workflows are immutable. Recover by starting a new workflow with the updated node definitions, or extend if a reporting leaf checkpoint naturally completed the graph. Next time issue control(pause) BEFORE composing the spec.", ).pipe(Effect.orDie) const ignored = r.ignore.length > 0 ? `\nIgnored (terminal, immutable — add replacements under new ids to retry): ${r.ignore.join(", ")}` : "" return { @@ -345,11 +346,22 @@ export const WorkflowTool = Tool.define< }), ) -function readWorkflowSpec(specPath: string | undefined, directory: string, ctx: Tool.Context) { +function readWorkflowSpec( + spec: Record | undefined, + specPath: string | undefined, + directory: string, + ctx: Tool.Context, +) { return Effect.gen(function* () { + if (spec && specPath) { + return yield* Effect.fail(new Error( + "Workflow configuration accepts exactly one source: remove either 'spec' or 'spec_path'.", + )) + } + if (spec) return { path: "", value: spec } if (!specPath) { return yield* Effect.fail(new Error( - `Workflow configuration requires 'spec_path'. Pass a saved workflow name (workflow(action: "list") shows them) or write the YAML spec to a file and retry with its path.`, + `Workflow configuration requires exactly one of 'spec' or 'spec_path'. Pass an inline structured spec for a one-off graph, or a saved workflow name/path through 'spec_path'.`, )) } const filepath = yield* resolveSpecPath(specPath, directory, ctx) diff --git a/packages/opencode/test/dag/dag-summary-publisher-behavior.test.ts b/packages/opencode/test/dag/dag-summary-publisher-behavior.test.ts index b26d0af2e3..e585761945 100644 --- a/packages/opencode/test/dag/dag-summary-publisher-behavior.test.ts +++ b/packages/opencode/test/dag/dag-summary-publisher-behavior.test.ts @@ -1,5 +1,5 @@ import { describe, expect } from "bun:test" -import { DateTime, Effect, Layer } from "effect" +import { DateTime, Deferred, Effect, Layer } from "effect" import { DagStore, type WorkflowRow, type WorkflowSummary } from "@opencode-ai/core/dag/store" import { DagEvent } from "@opencode-ai/schema/dag-event" import { EventV2Bridge } from "@/event-v2-bridge" @@ -18,6 +18,11 @@ interface SummaryEmission { interface StoreControl { failures: number + failuresAfterGate: number + readGate?: { + started: Deferred.Deferred + release: Deferred.Deferred + } reads: Map lookups: Map projects: Map @@ -29,15 +34,16 @@ interface EventControl { listener?: (event: never) => Effect.Effect } -function control() { +function control(): StoreControl { return { failures: 0, + failuresAfterGate: 0, reads: new Map(), lookups: new Map(), projects: new Map(), sessions: new Map(), summaries: new Map(), - } satisfies StoreControl + } } function workflow(id: string, sessionId: string, projectId: string): WorkflowRow { @@ -81,13 +87,24 @@ function runtime(state: StoreControl, bus: EventControl) { return sid ? workflow(dagID, sid, state.projects.get(dagID) ?? "global") : undefined }), getWorkflowSummaries: (sessionID) => - Effect.sync(() => { + Effect.gen(function* () { state.reads.set(sessionID, (state.reads.get(sessionID) ?? 0) + 1) if (state.failures > 0) { state.failures -= 1 throw new Error("simulated summary read failure") } - return state.summaries.get(sessionID) ?? [] + const summaries = state.summaries.get(sessionID) ?? [] + const gate = state.readGate + state.readGate = undefined + if (gate) { + yield* Deferred.succeed(gate.started, undefined) + yield* Deferred.await(gate.release) + } + if (state.failuresAfterGate > 0) { + state.failuresAfterGate -= 1 + throw new Error("simulated summary read failure after gate") + } + return summaries }), }) const events = Layer.mock(EventV2Bridge.Service, { @@ -232,6 +249,117 @@ describe("DagSummaryPublisher behavior", () => { ).pipe(Effect.provide(runtime(state, bus))) }) + it.instance("an event arriving during an in-flight read schedules a fresh recompute", () => { + const state = control() + const bus = {} satisfies EventControl + state.sessions.set("dag-inflight", "ses-inflight") + state.summaries.set("ses-inflight", [summary("dag-inflight", 1)]) + + return withCollector((collector) => + Effect.gen(function* () { + const started = yield* Deferred.make() + const release = yield* Deferred.make() + state.readGate = { started, release } + + yield* (yield* DagSummaryPublisher.Service).init() + yield* publishNodeEvents(bus, "dag-inflight", 1) + yield* Deferred.await(started).pipe(Effect.timeout("1 second")) + + state.summaries.set("ses-inflight", [summary("dag-inflight", 2)]) + yield* publishNodeEvents(bus, "dag-inflight", 1) + yield* Effect.sleep("20 millis") + yield* Deferred.succeed(release, undefined) + + yield* pollWithTimeout( + Effect.sync(() => (collector.emissions.at(-1)?.summaries[0]?.completedNodes === 2 ? true : undefined)), + () => + `event coalesced during an in-flight read was lost (lookups=${state.lookups.get("dag-inflight") ?? 0}, reads=${state.reads.get("ses-inflight") ?? 0}, emissions=${collector.emissions.length})`, + "500 millis", + ) + + expect(state.reads.get("ses-inflight")).toBe(2) + expect(collector.emissions.at(-1)?.summaries).toEqual([summary("dag-inflight", 2)]) + }), + ).pipe(Effect.provide(runtime(state, bus))) + }) + + it.instance("another DAG event arriving during a shared session read schedules a fresh recompute", () => { + const state = control() + const bus = {} satisfies EventControl + state.sessions.set("dag-session-a", "ses-shared") + state.sessions.set("dag-session-b", "ses-shared") + state.summaries.set("ses-shared", [summary("dag-session-a", 1)]) + + return withCollector((collector) => + Effect.gen(function* () { + const started = yield* Deferred.make() + const release = yield* Deferred.make() + state.readGate = { started, release } + + yield* (yield* DagSummaryPublisher.Service).init() + yield* publishNodeEvents(bus, "dag-session-a", 1) + yield* Deferred.await(started).pipe(Effect.timeout("1 second")) + + state.summaries.set("ses-shared", [summary("dag-session-a", 2)]) + yield* publishNodeEvents(bus, "dag-session-b", 1) + yield* Effect.sleep("80 millis") + yield* Deferred.succeed(release, undefined) + + yield* pollWithTimeout( + Effect.sync(() => (collector.emissions.at(-1)?.summaries[0]?.completedNodes === 2 ? true : undefined)), + () => + `shared session event was lost (reads=${state.reads.get("ses-shared") ?? 0}, emissions=${collector.emissions.length})`, + "500 millis", + ) + + expect(state.reads.get("ses-shared")).toBe(2) + expect(state.lookups).toEqual( + new Map([ + ["dag-session-a", 1], + ["dag-session-b", 1], + ]), + ) + }), + ).pipe(Effect.provide(runtime(state, bus))) + }) + + it.instance("an event arriving during a failed in-flight read schedules a fresh recompute", () => { + const state = control() + const bus = {} satisfies EventControl + state.failuresAfterGate = 1 + state.sessions.set("dag-failed-inflight", "ses-failed-inflight") + state.summaries.set("ses-failed-inflight", [summary("dag-failed-inflight", 1)]) + + return withCollector((collector) => + Effect.gen(function* () { + const started = yield* Deferred.make() + const release = yield* Deferred.make() + state.readGate = { started, release } + + yield* (yield* DagSummaryPublisher.Service).init() + yield* publishNodeEvents(bus, "dag-failed-inflight", 1) + yield* Deferred.await(started).pipe(Effect.timeout("1 second")) + + state.summaries.set("ses-failed-inflight", [summary("dag-failed-inflight", 2)]) + yield* publishNodeEvents(bus, "dag-failed-inflight", 1) + yield* Effect.sleep("20 millis") + yield* Deferred.succeed(release, undefined) + + yield* pollWithTimeout( + Effect.sync(() => (collector.emissions.at(-1)?.summaries[0]?.completedNodes === 2 ? true : undefined)), + () => + `event coalesced during a failed in-flight read was lost (reads=${state.reads.get("ses-failed-inflight") ?? 0}, emissions=${collector.emissions.length})`, + "500 millis", + ) + + expect(state.reads.get("ses-failed-inflight")).toBe(2) + expect(collector.emissions).toEqual([ + { sessionID: "ses-failed-inflight", summaries: [summary("dag-failed-inflight", 2)] }, + ]) + }), + ).pipe(Effect.provide(runtime(state, bus))) + }) + it.instance("different sessions coalesce independently", () => { const state = control() const bus = {} satisfies EventControl diff --git a/packages/opencode/test/dag/dag-summary-publisher.test.ts b/packages/opencode/test/dag/dag-summary-publisher.test.ts index 101d3ccc34..b84e74638e 100644 --- a/packages/opencode/test/dag/dag-summary-publisher.test.ts +++ b/packages/opencode/test/dag/dag-summary-publisher.test.ts @@ -54,11 +54,11 @@ describe("DagSummaryPublisher contract (stateless derived view)", () => { path.resolve("src/dag/runtime/summary-publisher.ts"), "utf-8", ) - // No module-level mutable Map/Set. The `pending` Set lives inside + // No module-level mutable Map/Set. The `pending` Map lives inside // the InstanceState closure, not at module level. expect(src).not.toMatch(/^const\s+\w+\s*=\s*new\s+(Map|Set)\b/m) expect(src).not.toMatch(/^let\s+\w+\s*=\s*new\s+(Map|Set)\b/m) - // The pending Set is declared inside the InstanceState.make closure. - expect(src).toMatch(/const pending = new Set/) + // The pending Map is declared inside the InstanceState.make closure. + expect(src).toMatch(/const pending = new Map/) }) }) diff --git a/packages/opencode/test/dag/workflow-tool.test.ts b/packages/opencode/test/dag/workflow-tool.test.ts index 358db14d92..610b2db967 100644 --- a/packages/opencode/test/dag/workflow-tool.test.ts +++ b/packages/opencode/test/dag/workflow-tool.test.ts @@ -317,6 +317,18 @@ function writeWorkflowSpec(name: string, value: unknown) { ) } +function toolContext() { + return { + sessionID: SessionID.make("ses_workflow_parent"), + messageID: MessageID.ascending(), + agent: "build", + abort: new AbortController().signal, + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, + } satisfies Tool.Context +} + describe("workflow tool schema (negative tests)", () => { it("action field accepts start/extend/control/status/list", () => { const decode = Schema.decodeUnknownSync(Parameters) @@ -328,6 +340,18 @@ describe("workflow tool schema (negative tests)", () => { expect(() => decode({ action: "list" })).not.toThrow() }) + it("retains an inline structured spec", () => { + const decode = Schema.decodeUnknownSync(Parameters) + const spec = { + config: { + name: "inline-schema", + nodes: [], + }, + } + + expect(decode({ action: "start", spec })).toEqual({ action: "start", spec }) + }) + it("action field rejects unknown actions", () => { const decode = Schema.decodeUnknownSync(Parameters) expect(() => decode({ action: "delete" })).toThrow() @@ -357,7 +381,7 @@ describe("workflow tool schema (negative tests)", () => { expect(() => decode({ action: "control", workflow_id: "wf-1", operation: "start" })).toThrow() }) - it("keeps workflow graph and admission internals out of tool-call parameters", () => { + it("keeps workflow graph and admission fields inside spec", () => { const decode = Schema.decodeUnknownSync(Parameters) expect(decode({ action: "start", @@ -419,6 +443,125 @@ describe("workflow tool execution", () => { }), ) + runtime.effect("starts from an inline structured spec without a file", () => + Effect.gen(function* () { + published.length = 0 + const info = yield* WorkflowTool + const workflow = yield* info.init() + const result = yield* workflow.execute( + Schema.decodeUnknownSync(Parameters)({ + action: "start", + spec: { + config: { + name: "inline-start", + nodes: [], + }, + }, + }), + toolContext(), + ) + + expect(result.title).toBe("Workflow started: inline-start") + expect(result.metadata.workflowId).toBeDefined() + expect(published.some((event) => event.type === DagEvent.WorkflowCreated.type)).toBe(true) + }), + ) + + runtime.effect("extends from an inline structured spec without a file", () => + Effect.gen(function* () { + published.length = 0 + const info = yield* WorkflowTool + const workflow = yield* info.init() + const result = yield* workflow.execute( + Schema.decodeUnknownSync(Parameters)({ + action: "extend", + workflow_id: "dag_defaults", + spec: { + nodes: [{ + id: "inline-added", + name: "Inline added", + worker_type: "general", + depends_on: [], + prompt_template: { inline: "work" }, + }], + }, + }), + toolContext(), + ) + + expect(result.title).toBe("Workflow extended: 1 nodes added") + expect(published.find((event) => event.type === DagEvent.NodeRegistered.type)?.data).toEqual( + expect.objectContaining({ nodeID: "inline-added" }), + ) + }), + ) + + runtime.effect("replans from an inline structured spec without a file", () => + Effect.gen(function* () { + published.length = 0 + const info = yield* WorkflowTool + const workflow = yield* info.init() + const result = yield* workflow.execute( + Schema.decodeUnknownSync(Parameters)({ + action: "control", + workflow_id: "dag_defaults", + operation: "replan", + spec: { + fragment: { + name: "inline-replan", + nodes: [{ + id: "inline-replanned", + name: "Inline replanned", + worker_type: "general", + depends_on: [], + prompt_template: { inline: "work" }, + }], + }, + }, + }), + toolContext(), + ) + + expect(result.title).toContain("Workflow replanned: +1") + expect(published.find((event) => event.type === DagEvent.NodeRegistered.type)?.data).toEqual( + expect.objectContaining({ nodeID: "inline-replanned" }), + ) + }), + ) + + runtime.effect("rejects ambiguous or missing spec sources before side effects", () => + Effect.gen(function* () { + const info = yield* WorkflowTool + const workflow = yield* info.init() + const cases = [ + { + params: { + action: "start", + spec: { config: { name: "ambiguous", nodes: [] } }, + spec_path: "saved-workflow", + }, + message: "accepts exactly one source", + }, + { + params: { action: "start" }, + message: "requires exactly one of 'spec' or 'spec_path'", + }, + ] + + for (const item of cases) { + published.length = 0 + const exit = yield* workflow.execute( + Schema.decodeUnknownSync(Parameters)(item.params), + toolContext(), + ).pipe(Effect.exit) + + expect(Exit.isFailure(exit)).toBe(true) + if (Exit.isFailure(exit)) expect(Cause.pretty(exit.cause)).toContain(item.message) + expect(published).toHaveLength(0) + } + }), + ) + runtime.effect("status and recovery reads retain consumed deep admission audit fields", () => Effect.gen(function* () { const info = yield* WorkflowTool @@ -1213,4 +1356,3 @@ describe("workflow tool saved workflows", () => { ), ) }) -