diff --git a/packages/core/src/plugin/command/workflow-routing.md b/packages/core/src/plugin/command/workflow-routing.md index 878aea801..85b9db97b 100644 --- a/packages/core/src/plugin/command/workflow-routing.md +++ b/packages/core/src/plugin/command/workflow-routing.md @@ -108,6 +108,10 @@ Top level is `title`/`mode`/`admission` (optional) and `config` (required); `kind` (required), `depends_on`, `instruction`, `worker_type`, `required`, `report_to_parent` — never `worker`, `prompt`, or `agent`. +A `report_to_parent` node with dependents is a reporting checkpoint: gate each +dependent on its output via `condition`, keep it a reporting leaf, or drop +`report_to_parent`. + Validate that `spec_path` before start. Fix every diagnostic in the same file and revalidate; validation creates no workflow. A successful start returns the exact workflow ID. The parent owns the graph, controls, and final report; diff --git a/packages/opencode/src/dag/CONTEXT.md b/packages/opencode/src/dag/CONTEXT.md index 3cd98f308..38ac3de76 100644 --- a/packages/opencode/src/dag/CONTEXT.md +++ b/packages/opencode/src/dag/CONTEXT.md @@ -13,6 +13,7 @@ Workflow Orchestration turns one user objective into one durable DAG. Its model- | Orchestration Router | The product-owned parent guidance that qualifies an objective and selects one Workflow Route without external Skill discovery. | | Block Composer | The Orchestration Router decision that selects the smallest Block graph justified by current evidence. | | Decision Checkpoint | One parent-owned confirmation for unresolved user choices that materially change behavior, scope, acceptance, or an irreversible boundary. | +| Reporting Checkpoint | A `report_to_parent: true` node with dependents; its dependents must gate on its output via `condition`, or it must be a reporting leaf. | | Workflow Brief | The recommended route, scope, acceptance evidence, assumptions, risks, and material alternatives presented at a Decision Checkpoint. | | Block | A reusable high-level orchestration capability such as explore, plan, debug, coding, verify, or review. Blocks compile into Nodes. | | Node | A low-level durable unit of child-agent work with dependencies, prompt input, policy, and output contract. | @@ -31,6 +32,7 @@ Workflow Orchestration turns one user objective into one durable DAG. Its model- - Model-facing graph actions expose only `spec_path`; graph fields live in YAML so provider tool-call serialization cannot turn a nested graph into a string. - Legacy YAML may be adapted at the file boundary without making legacy fields valid inline input. - Runtime Admission and Workflow Authoring Check have separate names, state, and responsibilities. +- Dependents of a reporting checkpoint must be gated on its output; authoring rejects ungated shapes at start/validate (enforcement point: authoring boundary only, runtime create deliberately unchanged). ## Boundaries @@ -43,3 +45,5 @@ Workflow Orchestration turns one user objective into one durable DAG. Its model- ## Decisions - [ADR-0001: One Workflow Authoring Check authority](docs/adr/0001-workflow-authoring-check.md) +- [ADR-0002: Parallel workspace writers with an implementation aggregator](docs/adr/0002-parallel-writers-aggregator.md) +- [ADR-0003: Reporting checkpoint gating at the authoring boundary](docs/adr/0003-reporting-checkpoint-gating.md) diff --git a/packages/opencode/src/dag/docs/adr/0003-reporting-checkpoint-gating.md b/packages/opencode/src/dag/docs/adr/0003-reporting-checkpoint-gating.md new file mode 100644 index 000000000..41b1e50b6 --- /dev/null +++ b/packages/opencode/src/dag/docs/adr/0003-reporting-checkpoint-gating.md @@ -0,0 +1,70 @@ +# ADR-0003: Reporting checkpoint gating at the authoring boundary + +- Status: Accepted +- Date: 2026-08-17 + +## Context + +On 2026-08-17 a hand-authored 15-node workflow (issue #320) ran 75 minutes to +`completed` although every one of its five decision checkpoints returned +`verdict: replan`. The checkpoint nodes carried `report_to_parent: true` but +their stage dependents declared only `depends_on`, no `condition`. The engine +spawns a dependent the moment all of its dependencies complete, so each next +stage started milliseconds (≈12ms) after its checkpoint settled; the wake to +the parent was terminal-only advisory signal, never a gate. The authoring +model ignored warning-level feedback, which is how the ungated shape shipped. + +Block-compiled graphs already gate dependents on checkpoint verdicts (the +issue #294 REJECT-checkpoint shape); hand-built node graphs had no equivalent +check. + +## Decision + +A `report_to_parent: true` node with dependents is a **reporting checkpoint**. +Each dependent must gate on the checkpoint's output via `condition` +(`input_mapping` does not count — it feeds data, it does not gate), or the +checkpoint must be a reporting leaf, or the node must drop `report_to_parent`. +`node_defaults.report_to_parent` is honored: a node inheriting the default +reports the same way. + +Enforcement lives in `checkpointGateDiagnostics`, wired only into +`validatePostCompile`'s structural branch — the authoring start/validate path. +Every ungated dependent emits one error-severity `dag.invalid` diagnostic in +both `portable` and `environment` profiles, so `start` and `validate` reject +the shape before any durable graph exists. + +Enforcement is authoring-only by design. `Dag.create` and the replan/extend +fragment paths stay untouched: the verdict vocabulary is open, the ACCEPT path +must not wait for the parent, and runtime enforcement would change the +semantics of every existing graph, including issue #294's wake-chain and +reopen-extend behavior. + +## Consequences + +- Ungated reporting checkpoints fail fast at start/validate with a diagnostic + naming the checkpoint, the dependent, and the three legal fixes. +- Runtime create, wake chains, and reopen-extend semantics are unchanged; + trusted internal callers retain full runtime flexibility. +- Saved and curated workflows were audited: 14 curated block workflows are + unaffected; only `ultra-flow-route.yaml` and `release-route.yaml` trip the + new check and are tracked in opencode-dag-config#14. + +## Alternatives Considered + +- Runtime enforcement at `Dag.create`: rejected — the verdict vocabulary is + open-ended, the ACCEPT path must not block waiting for the parent, and it + would change the behavior of every existing graph. +- Warning-severity diagnostic: rejected — the authoring model ignores + warnings; that is precisely how the incident happened. +- A new explicit `gate` field on dependents: rejected — `condition` already + expresses output gating and the block compiler already emits it; a second + mechanism would split the gating vocabulary. + +## Deferred + +- Replan/extend fragments are not checkpoint-gate-checked (coverage gap; no + date). Runtime flexibility was prioritized; fragment authoring remains + advisory. +- Deprecation of advisory wake chains (no date): `report_to_parent` without + gated dependents stays legal but is a smell worth revisiting once fragment + coverage exists. diff --git a/packages/opencode/src/dag/validation.ts b/packages/opencode/src/dag/validation.ts index d1126e41b..c92fae884 100644 --- a/packages/opencode/src/dag/validation.ts +++ b/packages/opencode/src/dag/validation.ts @@ -575,6 +575,38 @@ function conditionDiagnostics(nodes: readonly NodeConfig[]): Diagnostic[] { ] } +/** A report_to_parent node wakes the parent for adjudication; a dependent + * without a condition on that node's output is spawned the moment the + * checkpoint completes, so the checkpoint verdict can never act first. + * Block-compiled graphs gate dependents on the verdict (issue #294 + * REJECT-checkpoint shape); hand-built node graphs must do the same or keep + * the checkpoint as a reporting leaf. */ +export function checkpointGateDiagnostics( + nodes: readonly NodeConfig[], + defaults?: { readonly report_to_parent?: boolean }, +): Diagnostic[] { + const reportsToParent = (node: NodeConfig) => + node.report_to_parent ?? defaults?.report_to_parent ?? DEFAULT_WORKFLOW_CONFIG.reportToParent + return nodes.flatMap((checkpoint) => { + if (!reportsToParent(checkpoint)) return [] + return nodes + .filter((dependent) => dependent.depends_on.includes(checkpoint.id)) + .filter((dependent) => conditionReference(dependent.condition) !== checkpoint.id) + .map((dependent) => + diagnostic({ + code: DIAGNOSTIC_CODES.dagInvalid, + path: `nodes[${dependent.id}].condition`, + message: + `reporting checkpoint "${checkpoint.id}" has dependent "${dependent.id}" that is not gated on its output` + + ` — the engine spawns "${dependent.id}" as soon as "${checkpoint.id}" completes, so the checkpoint verdict cannot be acted on first`, + hint: + `Gate "${dependent.id}" with condition: "${checkpoint.id}.output. == ..." (e.g. on its verdict),` + + ` keep "${checkpoint.id}" a reporting leaf, or set report_to_parent: false on "${checkpoint.id}" if downstream must run unconditionally`, + }), + ) + }) +} + function bindingDiagnostics(nodes: readonly NodeConfig[]): Diagnostic[] { return templateBindingErrors(nodes).map((error) => diagnostic({ @@ -926,7 +958,7 @@ export function validatePostCompile(input: { config: { mode?: ExecutionMode max_total_nodes?: number - node_defaults?: { required?: boolean; model?: { modelID: string; providerID: string } } + node_defaults?: { required?: boolean; report_to_parent?: boolean; model?: { modelID: string; providerID: string } } } nodes: readonly NodeConfig[] /** The original blocks when the graph used the high-level interface. */ @@ -942,11 +974,14 @@ export function validatePostCompile(input: { const diagnostics = input.structural === false ? [] - : structuralDiagnostics({ - nodes: input.nodes, - mode: input.config.mode, - max_total_nodes: input.config.max_total_nodes, - }) + : [ + ...structuralDiagnostics({ + nodes: input.nodes, + mode: input.config.mode, + max_total_nodes: input.config.max_total_nodes, + }), + ...checkpointGateDiagnostics(input.nodes, input.config.node_defaults), + ] if (input.profile === "portable") diagnostics.push(...nonportablePromptDiagnostics(input.nodes)) if (input.profile === "environment") { diagnostics.push( diff --git a/packages/opencode/test/dag/dag-checkpoint-gate.test.ts b/packages/opencode/test/dag/dag-checkpoint-gate.test.ts new file mode 100644 index 000000000..18fcebecf --- /dev/null +++ b/packages/opencode/test/dag/dag-checkpoint-gate.test.ts @@ -0,0 +1,139 @@ +import { expect } from "bun:test" +import { Effect } from "effect" +import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" +import { WorkflowAuthoring } from "../../src/dag/authoring" +import { testEffect } from "../lib/effect" + +const it = testEffect(CrossSpawnSpawner.defaultLayer) + +// A report_to_parent (wake-eligible) checkpoint hands its verdict to the +// parent. When a dependent lacks a condition on that checkpoint's output, the +// engine spawns it the moment the checkpoint completes and the verdict can +// never act first — exactly the shape that ran a 6-stage "ultra flow" to +// completion after every decision checkpoint returned replan. Block-compiled +// graphs gate dependents on the verdict (issue #294 REJECT-checkpoint shape); +// hand-built node graphs must do the same. + +function spec(config: Record) { + return { title: "checkpoint gate", config } +} + +function checkpoint(id: string) { + return { + id, + name: id, + worker_type: "general", + depends_on: [], + report_to_parent: true, + prompt_template: { inline: id }, + output_schema: { + type: "object", + properties: { verdict: { type: "string" } }, + required: ["verdict"], + }, + } +} + +function stage(id: string, dependsOn: string[], condition?: string) { + return { + id, + name: id, + worker_type: "build", + depends_on: dependsOn, + prompt_template: { inline: id }, + ...(condition ? { condition } : {}), + } +} + +function validate(value: unknown) { + return WorkflowAuthoring.make().prepare({ + action: "start", + source: { kind: "inline", value, source: "" }, + }) +} + +it.effect("rejects a reporting checkpoint whose dependent is not gated on its output", () => + Effect.gen(function* () { + const result = yield* validate( + spec({ + name: "ungated-checkpoint", + nodes: [checkpoint("cp-design-decision"), stage("stage-development", ["cp-design-decision"])], + }), + ) + expect(result.valid).toBe(false) + expect(result.errors.some((d) => d.message.includes('"cp-design-decision"') && d.message.includes('"stage-development"'))).toBe(true) + }), +) + +it.effect("accepts a dependent gated by a condition on the checkpoint output", () => + Effect.gen(function* () { + const result = yield* validate( + spec({ + name: "gated-checkpoint", + nodes: [ + checkpoint("cp-design-decision"), + stage("stage-development", ["cp-design-decision"], 'cp-design-decision.output.verdict == "continue"'), + ], + }), + ) + expect(result.errors.filter((d) => d.message.includes("not gated"))).toEqual([]) + }), +) + +it.effect("accepts a reporting checkpoint as a leaf", () => + Effect.gen(function* () { + const result = yield* validate( + spec({ + name: "leaf-checkpoint", + nodes: [stage("stage-design", []), { ...checkpoint("cp-after-design"), depends_on: ["stage-design"] }], + }), + ) + expect(result.errors.filter((d) => d.message.includes("not gated"))).toEqual([]) + }), +) + +it.effect("accepts an ungated dependent when the checkpoint does not report to parent", () => + Effect.gen(function* () { + const result = yield* validate( + spec({ + name: "quiet-node", + nodes: [{ ...checkpoint("analysis"), report_to_parent: false }, stage("summary", ["analysis"])], + }), + ) + expect(result.errors.filter((d) => d.message.includes("not gated"))).toEqual([]) + }), +) + +it.effect("flags ungated dependents inherited from node_defaults.report_to_parent", () => + Effect.gen(function* () { + const result = yield* validate( + spec({ + name: "defaults-inherited", + node_defaults: { report_to_parent: true }, + nodes: [ + { ...checkpoint("cp"), report_to_parent: undefined }, + stage("after", ["cp"]), + ], + }), + ) + expect(result.valid).toBe(false) + expect(result.errors.some((d) => d.message.includes('"cp"') && d.message.includes('"after"'))).toBe(true) + }), +) + +it.effect("flags a condition that gates a different dependency than the checkpoint", () => + Effect.gen(function* () { + const result = yield* validate( + spec({ + name: "wrong-gate", + nodes: [ + checkpoint("cp-design-decision"), + stage("stage-design", []), + stage("stage-development", ["cp-design-decision", "stage-design"], 'stage-design.output.ready == "yes"'), + ], + }), + ) + expect(result.valid).toBe(false) + expect(result.errors.some((d) => d.message.includes('"cp-design-decision"') && d.message.includes('"stage-development"'))).toBe(true) + }), +)