From 55f0e7b2f9d025cb4446712ac8226c47cb1a95d1 Mon Sep 17 00:00:00 2001 From: Archon Date: Mon, 10 Aug 2026 19:35:23 +0000 Subject: [PATCH 1/4] =?UTF-8?q?feat(workflows):=20structural=20workflow=20?= =?UTF-8?q?signature=20=E2=80=94=20inputs/returns/with=20on=20workflow:=20?= =?UTF-8?q?nodes=20(#2470)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a declarative signature surface to the workflow language (Signature Phase 2): a workflow declares `inputs:` (what it takes) and `returns:` (which node's output is its result); callers supply values via `with:` — now accepted on `workflow:` sub-run nodes, not just `include:`. This coordinates, it does not compute (Workflow Language Constitution — cited). Changes: - Schema: `inputs:`/`returns:` on workflowBaseSchema; `with:` on workflow nodes with include-style shape validation; reject `with`+`input`; `fan_out.as`↔`with` collision check; delete the dead `fan_out.as` placeholder rejection (#2224 merged); `inputEnvKey` - Loader: parse/validate inputs (warn-and-drop, reject required+default), returns top-level-id existence check, env-key mangling collision reject, scan `workflow:` `with:` values for dangling refs, four-surface KEEP-IN-SYNC comment - Include-expander: `returns:` drives primarySink only (sinks/depends_on unchanged, may be a non-sink); validate `with:` against declared `inputs:` (defaults/missing-required/ undeclared-key) only when inputs declared — undeclared blocks byte-for-byte unchanged - Runtime: strict `$INPUTS.` substitution in the `!shellSafe` branch (did-you-mean hint, throws on unknown); `INPUTS_` env for bash/script sub-run nodes; persist `metadata.inputs` at spawn so `$INPUTS` reconstitutes on cold resume; fan-out static `with:` + per-item `$INPUTS.` channel - `returns:` rebinds a child run's terminal output (parent_run_id-gated; blank → '' + WARN, no sink fallthrough); loop_group per-iteration scan deliberately unchanged - Bare-run guard: a required-input block still lists/loads but a top-level run fails before any worktree/clone/AI cost (CLI + orchestrator) - Validator: bundled-set-only `workflow:` target check via the runtime fuzzy resolver - Docs: authoring-guide Workflow Signature section + binding-time table + `INPUTS_*` env mangling; constitution admits inputs/returns and records cross-file schema checking rejected - Tests: signature parse/validate, returns→primarySink, with vs inputs, $INPUTS runtime, bundled-target check, bare-run guard, env-key mangling; #2459 parity ratchet fixtures added Closes #2470 Co-Authored-By: Claude Opus 4.8 --- packages/cli/src/commands/workflow.ts | 10 +- .../src/orchestrator/orchestrator-agent.ts | 19 ++ .../docs/guides/authoring-workflows.md | 113 ++++++++++- .../workflow-language-constitution.md | 8 +- packages/workflows/src/dag-executor.ts | 153 +++++++++++++-- .../workflows/src/executor-shared.test.ts | 52 ++++++ packages/workflows/src/executor-shared.ts | 38 +++- packages/workflows/src/executor.ts | 9 + .../workflows/src/include-expander.test.ts | 83 +++++++++ packages/workflows/src/include-expander.ts | 75 +++++++- packages/workflows/src/loader.test.ts | 175 ++++++++++++++++-- packages/workflows/src/loader.ts | 108 ++++++++++- packages/workflows/src/schemas.test.ts | 26 +++ packages/workflows/src/schemas/dag-node.ts | 108 +++++++---- packages/workflows/src/schemas/index.ts | 3 + .../workflows/src/schemas/workflow-run.ts | 19 ++ packages/workflows/src/schemas/workflow.ts | 50 +++++ .../src/utils/workflow-requirements.test.ts | 38 +++- .../src/utils/workflow-requirements.ts | 53 +++++- packages/workflows/src/validator.test.ts | 28 +++ packages/workflows/src/validator.ts | 67 ++++++- 21 files changed, 1148 insertions(+), 87 deletions(-) diff --git a/packages/cli/src/commands/workflow.ts b/packages/cli/src/commands/workflow.ts index be4c1051b5..2ffb1a0591 100644 --- a/packages/cli/src/commands/workflow.ts +++ b/packages/cli/src/commands/workflow.ts @@ -43,7 +43,10 @@ import { createChildWorktreeResolver } from '@archon/core/workflows/child-isolat import { discoverWorkflowsWithConfig } from '@archon/workflows/workflow-discovery'; import { resolveWorkflowName } from '@archon/workflows/router'; import { executeWorkflow, hydrateResumableRun } from '@archon/workflows/executor'; -import { assertWorkflowRequirementsMet } from '@archon/workflows/utils/workflow-requirements'; +import { + assertWorkflowRequirementsMet, + assertWorkflowInputsSatisfiable, +} from '@archon/workflows/utils/workflow-requirements'; import { getWorkflowEventEmitter, type WorkflowEmitterEvent, @@ -1014,6 +1017,11 @@ export async function workflowRunCommand( assertNoWorktreeOptionsForFolder(options.folder === true, options); assertWorkflowNotWorktreePinnedForFolder(options.folder === true, pinnedEnabled, workflow.name); + // Signature gate (#2470): a workflow declaring `required` inputs is a reusable block — + // only a caller's `with:` can satisfy them, so a bare top-level run fails here, before + // the --detach fork and any worktree/clone/AI cost. It still lists/loads normally. + assertWorkflowInputsSatisfiable(workflow); + // Capability gate: hard-fail before the --detach fork and any worktree/clone/ // AI cost if the workflow declares `requires: [github]` and the acting CLI // user hasn't connected. No-op on solo PAT installs. Mirrors the orchestrator diff --git a/packages/core/src/orchestrator/orchestrator-agent.ts b/packages/core/src/orchestrator/orchestrator-agent.ts index f7ed3a2573..dbc69dd16d 100644 --- a/packages/core/src/orchestrator/orchestrator-agent.ts +++ b/packages/core/src/orchestrator/orchestrator-agent.ts @@ -44,6 +44,8 @@ import { executeWorkflow, hydrateResumableRun } from '@archon/workflows/executor import { assertWorkflowRequirementsMet, WorkflowRequirementError, + assertWorkflowInputsSatisfiable, + WorkflowMissingInputsError, } from '@archon/workflows/utils/workflow-requirements'; import type { WorkflowDefinition, @@ -737,6 +739,23 @@ async function dispatchOrchestratorWorkflow( }) : undefined; + // Signature gate (#2470): a workflow declaring `required` inputs is a reusable block — + // only a caller's `with:` satisfies them, so a bare top-level invocation fails here, + // before any worktree/clone/AI cost. It still lists/loads normally (builder + discovery). + try { + assertWorkflowInputsSatisfiable(workflow); + } catch (err) { + if (err instanceof WorkflowMissingInputsError) { + getLog().info( + { workflowName: workflow.name, conversationId, userId, missing: err.missing }, + 'workflow.required_inputs_unsatisfiable' + ); + await platform.sendMessage(conversationId, err.message); + return; + } + throw err; + } + // Capability gate: hard-fail before any worktree/clone/AI cost if the // workflow declares `requires: [github]` and the originating user hasn't // connected. No-op when per-user GitHub is disabled (solo PAT installs). diff --git a/packages/docs-web/src/content/docs/guides/authoring-workflows.md b/packages/docs-web/src/content/docs/guides/authoring-workflows.md index 0cecb507e6..5c6b8599a4 100644 --- a/packages/docs-web/src/content/docs/guides/authoring-workflows.md +++ b/packages/docs-web/src/content/docs/guides/authoring-workflows.md @@ -144,6 +144,10 @@ tags: [GitLab, Review] # Optional: explicit Web UI filter tags. Overri # keyword-based tag inference. An empty list (`tags: []`) # suppresses inference and shows no tags. Omit to fall # back to inferred tags (the default). +inputs: # Optional: declared signature — what this block takes. + diff: { required: true } # A caller supplies values via `with:`; the block reads + style: { default: strict } # them as `$INPUTS.`. See "Workflow Signature". +returns: synthesize # Optional: the node id whose output IS this block's result. # Required for DAG-based nodes: @@ -198,7 +202,7 @@ nodes: | `approval` | object | Pauses workflow for human review. See [Approval Nodes](/guides/approval-nodes/) | | `cancel` | string | Terminates the workflow run with a reason string. Uses existing cancellation plumbing — in-flight parallel nodes are stopped | | `include` | string | Name of another workflow whose nodes are inlined into this DAG at load time as a namespaced sub-DAG. See [Reusing a Shared Sub-DAG](#reusing-a-shared-sub-dag-with-include) | -| `workflow` | string | Name of another workflow to run as a governed **child sub-run** at execution time — its own run record, gates, artifacts, and cost. Optional `input` (data string), `isolation` (`'inherit'` \| `'worktree'`), and `fan_out` (one child per item of a runtime list). See [Composing a Governed Sub-Run](#composing-a-governed-sub-run-with-workflow) | +| `workflow` | string | Name of another workflow to run as a governed **child sub-run** at execution time — its own run record, gates, artifacts, and cost. Optional `input` (untyped data string → child's `$ARGUMENTS`) **or** `with:` (named inputs → child's `$INPUTS.`; mutually exclusive with `input`), `isolation` (`'inherit'` \| `'worktree'`), and `fan_out` (one child per item of a runtime list; optional `as:` names the per-item `$INPUTS` channel). See [Composing a Governed Sub-Run](#composing-a-governed-sub-run-with-workflow) and [Workflow Signature](#workflow-signature-inputs-returns-and-inputs) | **Common fields** — apply to all node types: @@ -985,7 +989,11 @@ underscores, or hyphens. Values must be strings and are inserted verbatim during expansion — they are **never expressions**: nothing is evaluated, computed, or interpreted, and the value is spliced in as text exactly as written. An inserted `$node.output` reference remains a reference and resolves through the normal runtime output substitution. A missing -input is a load error; extra caller keys are ignored until workflow input declarations ship. +input referenced by the block is a load error. Whether **extra** caller keys are allowed +depends on whether the block declares `inputs:` (see +[Workflow Signature](#workflow-signature-inputs-returns-and-inputs)): a block with no +`inputs:` ignores unrecognized keys; a block that declares `inputs:` rejects an undeclared +key at load. Substitution applies everywhere the value could reach the model or the shell, including inside Markdown code fences and inline code spans — `$INPUTS.` has no @@ -1002,8 +1010,10 @@ the prompt text. Use an inline `prompt:` when the block needs include inputs. This check is best-effort, so a clean load is not a guarantee. It covers the block's top-level `command:`/`loop.command` nodes only, so a command nested inside a `loop_group` body is not scanned; and a command file that cannot be resolved at load time is logged as a -warning and skipped rather than failing the workflow. This restriction applies to `include:`; -named `with:` mappings for `workflow:` sub-runs have not shipped. +warning and skipped rather than failing the workflow. This restriction applies to `include:` +only — a `workflow:` sub-run's named inputs **do** reach `command:` bodies, because they +resolve at runtime rather than at load time (see the binding-time table in +[Workflow Signature](#workflow-signature-inputs-returns-and-inputs)). ### Non-goals (Phase 1) @@ -1024,6 +1034,86 @@ for a standalone run. --- +## Workflow Signature: `inputs:`, `returns:`, and `$INPUTS` + +A workflow can declare a **structural signature** — what it takes and what it returns — so a +reusable block has an explicit, caller-facing contract instead of relying on positional +accidents. Two workflow-level fields: + +```yaml +name: archon-review-block +description: Reusable review block (building block — not for standalone runs) +inputs: + diff: + required: true + description: the diff to review + style: + default: strict +returns: synthesize # the node whose output IS this block's result +nodes: + - id: gather + prompt: Gather context for $INPUTS.diff (style $INPUTS.style). + - id: synthesize + prompt: Synthesize a review from $gather.output. + depends_on: [gather] + - id: implement-fixes + prompt: Apply the fixes. + depends_on: [synthesize] +``` + +- **`inputs:`** — a map of input name → `{ required?, default?, description? }`. `required: true` + and `default:` are mutually exclusive (a required input has no default; declaring both drops + the key at load with a warning). A caller supplies values with `with:` on the `include:` or + `workflow:` node that references this workflow. When a block declares `inputs:`, callers are + validated: a missing **required** input and an **undeclared** caller key are both load errors, + and a declared `default:` fills an omitted input. A workflow with **no** `inputs:` keeps the + old lenient behavior (unknown caller keys ignored). +- **`returns:`** — the **node id** whose output IS the workflow's result. It selects by id, so + it works for any node type and even a **non-sink** node (a node other nodes depend on). For an + `include:` block, `$blk.output` resolves to the `returns:` node; `depends_on: [blk]` still + waits on every terminal node. For a `workflow:` sub-run child, the child's terminal output + (threaded back as `$node.output`) becomes the `returns:` node's output. + +### Binding time: includes resolve at load, sub-runs at runtime + +`$INPUTS.` is delivered by **two deliberately separate paths**, and the difference decides +which surfaces can read it: + +| Caller | When `$INPUTS` resolves | Reaches `prompt:`/`bash:`/`script:` | Reaches `command:` file bodies | +|--------|-------------------------|-------------------------------------|--------------------------------| +| `include:` | **Load time** (the block is inlined; values are spliced into node text) | Yes | **No** — a command file is read after expansion, so `$INPUTS` in one is a load error | +| `workflow:` sub-run | **Runtime** (values become `$INPUTS` variables on the child run) | Yes | **Yes** — every child node flows through runtime substitution | + +That asymmetry is the point: a sub-run's named inputs reach `command:` bodies precisely because +they resolve at runtime, where an include's load-time macro cannot. Sub-run inputs are also +persisted to the child run's metadata at spawn, so `$INPUTS` reconstitutes on a cold resume. + +### `$INPUTS` in `bash:`/`script:` nodes uses env vars + +`$INPUTS.` text is substituted only into non-shell (AI/prompt) surfaces — a sub-run's +input value can derive from AI output, exactly the user-controlled class kept out of shell +source. A `bash:`/`script:` node instead reads each input as an **environment variable** named +`INPUTS_`: hyphens become underscores and the name is upper-cased, so `plan` → +`$INPUTS_PLAN` and `base-branch` → `$INPUTS_BASE_BRANCH`. (Because `-` and `_` both fold to `_`, +two input names that collide on one env key — e.g. `foo-bar` and `foo_bar` — are a load error.) + +```yaml +# in a workflow: sub-run child +nodes: + - id: check + bash: | + echo "planning: $INPUTS_PLAN" # NOT $INPUTS.plan — bash reads the env var +``` + +### Bare runs of a required-input block fail fast + +A workflow that declares a **required** input is a reusable block: only a caller's `with:` can +satisfy it. It still **loads and lists** (the builder and discovery need it visible), but a bare +**top-level** run fails immediately — before any worktree, clone, or AI cost — naming the missing +inputs and pointing at `include:`/`workflow:`. Reference it from another workflow instead. + +--- + ## Composing a Governed Sub-Run with `workflow:` A `workflow:` node runs another workflow as a **child sub-run** — a genuinely separate @@ -1051,6 +1141,21 @@ nodes: context: fresh ``` +A `workflow:` node has **one** input channel per invocation: either the untyped `input:` string +(delivered as the child's `$ARGUMENTS`) **or** the named `with:` map (delivered as the child's +`$INPUTS.` — see [Workflow Signature](#workflow-signature-inputs-returns-and-inputs)). +Setting both on one node is a load error. Use `with:` when the child declares named `inputs:` or +when its `command:` bodies need named values: + +```yaml + - id: implement-qa + workflow: qa-block + with: + plan: "$plan.output" + mode: fast + depends_on: [plan] +``` + ### `include:` vs `workflow:` — which to use Both reuse another workflow. They differ in **governance**, not syntax: diff --git a/packages/docs-web/src/content/docs/reference/workflow-language-constitution.md b/packages/docs-web/src/content/docs/reference/workflow-language-constitution.md index cab4174a76..6eed008bce 100644 --- a/packages/docs-web/src/content/docs/reference/workflow-language-constitution.md +++ b/packages/docs-web/src/content/docs/reference/workflow-language-constitution.md @@ -77,7 +77,7 @@ The test the rule actually applies is *"does one child's outcome end another's?" | `loop:` / `loop_group:` | ✅ admitted | Iteration structure the engine must own for events, gates, and cost accounting | | `include:` (load-time inlining, [#2121](https://github.com/coleam00/Archon/issues/2121)) | ✅ admitted | Textual composition, zero new runtime semantics — the engine sees a flat DAG | | ~~`first_success` racing join~~ ([#1764](https://github.com/coleam00/Archon/issues/1764), implemented in [#2250](https://github.com/coleam00/Archon/pull/2250)) | ❌ **rejected 2026-08-04** — reverses an earlier ✅ | Admitted originally as "a join rule — coordination", which is true of its *shape* and misses what it does: the winner aborts and cancels the losers, so one child's outcome ends its siblings'. That is the coupling [the independence rule](#the-independence-rule) forbids, and it cannot be reshaped — racing without terminating the losers is not racing. The want underneath it (several genuinely different attempts, best result forward) is real and is served by N distinct nodes with their own models converging on a collector node, which needs no mutual cancellation | -| Runtime sub-runs (`workflow:`, #2121 Phase 2) | ✅ shipped | A sub-run is a governance object (own run record, own gates, own audit trail). Slice 1: shared checkout, `input:` string, gate-aware pause/resume. Slice 2 adds opt-in per-child isolation (`isolation: worktree`) and data-driven fan-out (`fan_out:`); `with:` remains deferred and racing is rejected outright (row above) | +| Runtime sub-runs (`workflow:`, #2121 Phase 2) | ✅ shipped | A sub-run is a governance object (own run record, own gates, own audit trail). Slice 1: shared checkout, `input:` string, gate-aware pause/resume. Slice 2 adds opt-in per-child isolation (`isolation: worktree`) and data-driven fan-out (`fan_out:`); named `with:` inputs shipped in [#2470](https://github.com/coleam00/Archon/issues/2470) (row below), and racing is rejected outright (row above) | | Data-driven fan-out (`fan_out:`, [#2224](https://github.com/coleam00/Archon/pull/2224)) | ✅ shipped | The expansion is *data*, not structure: the target is a static workflow name and only the child COUNT comes from a runtime array, so the parent DAG the executor runs stays flat and static. Each child is a real run record with its own gates, artifacts and cost — the sub-run escape this page already names for runtime-resolved structure (see [Composition metastasis](#2-composition-metastasis-structure-features-become-functions)). `max_parallel` and `join` are coordination (concurrency bound, join rule); nothing in the block computes | | Per-node isolation **inferred** from another field (auto-`worktree` because a node fans out, or has a concurrent sibling) | ❌ rejected | The engine never infers isolation. How many children a node spawns says nothing about whether they write — N review or research children over a shared checkout is the common case. The engine's job is to make the author's declaration hold, not to guess what they must have meant; `isolation: worktree` and `mutates_checkout: false` are where those two claims get made. (Run-level worktree-by-default is a different thing and stands: a whole run against a repo has an owner and a lifecycle.) | | Fail-fast sibling cancellation on a failing join | ❌ rejected | One child's failure cancelled its in-flight siblings mid-run so a doomed join stopped burning tokens. Defensible under "one job split N ways"; wrong under [independence](#the-independence-rule) — the siblings' output is exactly what a partial failure is supposed to preserve. Every index now spawns and every child reaches its own terminal state before the join reduces. The trade is explicit: worst-case spend is `items.length`, not "until the first failure", which is what makes a run-tree budget ceiling ([#1961](https://github.com/coleam00/Archon/issues/1961)) load-bearing rather than theoretical | @@ -89,7 +89,9 @@ The test the rule actually applies is *"does one child's outcome end another's?" | Parentheses & nested boolean grouping in `when:` | ❌ rejected (see policy below) | The first step of home-growing an expression language | | Templating (Jinja-style interpolation, computed node ids) | ❌ rejected | Evaluation inside declaration — the Helm road | | Dynamic include targets (`include: $x.output`) | ❌ rejected | Turns structure into a runtime value; the engine can no longer statically validate the graph | -| `with:` include parameters | ✅ shipped (data-only) | Identifier-keyed string values are substituted during load-time expansion; inserted `$node.output` values continue through normal runtime output substitution. `workflow.with` is not yet shipped | +| `with:` include parameters | ✅ shipped (data-only) | Identifier-keyed string values are substituted during load-time expansion; inserted `$node.output` values continue through normal runtime output substitution | +| Workflow signature — `inputs:` + `returns:` + `with:` on `workflow:` ([#2470](https://github.com/coleam00/Archon/issues/2470)) | ✅ admitted (data-only) | Declarative composition metadata the engine must see to wire and validate: `inputs:` is a caller-facing contract (the engine validates `with:` against it — missing-required / undeclared-key are load errors), `returns:` names the node whose output IS the block's result (an id the engine resolves, not a computation), and `with:` on a `workflow:` node delivers named values as the child's runtime `$INPUTS.`. It coordinates — it does not compute. Passes the admissibility test: the engine needs it to govern; it is declarative data; a script node could not express a caller-facing signature | +| Cross-file schema checking (validate a caller's `output_format` against a callee's `returns`/`inputs` types) | ❌ rejected | Considered and rejected with `inputs:`/`returns:` (#2470). `returns:` decides *which* output; `output_format` decides *how fields are read* — there is no coupling to check, and adding one would make the loader reason across file boundaries about value shapes it cannot see statically. Type agreement between a caller and callee is the author's responsibility, surfaced at runtime by the existing strict `$node.output.field` access, not a new load-time cross-file analysis | ## The five smells — and the management lever for each @@ -107,7 +109,7 @@ These are the specific mechanisms by which workflow languages rot. Each is liste **Mechanism.** Reuse primitives are the most dangerous axis because they converge on function application: includes become calls, parameters become arguments, loop-carried state becomes variables — and suddenly the config format has scoping rules, evaluation order, and abstraction. This is how Helm charts became programs. -**Archon today.** `loop_group` already carries loop-state (`$LOOP_PREV`); `include:` adds textual reuse. Both are held on the declarative side deliberately: `include` is load-time expansion with zero new runtime semantics, and its shipped `with:` surface is a data-only string mapping resolved during expansion. Expressions, deep output access across the include boundary, `workflow.with`, and dynamic targets remain unsupported. +**Archon today.** `loop_group` already carries loop-state (`$LOOP_PREV`); `include:` adds textual reuse. Both are held on the declarative side deliberately: `include` is load-time expansion with zero new runtime semantics, and its shipped `with:` surface is a data-only string mapping resolved during expansion. The workflow **signature** (`inputs:`/`returns:`/`with:` on `workflow:`, #2470) is likewise data-only: a caller-facing contract the engine validates and a return-node id it resolves, never a computation. Expressions, deep output access across the include boundary, dynamic targets, and cross-file schema checking remain unsupported. **Lever — composition must be resolvable at load time.** Any reuse feature must fully resolve before execution begins (the engine executes a flat, static DAG). Parameterization, if ever added, is data-only mapping. Anything requiring runtime resolution of *structure* is Phase-2 sub-run territory — where it becomes a governance object with its own run record, not a language feature. diff --git a/packages/workflows/src/dag-executor.ts b/packages/workflows/src/dag-executor.ts index e6a473a8df..b55e08cb94 100644 --- a/packages/workflows/src/dag-executor.ts +++ b/packages/workflows/src/dag-executor.ts @@ -71,6 +71,7 @@ import { isPersistableNode, readSubrunMetadata, isApprovalContext, + inputEnvKey, } from './schemas'; import { formatToolCall } from './utils/tool-formatter'; import { createLogger, captureWorkflowCompleted } from '@archon/paths'; @@ -138,6 +139,25 @@ function dagNodeTelemetryType(node: DagNode): WorkflowNodeType { return 'prompt'; } +/** + * Resolve this run's named inputs (#2470) from persisted sub-run metadata. Non-empty + * only for `workflow:` sub-run children (the parent stamps `metadata.inputs` at spawn); + * a top-level run has none. Threaded into every AI/prompt substitution so `$INPUTS.` + * resolves, and mangled to `INPUTS_` env vars for bash/script nodes. + */ +function resolveRunInputs(workflowRun: WorkflowRun): Record | undefined { + return readSubrunMetadata(workflowRun.metadata as Record | undefined).inputs; +} + +/** Env-var bag delivering this run's named inputs to bash/script sub-run nodes (#2470). */ +function inputEnvVars(workflowRun: WorkflowRun): NodeJS.ProcessEnv { + const inputs = resolveRunInputs(workflowRun); + if (!inputs) return {}; + const env: NodeJS.ProcessEnv = {}; + for (const [name, value] of Object.entries(inputs)) env[inputEnvKey(name)] = value; + return env; +} + interface RunningTool { toolName: string; startedAt: number; @@ -390,6 +410,13 @@ export interface RunChildWorkflowArgs { itemHash?: string; /** Present only when re-driving a FAILED child on parent resume (D5 recovery path). */ resumeFailedChild?: WorkflowRun; + /** + * Named inputs (#2470) — the resolved `with:` map the parent supplied, plus (for a + * fan-out child) the per-item `fan_out.as` entry. Persisted to the child's + * `metadata.inputs` at spawn so `$INPUTS.` resolves at runtime and reconstitutes + * on cold resume. Undefined/empty when the node declares no `with:`/`as`. + */ + inputs?: Record; } /** @@ -1389,7 +1416,7 @@ async function executeNodeInternal( docsDir, issueContext, `dag node '${node.id}' prompt`, - { stateDir } + { stateDir, inputs: resolveRunInputs(workflowRun) } ); } catch (error) { const err = error as Error; @@ -2656,6 +2683,10 @@ async function executeBashNode( // host token via runSubprocess's process.env layering — the scrub is unaffected. const subprocessEnv: NodeJS.ProcessEnv = { ...(envVars ?? {}), + // Named sub-run inputs as INPUTS_ env vars (#2470). Spread after + // envVars so a configured project env var can never shadow an input's delivery, + // and before the engine-reserved keys so those still win (same ordering rationale). + ...inputEnvVars(workflowRun), ARTIFACTS_DIR: artifactsDir, STATE_DIR: stateDir, LOG_DIR: logDir, @@ -2927,6 +2958,9 @@ async function executeScriptNode( // and still override the ambient host token via runSubprocess (scrub unaffected). const subprocessEnv: NodeJS.ProcessEnv = { ...(envVars ?? {}), + // Named sub-run inputs as INPUTS_ env vars (#2470) — same ordering + // rationale as executeBashNode: after envVars, before the engine-reserved keys. + ...inputEnvVars(workflowRun), ARTIFACTS_DIR: artifactsDir, STATE_DIR: stateDir, LOG_DIR: logDir, @@ -3579,6 +3613,10 @@ async function executeLoopGroupNode( // Determine this iteration's terminal output (first completed terminal node in // definition order — mirrors the top-level run's terminal-output selection). + // DELIBERATELY NOT `returns:`-aware (#2470): a loop_group's per-iteration output is + // the iteration's own result, not a caller contract — `returns:` selects a WORKFLOW's + // result and only rebinds a child run's terminal output (see executeDagWorkflow). Leave + // this positional scan as-is; do not "fix" the inconsistency. const allDeps = new Set(iterBodyNodes.flatMap(n => n.depends_on ?? [])); const terminalOutput = iterBodyNodes .filter(n => !allDeps.has(n.id)) @@ -4342,7 +4380,7 @@ async function executeLoopNode( i === startIteration ? loopUserInput : '', undefined, // rejectionReason i === startIteration ? '' : lastIterationOutput, - { stateDir } + { stateDir, inputs: resolveRunInputs(workflowRun) } ); const finalPrompt = substituteNodeOutputRefs(substitutedPrompt, nodeOutputs); @@ -5211,7 +5249,7 @@ async function executeApprovalNode( undefined, // loopUserInput rejectionReason, undefined, // loopPrevOutput - { stateDir } + { stateDir, inputs: resolveRunInputs(workflowRun) } ); // Build a synthetic PromptNode to reuse executeNodeInternal. @@ -5409,6 +5447,32 @@ async function executeWorkflowNode( ); const input = substituteNodeOutputRefs(substitutedInput, ctx.nodeOutputs); + // Resolve the node's `with:` map (#2470) into concrete strings — same two-pass + // resolution as `input`: workflow vars (non-shellSafe: these values become the child's + // `$INPUTS`, not shell source) then `$node.output` refs. The result is persisted to the + // child's metadata.inputs at spawn and reconstituted on cold resume. Throws on a bad ref + // exactly as the input surface does — caught by the caller's try/catch → fail closed. + let resolvedInputs: Record | undefined; + if (node.with !== undefined) { + resolvedInputs = {}; + for (const [name, rawValue] of Object.entries(node.with)) { + const { prompt: substituted } = substituteWorkflowVariables( + rawValue, + parentRun.id, + parentRun.user_message ?? '', + ctx.artifactsDir, + ctx.baseBranch, + ctx.docsDir, + ctx.issueContext, + undefined, + undefined, + undefined, + { stateDir: ctx.stateDir, inputs: resolveRunInputs(parentRun) } + ); + resolvedInputs[name] = substituteNodeOutputRefs(substituted, ctx.nodeOutputs); + } + } + // Producer's declared field set (only when output_format declares object // properties) so a downstream `$node.output.field` on a JSON-emitting child // resolves declared-optional-absent → '' vs a typo → throw. @@ -5570,6 +5634,7 @@ async function executeWorkflowNode( userId: parentRun.user_id ?? undefined, codebaseId: parentRun.codebase_id ?? undefined, isolation: node.isolation, + ...(resolvedInputs !== undefined ? { inputs: resolvedInputs } : {}), }; try { @@ -5987,6 +6052,36 @@ async function executeFanOutWorkflowNode( return failResult(msg); } + // Resolve the node's static `with:` map (#2470) once — the same $INPUTS applied to EVERY + // fan-out child. Per-item, the `fan_out.as` channel adds `$INPUTS. = ` on top + // (load-time collision-checked so `as` never overwrites a `with:` key). Resolved here + // rather than per-child because the values don't depend on the item. + const fanOutStaticInputs: Record = {}; + try { + if (node.with !== undefined) { + for (const [name, rawValue] of Object.entries(node.with)) { + const { prompt: substituted } = substituteWorkflowVariables( + rawValue, + parentRun.id, + parentRun.user_message ?? '', + ctx.artifactsDir, + ctx.baseBranch, + ctx.docsDir, + ctx.issueContext, + undefined, + undefined, + undefined, + { stateDir: ctx.stateDir, inputs: resolveRunInputs(parentRun) } + ); + fanOutStaticInputs[name] = substituteNodeOutputRefs(substituted, ctx.nodeOutputs); + } + } + } catch (err) { + const msg = `fan_out 'with:' on '${node.id}' could not be resolved: ${(err as Error).message}`; + await notify(`❌ **Fan-out failed** (node \`${node.id}\`): ${msg}`); + return failResult(msg); + } + // 2. Empty array → a valid zero-width expansion (#977 acceptance): complete with '[]'. if (items.length === 0) { getLog().info({ parentRunId: parentRun.id, nodeId: node.id }, 'workflow.fan_out_empty'); @@ -6205,6 +6300,13 @@ async function executeFanOutWorkflowNode( return childOutcomeFromRun(existing); } const input = itemToInput(item); + // Per-child $INPUTS (#2470): the static `with:` map plus the per-item `fan_out.as` + // channel (the item value under `$INPUTS.`). `as` is load-time guaranteed not to + // collide with a `with:` key, so this spread order is unambiguous. + const childInputs: Record = { + ...fanOutStaticInputs, + ...(fanOut.as !== undefined ? { [fanOut.as]: input } : {}), + }; // A fan-out-recoverable-cancelled child (gate/sibling) can't be resumed while // 'cancelled' (resumeWorkflowRun rejects that status) — clear it to 'failed' first, // then re-drive through the failed path. Our own tagged cancel is terminal state we @@ -6236,6 +6338,7 @@ async function executeFanOutWorkflowNode( isolation: node.isolation, childIndex: i, itemHash: hashFanOutItem(input), + ...(Object.keys(childInputs).length > 0 ? { inputs: childInputs } : {}), ...(resumeChild ? { resumeFailedChild: resumeChild } : {}), }); // A paused child is cancelled HERE rather than at the join, and the timing is @@ -7859,6 +7962,8 @@ export async function executeDagWorkflow( model?: string; /** Terminal-success evidence gate (#2230) — read at the completion path. */ evidence_policy?: WorkflowEvidencePolicy; + /** Declared `returns:` node id (#2470) — rebinds a CHILD run's terminal output. */ + returns?: string; } & WorkflowLevelOptions, workflowRun: WorkflowRun, workflowProvider: string, @@ -8363,15 +8468,39 @@ export async function executeDagWorkflow( if (gate === 'paused') return; } - // Terminal output (first sink node, non-blank, definition order) — the run's - // "summary". Computed BEFORE completeWorkflowRun so a sub-run can persist it into - // its own metadata: a `workflow:` parent re-reads it from there on auto-resume - // (the child's executeWorkflow return value is discarded across the human gate). - const allDependencies = new Set(workflow.nodes.flatMap(n => n.depends_on ?? [])); - const terminalOutput = workflow.nodes - .filter(n => !allDependencies.has(n.id)) - .map(n => nodeOutputs.get(n.id)) - .find(o => o?.state === 'completed' && o.output.trim().length > 0)?.output; + // Terminal output (the run's "summary"). Computed BEFORE completeWorkflowRun so a + // sub-run can persist it into its own metadata: a `workflow:` parent re-reads it from + // there on auto-resume (the child's executeWorkflow return value is discarded across the + // human gate). + // + // #2470: when a CHILD run's workflow declares `returns:`, its terminal output is THAT + // node's output — even a non-sink — instead of the positional first-sink scan. Gated on + // parent_run_id: a top-level run's summary stays the sink-scan chat/CLI affordance, not a + // caller contract. A `returns` node that didn't complete / produced blank output threads + // '' with a WARN and does NOT fall through to the sink scan (that would resurrect the + // positional accident under a new name). The loop_group per-iteration terminal scan + // (~executeLoopGroupNode) is byte-identical and DELIBERATELY unchanged — its result is + // the iteration's, never a caller's. + let terminalOutput: string | undefined; + if (workflow.returns !== undefined && workflowRun.parent_run_id) { + const returnsOutput = nodeOutputs.get(workflow.returns); + const value = returnsOutput?.state === 'completed' ? returnsOutput.output : undefined; + if (value !== undefined && value.trim().length > 0) { + terminalOutput = value; + } else { + getLog().warn( + { workflowRunId: workflowRun.id, returns: workflow.returns }, + 'workflow.returns_node_blank_output' + ); + terminalOutput = ''; + } + } else { + const allDependencies = new Set(workflow.nodes.flatMap(n => n.depends_on ?? [])); + terminalOutput = workflow.nodes + .filter(n => !allDependencies.has(n.id)) + .map(n => nodeOutputs.get(n.id)) + .find(o => o?.state === 'completed' && o.output.trim().length > 0)?.output; + } // Update DB and emit completion try { diff --git a/packages/workflows/src/executor-shared.test.ts b/packages/workflows/src/executor-shared.test.ts index 610fe670d7..8e8ce508e9 100644 --- a/packages/workflows/src/executor-shared.test.ts +++ b/packages/workflows/src/executor-shared.test.ts @@ -117,6 +117,58 @@ describe('substituteWorkflowVariables', () => { expect(prompt).toBe('No state reference here'); }); + it('substitutes a known $INPUTS. from options.inputs (#2470)', () => { + const { prompt } = substituteWorkflowVariables( + 'Plan: $INPUTS.plan and mode $INPUTS.mode', + 'run-1', + 'msg', + '/tmp/artifacts', + 'main', + 'docs/', + undefined, + undefined, + undefined, + undefined, + { inputs: { plan: 'do the thing', mode: 'fast' } } + ); + expect(prompt).toBe('Plan: do the thing and mode fast'); + }); + + it('throws with a did-you-mean hint on an unknown $INPUTS name (#2470)', () => { + expect(() => + substituteWorkflowVariables( + 'Use $INPUTS.pln', + 'run-1', + 'msg', + '/tmp/artifacts', + 'main', + 'docs/', + undefined, + undefined, + undefined, + undefined, + { inputs: { plan: 'x' } } + ) + ).toThrow('$INPUTS.plan'); + }); + + it('does NOT substitute $INPUTS under shellSafe — env delivery is the shell path (#2470/#2115)', () => { + const { prompt } = substituteWorkflowVariables( + 'echo "$INPUTS.plan"', + 'run-1', + 'msg', + '/tmp/artifacts', + 'main', + 'docs/', + undefined, + undefined, + undefined, + undefined, + { shellSafe: true, inputs: { plan: 'x' } } + ); + expect(prompt).toBe('echo "$INPUTS.plan"'); + }); + it('replaces $BASE_BRANCH with config value', () => { const { prompt } = substituteWorkflowVariables( 'Merge into $BASE_BRANCH', diff --git a/packages/workflows/src/executor-shared.ts b/packages/workflows/src/executor-shared.ts index bfcd594354..500c11869e 100644 --- a/packages/workflows/src/executor-shared.ts +++ b/packages/workflows/src/executor-shared.ts @@ -13,6 +13,17 @@ import { BUNDLED_COMMANDS, isBinaryBuild } from './defaults/bundled-defaults'; import { createLogger } from '@archon/paths'; import { isValidCommandName } from './command-validation'; import type { LoadCommandResult } from './schemas'; +import { INPUT_NAME_SOURCE } from './schemas/dag-node'; +import { similarNodeIds } from './output-ref'; + +/** + * Runtime `$INPUTS.` reference — the sub-run twin of the include-expander's + * load-time INPUTS_REF, built from the same identifier grammar so a name that + * validates as a `with:` key can never fail to match here. Resolved only for + * `workflow:` sub-runs (child runs get `metadata.inputs`), and only into non-shell + * surfaces (shell nodes get `INPUTS_` env vars instead — see #2470). + */ +const INPUTS_RUNTIME_REF = new RegExp(String.raw`\$INPUTS\.(${INPUT_NAME_SOURCE})`, 'g'); /** Lazy-initialized logger */ let cachedLog: ReturnType | undefined; @@ -424,6 +435,9 @@ export const CONTEXT_VAR_PATTERN_STR = * - $LOOP_PREV_OUTPUT - Cleaned output of the previous loop iteration. Empty string on the * first iteration (no prior output exists). Useful for fresh_context loops that need * to reference what the previous pass produced or why it failed. + * - $INPUTS. - Named sub-run inputs (#2470), supplied by a caller's `with:` on a + * `workflow:` node. Resolved from `options.inputs` in the non-shell branch only; an + * unknown name THROWS. Shell (bash/script) nodes read `INPUTS_` env vars. * * When issueContext is undefined, context variables are replaced with empty string * to avoid sending literal "$CONTEXT" to the AI. @@ -439,7 +453,7 @@ export function substituteWorkflowVariables( loopUserInput?: string, rejectionReason?: string, loopPrevOutput?: string, - options?: { shellSafe?: boolean; stateDir?: string } + options?: { shellSafe?: boolean; stateDir?: string; inputs?: Record } ): { prompt: string; contextSubstituted: boolean } { // Fail fast if the prompt references $BASE_BRANCH but no base branch could be resolved if (!baseBranch && prompt.includes('$BASE_BRANCH')) { @@ -483,6 +497,26 @@ export function substituteWorkflowVariables( .replace(/\$LOOP_USER_INPUT/g, loopUserInput ?? '') .replace(/\$REJECTION_REASON/g, rejectionReason ?? '') .replace(/\$LOOP_PREV_OUTPUT/g, loopPrevOutput ?? ''); + + // $INPUTS. — named sub-run inputs (#2470). Substituted ONLY in the non-shell + // branch: a sub-run's input value can derive from AI output (e.g. `with: {plan: + // $plan.output}`), the exact user-controlled class shellSafe keeps out of shell + // source (#2115). Bash/script bodies read INPUTS_ env vars instead. + // An unknown name THROWS (mirrors $node.output.field strictness) rather than + // substituting '' — a typo'd input silently emptying is worse than a load-visible error. + const inputs = options?.inputs; + result = result.replace(INPUTS_RUNTIME_REF, (_match, name: string) => { + if (inputs && Object.hasOwn(inputs, name)) return inputs[name]; + const known = inputs ? Object.keys(inputs) : []; + const hint = similarNodeIds(name, known); + const suffix = + hint.length > 0 + ? ` Did you mean ${hint.map(h => `$INPUTS.${h}`).join(', ')}?` + : known.length > 0 + ? ` Available inputs: ${known.map(k => `$INPUTS.${k}`).join(', ')}.` + : ' This run has no declared inputs.'; + throw new Error(`Unknown input '$INPUTS.${name}'.${suffix}`); + }); } // Check if context variables exist (use fresh regex to avoid lastIndex issues) @@ -534,7 +568,7 @@ export function buildPromptWithContext( docsDir: string, issueContext: string | undefined, logLabel: string, - options?: { shellSafe?: boolean; stateDir?: string } + options?: { shellSafe?: boolean; stateDir?: string; inputs?: Record } ): string { const { prompt, contextSubstituted } = substituteWorkflowVariables( template, diff --git a/packages/workflows/src/executor.ts b/packages/workflows/src/executor.ts index 33f238cac0..0f3f676968 100644 --- a/packages/workflows/src/executor.ts +++ b/packages/workflows/src/executor.ts @@ -653,6 +653,7 @@ async function runChildWorkflow( childIndex, itemHash, resumeFailedChild, + inputs, } = args; // Every failure below returns a `{ status: 'failed' }` outcome (never throws); @@ -829,6 +830,14 @@ async function runChildWorkflow( // alongside so resume can WARN on a non-deterministic producer (same index, new item). ...(childIndex !== undefined ? { [SUBRUN_METADATA_KEYS.childIndex]: childIndex } : {}), ...(itemHash !== undefined ? { [SUBRUN_METADATA_KEYS.fanOutItemHash]: itemHash } : {}), + // Named inputs (#2470) — persisted at spawn so the child's `$INPUTS.` + // resolves from `metadata.inputs` at runtime (resolveRunInputs) and survives a + // COLD resume: both resume paths (hydrateResumableRun and the zero-completed-node + // resumeWorkflowRun fallback) reload THIS run row, so the map is intact without + // re-resolving parent refs that may be out of scope. Stamped only when non-empty. + ...(inputs !== undefined && Object.keys(inputs).length > 0 + ? { [SUBRUN_METADATA_KEYS.inputs]: inputs } + : {}), // Record the child's own worktree env + branch (mirrors the container path's // isolation_env_id) so `isolation list` correlation + PR-E console grouping // can find it. Absent for `inherit`/shared-checkout children. diff --git a/packages/workflows/src/include-expander.test.ts b/packages/workflows/src/include-expander.test.ts index b5db1d1b18..85b81f96f7 100644 --- a/packages/workflows/src/include-expander.test.ts +++ b/packages/workflows/src/include-expander.test.ts @@ -982,3 +982,86 @@ describe('expandWorkflowIncludes — determinism', () => { expect(workflows.get('plain')).toBe(plain); }); }); + +// --------------------------------------------------------------------------- +// returns: + declared inputs: (#2470) +// --------------------------------------------------------------------------- + +/** Add workflow-level signature fields to a block. */ +function withSignature( + base: WorkflowDefinition, + sig: { returns?: string; inputs?: WorkflowDefinition['inputs'] } +): WorkflowDefinition { + return { ...base, ...sig }; +} + +describe('expandWorkflowIncludes — returns drives primarySink (#2470)', () => { + test('$blk.output resolves to the declared returns node (a non-sink); depends_on still waits on the sink', () => { + // Block: synthesize -> implement (implement is the sole sink; synthesize is NOT). + const block = withSignature( + wf('review-block', [ + { id: 'synthesize', prompt: 'synthesize' }, + { id: 'implement', prompt: 'implement $synthesize.output', depends_on: ['synthesize'] }, + ]), + { returns: 'synthesize' } + ); + const parent = wf('parent', [ + { id: 'blk', include: 'review-block' }, + { id: 'consume', prompt: 'result: $blk.output', depends_on: ['blk'] }, + ]); + + const { workflows, errors } = expandWorkflowIncludes(mapOf(block, parent)); + expect(errors).toHaveLength(0); + const consume = nodeById(workflows.get('parent')!, 'consume')!; + // $blk.output → the returns node (synthesize), NOT the positional first sink (implement). + expect('prompt' in consume ? consume.prompt : '').toBe('result: $blk__synthesize.output'); + // depends_on: [blk] still expands to the block's sink (implement), so the wait is intact. + expect(consume.depends_on).toContain('blk__implement'); + }); +}); + +describe('expandWorkflowIncludes — with vs declared inputs (#2470)', () => { + test('applies a declared default for an omitted input', () => { + const block = withSignature(wf('blk', [{ id: 'work', prompt: 'style: $INPUTS.style' }]), { + inputs: { style: { default: 'strict' } }, + }); + const parent = wf('parent', [{ id: 'blk', include: 'blk' }]); + const { workflows, errors } = expandWorkflowIncludes(mapOf(block, parent)); + expect(errors).toHaveLength(0); + const work = nodeById(workflows.get('parent')!, 'blk__work')!; + expect('prompt' in work ? work.prompt : '').toBe('style: strict'); + }); + + test('errors on a missing required input', () => { + const block = withSignature(wf('blk', [{ id: 'work', prompt: 'diff: $INPUTS.diff' }]), { + inputs: { diff: { required: true } }, + }); + const parent = wf('parent', [{ id: 'blk', include: 'blk' }]); + const { workflows, errors } = expandWorkflowIncludes(mapOf(block, parent)); + expect(workflows.has('parent')).toBe(false); + expect(errors.find(e => e.filename === 'parent')?.error).toContain("requires input 'diff'"); + }); + + test('errors on a caller with: key the block does not declare', () => { + const block = withSignature(wf('blk', [{ id: 'work', prompt: 'x' }]), { + inputs: { known: { default: 'v' } }, + }); + const parent = wf('parent', [{ id: 'blk', include: 'blk', with: { unknown: 'oops' } }]); + const { workflows, errors } = expandWorkflowIncludes(mapOf(block, parent)); + expect(workflows.has('parent')).toBe(false); + expect(errors.find(e => e.filename === 'parent')?.error).toContain( + "does not declare input 'unknown'" + ); + }); + + test('a block with NO declared inputs keeps Phase-1 passthrough (undeclared key accepted)', () => { + const block = wf('blk', [{ id: 'work', prompt: 'v: $INPUTS.v' }]); + const parent = wf('parent', [ + { id: 'blk', include: 'blk', with: { v: 'hello', extra: 'ignored' } }, + ]); + const { workflows, errors } = expandWorkflowIncludes(mapOf(block, parent)); + expect(errors).toHaveLength(0); + const work = nodeById(workflows.get('parent')!, 'blk__work')!; + expect('prompt' in work ? work.prompt : '').toBe('v: hello'); + }); +}); diff --git a/packages/workflows/src/include-expander.ts b/packages/workflows/src/include-expander.ts index ae3543d210..4227e2078b 100644 --- a/packages/workflows/src/include-expander.ts +++ b/packages/workflows/src/include-expander.ts @@ -12,8 +12,11 @@ * - the include node's own `depends_on`/`when`/`trigger_rule` attach to the * sub-DAG's ENTRY nodes (those with no internal upstream) * - other parent nodes that referenced the include id resolve `depends_on: [I]` - * to the sub-DAG's SINKS and `$I.output` to its PRIMARY sink (first sink in - * definition order — the same terminal-selection rule loop_group uses) + * to the sub-DAG's SINKS and `$I.output` to its PRIMARY sink. The primary sink is + * the block's declared `returns:` node when it sets one (#2470) — which may be a + * NON-sink node — otherwise the first sink in definition order. `returns:` moves + * ONLY the primary sink; `depends_on: [I]` still waits on every sink. (loop_group's + * own first-sink terminal rule is deliberately unchanged.) * * Targets are resolved recursively (a target may itself `include:` others), * depth-capped and cycle-detected. Because expansion runs BEFORE any @@ -295,12 +298,59 @@ interface ExpandedInclude { primarySink: string; } +/** + * Resolve a caller's `with:` map against a block's declared `inputs:` (#2470). + * Only active when the block declares `inputs:` — an undeclared block keeps Phase-1 + * behaviour byte-for-byte (the caller's `with:` passes through verbatim). When declared: + * applies each input's `default` for an omitted name, errors on an unsupplied `required` + * input, and errors on a caller `with:` key the block doesn't declare. + */ +function resolveIncludeInputs( + includeNode: IncludeNode, + child: WorkflowDefinition +): Record { + const callerWith = includeNode.with ?? {}; + const declared = child.inputs; + if (declared === undefined) return callerWith; + + // Reject caller keys the block does not declare. + const undeclared = Object.keys(callerWith).filter(k => !Object.hasOwn(declared, k)); + if (undeclared.length > 0) { + const names = undeclared.sort(); + throw new IncludeExpansionError( + `Node '${includeNode.id}': included block '${child.name}' does not declare input${names.length === 1 ? '' : 's'} ${names.map(n => `'${n}'`).join(', ')}. Declared inputs: ${Object.keys(declared).sort().join(', ') || '(none)'}.` + ); + } + + const resolved: Record = {}; + const missingRequired: string[] = []; + for (const [name, spec] of Object.entries(declared)) { + if (Object.hasOwn(callerWith, name)) { + resolved[name] = callerWith[name]; + } else if (spec.default !== undefined) { + resolved[name] = spec.default; + } else if (spec.required === true) { + missingRequired.push(name); + } + // Declared, not supplied, not required, no default: omitted. If the block body + // references `$INPUTS.`, applyInputsMacro reports it as missing and fails. + } + if (missingRequired.length > 0) { + const names = missingRequired.sort(); + throw new IncludeExpansionError( + `Node '${includeNode.id}': included block '${child.name}' requires input${names.length === 1 ? '' : 's'} ${names.map(n => `'${n}'`).join(', ')}. Pass ${names.length === 1 ? 'it' : 'them'} through 'with:'.` + ); + } + return resolved; +} + /** * Inline one include node's fully-expanded child into namespaced parent nodes. - * Never mutates `childNodes` (each node is deep-cloned first), so a building block + * Never mutates the child's nodes (each node is deep-cloned first), so a building block * shared by two parents is namespaced independently. */ -function inlineInclude(includeNode: IncludeNode, childNodes: DagNode[]): ExpandedInclude { +function inlineInclude(includeNode: IncludeNode, child: WorkflowDefinition): ExpandedInclude { + const childNodes = child.nodes; const prefix = `${includeNode.id}__`; const childTopLevelIds = new Set(childNodes.map(n => n.id)); const rename = (id: string): string => (childTopLevelIds.has(id) ? prefix + id : id); @@ -311,6 +361,7 @@ function inlineInclude(includeNode: IncludeNode, childNodes: DagNode[]): Expande const parentDeps = includeNode.depends_on ?? []; const missingInputs = new Set(); + const resolvedInputs = resolveIncludeInputs(includeNode, child); const namespaced = childNodes.map(cn => { const clone = structuredClone(cn); @@ -320,7 +371,7 @@ function inlineInclude(includeNode: IncludeNode, childNodes: DagNode[]): Expande // load-bearing: a caller ref such as `$gather.output` must remain parent-scoped even // when the included block also has a node named `gather`. rewriteNodeOutputRefs(clone, rename); - applyInputsMacro(clone, includeNode.with ?? {}, missingInputs); + applyInputsMacro(clone, resolvedInputs, missingInputs); clone.id = prefix + cn.id; if (wasEntry) { @@ -368,8 +419,13 @@ function inlineInclude(includeNode: IncludeNode, childNodes: DagNode[]): Expande return { namespaced, sinks: sinkOriginalIds.map(id => prefix + id), + // `$blk.output` resolves to the block's declared `returns:` node when set (#2470) — + // even a NON-sink node — otherwise the first sink in definition order. `returns:` + // was validated at load to name a top-level child node, so `prefix + returns` is a + // real namespaced id. Only `primarySink` moves; `sinks` (and thus `depends_on:[blk]`) + // still covers every terminal node. // A valid non-empty DAG always has ≥1 sink; sinkOriginalIds[0] is defined. - primarySink: prefix + (sinkOriginalIds[0] ?? ''), + primarySink: prefix + (child.returns ?? sinkOriginalIds[0] ?? ''), }; } @@ -385,6 +441,11 @@ const NON_DROPPED_WORKFLOW_KEYS: ReadonlySet = new Set([ 'description', 'nodes', 'tags', + // #2470: both are CONSUMED by inlining, not dropped — `returns` drives the block's + // primarySink and `inputs` validates the caller's `with:`. Warning "dropped" would be + // misleading. + 'returns', + 'inputs', ]); /** Isolation/concurrency-safety fields — a silent drop of these is the most dangerous. */ @@ -566,7 +627,7 @@ export function expandWorkflowIncludes( } warnDroppedWorkflowLevelFields(node, child); if (commandContents) scanBlockCommandRefs(node, child, commandContents); - const inlined = inlineInclude(node, child.nodes); + const inlined = inlineInclude(node, child); sinksByIncludeId.set(node.id, inlined.sinks); primarySinkByIncludeId.set(node.id, inlined.primarySink); newNodes.push(...inlined.namespaced); diff --git a/packages/workflows/src/loader.test.ts b/packages/workflows/src/loader.test.ts index 54fa4ad4f2..4c22f78284 100644 --- a/packages/workflows/src/loader.test.ts +++ b/packages/workflows/src/loader.test.ts @@ -3296,11 +3296,11 @@ nodes: expect(err?.error).toContain("references unknown node '$ghost.output'"); }); - it("rejects 'with:' on a workflow node (deferred to slice 2)", async () => { + it("accepts 'with:' on a workflow node (#2470)", async () => { const result = await loadOne( - 'with-reject', + 'with-accept', ` -name: with-reject +name: with-accept description: with on a workflow node nodes: - id: sub @@ -3309,9 +3309,30 @@ nodes: foo: bar ` ); - const err = result.errors.find(e => e.filename === 'with-reject.yaml'); + const err = result.errors.find(e => e.filename === 'with-accept.yaml'); + expect(err).toBeUndefined(); + const wf = result.workflows.find(w => w.workflow.name === 'with-accept'); + const node = wf?.workflow.nodes.find(n => n.id === 'sub'); + expect(node && 'with' in node ? node.with : undefined).toEqual({ foo: 'bar' }); + }); + + it("rejects 'with:' and 'input:' together on a workflow node (#2470)", async () => { + const result = await loadOne( + 'with-input-reject', + ` +name: with-input-reject +description: with and input on a workflow node +nodes: + - id: sub + workflow: child-wf + input: hello + with: + foo: bar +` + ); + const err = result.errors.find(e => e.filename === 'with-input-reject.yaml'); expect(err).toBeDefined(); - expect(err?.error).toContain("'with:'"); + expect(err?.error).toContain("'with:' and 'input:'"); }); it("rejects 'retry:' on a workflow node", async () => { @@ -3564,12 +3585,12 @@ nodes: expect(err?.error).toContain('collector'); }); - it("rejects 'fan_out.as' ($INPUTS channel staged for PR-B) instead of ignoring it", async () => { + it("accepts 'fan_out.as' now that the $INPUTS channel exists (#2470)", async () => { const result = await loadOne( 'fan-as', ` name: fan-as -description: as names an $INPUTS channel that does not exist yet +description: as names the per-item $INPUTS channel nodes: - id: plan prompt: "emit tasks" @@ -3581,13 +3602,36 @@ nodes: as: task ` ); - // Accepting it silently would deliver a literal '$INPUTS.task' to the model — the - // field reads as a working feature while doing nothing. const err = result.errors.find(e => e.filename === 'fan-as.yaml'); + expect(err).toBeUndefined(); + const wf = result.workflows.find(w => w.workflow.name === 'fan-as'); + const node = wf?.workflow.nodes.find(n => n.id === 'work'); + expect(node && 'fan_out' in node ? node.fan_out?.as : undefined).toBe('task'); + }); + + it("rejects 'fan_out.as' colliding with a 'with:' key (#2470)", async () => { + const result = await loadOne( + 'fan-as-collide', + ` +name: fan-as-collide +description: as collides with a with key +nodes: + - id: plan + prompt: "emit tasks" + - id: work + workflow: child-wf + depends_on: [plan] + with: + task: static + fan_out: + items: "$plan.output.tasks" + as: task +` + ); + const err = result.errors.find(e => e.filename === 'fan-as-collide.yaml'); expect(err).toBeDefined(); expect(err?.error).toContain('fan_out.as'); - expect(err?.error).toContain('#2214'); - expect(err?.error).toContain('$ARGUMENTS'); + expect(err?.error).toContain('collides'); }); it("rejects 'max_parallel: 0' (must be >= 1)", async () => { @@ -4894,6 +4938,109 @@ nodes: }); }); +// --------------------------------------------------------------------------- +// Workflow signature: inputs / returns (#2470) +// --------------------------------------------------------------------------- + +describe('workflow signature: inputs / returns (#2470)', () => { + it('parses declared inputs and returns', () => { + const { workflow, error } = parseWorkflow( + ` +name: sig +description: signature block +returns: build +inputs: + diff: + required: true + description: the diff to review + style: + default: strict +nodes: + - id: build + prompt: "do it with $INPUTS.diff and $INPUTS.style" +`, + 'sig.yaml' + ); + expect(error).toBeNull(); + expect(workflow?.returns).toBe('build'); + expect(workflow?.inputs?.diff?.required).toBe(true); + expect(workflow?.inputs?.style?.default).toBe('strict'); + }); + + it('rejects returns naming a non-existent top-level node', () => { + const { workflow, error } = parseWorkflow( + ` +name: bad-returns +description: returns names nothing +returns: nope +nodes: + - id: build + prompt: "hi" +`, + 'bad-returns.yaml' + ); + expect(workflow).toBeNull(); + expect(error?.error).toContain("returns: 'nope'"); + }); + + it('drops a contradictory required+default input (warn-and-drop)', () => { + const { workflow, error } = parseWorkflow( + ` +name: contradiction +description: required and default together +inputs: + x: + required: true + default: v +nodes: + - id: build + prompt: "hi" +`, + 'contradiction.yaml' + ); + expect(error).toBeNull(); + // The single contradictory key is dropped, leaving no inputs. + expect(workflow?.inputs).toBeUndefined(); + }); + + it('rejects two input names that mangle to the same env key', () => { + const { workflow, error } = parseWorkflow( + ` +name: collide +description: env-key collision +inputs: + foo-bar: + description: hyphen form + foo_bar: + description: underscore form +nodes: + - id: build + prompt: "hi" +`, + 'collide.yaml' + ); + expect(workflow).toBeNull(); + expect(error?.error).toContain('INPUTS_FOO_BAR'); + }); + + it('flags a dangling $node.output ref inside a workflow: with value', () => { + const { workflow, error } = parseWorkflow( + ` +name: with-ref +description: with value references an unknown node +nodes: + - id: sub + workflow: child-wf + with: + plan: "$nosuch.output" +`, + 'with-ref.yaml' + ); + expect(workflow).toBeNull(); + expect(error?.error).toContain('nosuch'); + }); +}); + // --------------------------------------------------------------------------- // Workflow-level field parity (#2457) // --------------------------------------------------------------------------- @@ -4975,6 +5122,12 @@ describe('workflow-level field parity (#2457)', () => { yaml: 'requires:\n - github', present: w => w.requires?.includes('github') === true, }, + inputs: { + yaml: 'inputs:\n diff:\n required: true', + present: w => w.inputs?.diff?.required === true, + }, + // `returns` must name a real top-level node id — the fixture's single node is `only`. + returns: { yaml: 'returns: only', present: w => w.returns === 'only' }, }; const schemaKeys = Object.keys(workflowDefinitionSchema.shape); diff --git a/packages/workflows/src/loader.ts b/packages/workflows/src/loader.ts index 4d9c776a31..4afb38c4ad 100644 --- a/packages/workflows/src/loader.ts +++ b/packages/workflows/src/loader.ts @@ -40,11 +40,17 @@ import { webSearchModeSchema, workflowRequirementSchema, workflowEvidencePolicySchema, + workflowInputSpecSchema, KNOWN_WORKFLOW_KEYS, KNOWN_WORKFLOW_NESTED_KEYS, WORKFLOW_ONLY_KEYS, } from './schemas/workflow'; -import type { WorkflowRequirement, WorkflowEvidencePolicy } from './schemas/workflow'; +import type { + WorkflowRequirement, + WorkflowEvidencePolicy, + WorkflowInputSpec, +} from './schemas/workflow'; +import { INPUT_NAME_PATTERN, inputEnvKey } from './schemas/dag-node'; import { workflowNodeHooksSchema } from './schemas/hooks'; import { z } from '@hono/zod-openapi'; @@ -399,11 +405,12 @@ export function validateDagStructure( // loop_group.until_bash, workflow.input, workflow.fan_out.items). A dangling ref in // any of them silently substitutes to '' at run time, so all must be validated here. // - // KEEP IN SYNC (three ref-surface enumerations must agree): + // KEEP IN SYNC (four ref-surface enumerations must agree): // 1. this scan (loader validateDagStructure) — validates refs, // 2. rewriteNodeOutputRefs (include-expander.ts) — renames refs on inline, - // 3. the substituteNodeOutputRefs call sites (dag-executor.ts) — resolves refs at run. - // Adding a substituted field to one means updating all three. + // 3. the substituteNodeOutputRefs call sites (dag-executor.ts) — resolves refs at run, + // 4. applyInputsMacro (include-expander.ts) — inlines include `with:` values (#2470). + // Adding a substituted field to one means updating all four. // // Prose fields (prompt / loop.prompt) may contain triple-backtick fenced blocks or // single-backtick inline code that are documentation meant to render literally to @@ -428,6 +435,13 @@ export function validateDagStructure( if (isWorkflowNode(node)) { if (node.input) sources.push(node.input); if (node.fan_out) sources.push(node.fan_out.items); + // A `workflow:` node's `with:` values (#2470) are live ref surfaces: unlike + // an `include:` node's `with:` (inlined by the macro and caught post-expansion + // by this same scan), sub-run `with:` values are never inlined — they resolve + // at runtime into `$INPUTS.` — so scan them here for dangling refs. + if (node.with) { + for (const value of Object.values(node.with)) sources.push(value); + } } if (isCancelNode(node)) sources.push(node.cancel); if (isApprovalNode(node)) sources.push(node.approval.message); @@ -893,6 +907,90 @@ export function parseWorkflow(content: string, filename: string): ParseResult { getLog().warn({ filename, value: raw.requires }, 'invalid_workflow_requires_block_ignored'); } + // Parse optional inputs — the declared signature (#2470). Per-key warn-and-drop: + // an invalid spec, a non-identifier name (env mangling needs identifier names), or + // a contradictory `required: true` + `default:` pair is dropped with a warning. The + // surviving record is set only when non-empty. Absent/invalid block leaves `inputs` + // undefined — a workflow with no declared inputs keeps Phase-1 behaviour untouched. + let inputs: Record | undefined; + const rawInputs = raw.inputs; + if (rawInputs !== undefined) { + if ( + typeof rawInputs === 'object' && + rawInputs !== null && + !Array.isArray(rawInputs) && + (Object.getPrototypeOf(rawInputs) === Object.prototype || + Object.getPrototypeOf(rawInputs) === null) + ) { + const valid: Record = {}; + for (const [name, spec] of Object.entries(rawInputs as Record)) { + if (!INPUT_NAME_PATTERN.test(name)) { + getLog().warn({ filename, name }, 'invalid_workflow_input_name_ignored'); + continue; + } + const parsed = workflowInputSpecSchema.safeParse(spec); + if (!parsed.success) { + getLog().warn({ filename, name, value: spec }, 'invalid_workflow_input_spec_ignored'); + continue; + } + if (parsed.data.required === true && parsed.data.default !== undefined) { + getLog().warn( + { filename, name }, + 'contradictory_workflow_input_required_with_default_ignored' + ); + continue; + } + valid[name] = parsed.data; + } + // Reject env-key mangling collisions: two names that fold to the same + // INPUTS_ env key would silently clobber each other for + // bash/script sub-run nodes (Task 17). Catch it here where all names are + // visible; a colliding pair is a hard load error, not a warn-and-drop. + const envKeyOwners = new Map(); + for (const name of Object.keys(valid)) { + const envKey = inputEnvKey(name); + const existing = envKeyOwners.get(envKey); + if (existing !== undefined) { + return { + workflow: null, + error: { + filename, + error: `Workflow inputs '${existing}' and '${name}' both map to env var '${envKey}' — rename one so each input has a unique env key`, + errorType: 'validation_error', + }, + }; + } + envKeyOwners.set(envKey, name); + } + if (Object.keys(valid).length > 0) inputs = valid; + } else { + getLog().warn({ filename, value: rawInputs }, 'invalid_workflow_inputs_block_ignored'); + } + } + + // Parse optional returns — the node id whose output IS this workflow's result + // (#2470). Accept a non-empty string; warn-and-drop otherwise. The referenced id + // must name a top-level node — checked below once dagNodes is assembled. + let returns: string | undefined; + if (typeof raw.returns === 'string' && raw.returns.trim().length > 0) { + returns = raw.returns.trim(); + } else if (raw.returns !== undefined) { + getLog().warn({ filename, value: raw.returns }, 'invalid_workflow_returns_value_ignored'); + } + // `returns` must name a top-level node id. Done here (not in validateDagStructure, + // which takes nodes and is reused for loop_group bodies / the expander with no + // `returns` in scope) now that dagNodes is computed. + if (returns !== undefined && !dagNodes.some(n => n.id === returns)) { + return { + workflow: null, + error: { + filename, + error: `Workflow declares returns: '${returns}' but no top-level node has that id`, + errorType: 'validation_error', + }, + }; + } + // Parse workflow-level fallback fields. Same warn-and-drop pattern as // `modelReasoningEffort` / `webSearchMode` above. These are declared on // `workflowBaseSchema` and consumed by the DAG executor's @@ -1010,6 +1108,8 @@ export function parseWorkflow(content: string, filename: string): ParseResult { ...(evidencePolicy !== undefined ? { evidence_policy: evidencePolicy } : {}), ...(tags !== undefined ? { tags } : {}), ...(requires !== undefined ? { requires } : {}), + ...(inputs !== undefined ? { inputs } : {}), + ...(returns !== undefined ? { returns } : {}), }, error: null, warnings: parseWarnings, diff --git a/packages/workflows/src/schemas.test.ts b/packages/workflows/src/schemas.test.ts index 4570c8e57f..1a2cf5b93b 100644 --- a/packages/workflows/src/schemas.test.ts +++ b/packages/workflows/src/schemas.test.ts @@ -15,6 +15,8 @@ import { BASH_NODE_AI_FIELDS, approvalOnRejectSchema, dagNodeSchema, + inputEnvKey, + readSubrunMetadata, } from './schemas'; import type { WorkflowDefinition, @@ -1053,3 +1055,27 @@ describe('INCLUDE_NODE_IGNORED_FIELDS', () => { } }); }); + +describe('inputEnvKey (#2470)', () => { + test('mangles an input name to INPUTS_', () => { + expect(inputEnvKey('plan')).toBe('INPUTS_PLAN'); + expect(inputEnvKey('base-branch')).toBe('INPUTS_BASE_BRANCH'); + expect(inputEnvKey('foo_bar')).toBe('INPUTS_FOO_BAR'); + // hyphen and underscore fold to the same key — the loader rejects such a pair. + expect(inputEnvKey('foo-bar')).toBe(inputEnvKey('foo_bar')); + }); +}); + +describe('readSubrunMetadata — inputs (#2470)', () => { + test('reads a well-formed inputs map', () => { + const md = readSubrunMetadata({ inputs: { plan: 'do it', mode: 'fast' } }); + expect(md.inputs).toEqual({ plan: 'do it', mode: 'fast' }); + }); + + test('treats a non-string-valued or non-object inputs as unset', () => { + expect(readSubrunMetadata({ inputs: { plan: 5 } }).inputs).toBeUndefined(); + expect(readSubrunMetadata({ inputs: ['a'] }).inputs).toBeUndefined(); + expect(readSubrunMetadata({}).inputs).toBeUndefined(); + expect(readSubrunMetadata(undefined).inputs).toBeUndefined(); + }); +}); diff --git a/packages/workflows/src/schemas/dag-node.ts b/packages/workflows/src/schemas/dag-node.ts index ced1528851..02dd9ca9a7 100644 --- a/packages/workflows/src/schemas/dag-node.ts +++ b/packages/workflows/src/schemas/dag-node.ts @@ -459,7 +459,19 @@ export type CancelNode = z.infer & { * grammar elsewhere in the tree encodes a different concept and stays separate. */ export const INPUT_NAME_SOURCE = String.raw`[a-zA-Z_][a-zA-Z0-9_-]*`; -const INPUT_NAME_PATTERN = new RegExp(`^${INPUT_NAME_SOURCE}$`); +export const INPUT_NAME_PATTERN = new RegExp(`^${INPUT_NAME_SOURCE}$`); + +/** + * Env-var key an input name is delivered under to bash/script sub-run nodes (#2470): + * `INPUTS_` + UPPER_SNAKE(name) (hyphens → underscores, uppercased). Because `-` and + * `_` both fold to `_`, two distinct names (`foo-bar`, `foo_bar`) can collide on one + * env key — the loader rejects that at load time (all names for one workflow are + * visible there). Bash/script bodies read `$INPUTS_`; `$INPUTS.` + * text is only substituted into non-shell (AI/prompt) surfaces. + */ +export function inputEnvKey(name: string): string { + return `INPUTS_${name.replace(/-/g, '_').toUpperCase()}`; +} /** * Include node schema — a load-time directive that inlines another workflow's @@ -518,11 +530,10 @@ export type IncludeNode = z.infer & { * forbids, and racing cannot be reshaped without it. The enum value is retained only * so existing YAML gets a message explaining the rejection. * - * `as` is a forward seam reserved for PR-B (#2214, `with:`/`$INPUTS`): it will name the - * per-item value as `$INPUTS.` inside the child. The key is accepted here so PR-B - * needs no schema migration, but until PR-B lands the superRefine REJECTS it at load — - * it has no runtime effect, and silently ignoring it would deliver a literal - * `$INPUTS.` to the model. The item travels as the child's `$ARGUMENTS` today. + * `as` names the per-item value as `$INPUTS.` inside each child (#2470 lifted the + * PR-B/#2214 placeholder now that #2224 merged). It is generally accepted; the superRefine + * rejects it only when it collides with a `with:` key of the same name (both would populate + * the same `$INPUTS.` slot). When `as` is unset the item still travels as `$ARGUMENTS`. */ export const fanOutConfigSchema = z.object({ items: z @@ -556,6 +567,9 @@ export type FanOutConfig = z.infer; export const workflowNodeSchema = dagNodeBaseSchema.extend({ workflow: z.string().min(1, "'workflow' must be a non-empty workflow name"), input: z.string().optional(), + // Named inputs passed to the child sub-run as `$INPUTS.` (#2470). Mutually + // exclusive with `input:`. Same identifier-keyed string-map shape as `include.with`. + with: z.record(z.string(), z.string()).optional(), isolation: z.enum(['inherit', 'worktree']).optional(), fan_out: fanOutConfigSchema.optional(), }); @@ -803,9 +817,11 @@ export const dagNodeSchema = dagNodeFlatSchema return z.NEVER; } - // `include.with` is a load-time, identifier-keyed string map. Keep the flat - // field raw so other node variants can still strip it contextually. - if (hasInclude && data.with !== undefined) { + // `with:` is an identifier-keyed string map on BOTH include and workflow nodes + // (#2470). For includes it inlines at load time (applyInputsMacro); for sub-runs it + // becomes `$INPUTS.` runtime variables on the child. Same shape validation for + // both; the flat field stays `z.unknown()` so other node variants strip it contextually. + const validateWithShape = (kind: 'include' | 'workflow'): void => { const prototype = typeof data.with === 'object' && data.with !== null ? Object.getPrototypeOf(data.with) @@ -818,33 +834,39 @@ export const dagNodeSchema = dagNodeFlatSchema ) { ctx.addIssue({ code: z.ZodIssueCode.custom, - message: "'with' on include nodes must be an object mapping input names to strings", + message: `'with' on ${kind} nodes must be an object mapping input names to strings`, path: ['with'], }); - } else { - for (const [key, value] of Object.entries(data.with)) { - if (!INPUT_NAME_PATTERN.test(key)) { - ctx.addIssue({ - code: z.ZodIssueCode.custom, - message: `invalid include input name '${key}'; use letters, numbers, underscores, or hyphens and start with a letter or underscore`, - path: ['with'], - }); - } - if (typeof value !== 'string') { - ctx.addIssue({ - code: z.ZodIssueCode.custom, - message: `include input '${key}' must be a string`, - path: ['with'], - }); - } + return; + } + for (const [key, value] of Object.entries(data.with)) { + if (!INPUT_NAME_PATTERN.test(key)) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: `invalid ${kind} input name '${key}'; use letters, numbers, underscores, or hyphens and start with a letter or underscore`, + path: ['with'], + }); + } + if (typeof value !== 'string') { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: `${kind} input '${key}' must be a string`, + path: ['with'], + }); } } - } - if (hasWorkflow && data.with !== undefined) { + }; + if (hasInclude && data.with !== undefined) validateWithShape('include'); + if (hasWorkflow && data.with !== undefined) validateWithShape('workflow'); + // A `workflow:` node has ONE input channel per invocation: either the untyped + // `input:` string ($ARGUMENTS) or the named `with:` map ($INPUTS.). Accepting + // both would require a precedence rule between two overlapping channels — the exact + // ambiguity the constitution's smell #5 warns against — so reject them together. + if (hasWorkflow && data.with !== undefined && data.input !== undefined) { ctx.addIssue({ code: z.ZodIssueCode.custom, message: - "'with:' named-parameter mapping is not yet supported on workflow nodes (slice 2). Use 'input:' instead.", + "'with:' and 'input:' cannot both be set on a workflow node — 'with:' supplies named $INPUTS, 'input:' supplies the child's $ARGUMENTS. Use one.", path: ['with'], }); } @@ -901,18 +923,22 @@ export const dagNodeSchema = dagNodeFlatSchema path: ['fan_out', 'join'], }); } - // `as` names the per-item value for the `$INPUTS.` channel that PR-B (#2214) will - // add. Accept the key in the schema (so PR-B lifts a guard rather than migrating YAML) - // but reject it fail-fast now, exactly as `first_success` above. `$INPUTS` exists - // nowhere in the engine today, so an author writing `as: task` and `$INPUTS.task` in - // the child gets the literal string delivered to the model — silently wrong output - // with no error. A field that quietly does nothing reads as a working feature. - if (hasWorkflow && data.fan_out?.as !== undefined) { + // `fan_out.as` names the per-item value as `$INPUTS.` inside each child (#2470 + // lifts the PR-B/#2214 placeholder — #2224 merged). It is now generally accepted; the + // only rejection is a COLLISION with a `with:` key, since both would populate the same + // `$INPUTS.` slot on the child with a precedence rule between them — the same + // two-channels-one-name ambiguity the `with:`+`input:` guard rejects above. + if ( + hasWorkflow && + data.fan_out?.as !== undefined && + typeof data.with === 'object' && + data.with !== null && + !Array.isArray(data.with) && + Object.prototype.hasOwnProperty.call(data.with, data.fan_out.as) + ) { ctx.addIssue({ code: z.ZodIssueCode.custom, - message: - "'fan_out.as' (the $INPUTS channel) is not yet supported (PR-B, #2214). Remove it — " + - "each item is delivered to the child as $ARGUMENTS, which the child's prompts can use today.", + message: `'fan_out.as: ${data.fan_out.as}' collides with a 'with:' key of the same name — both would populate $INPUTS.${data.fan_out.as}. Rename one.`, path: ['fan_out', 'as'], }); } @@ -1131,6 +1157,10 @@ export const dagNodeSchema = dagNodeFlatSchema ...(data.output_format !== undefined ? { output_format: data.output_format } : {}), workflow: data.workflow.trim(), ...(data.input !== undefined ? { input: data.input } : {}), + // `with:` supplies named $INPUTS to the child sub-run (#2470), validated in shape + // by the superRefine above and mutually exclusive with `input:`. Mirrors the + // include transform's `with` assembly. + ...(data.with !== undefined ? { with: data.with as Record } : {}), // Isolation is EXPLICIT-ONLY — never inferred, including from `fan_out`. How many // children a node spawns says nothing about whether they write; N review or // research children over the shared checkout is the common case. A shared-checkout diff --git a/packages/workflows/src/schemas/index.ts b/packages/workflows/src/schemas/index.ts index 9efd4bbaf7..60d97599e5 100644 --- a/packages/workflows/src/schemas/index.ts +++ b/packages/workflows/src/schemas/index.ts @@ -45,6 +45,7 @@ export { fanOutConfigSchema, dagNodeSchema, INPUT_NAME_SOURCE, + inputEnvKey, isCommandNode, isBashNode, isLoopNode, @@ -103,6 +104,7 @@ export { webSearchModeSchema, workflowRequirementSchema, workflowEvidencePolicySchema, + workflowInputSpecSchema, workflowBaseSchema, workflowDefinitionSchema, KNOWN_WORKFLOW_KEYS, @@ -114,6 +116,7 @@ export type { WebSearchMode, WorkflowRequirement, WorkflowEvidencePolicy, + WorkflowInputSpec, WorkflowBase, WorkflowDefinition, } from './workflow'; diff --git a/packages/workflows/src/schemas/workflow-run.ts b/packages/workflows/src/schemas/workflow-run.ts index ca16400375..17985dd489 100644 --- a/packages/workflows/src/schemas/workflow-run.ts +++ b/packages/workflows/src/schemas/workflow-run.ts @@ -156,11 +156,15 @@ export type WorkflowRun = z.infer; * child, which is what distinguishes the two on re-entry. * `fan_out_item_hash` — hash of the item the child was spawned with, so a resume can warn * when a non-deterministic producer changed it under the same index. + * `inputs` — the resolved `with:` map (name → string) the parent supplied (#2470), + * persisted at spawn so the child's `$INPUTS.` reconstitutes on a + * cold resume without re-resolving parent refs that may be out of scope. */ export const SUBRUN_METADATA_KEYS = { parentNodeId: 'parent_node_id', childIndex: 'child_index', fanOutItemHash: 'fan_out_item_hash', + inputs: 'inputs', } as const; /** Typed view of the sub-run keys on a run's metadata; each is undefined when unset. */ @@ -168,14 +172,29 @@ export function readSubrunMetadata(metadata: Record | undefined parentNodeId: string | undefined; childIndex: number | undefined; fanOutItemHash: string | undefined; + inputs: Record | undefined; } { const parentNodeId = metadata?.[SUBRUN_METADATA_KEYS.parentNodeId]; const childIndex = metadata?.[SUBRUN_METADATA_KEYS.childIndex]; const fanOutItemHash = metadata?.[SUBRUN_METADATA_KEYS.fanOutItemHash]; + const rawInputs = metadata?.[SUBRUN_METADATA_KEYS.inputs]; + // Accept only a plain object of string values; anything else reads as unset. The + // writer always stores a Record, so a non-conforming value is + // corrupt/foreign metadata, not a shape this reader should try to coerce. + let inputs: Record | undefined; + if ( + typeof rawInputs === 'object' && + rawInputs !== null && + !Array.isArray(rawInputs) && + Object.values(rawInputs as Record).every(v => typeof v === 'string') + ) { + inputs = rawInputs as Record; + } return { parentNodeId: typeof parentNodeId === 'string' ? parentNodeId : undefined, childIndex: typeof childIndex === 'number' ? childIndex : undefined, fanOutItemHash: typeof fanOutItemHash === 'string' ? fanOutItemHash : undefined, + inputs, }; } diff --git a/packages/workflows/src/schemas/workflow.ts b/packages/workflows/src/schemas/workflow.ts index 688ee6eeba..fb0f773660 100644 --- a/packages/workflows/src/schemas/workflow.ts +++ b/packages/workflows/src/schemas/workflow.ts @@ -114,6 +114,29 @@ export const workflowEvidencePolicySchema = z.object({ export type WorkflowEvidencePolicy = z.infer; +// --------------------------------------------------------------------------- +// Workflow signature — declared inputs (#2470, Signature Phase 2) +// --------------------------------------------------------------------------- + +/** + * Declaration of a single input a workflow accepts. All fields optional: + * - `required` — a caller MUST supply this via `with:`; a bare top-level run + * of a workflow with an unsatisfied required input fails before any cost. + * - `default` — value used when a caller omits the input (mutually exclusive + * with `required: true`; the loader drops any key that declares both). + * - `description` — human documentation only, unused by the engine. + * + * This is declarative metadata the engine needs to wire and validate `with:` + * against — it coordinates, it does not compute (Workflow Language Constitution). + */ +export const workflowInputSpecSchema = z.object({ + required: z.boolean().optional(), + default: z.string().optional(), + description: z.string().optional(), +}); + +export type WorkflowInputSpec = z.infer; + // --------------------------------------------------------------------------- // WorkflowBase — common fields shared by all workflow types // --------------------------------------------------------------------------- @@ -155,6 +178,23 @@ export const workflowBaseSchema = z.object({ * when per-user GitHub is enabled; a no-op for solo PAT installs. */ requires: z.array(workflowRequirementSchema).optional(), + /** + * Declared inputs this workflow accepts (#2470). A caller supplies values via + * `with:` on the `include:`/`workflow:` node that references this workflow; + * for sub-runs the values become `$INPUTS.` runtime variables on the + * child. When a workflow declares `inputs:`, callers are validated against it + * (missing required / undeclared key = load error); a workflow with no + * `inputs:` keeps Phase-1 behaviour untouched. + */ + inputs: z.record(z.string(), workflowInputSpecSchema).optional(), + /** + * The node id whose output IS this workflow's result (#2470). Drives + * `$blk.output` for include blocks (the block's `primarySink`, overriding the + * positional first-sink default) and the terminal output of a `workflow:` + * sub-run child. Selecting by id (not text) works for every node type, + * including a non-sink node. + */ + returns: z.string().min(1).optional(), }); export type WorkflowBase = z.infer; @@ -224,6 +264,16 @@ export const KNOWN_WORKFLOW_NESTED_KEYS: ReadonlyMap = ne 'evidence_policy', { kind: 'object', keys: new Set(Object.keys(workflowEvidencePolicySchema.shape)) }, ], + // First `record` entry in this map: `inputs` is a record of input-name → spec, + // so unknown keys under an individual spec (e.g. `inputs.diff.typo`) warn. + // `returns` is a plain string and needs no nested registration. + [ + 'inputs', + { + kind: 'record', + entry: { kind: 'object', keys: new Set(Object.keys(workflowInputSpecSchema.shape)) }, + }, + ], ]); // --------------------------------------------------------------------------- diff --git a/packages/workflows/src/utils/workflow-requirements.test.ts b/packages/workflows/src/utils/workflow-requirements.test.ts index 0908cf16e9..64f2721b69 100644 --- a/packages/workflows/src/utils/workflow-requirements.test.ts +++ b/packages/workflows/src/utils/workflow-requirements.test.ts @@ -1,5 +1,10 @@ import { describe, test, expect } from 'bun:test'; -import { assertWorkflowRequirementsMet, WorkflowRequirementError } from './workflow-requirements'; +import { + assertWorkflowRequirementsMet, + WorkflowRequirementError, + assertWorkflowInputsSatisfiable, + WorkflowMissingInputsError, +} from './workflow-requirements'; describe('assertWorkflowRequirementsMet', () => { test('passes when there are no requirements', () => { @@ -28,3 +33,34 @@ describe('assertWorkflowRequirementsMet', () => { expect((thrown as WorkflowRequirementError).message).toContain('connect github'); }); }); + +describe('assertWorkflowInputsSatisfiable (#2470)', () => { + test('passes with no declared inputs', () => { + expect(() => assertWorkflowInputsSatisfiable({})).not.toThrow(); + expect(() => assertWorkflowInputsSatisfiable({ inputs: {} })).not.toThrow(); + }); + + test('passes when all declared inputs are optional or defaulted', () => { + expect(() => + assertWorkflowInputsSatisfiable({ + inputs: { a: { default: 'x' }, b: { description: 'optional' } }, + }) + ).not.toThrow(); + }); + + test('throws naming missing required inputs on a bare top-level run', () => { + let thrown: unknown; + try { + assertWorkflowInputsSatisfiable({ + name: 'block', + inputs: { diff: { required: true }, plan: { required: true }, style: { default: 's' } }, + }); + } catch (err) { + thrown = err; + } + expect(thrown).toBeInstanceOf(WorkflowMissingInputsError); + expect((thrown as WorkflowMissingInputsError).missing).toEqual(['diff', 'plan']); + expect((thrown as WorkflowMissingInputsError).message).toContain("'diff', 'plan'"); + expect((thrown as WorkflowMissingInputsError).message).toContain('with:'); + }); +}); diff --git a/packages/workflows/src/utils/workflow-requirements.ts b/packages/workflows/src/utils/workflow-requirements.ts index 73d77eb5d7..e72f668e5d 100644 --- a/packages/workflows/src/utils/workflow-requirements.ts +++ b/packages/workflows/src/utils/workflow-requirements.ts @@ -9,7 +9,7 @@ * check) and pass it in. The orchestrator/CLI/web entrypoints own the I/O; this * just encodes the policy so all three behave identically. */ -import type { WorkflowRequirement } from '../schemas/workflow'; +import type { WorkflowRequirement, WorkflowInputSpec } from '../schemas/workflow'; /** Minimal shape needed to evaluate requirements — avoids a full WorkflowDefinition dep. */ export interface RequirementBearingWorkflow { @@ -51,3 +51,54 @@ export function assertWorkflowRequirementsMet( throw new WorkflowRequirementError('github'); } } + +// --------------------------------------------------------------------------- +// Declared-input satisfiability (#2470) +// --------------------------------------------------------------------------- + +/** Minimal shape needed to evaluate declared inputs — avoids a full WorkflowDefinition dep. */ +export interface InputBearingWorkflow { + name?: string; + inputs?: Record; +} + +/** + * Thrown when a workflow that declares `required` inputs is invoked at the TOP LEVEL, + * where no caller `with:` can satisfy them. `message` is user-facing and names the + * missing inputs plus the two ways to supply them (`include:`/`workflow:` with `with:`). + */ +export class WorkflowMissingInputsError extends Error { + constructor( + public readonly workflowName: string | undefined, + public readonly missing: readonly string[] + ) { + const names = missing.map(n => `'${n}'`).join(', '); + super( + `This workflow declares required input${missing.length === 1 ? '' : 's'} ${names} that only a ` + + 'caller can supply. It is a reusable block: reference it from another workflow with an ' + + '`include:` or `workflow:` node and pass the input(s) via `with:` (e.g. `with: { ' + + `${missing[0]}: $someNode.output }\`). No worktree was created and no AI cost was incurred.` + ); + this.name = 'WorkflowMissingInputsError'; + } +} + +/** + * Throw {@link WorkflowMissingInputsError} when a TOP-LEVEL invocation cannot satisfy the + * workflow's declared `required` inputs (#2470). A `required` input never carries a default + * (the loader drops that contradiction), so a bare run can never satisfy one — the block is + * meant to be called via `include:`/`workflow:` `with:`. The workflow still LOADS and LISTS + * normally (discovery/builder need it visible); only top-level invocation fails, before any + * worktree/clone/AI cost. A workflow with no declared inputs always passes. + */ +export function assertWorkflowInputsSatisfiable(workflow: InputBearingWorkflow): void { + const inputs = workflow.inputs; + if (!inputs) return; + const missing = Object.entries(inputs) + .filter(([, spec]) => spec.required === true) + .map(([name]) => name) + .sort(); + if (missing.length > 0) { + throw new WorkflowMissingInputsError(workflow.name, missing); + } +} diff --git a/packages/workflows/src/validator.test.ts b/packages/workflows/src/validator.test.ts index f6487bd906..2908db480f 100644 --- a/packages/workflows/src/validator.test.ts +++ b/packages/workflows/src/validator.test.ts @@ -178,6 +178,34 @@ describe('validateWorkflowResources — command nodes', () => { }); }); +// ============================================================================= +// validateWorkflowResources — bundled sub-run target check (#2470) +// ============================================================================= + +describe('validateWorkflowResources — bundled workflow: target check', () => { + test('bundled workflow with a workflow: node to a non-existent bundled name fails', async () => { + const workflow = makeWorkflow('test', [ + { id: 'sub', workflow: 'definitely-not-a-bundled-workflow' } as DagNode, + ]); + const issues = await validateWorkflowResources(workflow, tmpDir, { + workflowSource: 'bundled', + }); + expect( + issues.some(i => i.field === 'workflow' && i.message.includes('not a bundled workflow')) + ).toBe(true); + }); + + test('project workflow with a workflow: node to a non-existent name is NOT checked (runtime-resolved)', async () => { + const workflow = makeWorkflow('test', [ + { id: 'sub', workflow: 'definitely-not-a-bundled-workflow' } as DagNode, + ]); + const issues = await validateWorkflowResources(workflow, tmpDir, { + workflowSource: 'project', + }); + expect(issues.some(i => i.field === 'workflow')).toBe(false); + }); +}); + // ============================================================================= // validateWorkflowResources — portable model refs // ============================================================================= diff --git a/packages/workflows/src/validator.ts b/packages/workflows/src/validator.ts index 56ef183bc4..b5999e2cf0 100644 --- a/packages/workflows/src/validator.ts +++ b/packages/workflows/src/validator.ts @@ -19,7 +19,7 @@ import { findMarkdownFilesRecursive, } from '@archon/paths'; import { execFileAsync } from '@archon/git'; -import { BUNDLED_COMMANDS, isBinaryBuild } from './defaults/bundled-defaults'; +import { BUNDLED_COMMANDS, BUNDLED_WORKFLOWS, isBinaryBuild } from './defaults/bundled-defaults'; import { isValidCommandName } from './command-validation'; import { levenshtein, findSimilar } from './utils/fuzzy-match'; import { getProviderCapabilities, isRegisteredProvider, skillSearchRoots } from '@archon/providers'; @@ -30,7 +30,16 @@ function getLog(): ReturnType { if (!cachedLog) cachedLog = createLogger('workflow.validator'); return cachedLog; } -import { isBashNode, isLoopNode, isLoopGroupNode, isScriptNode, isIncludeNode } from './schemas'; +import { + isBashNode, + isLoopNode, + isLoopGroupNode, + isScriptNode, + isIncludeNode, + isWorkflowNode, +} from './schemas'; +import { parseWorkflow } from './loader'; +import { resolveWorkflowName } from './router'; import type { WorkflowDefinition, DagNode, WorkflowSource } from './schemas'; import type { ScriptRuntime } from './script-discovery'; import { discoverScriptsForCwd } from './script-discovery'; @@ -282,6 +291,26 @@ function resolveProvider( return workflowProvider ?? defaultProvider; } +/** + * Bundled workflow definitions, parsed once and cached (#2470). Used only by the + * bundled-set-only `workflow:` target check below — a bundled workflow's sub-run target + * must itself resolve within the bundled set (a bundled workflow can't depend on a + * project/global workflow that may not exist on another install). Resolution reuses the + * runtime fuzzy `resolveWorkflowName` so a legal suffix/substring ref isn't reported broken. + * parseWorkflow never expands includes, so `workflow.name` is the authoritative id here. + */ +let bundledWorkflowDefsCache: WorkflowDefinition[] | undefined; +function getBundledWorkflowDefs(): WorkflowDefinition[] { + if (bundledWorkflowDefsCache) return bundledWorkflowDefsCache; + const defs: WorkflowDefinition[] = []; + for (const [filename, content] of Object.entries(BUNDLED_WORKFLOWS)) { + const { workflow } = parseWorkflow(content, filename); + if (workflow) defs.push(workflow); + } + bundledWorkflowDefsCache = defs; + return defs; +} + /** * Validate a workflow's external resource references (Level 3). * @@ -375,6 +404,40 @@ export async function validateWorkflowResources( } if ('model' in node && node.model) validateModelRef(node.model, node.id); + // --- Bundled `workflow:` sub-run target check (#2470) --- + // A BUNDLED workflow ships with the binary and runs on any install, so its sub-run + // targets must resolve within the bundled set — a reference to a project/global + // workflow could be absent elsewhere. Only the bundled set is checked at load/CI; + // project sub-run targets stay RUNTIME-resolved on purpose (a load-time existence + // check would silently kill mid-flight authoring — constitution case-law), and the + // check uses the SAME fuzzy resolver as runtime so a legal suffix ref isn't flagged. + if (config?.workflowSource === 'bundled' && isWorkflowNode(node)) { + let resolvedTarget: WorkflowDefinition | undefined; + let ambiguityMessage: string | undefined; + try { + resolvedTarget = resolveWorkflowName(node.workflow, getBundledWorkflowDefs()); + } catch (err) { + ambiguityMessage = (err as Error).message; + } + if (ambiguityMessage !== undefined) { + issues.push({ + level: 'error', + nodeId: node.id, + field: 'workflow', + message: `Node '${node.id}' sub-run target '${node.workflow}' is ambiguous within the bundled set: ${ambiguityMessage}`, + hint: 'Use the full bundled workflow name.', + }); + } else if (!resolvedTarget) { + issues.push({ + level: 'error', + nodeId: node.id, + field: 'workflow', + message: `Node '${node.id}' targets sub-run '${node.workflow}', which is not a bundled workflow`, + hint: 'A bundled workflow may only reference other bundled workflows (project/global targets are not guaranteed to exist on every install).', + }); + } + } + // --- Command nodes: check file exists --- if ('command' in node && typeof node.command === 'string') { if (!isValidCommandName(node.command)) { From ba329f6c7f76a3174d6e5ce53901b2f473302337 Mon Sep 17 00:00:00 2001 From: Archon Date: Mon, 10 Aug 2026 20:22:49 +0000 Subject: [PATCH 2/4] simplify: reduce complexity in changed files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hoist resolveRunInputs(parentRun) out of the per-entry with: resolution loops in executeWorkflowNode and executeFanOutWorkflowNode — it re-parsed run metadata on every iteration. Computed once into a named local now. Co-Authored-By: Claude Opus 4.8 --- .../defaults/archon-hyperframes-video.yaml | 226 ++++++++++++++++++ packages/workflows/src/dag-executor.ts | 15 +- 2 files changed, 238 insertions(+), 3 deletions(-) create mode 100644 .archon/workflows/defaults/archon-hyperframes-video.yaml diff --git a/.archon/workflows/defaults/archon-hyperframes-video.yaml b/.archon/workflows/defaults/archon-hyperframes-video.yaml new file mode 100644 index 0000000000..7b64258234 --- /dev/null +++ b/.archon/workflows/defaults/archon-hyperframes-video.yaml @@ -0,0 +1,226 @@ +name: archon-hyperframes-video +description: | + Use when: User wants to generate or modify a HyperFrames video using HeyGen's AI pipeline. + Triggers: "create hyperframe video", "hyperframes video", "website to video", + "heygen video", "hyperframes", "website video". + Does: Installs HyperFrames skill if needed -> captures website assets -> runs AI pipeline + (Design, Script, Storyboard, VO+Timing, Build, Validate) -> previews output -> + renders final MP4 video -> summarizes the result. + Requires: Node.js / npx available in PATH. + Arguments: $1=URL (required), $2=creative direction (e.g. "bold cinematic dark theme"), + $3=video type (social-ad|product-launch|product-tour|brand-reel|feature-announcement|teaser), + $4=output filename without extension (default: hyperframes-output), + $5=project directory (optional, defaults to current directory). + Optional: Set GEMINI_API_KEY env var for enhanced AI vision analysis (~$0.04/capture). + +nodes: + # ── Layer 0: Validate args and set up working directory ────────────────── + - id: setup + bash: | + URL="${1:-}" + CREATIVE="${2:-bold, cinematic}" + VIDEO_TYPE="${3:-product-launch}" + OUTPUT_NAME="${4:-hyperframes-output}" + PROJECT_DIR="${5:-$(pwd)}" + + if [ -z "$URL" ]; then + echo "ERROR: A URL is required as the first argument." + echo "Usage: archon-hyperframes-video [creative-direction] [video-type] [output-name] [project-dir]" + exit 1 + fi + + # Validate URL format + if ! echo "$URL" | grep -qE '^https?://'; then + echo "ERROR: URL must start with http:// or https://" + exit 1 + fi + + echo "URL=$URL" + echo "CREATIVE=$CREATIVE" + echo "VIDEO_TYPE=$VIDEO_TYPE" + echo "OUTPUT_NAME=$OUTPUT_NAME" + echo "PROJECT_DIR=$PROJECT_DIR" + mkdir -p "$PROJECT_DIR" + echo "SETUP_OK" + timeout: 10000 + + # ── Layer 1: Check/install HyperFrames skill ────────────────────────────── + - id: install-skill + bash: | + if npx hyperframes --version >/dev/null 2>&1; then + echo "HyperFrames already installed: $(npx hyperframes --version 2>/dev/null || echo 'version unknown')" + echo "SKILL_READY" + else + echo "Installing HyperFrames skill..." + npx skills add heygen-com/hyperframes + RESULT=$? + if [ $RESULT -eq 0 ]; then + echo "SKILL_INSTALLED" + else + echo "ERROR: Failed to install HyperFrames skill. Check your network connection." + exit 1 + fi + fi + depends_on: [setup] + timeout: 120000 + + # ── Layer 2: Capture website assets ─────────────────────────────────────── + - id: capture + bash: | + # Extract URL from setup output + URL=$(echo "$setup_output" | grep '^URL=' | cut -d= -f2-) + PROJECT_DIR=$(echo "$setup_output" | grep '^PROJECT_DIR=' | cut -d= -f2-) + + cd "$PROJECT_DIR" + + echo "Capturing assets from: $URL" + echo "This may take up to 2 minutes for complex sites..." + + if [ -n "$GEMINI_API_KEY" ]; then + echo "Gemini vision analysis enabled." + fi + + npx hyperframes capture "$URL" --timeout 120000 + RESULT=$? + + if [ $RESULT -eq 0 ]; then + echo "" + echo "CAPTURE_SUCCESS" + ls -la 2>/dev/null | head -20 + else + echo "CAPTURE_FAILED: Could not capture website. Check if the URL is accessible." + exit 1 + fi + depends_on: [install-skill] + timeout: 180000 + + # ── Layer 3: Run AI pipeline (Design → Script → Storyboard → VO → Build → Validate) ── + - id: generate + prompt: | + You are working with the HyperFrames skill to create a video from a website. + + Setup details: + $setup.output + + Capture result: + $capture.output + + Now run the complete HyperFrames AI pipeline using the captured website data. + Use the following creative direction and parameters: + + Creative direction: Extract the creative direction from the setup output (CREATIVE= line). + Video type: Extract the video type from the setup output (VIDEO_TYPE= line). + + Run the full 7-step pipeline: + 1. Design — generate brand reference documentation from the captured assets + 2. Script — create narration with hook, story, proof, and call-to-action structure + 3. Storyboard — define per-beat creative direction with mood, assets, and transitions + 4. VO + Timing — generate TTS audio with word-level timestamps + 5. Build — compile animated HTML compositions for each beat + 6. Validate — generate snapshot PNGs for visual verification + + Target video duration based on video type: + - social-ad: 10-15 seconds + - product-launch: 20-30 seconds + - product-tour: 30-60 seconds + - brand-reel: 15-30 seconds + - feature-announcement: 15-25 seconds + - teaser: 8-15 seconds + + After completing the pipeline, confirm which files were generated (especially STORYBOARD.md + and the compositions/ directory). If the storyboard was generated, read it back and + summarize the beats and creative direction for the user. + depends_on: [capture] + skills: + - heygen-com/hyperframes + + # ── Layer 4: Preview compositions ───────────────────────────────────────── + - id: preview + bash: | + PROJECT_DIR=$(echo "$setup_output" | grep '^PROJECT_DIR=' | cut -d= -f2-) + cd "$PROJECT_DIR" + + echo "Running preview to verify compositions..." + npx hyperframes preview 2>&1 + RESULT=$? + + if [ $RESULT -eq 0 ]; then + echo "" + echo "PREVIEW_SUCCESS" + else + echo "PREVIEW_WARNING: Preview had issues, but attempting render anyway." + echo "PREVIEW_PARTIAL" + fi + depends_on: [generate] + timeout: 120000 + + # ── Layer 5: Render final video ──────────────────────────────────────────── + - id: render + bash: | + PROJECT_DIR=$(echo "$setup_output" | grep '^PROJECT_DIR=' | cut -d= -f2-) + OUTPUT_NAME=$(echo "$setup_output" | grep '^OUTPUT_NAME=' | cut -d= -f2-) + OUTPUT_FILE="${OUTPUT_NAME}.mp4" + + cd "$PROJECT_DIR" + + echo "Rendering final video to: $OUTPUT_FILE" + npx hyperframes render --output "$OUTPUT_FILE" 2>&1 + RESULT=$? + + if [ $RESULT -eq 0 ]; then + echo "" + echo "RENDER_SUCCESS" + ls -la "$OUTPUT_FILE" 2>/dev/null || echo "(output file info unavailable)" + else + echo "RENDER_FAILED: Video render did not complete. Check compositions/ for partial output." + exit 1 + fi + depends_on: [preview] + timeout: 300000 + + # ── Layer 6: Take key-frame snapshots ───────────────────────────────────── + - id: snapshot + bash: | + PROJECT_DIR=$(echo "$setup_output" | grep '^PROJECT_DIR=' | cut -d= -f2-) + OUTPUT_NAME=$(echo "$setup_output" | grep '^OUTPUT_NAME=' | cut -d= -f2-) + + cd "$PROJECT_DIR" + + echo "Extracting key-frame snapshots..." + # Extract at 10%, 50%, and 80% through the video for a representative preview + npx hyperframes snapshot "$OUTPUT_NAME" --at 0.1,0.5,0.8 2>&1 || \ + npx hyperframes snapshot . 2>&1 || \ + echo "SNAPSHOT_SKIPPED: Snapshot extraction not available or failed (non-critical)" + + ls -la snapshots/ 2>/dev/null || echo "(no snapshots directory)" + echo "SNAPSHOT_DONE" + depends_on: [render] + timeout: 60000 + + # ── Layer 7: Summary ─────────────────────────────────────────────────────── + - id: summary + prompt: | + A HyperFrames video was generated from a website. + + Original request: + $ARGUMENTS + + Setup: $setup.output + Capture: $capture.output + Pipeline generation: $generate.output + Preview: $preview.output + Render: $render.output + Snapshots: $snapshot.output + + Summarize the result for the user: + 1. The source URL and creative direction used + 2. The video type and target duration + 3. Whether all pipeline steps completed successfully + 4. Where the output video file is located + 5. Any key-frame snapshots produced + 6. How to iterate: edit STORYBOARD.md and re-run the build step for specific beats, + or run 'npx hyperframes capture ' again to refresh the source assets. + + If any steps failed, explain what went wrong and suggest remediation steps. + depends_on: [snapshot] + model: haiku diff --git a/packages/workflows/src/dag-executor.ts b/packages/workflows/src/dag-executor.ts index b55e08cb94..1de60b7e81 100644 --- a/packages/workflows/src/dag-executor.ts +++ b/packages/workflows/src/dag-executor.ts @@ -5429,6 +5429,10 @@ async function executeWorkflowNode( return executeFanOutWorkflowNode(node, ctx, node.fan_out, ctx.runChildWorkflow); } + // This run's named inputs (#2470), resolved once — threaded identically into the + // `input:` string and every `with:` value below. + const parentInputs = resolveRunInputs(parentRun); + // Resolve the input data string (workflow vars + $node.output refs), exactly as // prompt/bash nodes resolve their text surface. const rawInput = node.input ?? ''; @@ -5443,7 +5447,11 @@ async function executeWorkflowNode( undefined, // loopUserInput undefined, // rejectionReason undefined, // loopPrevOutput - { stateDir: ctx.stateDir } + // Thread the parent run's inputs so `$INPUTS.` resolves in an `input:` string + // exactly as it does in the sibling `with:` values below (a nested sub-run forwarding + // a parent input into a grandchild's $ARGUMENTS). Without this the token would throw + // "This run has no declared inputs" on a run that DOES have inputs (#2470 parity). + { stateDir: ctx.stateDir, inputs: parentInputs } ); const input = substituteNodeOutputRefs(substitutedInput, ctx.nodeOutputs); @@ -5467,7 +5475,7 @@ async function executeWorkflowNode( undefined, undefined, undefined, - { stateDir: ctx.stateDir, inputs: resolveRunInputs(parentRun) } + { stateDir: ctx.stateDir, inputs: parentInputs } ); resolvedInputs[name] = substituteNodeOutputRefs(substituted, ctx.nodeOutputs); } @@ -6057,6 +6065,7 @@ async function executeFanOutWorkflowNode( // (load-time collision-checked so `as` never overwrites a `with:` key). Resolved here // rather than per-child because the values don't depend on the item. const fanOutStaticInputs: Record = {}; + const parentInputs = resolveRunInputs(parentRun); try { if (node.with !== undefined) { for (const [name, rawValue] of Object.entries(node.with)) { @@ -6071,7 +6080,7 @@ async function executeFanOutWorkflowNode( undefined, undefined, undefined, - { stateDir: ctx.stateDir, inputs: resolveRunInputs(parentRun) } + { stateDir: ctx.stateDir, inputs: parentInputs } ); fanOutStaticInputs[name] = substituteNodeOutputRefs(substituted, ctx.nodeOutputs); } From adaff47a9f9add9c23986739e7316685cb626134 Mon Sep 17 00:00:00 2001 From: Leex Date: Wed, 12 Aug 2026 12:13:02 +0200 Subject: [PATCH 3/4] chore: remove out-of-scope hyperframes default workflow Was accidentally included in this PR and caused check:bundled to fail. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01R4KmwzY3iYy4ZBUw7RkwGY --- .../defaults/archon-hyperframes-video.yaml | 226 ------------------ 1 file changed, 226 deletions(-) delete mode 100644 .archon/workflows/defaults/archon-hyperframes-video.yaml diff --git a/.archon/workflows/defaults/archon-hyperframes-video.yaml b/.archon/workflows/defaults/archon-hyperframes-video.yaml deleted file mode 100644 index 7b64258234..0000000000 --- a/.archon/workflows/defaults/archon-hyperframes-video.yaml +++ /dev/null @@ -1,226 +0,0 @@ -name: archon-hyperframes-video -description: | - Use when: User wants to generate or modify a HyperFrames video using HeyGen's AI pipeline. - Triggers: "create hyperframe video", "hyperframes video", "website to video", - "heygen video", "hyperframes", "website video". - Does: Installs HyperFrames skill if needed -> captures website assets -> runs AI pipeline - (Design, Script, Storyboard, VO+Timing, Build, Validate) -> previews output -> - renders final MP4 video -> summarizes the result. - Requires: Node.js / npx available in PATH. - Arguments: $1=URL (required), $2=creative direction (e.g. "bold cinematic dark theme"), - $3=video type (social-ad|product-launch|product-tour|brand-reel|feature-announcement|teaser), - $4=output filename without extension (default: hyperframes-output), - $5=project directory (optional, defaults to current directory). - Optional: Set GEMINI_API_KEY env var for enhanced AI vision analysis (~$0.04/capture). - -nodes: - # ── Layer 0: Validate args and set up working directory ────────────────── - - id: setup - bash: | - URL="${1:-}" - CREATIVE="${2:-bold, cinematic}" - VIDEO_TYPE="${3:-product-launch}" - OUTPUT_NAME="${4:-hyperframes-output}" - PROJECT_DIR="${5:-$(pwd)}" - - if [ -z "$URL" ]; then - echo "ERROR: A URL is required as the first argument." - echo "Usage: archon-hyperframes-video [creative-direction] [video-type] [output-name] [project-dir]" - exit 1 - fi - - # Validate URL format - if ! echo "$URL" | grep -qE '^https?://'; then - echo "ERROR: URL must start with http:// or https://" - exit 1 - fi - - echo "URL=$URL" - echo "CREATIVE=$CREATIVE" - echo "VIDEO_TYPE=$VIDEO_TYPE" - echo "OUTPUT_NAME=$OUTPUT_NAME" - echo "PROJECT_DIR=$PROJECT_DIR" - mkdir -p "$PROJECT_DIR" - echo "SETUP_OK" - timeout: 10000 - - # ── Layer 1: Check/install HyperFrames skill ────────────────────────────── - - id: install-skill - bash: | - if npx hyperframes --version >/dev/null 2>&1; then - echo "HyperFrames already installed: $(npx hyperframes --version 2>/dev/null || echo 'version unknown')" - echo "SKILL_READY" - else - echo "Installing HyperFrames skill..." - npx skills add heygen-com/hyperframes - RESULT=$? - if [ $RESULT -eq 0 ]; then - echo "SKILL_INSTALLED" - else - echo "ERROR: Failed to install HyperFrames skill. Check your network connection." - exit 1 - fi - fi - depends_on: [setup] - timeout: 120000 - - # ── Layer 2: Capture website assets ─────────────────────────────────────── - - id: capture - bash: | - # Extract URL from setup output - URL=$(echo "$setup_output" | grep '^URL=' | cut -d= -f2-) - PROJECT_DIR=$(echo "$setup_output" | grep '^PROJECT_DIR=' | cut -d= -f2-) - - cd "$PROJECT_DIR" - - echo "Capturing assets from: $URL" - echo "This may take up to 2 minutes for complex sites..." - - if [ -n "$GEMINI_API_KEY" ]; then - echo "Gemini vision analysis enabled." - fi - - npx hyperframes capture "$URL" --timeout 120000 - RESULT=$? - - if [ $RESULT -eq 0 ]; then - echo "" - echo "CAPTURE_SUCCESS" - ls -la 2>/dev/null | head -20 - else - echo "CAPTURE_FAILED: Could not capture website. Check if the URL is accessible." - exit 1 - fi - depends_on: [install-skill] - timeout: 180000 - - # ── Layer 3: Run AI pipeline (Design → Script → Storyboard → VO → Build → Validate) ── - - id: generate - prompt: | - You are working with the HyperFrames skill to create a video from a website. - - Setup details: - $setup.output - - Capture result: - $capture.output - - Now run the complete HyperFrames AI pipeline using the captured website data. - Use the following creative direction and parameters: - - Creative direction: Extract the creative direction from the setup output (CREATIVE= line). - Video type: Extract the video type from the setup output (VIDEO_TYPE= line). - - Run the full 7-step pipeline: - 1. Design — generate brand reference documentation from the captured assets - 2. Script — create narration with hook, story, proof, and call-to-action structure - 3. Storyboard — define per-beat creative direction with mood, assets, and transitions - 4. VO + Timing — generate TTS audio with word-level timestamps - 5. Build — compile animated HTML compositions for each beat - 6. Validate — generate snapshot PNGs for visual verification - - Target video duration based on video type: - - social-ad: 10-15 seconds - - product-launch: 20-30 seconds - - product-tour: 30-60 seconds - - brand-reel: 15-30 seconds - - feature-announcement: 15-25 seconds - - teaser: 8-15 seconds - - After completing the pipeline, confirm which files were generated (especially STORYBOARD.md - and the compositions/ directory). If the storyboard was generated, read it back and - summarize the beats and creative direction for the user. - depends_on: [capture] - skills: - - heygen-com/hyperframes - - # ── Layer 4: Preview compositions ───────────────────────────────────────── - - id: preview - bash: | - PROJECT_DIR=$(echo "$setup_output" | grep '^PROJECT_DIR=' | cut -d= -f2-) - cd "$PROJECT_DIR" - - echo "Running preview to verify compositions..." - npx hyperframes preview 2>&1 - RESULT=$? - - if [ $RESULT -eq 0 ]; then - echo "" - echo "PREVIEW_SUCCESS" - else - echo "PREVIEW_WARNING: Preview had issues, but attempting render anyway." - echo "PREVIEW_PARTIAL" - fi - depends_on: [generate] - timeout: 120000 - - # ── Layer 5: Render final video ──────────────────────────────────────────── - - id: render - bash: | - PROJECT_DIR=$(echo "$setup_output" | grep '^PROJECT_DIR=' | cut -d= -f2-) - OUTPUT_NAME=$(echo "$setup_output" | grep '^OUTPUT_NAME=' | cut -d= -f2-) - OUTPUT_FILE="${OUTPUT_NAME}.mp4" - - cd "$PROJECT_DIR" - - echo "Rendering final video to: $OUTPUT_FILE" - npx hyperframes render --output "$OUTPUT_FILE" 2>&1 - RESULT=$? - - if [ $RESULT -eq 0 ]; then - echo "" - echo "RENDER_SUCCESS" - ls -la "$OUTPUT_FILE" 2>/dev/null || echo "(output file info unavailable)" - else - echo "RENDER_FAILED: Video render did not complete. Check compositions/ for partial output." - exit 1 - fi - depends_on: [preview] - timeout: 300000 - - # ── Layer 6: Take key-frame snapshots ───────────────────────────────────── - - id: snapshot - bash: | - PROJECT_DIR=$(echo "$setup_output" | grep '^PROJECT_DIR=' | cut -d= -f2-) - OUTPUT_NAME=$(echo "$setup_output" | grep '^OUTPUT_NAME=' | cut -d= -f2-) - - cd "$PROJECT_DIR" - - echo "Extracting key-frame snapshots..." - # Extract at 10%, 50%, and 80% through the video for a representative preview - npx hyperframes snapshot "$OUTPUT_NAME" --at 0.1,0.5,0.8 2>&1 || \ - npx hyperframes snapshot . 2>&1 || \ - echo "SNAPSHOT_SKIPPED: Snapshot extraction not available or failed (non-critical)" - - ls -la snapshots/ 2>/dev/null || echo "(no snapshots directory)" - echo "SNAPSHOT_DONE" - depends_on: [render] - timeout: 60000 - - # ── Layer 7: Summary ─────────────────────────────────────────────────────── - - id: summary - prompt: | - A HyperFrames video was generated from a website. - - Original request: - $ARGUMENTS - - Setup: $setup.output - Capture: $capture.output - Pipeline generation: $generate.output - Preview: $preview.output - Render: $render.output - Snapshots: $snapshot.output - - Summarize the result for the user: - 1. The source URL and creative direction used - 2. The video type and target duration - 3. Whether all pipeline steps completed successfully - 4. Where the output video file is located - 5. Any key-frame snapshots produced - 6. How to iterate: edit STORYBOARD.md and re-run the build step for specific beats, - or run 'npx hyperframes capture ' again to refresh the source assets. - - If any steps failed, explain what went wrong and suggest remediation steps. - depends_on: [snapshot] - model: haiku From dc560035fd746db41908a1e454d9a085f570353c Mon Sep 17 00:00:00 2001 From: Leex Date: Wed, 12 Aug 2026 12:33:09 +0200 Subject: [PATCH 4/4] =?UTF-8?q?fix(workflows):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20with:=20expansion,=20runtime=20input=20contract,=20?= =?UTF-8?q?docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the changes requested on #2523. - include-expander: rewriteNodeOutputRefs() and applyInputsMacro() now walk every value in a workflow: node's with: map, so an included block that calls a child workflow can forward $INPUTS and reference its own nodes. Previously an included block's with: value referencing a child-local node failed with "references unknown node '$local.output'". - Extract the declared-input contract into workflow-inputs.ts and enforce it on the runtime workflow: path as well: declared defaults applied, unsupplied required inputs and undeclared keys rejected, resolved after child discovery and before isolation/worktree or run-row creation. Both call surfaces now go through one implementation, so include: and workflow: accept identical maps by construction. - Apply declared defaults to bare top-level runs, which have no parent to stamp metadata.inputs. - CLAUDE.md: with: is no longer rejected on workflow: nodes and fan_out.as is no longer reserved; document the workflow-level inputs:/returns: signature. Regression coverage: included block with a workflow: node whose with: values use both $INPUTS and a child-local node ref; sub-run defaults, missing-required and undeclared inputs; runtime $INPUTS delivery into AI surfaces; returns: rebinding on a sub-run; cold-resume metadata reconstitution. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01R4KmwzY3iYy4ZBUw7RkwGY --- CLAUDE.md | 3 +- packages/workflows/src/executor.ts | 52 +- .../workflows/src/include-expander.test.ts | 42 ++ packages/workflows/src/include-expander.ts | 55 +- packages/workflows/src/subrun.test.ts | 484 ++++++++++++++++++ packages/workflows/src/workflow-inputs.ts | 105 ++++ 6 files changed, 702 insertions(+), 39 deletions(-) create mode 100644 packages/workflows/src/workflow-inputs.ts diff --git a/CLAUDE.md b/CLAUDE.md index a751b2e983..fe46835397 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -450,8 +450,9 @@ Structured logging uses Pino via `createLogger('')` from `@archon/paths` 2. **Workflows** (YAML-based): - Stored in `.archon/workflows/`; recommended copyable layout is `.archon/workflows///` with one YAML plus optional `commands/` and `scripts/`. Both directory names are author-chosen. Flat and one-level grouped YAML remain supported. - Multi-step AI execution chains, discovered at runtime - - **`nodes:` (DAG format)**: Nodes with explicit `depends_on` edges; independent nodes in the same topological layer run concurrently. Node types: `command:` (named command file), `prompt:` (inline prompt), `bash:` (shell script, stdout captured as `$nodeId.output`, no AI, receives managed per-project env vars in its subprocess environment when configured), `loop:` (iterative AI prompt until completion signal), `loop_group:` (multi-node sub-DAG body repeated per iteration until `until` signal / `until_bash` exit 0 / `max_iterations`; body is sealed for `depends_on` but may read outer outputs via `$nodeId.output` and the previous iteration via `$LOOP_PREV..output`; a failed body node fails the group immediately; group-level `model`/`provider` become body defaults), `approval:` (human gate; pauses until user approves or rejects; `capture_response: true` stores the user's comment as `$.output` for downstream nodes, default false), `script:` (inline TypeScript/Python or named script from `.archon/scripts/`, runs via `bun` or `uv`, stdout captured as `$nodeId.output`, no AI, receives managed per-project env vars in its subprocess environment when configured, supports `deps:` for dependency installation and `timeout:` in ms, requires `runtime: bun` or `runtime: uv`), `include:` (load-time inlining of another workflow's nodes as a flattened, namespaced sub-DAG — each included node becomes `__`; the include node's `depends_on`/`when`/`trigger_rule` attach to the block's entry nodes, and `$includeId.output` resolves to the block's terminal (primary) sink; expansion happens at discovery so the executor sees ordinary nodes; `with:` passes an identifier-keyed string map the block reads as `$INPUTS.`, substituted VERBATIM at load time (never expressions) across every inline text surface including inside code fences — an unsupplied name is a load error, and `$INPUTS` in a `command:`/`loop.command` file is rejected because a command body is read after expansion and can never be parameterized (best-effort: top-level command nodes only, unresolvable files warn and are skipped) — see the "Reusing a Shared Sub-DAG" guide), `workflow:` (runtime sub-run — starts another workflow by static name as a separate governed CHILD run with its own `workflow_runs` row (`parent_run_id`), artifacts, gates, cost, and audit trail; `input:` forwards a data string (substituted like prompt bodies) as the child's `$ARGUMENTS`; the child's terminal output threads back as `$nodeId.output`; a child gate pauses the whole tree (approve the CHILD by run id — the parent auto-resumes on child completion); `isolation:` chooses the child's checkout — `inherit` (default; shares the parent's) or `worktree` (its own git worktree + branch, opt-in only, never inferred; requires an injected child-isolation resolver, so it fails fast on folder projects and surfaces that don't wire one), `with:` and `retry:` rejected, disallowed inside a `loop_group` body; abandon cascade-cancels descendants; `fan_out:` runs ONE CHILD PER ITEM of a runtime list — `items` (a `$node.output` ref or literal JSON array), `max_parallel` (default 5, bounds concurrency not total count or spend), `join` (default `all_done`: every terminal outcome aggregates with failures as `{error,status}`; `all_success` for the genuinely dependent case), `as` reserved and rejected at load. Children are INDEPENDENT: every index spawns, each runs to its own terminal state, and none cancels another — the sole exception is a child that pauses at a gate, which is cancelled because a parent has one approval slot (gate before/after the fan-out, never inside a child). Racing (`join: first_success`) is rejected outright, not deferred. Concurrent children on a SHARED checkout collide on the path lock, so a spawn-time preflight refuses that expansion unless the child declares `mutates_checkout: false`, the node sets `isolation: worktree`, or `max_parallel: 1`) . Supports `when:` conditions, `trigger_rule` join semantics, `$nodeId.output` substitution, `output_format` for structured JSON output (SDK-enforced on Claude/Codex/OpenCode; best-effort prompt-augmentation + repair on Pi/Copilot — the parsed output is **validated against the declared schema for every provider**, best-effort providers (Pi/Copilot) re-ask up to 3× on a validation miss, and a node that declares `output_format` but returns no schema-valid output **fails** rather than degrading silently; `$nodeId.output.field` access is strict — a field not in the producer's schema, or a schemaless node whose output isn't JSON / lacks the key, fails the consuming node, while an author-declared-optional field resolves to `''`), `allowed_tools`/`denied_tools` for per-node tool restrictions (all providers except Codex), `hooks` for per-node SDK hook callbacks (Claude only), `mcp` for per-node MCP server config files (all providers except Pi, env vars expanded at execution time), and `skills` for per-node skill preloading via AgentDefinition wrapping (per-node injection on Claude/Pi/OpenCode/Copilot; Codex instead auto-discovers skills from `.agents/skills/` on the filesystem — the `skills:` list is informational for Codex nodes), `agents` for inline sub-agent definitions invokable via the Task tool (Claude only), and `effort`/`thinking` for reasoning depth (Claude/Pi/Copilot) plus the Claude-only SDK advanced options `maxBudgetUsd`/`systemPrompt`/`fallbackModel`/`betas`/`sandbox` (also settable at workflow level), and `persist_session` for cross-run provider session continuity (node-level opt-in; workflow-level default via `persist_sessions: true`; requires a provider with the `sessionResume` capability), and `output_type` (any node type) for engine-written typed output sidecars — when set, the executor writes `$ARTIFACTS_DIR/nodes/.md` + `.meta.json` after the node completes (best-effort) so downstream nodes and later runs can locate output by type instead of guessing filenames + - **`nodes:` (DAG format)**: Nodes with explicit `depends_on` edges; independent nodes in the same topological layer run concurrently. Node types: `command:` (named command file), `prompt:` (inline prompt), `bash:` (shell script, stdout captured as `$nodeId.output`, no AI, receives managed per-project env vars in its subprocess environment when configured), `loop:` (iterative AI prompt until completion signal), `loop_group:` (multi-node sub-DAG body repeated per iteration until `until` signal / `until_bash` exit 0 / `max_iterations`; body is sealed for `depends_on` but may read outer outputs via `$nodeId.output` and the previous iteration via `$LOOP_PREV..output`; a failed body node fails the group immediately; group-level `model`/`provider` become body defaults), `approval:` (human gate; pauses until user approves or rejects; `capture_response: true` stores the user's comment as `$.output` for downstream nodes, default false), `script:` (inline TypeScript/Python or named script from `.archon/scripts/`, runs via `bun` or `uv`, stdout captured as `$nodeId.output`, no AI, receives managed per-project env vars in its subprocess environment when configured, supports `deps:` for dependency installation and `timeout:` in ms, requires `runtime: bun` or `runtime: uv`), `include:` (load-time inlining of another workflow's nodes as a flattened, namespaced sub-DAG — each included node becomes `__`; the include node's `depends_on`/`when`/`trigger_rule` attach to the block's entry nodes, and `$includeId.output` resolves to the block's terminal (primary) sink; expansion happens at discovery so the executor sees ordinary nodes; `with:` passes an identifier-keyed string map the block reads as `$INPUTS.`, substituted VERBATIM at load time (never expressions) across every inline text surface including inside code fences — an unsupplied name is a load error, and `$INPUTS` in a `command:`/`loop.command` file is rejected because a command body is read after expansion and can never be parameterized (best-effort: top-level command nodes only, unresolvable files warn and are skipped) — see the "Reusing a Shared Sub-DAG" guide), `workflow:` (runtime sub-run — starts another workflow by static name as a separate governed CHILD run with its own `workflow_runs` row (`parent_run_id`), artifacts, gates, cost, and audit trail; `input:` forwards a data string (substituted like prompt bodies) as the child's `$ARGUMENTS`; the child's terminal output threads back as `$nodeId.output`; a child gate pauses the whole tree (approve the CHILD by run id — the parent auto-resumes on child completion); `isolation:` chooses the child's checkout — `inherit` (default; shares the parent's) or `worktree` (its own git worktree + branch, opt-in only, never inferred; requires an injected child-isolation resolver, so it fails fast on folder projects and surfaces that don't wire one), `with:` passes an identifier-keyed string map (mutually exclusive with `input:`) resolved at RUN time — workflow vars, then `$node.output` refs — and persisted to the child's `metadata.inputs`, where it reads as `$INPUTS.` in AI/prompt surfaces and as `INPUTS_` env vars in bash/script nodes; the resolved child's declared `inputs:` contract is enforced before any worktree or child run row exists (declared defaults applied, unsupplied `required` inputs and undeclared keys rejected), through the same shared implementation `include:` uses so both surfaces accept exactly the same maps; `retry:` rejected, disallowed inside a `loop_group` body; abandon cascade-cancels descendants; `fan_out:` runs ONE CHILD PER ITEM of a runtime list — `items` (a `$node.output` ref or literal JSON array), `max_parallel` (default 5, bounds concurrency not total count or spend), `join` (default `all_done`: every terminal outcome aggregates with failures as `{error,status}`; `all_success` for the genuinely dependent case), `as` names the per-item value as `$INPUTS.` inside each child (rejected at load only when it collides with a `with:` key of the same name — both would populate the same slot; when unset the item still travels as `$ARGUMENTS`). Children are INDEPENDENT: every index spawns, each runs to its own terminal state, and none cancels another — the sole exception is a child that pauses at a gate, which is cancelled because a parent has one approval slot (gate before/after the fan-out, never inside a child). Racing (`join: first_success`) is rejected outright, not deferred. Concurrent children on a SHARED checkout collide on the path lock, so a spawn-time preflight refuses that expansion unless the child declares `mutates_checkout: false`, the node sets `isolation: worktree`, or `max_parallel: 1`) . Supports `when:` conditions, `trigger_rule` join semantics, `$nodeId.output` substitution, `output_format` for structured JSON output (SDK-enforced on Claude/Codex/OpenCode; best-effort prompt-augmentation + repair on Pi/Copilot — the parsed output is **validated against the declared schema for every provider**, best-effort providers (Pi/Copilot) re-ask up to 3× on a validation miss, and a node that declares `output_format` but returns no schema-valid output **fails** rather than degrading silently; `$nodeId.output.field` access is strict — a field not in the producer's schema, or a schemaless node whose output isn't JSON / lacks the key, fails the consuming node, while an author-declared-optional field resolves to `''`), `allowed_tools`/`denied_tools` for per-node tool restrictions (all providers except Codex), `hooks` for per-node SDK hook callbacks (Claude only), `mcp` for per-node MCP server config files (all providers except Pi, env vars expanded at execution time), and `skills` for per-node skill preloading via AgentDefinition wrapping (per-node injection on Claude/Pi/OpenCode/Copilot; Codex instead auto-discovers skills from `.agents/skills/` on the filesystem — the `skills:` list is informational for Codex nodes), `agents` for inline sub-agent definitions invokable via the Task tool (Claude only), and `effort`/`thinking` for reasoning depth (Claude/Pi/Copilot) plus the Claude-only SDK advanced options `maxBudgetUsd`/`systemPrompt`/`fallbackModel`/`betas`/`sandbox` (also settable at workflow level), and `persist_session` for cross-run provider session continuity (node-level opt-in; workflow-level default via `persist_sessions: true`; requires a provider with the `sessionResume` capability), and `output_type` (any node type) for engine-written typed output sidecars — when set, the executor writes `$ARTIFACTS_DIR/nodes/.md` + `.meta.json` after the node completes (best-effort) so downstream nodes and later runs can locate output by type instead of guessing filenames - Workflow-level `requires: [github]` hard-blocks invocation (before any worktree/clone/AI cost) when the originating user hasn't connected their GitHub identity — enforced only when per-user GitHub is enabled (GitHub App + `TOKEN_ENCRYPTION_KEY`); a no-op for solo PAT installs + - Workflow-level **signature** (`inputs:` / `returns:`) declares the contract a workflow exposes to its callers. `inputs:` is a map of input name → `{ required?, default?, description? }`; a workflow that declares it validates every caller (`include: … with:` at load time, `workflow: … with:` at run time) through one shared resolver — defaults applied, missing `required` and undeclared keys rejected — while a workflow that declares NO `inputs:` keeps passthrough behavior. Declared defaults also apply to a bare top-level run, which has no caller to supply them. `returns:` names the node whose output `$.output` resolves to, overriding the positional first-sink default (`depends_on` still waits on the real sinks) - Provider inherited from `.archon/config.yaml` unless explicitly set; per-node `provider` and `model` overrides supported - Model and options can be set per workflow or inherited from config defaults - `interactive: true` at the workflow level forces foreground execution on web (required for approval-gate workflows in the web UI) diff --git a/packages/workflows/src/executor.ts b/packages/workflows/src/executor.ts index 0f3f676968..ec4731b1ea 100644 --- a/packages/workflows/src/executor.ts +++ b/packages/workflows/src/executor.ts @@ -25,12 +25,14 @@ import { isApprovalContext, isRunBlockedOnChild, SUBRUN_METADATA_KEYS, + readSubrunMetadata, } from './schemas'; import { executeDagWorkflow, childOutcomeFromRun } from './dag-executor'; import type { RunChildWorkflowArgs, ChildWorkflowOutcome } from './dag-executor'; import { discoverWorkflowsWithConfig } from './workflow-discovery'; import { maybeWarnLegacyStatePath, maybeWarnLegacyArtifactsPath } from './state-migration'; import { resolveWorkflowName } from './router'; +import { resolveDeclaredInputs, defaultRunInputs } from './workflow-inputs'; import { logWorkflowStart, logWorkflowError } from './logger'; import { formatDuration, parseDbTimestamp } from './utils/duration'; import { keepAwake } from './utils/keep-awake'; @@ -715,6 +717,26 @@ async function runChildWorkflow( ); } + // 2b. Enforce the RESOLVED child's declared `inputs:` contract (#2470) — the exact + // same resolution `include:` performs at load time, through the same shared + // implementation, so a `with:` map accepted by one surface is accepted by the + // other. It can only run here (not at load time) because the target is resolved + // late by design, so it sits before isolation/worktree creation and before the + // child run row exists: a contract violation must never leave an orphan worktree + // or a doomed child row behind. + let childInputs: Record | undefined; + try { + const resolved = resolveDeclaredInputs( + inputs ?? {}, + childWorkflow.inputs, + `Node '${nodeId}'`, + `sub-run workflow '${childWorkflow.name}'` + ); + childInputs = Object.keys(resolved).length > 0 ? resolved : undefined; + } catch (err) { + return failOutcome((err as Error).message); + } + // 3. Resolve the child's execution cwd (slice 2, PR-A). `isolation: 'worktree'` // runs the child in its own git worktree obtained from the injected resolver. // A resume whose child run row still exists reuses that row's recorded path @@ -835,9 +857,9 @@ async function runChildWorkflow( // COLD resume: both resume paths (hydrateResumableRun and the zero-completed-node // resumeWorkflowRun fallback) reload THIS run row, so the map is intact without // re-resolving parent refs that may be out of scope. Stamped only when non-empty. - ...(inputs !== undefined && Object.keys(inputs).length > 0 - ? { [SUBRUN_METADATA_KEYS.inputs]: inputs } - : {}), + // This is the CONTRACT-RESOLVED map (declared defaults applied), not the raw + // caller map — the child must see exactly what its `inputs:` block promises. + ...(childInputs !== undefined ? { [SUBRUN_METADATA_KEYS.inputs]: childInputs } : {}), // Record the child's own worktree env + branch (mirrors the container path's // isolation_env_id) so `isolation list` correlation + PR-E console grouping // can find it. Absent for `inherit`/shared-checkout children. @@ -1804,6 +1826,28 @@ export async function executeWorkflow( // Continue anyway - workflow is already recorded in database } + // Declared-input defaults for a run with no caller (#2470). Runtime `$INPUTS` + // otherwise comes only from `metadata.inputs`, which a parent stamps at spawn — so a + // workflow started directly (CLI / chat / web) would throw on its own + // `$INPUTS.` while the identical workflow invoked as a `workflow:` child + // resolved it. Derived from the definition rather than persisted, so it stays + // correct across a cold resume and when the defaults are later edited. Any caller- + // supplied value already on the row wins; only defaults are filled in. + const declaredDefaults = defaultRunInputs(workflow.inputs); + const runForDag: WorkflowRun = declaredDefaults + ? { + ...workflowRun, + metadata: { + ...(workflowRun.metadata as Record | undefined), + [SUBRUN_METADATA_KEYS.inputs]: { + ...declaredDefaults, + ...(readSubrunMetadata(workflowRun.metadata as Record | undefined) + .inputs ?? {}), + }, + }, + } + : workflowRun; + // Execute the DAG workflow const dagSummary = await executeDagWorkflow( deps, @@ -1811,7 +1855,7 @@ export async function executeWorkflow( conversationId, cwd, workflow, - workflowRun, + runForDag, resolvedProvider, resolvedModel, artifactsDir, diff --git a/packages/workflows/src/include-expander.test.ts b/packages/workflows/src/include-expander.test.ts index 85b81f96f7..002aebc3ee 100644 --- a/packages/workflows/src/include-expander.test.ts +++ b/packages/workflows/src/include-expander.test.ts @@ -1065,3 +1065,45 @@ describe('expandWorkflowIncludes — with vs declared inputs (#2470)', () => { expect('prompt' in work ? work.prompt : '').toBe('v: hello'); }); }); + +describe('expandWorkflowIncludes — workflow: node `with:` values (#2470)', () => { + test('namespaces child-local node refs and substitutes $INPUTS in every with: value', () => { + // Block: a local node whose output is forwarded to a child workflow alongside + // an `$INPUTS`-sourced value. Both surfaces live in `with:`, which the + // expander must walk exactly like `input:` and `fan_out.items`. + const block = withSignature( + wf('caller-blk', [ + { id: 'local', bash: 'echo hi' }, + { + id: 'call', + workflow: 'child', + depends_on: ['local'], + with: { payload: '$local.output', style: '$INPUTS.style' }, + }, + ]), + { inputs: { style: { default: 'strict' } } } + ); + const parent = wf('parent', [{ id: 'outer', include: 'caller-blk' }]); + + const { workflows, errors } = expandWorkflowIncludes(mapOf(block, parent)); + expect(errors).toHaveLength(0); + + const call = nodeById(workflows.get('parent')!, 'outer__call')!; + expect('with' in call ? call.with : undefined).toEqual({ + payload: '$outer__local.output', + style: 'strict', + }); + }); + + test('reports a missing required input referenced only from a with: value', () => { + const block = withSignature( + wf('caller-blk', [{ id: 'call', workflow: 'child', with: { diff: '$INPUTS.diff' } }]), + { inputs: { diff: { required: true } } } + ); + const parent = wf('parent', [{ id: 'outer', include: 'caller-blk' }]); + + const { workflows, errors } = expandWorkflowIncludes(mapOf(block, parent)); + expect(workflows.has('parent')).toBe(false); + expect(errors.find(e => e.filename === 'parent')?.error).toContain("requires input 'diff'"); + }); +}); diff --git a/packages/workflows/src/include-expander.ts b/packages/workflows/src/include-expander.ts index 4227e2078b..1acc6d8434 100644 --- a/packages/workflows/src/include-expander.ts +++ b/packages/workflows/src/include-expander.ts @@ -45,6 +45,7 @@ import { import { createLogger } from '@archon/paths'; import { validateDagStructure } from './loader'; import { getFileBackedCommandName } from './command-file'; +import { resolveDeclaredInputs } from './workflow-inputs'; /** * Resolve the logger on every call rather than caching it at module scope. @@ -196,10 +197,13 @@ function rewriteNodeOutputRefs(node: DagNode, rename: (id: string) => string): v } else if (isScriptNode(node)) { node.script = code(node.script); } else if (isWorkflowNode(node)) { - // workflow.input and workflow.fan_out.items are live code/expression ref surfaces - // (data strings), so refs inside an included block's `workflow:` node namespace - // verbatim. + // workflow.input, workflow.with values and workflow.fan_out.items are live + // code/expression ref surfaces (data strings), so refs inside an included block's + // `workflow:` node namespace verbatim. if (node.input !== undefined) node.input = code(node.input); + if (node.with !== undefined) { + for (const [key, value] of Object.entries(node.with)) node.with[key] = code(value); + } if (node.fan_out !== undefined) node.fan_out.items = code(node.fan_out.items); } else if (isCancelNode(node)) { node.cancel = code(node.cancel); @@ -281,6 +285,9 @@ function applyInputsMacro(node: DagNode, args: Record, missing: node.script = substitute(node.script); } else if (isWorkflowNode(node)) { if (node.input !== undefined) node.input = substitute(node.input); + if (node.with !== undefined) { + for (const [key, value] of Object.entries(node.with)) node.with[key] = substitute(value); + } if (node.fan_out !== undefined) node.fan_out.items = substitute(node.fan_out.items); } else if (isCancelNode(node)) { node.cancel = substitute(node.cancel); @@ -309,39 +316,19 @@ function resolveIncludeInputs( includeNode: IncludeNode, child: WorkflowDefinition ): Record { - const callerWith = includeNode.with ?? {}; - const declared = child.inputs; - if (declared === undefined) return callerWith; - - // Reject caller keys the block does not declare. - const undeclared = Object.keys(callerWith).filter(k => !Object.hasOwn(declared, k)); - if (undeclared.length > 0) { - const names = undeclared.sort(); - throw new IncludeExpansionError( - `Node '${includeNode.id}': included block '${child.name}' does not declare input${names.length === 1 ? '' : 's'} ${names.map(n => `'${n}'`).join(', ')}. Declared inputs: ${Object.keys(declared).sort().join(', ') || '(none)'}.` - ); - } - - const resolved: Record = {}; - const missingRequired: string[] = []; - for (const [name, spec] of Object.entries(declared)) { - if (Object.hasOwn(callerWith, name)) { - resolved[name] = callerWith[name]; - } else if (spec.default !== undefined) { - resolved[name] = spec.default; - } else if (spec.required === true) { - missingRequired.push(name); - } - // Declared, not supplied, not required, no default: omitted. If the block body - // references `$INPUTS.`, applyInputsMacro reports it as missing and fails. - } - if (missingRequired.length > 0) { - const names = missingRequired.sort(); - throw new IncludeExpansionError( - `Node '${includeNode.id}': included block '${child.name}' requires input${names.length === 1 ? '' : 's'} ${names.map(n => `'${n}'`).join(', ')}. Pass ${names.length === 1 ? 'it' : 'them'} through 'with:'.` + try { + return resolveDeclaredInputs( + includeNode.with ?? {}, + child.inputs, + `Node '${includeNode.id}'`, + `included block '${child.name}'` ); + } catch (err) { + // Re-typed so the per-workflow expansion loop treats a contract violation as the + // same resilient "drop one workflow, keep the rest" failure as every other + // expansion error, rather than escaping as an unhandled throw. + throw new IncludeExpansionError((err as Error).message); } - return resolved; } /** diff --git a/packages/workflows/src/subrun.test.ts b/packages/workflows/src/subrun.test.ts index f2492229cb..1ae93ad70b 100644 --- a/packages/workflows/src/subrun.test.ts +++ b/packages/workflows/src/subrun.test.ts @@ -3738,3 +3738,487 @@ nodes: expect(String(approval?.childRunId ?? '')).toBe(paused?.id ?? ''); }); }); + +// --------------------------------------------------------------------------- +// Runtime declared-input contract on `workflow:` nodes (#2470) +// +// `include:` enforces the callee's `inputs:` block at LOAD time; these lock the +// RUN-time twin so the same `with:` map is accepted or rejected identically no +// matter which surface calls the block. +// --------------------------------------------------------------------------- + +describe('workflow: declared input contract at runtime (#2470)', () => { + let cwd: string; + const originalArchonHome = process.env.ARCHON_HOME; + + async function writeWorkflow(name: string, yaml: string): Promise { + await writeFile(join(cwd, '.archon', 'workflows', `${name}.yaml`), yaml); + } + + async function discover(name: string): Promise { + const result = await discoverWorkflows(cwd, { loadDefaults: false }); + const wf = result.workflows.find(w => w.workflow.name === name); + if (!wf) throw new Error(`workflow ${name} not found: ${JSON.stringify(result.errors)}`); + return wf.workflow; + } + + /** A child that declares `inputs:` and echoes them, so delivery is observable. */ + async function writeDeclaringChild(inputsYaml: string): Promise { + await writeWorkflow( + 'child-declares', + ` +name: child-declares +description: child with a declared input contract +inputs: +${inputsYaml} +nodes: + - id: emit + bash: echo "style=$INPUTS_STYLE tone=$INPUTS_TONE" +` + ); + } + + /** The child run row spawned by `sub`, or undefined if none was created. */ + function childRun(store: InMemoryStore): WorkflowRun | undefined { + return [...store.runs.values()].find(r => r.workflow_name === 'child-declares'); + } + + beforeEach(async () => { + cwd = join(tmpdir(), `subin-${Date.now()}-${Math.random().toString(36).slice(2)}`); + await mkdir(join(cwd, '.archon', 'workflows'), { recursive: true }); + process.env.ARCHON_HOME = join(cwd, 'home'); + }); + + afterEach(async () => { + await rm(cwd, { recursive: true, force: true }).catch(() => {}); + if (originalArchonHome === undefined) delete process.env.ARCHON_HOME; + else process.env.ARCHON_HOME = originalArchonHome; + }); + + it('applies the child declared default for an omitted with: key', async () => { + await writeDeclaringChild(' style:\n default: strict\n tone:\n default: dry'); + await writeWorkflow( + 'parent-defaults', + ` +name: parent-defaults +description: supplies only one of two declared inputs +nodes: + - id: sub + workflow: child-declares + with: + style: loud +` + ); + + const store = new InMemoryStore(); + const result = await executeWorkflow( + makeDeps(store), + makePlatform(), + 'conv-plat', + cwd, + await discover('parent-defaults'), + 'goal', + 'conv-db' + ); + + expect(result.success).toBe(true); + // The caller's value wins; the omitted one is filled from the child's default — + // and the RESOLVED map (not the raw caller map) is what gets persisted, so it + // survives a cold resume that never re-reads the parent. + const child = childRun(store); + expect(child?.metadata?.inputs).toEqual({ style: 'loud', tone: 'dry' }); + // Delivered to the child's shell surface as INPUTS_. + const emitted = store.events.find( + e => e.workflow_run_id === child?.id && e.event_type === 'node_completed' + ); + expect(String(emitted?.data?.node_output)).toContain('style=loud tone=dry'); + }); + + it('fails the node when a required child input is not supplied', async () => { + await writeDeclaringChild(' style:\n required: true'); + await writeWorkflow( + 'parent-missing', + ` +name: parent-missing +description: omits a required child input +nodes: + - id: sub + workflow: child-declares +` + ); + + const store = new InMemoryStore(); + const result = await executeWorkflow( + makeDeps(store), + makePlatform(), + 'conv-plat', + cwd, + await discover('parent-missing'), + 'goal', + 'conv-db' + ); + + expect(result.success).toBe(false); + const failed = store.events.find(e => e.event_type === 'node_failed'); + expect(String(failed?.data?.error)).toContain("requires input 'style'"); + // Rejected BEFORE the child row exists — no doomed run left to clean up. + expect(childRun(store)).toBeUndefined(); + }); + + it('fails the node on a with: key the child does not declare', async () => { + await writeDeclaringChild(' style:\n default: strict'); + await writeWorkflow( + 'parent-undeclared', + ` +name: parent-undeclared +description: passes a key the child never declared +nodes: + - id: sub + workflow: child-declares + with: + stlye: typo +` + ); + + const store = new InMemoryStore(); + const result = await executeWorkflow( + makeDeps(store), + makePlatform(), + 'conv-plat', + cwd, + await discover('parent-undeclared'), + 'goal', + 'conv-db' + ); + + expect(result.success).toBe(false); + const failed = store.events.find(e => e.event_type === 'node_failed'); + expect(String(failed?.data?.error)).toContain("does not declare input 'stlye'"); + expect(childRun(store)).toBeUndefined(); + }); + + it('keeps Phase-1 passthrough for a child that declares no inputs', async () => { + await writeWorkflow( + 'child-undeclared', + ` +name: child-undeclared +description: no inputs block at all +nodes: + - id: emit + bash: echo "v=$INPUTS_V" +` + ); + await writeWorkflow( + 'parent-passthrough', + ` +name: parent-passthrough +description: passes anything to an undeclared child +nodes: + - id: sub + workflow: child-undeclared + with: + v: hello +` + ); + + const store = new InMemoryStore(); + const result = await executeWorkflow( + makeDeps(store), + makePlatform(), + 'conv-plat', + cwd, + await discover('parent-passthrough'), + 'goal', + 'conv-db' + ); + + expect(result.success).toBe(true); + const child = [...store.runs.values()].find(r => r.workflow_name === 'child-undeclared'); + expect(child?.metadata?.inputs).toEqual({ v: 'hello' }); + }); + + it('resolves declared defaults on a bare top-level run (no parent to stamp metadata)', async () => { + // Runtime `$INPUTS` otherwise comes only from sub-run metadata, so a workflow + // started directly would throw on its own `$INPUTS.` while the identical + // workflow invoked as a child resolved it. + await writeWorkflow( + 'top-level-defaults', + ` +name: top-level-defaults +description: declares a defaulted input and reads it +inputs: + style: + default: strict +nodes: + - id: emit + bash: echo "style=$INPUTS_STYLE" +` + ); + + const store = new InMemoryStore(); + const result = await executeWorkflow( + makeDeps(store), + makePlatform(), + 'conv-plat', + cwd, + await discover('top-level-defaults'), + 'goal', + 'conv-db' + ); + + expect(result.success).toBe(true); + const emitted = store.events.find(e => e.event_type === 'node_completed'); + expect(String(emitted?.data?.node_output)).toContain('style=strict'); + }); +}); + +// --------------------------------------------------------------------------- +// Runtime `$INPUTS` delivery into AI surfaces + cold-resume reconstitution (#2470) +// --------------------------------------------------------------------------- + +describe('workflow: runtime $INPUTS delivery and cold resume (#2470)', () => { + let cwd: string; + const originalArchonHome = process.env.ARCHON_HOME; + + async function writeWorkflow(name: string, yaml: string): Promise { + await writeFile(join(cwd, '.archon', 'workflows', `${name}.yaml`), yaml); + } + + async function discover(name: string): Promise { + const result = await discoverWorkflows(cwd, { loadDefaults: false }); + const wf = result.workflows.find(w => w.workflow.name === name); + if (!wf) throw new Error(`workflow ${name} not found: ${JSON.stringify(result.errors)}`); + return wf.workflow; + } + + /** Deps whose provider records every prompt it is handed, so AI-surface delivery is observable. */ + function makeRecordingDeps(store: IWorkflowStore): { deps: WorkflowDeps; prompts: string[] } { + const prompts: string[] = []; + const provider = { + ...makeProvider(), + sendQuery: mock(function* (prompt: string) { + prompts.push(prompt); + yield { type: 'assistant', content: 'ai-output' }; + yield { type: 'result', sessionId: 'sess', cost: 0.01, tokens: { input: 7, output: 3 } }; + }), + }; + return { + deps: { + ...makeDeps(store), + getAgentProvider: mock(() => provider) as unknown as WorkflowDeps['getAgentProvider'], + }, + prompts, + }; + } + + beforeEach(async () => { + cwd = join(tmpdir(), `subdel-${Date.now()}-${Math.random().toString(36).slice(2)}`); + await mkdir(join(cwd, '.archon', 'workflows'), { recursive: true }); + process.env.ARCHON_HOME = join(cwd, 'home'); + }); + + afterEach(async () => { + await rm(cwd, { recursive: true, force: true }).catch(() => {}); + if (originalArchonHome === undefined) delete process.env.ARCHON_HOME; + else process.env.ARCHON_HOME = originalArchonHome; + }); + + it("substitutes the child's $INPUTS. into an AI prompt surface", async () => { + await writeWorkflow( + 'child-ai-inputs', + ` +name: child-ai-inputs +description: reads a declared input from a prompt +inputs: + style: + default: strict +nodes: + - id: work + prompt: "write it in $INPUTS.style style" +` + ); + await writeWorkflow( + 'parent-ai-inputs', + ` +name: parent-ai-inputs +description: supplies the child input +nodes: + - id: sub + workflow: child-ai-inputs + with: + style: terse +` + ); + + const store = new InMemoryStore(); + const { deps, prompts } = makeRecordingDeps(store); + const result = await executeWorkflow( + deps, + makePlatform(), + 'conv-plat', + cwd, + await discover('parent-ai-inputs'), + 'goal', + 'conv-db' + ); + + expect(result.success).toBe(true); + // The caller's value reached the model; the literal token never did. + expect(prompts.some(p => p.includes('write it in terse style'))).toBe(true); + expect(prompts.some(p => p.includes('$INPUTS.style'))).toBe(false); + }); + + it('reconstitutes $INPUTS from the child run row on a COLD resume (no parent in the loop)', async () => { + // The child is resumed directly from its own persisted row — the parent never + // re-resolves `with:` (its refs may be long out of scope), so a post-gate node + // can only see `$INPUTS` if the resolved map survived on `metadata.inputs`. + await writeWorkflow( + 'child-cold', + ` +name: child-cold +description: gated child that reads an input AFTER the gate +interactive: true +inputs: + style: + default: strict +nodes: + - id: gate + approval: + message: "hold" + - id: after-gate + prompt: "resumed in $INPUTS.style style" + depends_on: [gate] +` + ); + await writeWorkflow( + 'parent-cold', + ` +name: parent-cold +description: spawns the gated child +interactive: true +nodes: + - id: sub + workflow: child-cold + with: + style: terse +` + ); + + const store = new InMemoryStore(); + const { deps, prompts } = makeRecordingDeps(store); + await executeWorkflow( + deps, + makePlatform(), + 'conv-plat', + cwd, + await discover('parent-cold'), + 'goal', + 'conv-db' + ); + + const child = [...store.runs.values()].find(r => r.workflow_name === 'child-cold'); + expect(child?.status).toBe('paused'); + expect(child?.metadata?.inputs).toEqual({ style: 'terse' }); + + // Cold path: approve, hydrate from the persisted row alone, re-drive the child. + store.approveGate(child!.id); + const hydrated = await hydrateResumableRun(deps, (await store.getWorkflowRun(child!.id))!); + expect(hydrated).not.toBeNull(); + await executeWorkflow( + deps, + makePlatform(), + 'conv-plat', + cwd, + await discover('child-cold'), + child!.user_message, + 'conv-db', + { ...hydrated! } + ); + + expect((await store.getWorkflowRun(child!.id))?.status).toBe('completed'); + expect(prompts.some(p => p.includes('resumed in terse style'))).toBe(true); + }); +}); + +// --------------------------------------------------------------------------- +// Runtime `returns:` rebinding on a `workflow:` sub-run (#2470) +// --------------------------------------------------------------------------- + +describe('workflow: returns rebinds the child terminal output (#2470)', () => { + let cwd: string; + const originalArchonHome = process.env.ARCHON_HOME; + + async function writeWorkflow(name: string, yaml: string): Promise { + await writeFile(join(cwd, '.archon', 'workflows', `${name}.yaml`), yaml); + } + + async function discover(name: string): Promise { + const result = await discoverWorkflows(cwd, { loadDefaults: false }); + const wf = result.workflows.find(w => w.workflow.name === name); + if (!wf) throw new Error(`workflow ${name} not found: ${JSON.stringify(result.errors)}`); + return wf.workflow; + } + + beforeEach(async () => { + cwd = join(tmpdir(), `subret-${Date.now()}-${Math.random().toString(36).slice(2)}`); + await mkdir(join(cwd, '.archon', 'workflows'), { recursive: true }); + process.env.ARCHON_HOME = join(cwd, 'home'); + }); + + afterEach(async () => { + await rm(cwd, { recursive: true, force: true }).catch(() => {}); + if (originalArchonHome === undefined) delete process.env.ARCHON_HOME; + else process.env.ARCHON_HOME = originalArchonHome; + }); + + it('threads the declared returns node output, not the last sink', async () => { + // `summary` is the declared return; `cleanup` is the positional sink that would + // otherwise supply the terminal output. + await writeWorkflow( + 'child-returns', + ` +name: child-returns +description: declares a non-sink return node +returns: summary +nodes: + - id: summary + bash: echo "THE-SUMMARY" + - id: cleanup + bash: echo "THE-CLEANUP" + depends_on: [summary] +` + ); + await writeWorkflow( + 'parent-returns', + ` +name: parent-returns +description: consumes the child's declared return +nodes: + - id: sub + workflow: child-returns +` + ); + + const store = new InMemoryStore(); + const result = await executeWorkflow( + makeDeps(store), + makePlatform(), + 'conv-plat', + cwd, + await discover('parent-returns'), + 'goal', + 'conv-db' + ); + + expect(result.success).toBe(true); + const parentRun = [...store.runs.values()].find(r => r.workflow_name === 'parent-returns'); + const subCompleted = store.events.find( + e => + e.workflow_run_id === parentRun?.id && + e.event_type === 'node_completed' && + e.step_name === 'sub' + ); + expect(String(subCompleted?.data?.node_output)).toContain('THE-SUMMARY'); + expect(String(subCompleted?.data?.node_output)).not.toContain('THE-CLEANUP'); + }); +}); diff --git a/packages/workflows/src/workflow-inputs.ts b/packages/workflows/src/workflow-inputs.ts new file mode 100644 index 0000000000..882d81b827 --- /dev/null +++ b/packages/workflows/src/workflow-inputs.ts @@ -0,0 +1,105 @@ +/** + * The declared-input contract (#2470), shared by both call surfaces. + * + * A workflow's `inputs:` block is one contract with two callers: + * - `include:` resolves it at LOAD time (include-expander.ts), splicing values into + * `$INPUTS.` before the DAG is ever persisted; + * - `workflow:` resolves it at RUN time (executor.ts), persisting the resolved map to + * the child's `metadata.inputs`. + * + * They must agree — a `with:` map accepted by one and rejected by the other would make + * the same block behave differently depending on how it was called. This module is the + * single implementation both go through, so parity is structural rather than a comment + * asking two files to stay in sync. + * + * Semantics (identical on both surfaces): a workflow that declares NO `inputs:` keeps + * Phase-1 passthrough (the caller's map is forwarded verbatim). One that declares + * `inputs:` applies each spec's `default` for an omitted name, rejects an unsupplied + * `required` input, and rejects a caller key the workflow does not declare. + */ +import type { WorkflowDefinition } from './schemas/workflow'; + +/** A workflow's declared `inputs:` block, or undefined when it declares none. */ +export type DeclaredInputs = WorkflowDefinition['inputs']; + +/** Thrown when a caller's `with:` map violates the callee's declared contract. */ +export class WorkflowInputContractError extends Error { + constructor(message: string) { + super(message); + this.name = 'WorkflowInputContractError'; + } +} + +/** Render `'a', 'b'` for an error message, sorted for a deterministic string. */ +function quoteNames(names: string[]): string { + return names + .slice() + .sort() + .map(n => `'${n}'`) + .join(', '); +} + +/** + * Resolve a caller's supplied map against a callee's declared `inputs:`. + * + * @param supplied - the caller's `with:` values, already substituted to concrete strings. + * @param declared - the callee's `inputs:` block (undefined ⇒ Phase-1 passthrough). + * @param context - message prefix identifying the call site, e.g. `Node 'review'`. + * @param calleeLabel - how the callee is named in errors, e.g. `included block 'blk'`. + * @throws WorkflowInputContractError on an undeclared key or a missing required input. + */ +export function resolveDeclaredInputs( + supplied: Record, + declared: DeclaredInputs, + context: string, + calleeLabel: string +): Record { + if (declared === undefined) return supplied; + + // Reject caller keys the callee does not declare. Checked before defaults so a typo + // ('stlye' for 'style') fails loudly instead of silently taking the default. + const undeclared = Object.keys(supplied).filter(k => !Object.hasOwn(declared, k)); + if (undeclared.length > 0) { + throw new WorkflowInputContractError( + `${context}: ${calleeLabel} does not declare input${undeclared.length === 1 ? '' : 's'} ${quoteNames(undeclared)}. Declared inputs: ${Object.keys(declared).sort().join(', ') || '(none)'}.` + ); + } + + const resolved: Record = {}; + const missingRequired: string[] = []; + for (const [name, spec] of Object.entries(declared)) { + if (Object.hasOwn(supplied, name)) { + resolved[name] = supplied[name]; + } else if (spec.default !== undefined) { + resolved[name] = spec.default; + } else if (spec.required === true) { + missingRequired.push(name); + } + // Declared, not supplied, not required, no default: omitted. A body that references + // `$INPUTS.` fails at the reference site rather than here. + } + if (missingRequired.length > 0) { + throw new WorkflowInputContractError( + `${context}: ${calleeLabel} requires input${missingRequired.length === 1 ? '' : 's'} ${quoteNames(missingRequired)}. Pass ${missingRequired.length === 1 ? 'it' : 'them'} through 'with:'.` + ); + } + return resolved; +} + +/** + * Declared inputs that a run carries with no caller at all — the defaults of a workflow + * started directly (CLI / chat / web), which has no parent to stamp `metadata.inputs`. + * + * Without this a top-level run of a workflow whose `inputs:` are all defaulted would throw + * on its own `$INPUTS.` references, while the identical workflow invoked as a + * `workflow:` child would resolve them. Required inputs are NOT synthesized here: a bare + * run supplies nothing, so the reference fails at its use site with the normal message. + */ +export function defaultRunInputs(declared: DeclaredInputs): Record | undefined { + if (declared === undefined) return undefined; + const defaults: Record = {}; + for (const [name, spec] of Object.entries(declared)) { + if (spec.default !== undefined) defaults[name] = spec.default; + } + return Object.keys(defaults).length > 0 ? defaults : undefined; +}