diff --git a/.archon/workflows/defaults/archon-workflow-builder.yaml b/.archon/workflows/defaults/archon-workflow-builder.yaml index 49f129be0c..82efe49cc1 100644 --- a/.archon/workflows/defaults/archon-workflow-builder.yaml +++ b/.archon/workflows/defaults/archon-workflow-builder.yaml @@ -144,7 +144,7 @@ nodes: # Use for deterministic data transforms the shell would mangle (JSON parsing, etc.) script: | // JSON is valid JS expression syntax — assign directly (String.raw breaks on backticks) - const data = $other-node.output; + const data = $.output; console.log(JSON.stringify({ count: data.items.length })); runtime: bun # required: 'bun' (.ts/.js) or 'uv' (.py) # deps: [requests] # uv only @@ -199,10 +199,10 @@ nodes: 5. Use `bash` nodes for deterministic shell operations (file checks, git commands, installs) 6. Use `script` nodes for typed data transforms (TypeScript JSON parsing, Python with deps) — stdout is captured as output, stderr is forwarded as a warning. - `$nodeId.output` is NOT shell-quoted in script bodies. - - **TypeScript/bun**: assign directly — `const data = $nodeId.output;` + `$.output` is NOT shell-quoted in script bodies. + - **TypeScript/bun**: assign directly — `const data = $.output;` (JSON is valid JS expression syntax; avoid String.raw — it breaks on backticks) - - **Python/uv**: use json.loads — `import json; data = json.loads("""$nodeId.output""")` + - **Python/uv**: use json.loads — `import json; data = json.loads("""$.output""")` Never interpolate into shell syntax. 7. Use `prompt` nodes for AI reasoning tasks 8. Use `approval` nodes to pause for human review at risky gates (plan→execute boundary, destructive actions) diff --git a/packages/docs-web/src/content/docs/book/quick-reference.md b/packages/docs-web/src/content/docs/book/quick-reference.md index 6c67e84f00..f15aa39061 100644 --- a/packages/docs-web/src/content/docs/book/quick-reference.md +++ b/packages/docs-web/src/content/docs/book/quick-reference.md @@ -158,8 +158,9 @@ All nodes share these base fields: | Field | Required | Type | Description | |-------|----------|------|-------------| | `input` | No | string | Data string forwarded as the child's `$ARGUMENTS`. Substituted like a `prompt:` body (`$nodeId.output`, workflow variables) | +| `with` | No | object | Named string values forwarded as the child's `$INPUTS`. Keys must be valid input identifiers. Mutually exclusive with `input` | | `isolation` | No | `'inherit' \| 'worktree'` | Which checkout the child runs in. Default (and `'inherit'`) shares the parent's. `'worktree'` gives the child its own worktree + branch — opt-in only, never inferred, and it fails the node rather than falling back to the shared checkout when a worktree can't be created (folder projects, surfaces with no resolver) | -| `fan_out` | No | object | Run 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), `join` (default `all_done`), `as` (reserved, rejected at load). Every child runs to its own terminal state; none cancels another | +| `fan_out` | No | object | Run 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), `join` (default `all_done`), `as` (names the item as `$INPUTS.` and must not collide with `with`). Every child runs to its own terminal state; none cancels another | `retry` is rejected on `workflow:` nodes, and `workflow:` is rejected inside a `loop_group` body. The child's terminal output threads back as `$nodeId.output`; a child approval gate pauses the whole tree — approve the **child** by run id and the parent auto-resumes. A child gate is the exception: it works for a 1:1 sub-run, but a child that pauses inside a `fan_out:` expansion **fails the node** instead — a parent has one approval slot and cannot hand it to N children, so gate before or after the fan-out node rather than inside a child of it. diff --git a/packages/docs-web/src/content/docs/guides/authoring-commands.md b/packages/docs-web/src/content/docs/guides/authoring-commands.md index 04f099a042..922a0d5276 100644 --- a/packages/docs-web/src/content/docs/guides/authoring-commands.md +++ b/packages/docs-web/src/content/docs/guides/authoring-commands.md @@ -26,7 +26,7 @@ A command is a **markdown file** that serves as a detailed instruction set for a ## File Format -Shared commands live in `.archon/commands/` relative to the working directory and are loaded at runtime. A packaged workflow keeps its commands beside the YAML in `/commands/`; those references resolve only inside the owning package and do not fall back to shared commands. +Shared commands live in `.archon/commands/` relative to the working directory. Ordinary command nodes load them at runtime. When a workflow is composed through `include:`, Archon resolves and snapshots its command bodies during load-time composition so references and declared inputs can be validated before the nodes join the parent's DAG. A packaged workflow keeps its commands beside the YAML in `/commands/`; those references resolve only inside the owning package and do not fall back to shared commands. > **`defaults/` is maintainer-territory:** `.archon/commands/defaults/` is reserved for commands shipped with Archon itself (embedded into the binary at build time). For your own commands use an owning workflow package's `commands/`, `.archon/commands/` (project-scoped), or `~/.archon/commands/` (home-scoped). Every file under `defaults/` must be committed in git — `bun run validate` will error if untracked files are found there. 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 f1e71ddc68..c095b34cfe 100644 --- a/packages/docs-web/src/content/docs/guides/authoring-workflows.md +++ b/packages/docs-web/src/content/docs/guides/authoring-workflows.md @@ -948,13 +948,17 @@ written the nodes by hand. There is no separate child run. `id: review` yields `review__verify-pr-base`, `review__sync`, `review__implement-fixes`, and so on. These namespaced ids are what appear in the event stream and in `archon workflow get `. -- **Edges.** Internal `depends_on` edges and `$id.output` references in inline node text are - rewired to the namespaced ids automatically. Named `command:` and `loop.command` files - remain external and cannot be rewritten; when a readable command body references a - top-level block node whose id will be namespaced, workflow loading fails. This best-effort - scan includes nested `loop_group` bodies; unresolved files warn and are skipped. The include - node's own `depends_on` / `when` / `trigger_rule` attach to the block's **entry** nodes - (those with no upstream inside the block). +- **Edges and command bodies.** Internal `depends_on` edges and `$id.output` references are + rewired to the namespaced ids automatically. This includes named `command:` and + `loop.command` files: discovery resolves their bodies and compiles them into the flat DAG + before namespacing. Compilation recurses through nested `loop_group` bodies. An unresolved + included command cannot start a fresh execution because its references cannot be proven safe. + Command nodes fail composition immediately; loop commands retain a private compilation error + so a paused loop can still resume from its persisted, validated prompt snapshot. + The include node's own `depends_on` / `when` / `trigger_rule` attach to the block's + **entry** nodes (those with no upstream inside the block). If both the include and an entry + define `when:` and either condition contains `||`, loading fails because the grammar cannot + group them without changing precedence; put the gate only on the include or inside the block. - **Sink asymmetry (a downstream node depending on the include).** A `depends_on: []` on a downstream node fans out to **all** of the block's sink nodes (every node with no dependents inside the block), so it waits for the whole block to finish. @@ -1006,21 +1010,28 @@ Substitution applies everywhere the value could reach the model or the shell, in inside Markdown code fences and inline code spans — `$INPUTS.` has no documentation-only meaning, so a fenced occurrence is still a live parameter. -#### Command bodies cannot use include inputs - -Phase 1 cannot parameterize a `command:` file or `loop.command` file used by an included -block. Command bodies are read at execution time, after load-time include expansion has -finished. When such a file can be read at load time and contains `$INPUTS.` anywhere — -including inside a code fence — workflow loading fails with a message directing you to inline -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 `command:` and -`loop.command` files throughout the block, including nested `loop_group` bodies. 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:` -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)). +#### Command bodies use the same explicit interface + +Named `command:` and `loop.command` files are the preferred home for substantial prompts and +can use both workflow-local `$node.output` references and declared `$INPUTS.` values. +For an `include:`, Archon resolves and snapshots the command body during load-time composition, +then applies the same input binding and node-id namespacing as an inline prompt. The authored +workflow stays command-first; the executor receives a deterministic flat DAG. + +Every live reference in the command body must belong to the included workflow's lexical node +scope. A direct `$caller.output` reference is rejected whether or not the parent happens to +have a node called `caller`; declare an input and pass it with `with:` instead. Failure to +resolve or read an included command is never a warning or best-effort bypass: a fresh execution +fails before an AI turn. A paused loop remains resumable from its saved prompt snapshot even if +the command is later deleted or made invalid. +Canonical references are live even inside Markdown code fences and inline code because runtime +substitution is syntax-agnostic. + +Named `script:` files are different: they are opaque programs, not prompt templates. Archon +does not scan or rewrite their source. An `include:` may bind `$INPUTS` in the YAML `script:` +selector, but flattening does not inject those values into the selected program's environment. +The documented `INPUTS_` environment variables apply to `workflow:` sub-runs, +whose concrete inputs are persisted in child-run metadata. ### Non-goals (Phase 1) @@ -1088,12 +1099,13 @@ 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 | +| `include:` | **Load time** (the block and command bodies are compiled into the flat DAG) | Yes | **Yes** — resolved command bodies receive the same input binding before execution | | `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. +Both composition paths support command-backed prompts; their binding time differs. Includes +snapshot and bind the resolved command body at load time, while sub-runs resolve inputs at +runtime. Sub-run inputs are persisted to the child run's metadata at spawn, so `$INPUTS` +reconstitutes on a cold resume. ### `$INPUTS` in `bash:`/`script:` nodes uses env vars @@ -1392,7 +1404,7 @@ can line results up against the input list positionally. | Field | Default | What it does | |-------|---------|--------------| | `items` | required | A `$node.output` (or `$node.output.field`) reference that must resolve to a **JSON array** at run time. Anything else — an object, a bare string, malformed JSON, a dangling ref — fails the node before any child is created. It never fans out over the characters of a string, and never silently degrades to zero items. An empty array is legal: the node completes immediately with `[]`. | -| `as` | — | Reserved for a future `$INPUTS.` channel ([#2214](https://github.com/coleam00/Archon/issues/2214)) and **rejected at load** until then, rather than accepted and ignored — writing `as: task` and then `$INPUTS.task` in the child would otherwise deliver the literal string to the model. The item reaches the child as `$ARGUMENTS`. | +| `as` | — | Names the current item as `$INPUTS.` in each child. It must not collide with a `with:` key. The item also reaches the child as `$ARGUMENTS`. | | `max_parallel` | `5` | How many children may be **in flight at once**. | | `join` | `all_done` | How N child outcomes reduce to one node outcome (below). | @@ -1605,8 +1617,8 @@ re-deriving it. ### Non-goals (this slice) -- **No `with:` named-parameter mapping** — use `input:` (a single data string). A - `workflow:` node with a `with:` key is rejected with a clear error. +- **Choose one input form** — `input:` sends a single data string as `$ARGUMENTS`; + `with:` supplies named `$INPUTS` values. The two forms are mutually exclusive. - **No racing** (`join: first_success`) — rejected outright, not deferred (see [Why there is no racing join](#why-there-is-no-racing-join)). - **Not inside a `loop_group` body** — a `workflow:` node, fanned out or not, is rejected there at load time ([#2439](https://github.com/coleam00/Archon/issues/2439)). diff --git a/packages/docs-web/src/content/docs/guides/loop-nodes.md b/packages/docs-web/src/content/docs/guides/loop-nodes.md index 255a0eb1a9..dc1456bc9d 100644 --- a/packages/docs-web/src/content/docs/guides/loop-nodes.md +++ b/packages/docs-web/src/content/docs/guides/loop-nodes.md @@ -127,12 +127,20 @@ owning workflow's `commands/` directory for packaged workflows or in `.archon/commands/` for legacy workflows, the same way it does for `command:` nodes. -The file is **read once per run** — loaded when the loop node starts and -reused for every iteration, including across interactive-gate pauses: the -loaded text is persisted with the pause, so editing or deleting the file while -a run sits paused neither changes nor breaks the resumed loop's prompt. A -missing, empty, or unreadable target fails the node immediately with an -actionable error — no iterations execute. +For an ordinary workflow, the file is **read once per run** when the loop node +starts. For a workflow composed through `include:`, Archon resolves and compiles +the command body during load-time composition so its node references and declared +inputs are proven before the child joins the parent's flat DAG. A matching file +that is unreadable fails closed; Archon never falls through to a lower-precedence +command with the same name. A missing, empty, unreadable, or non-hermetic included +command fails before a fresh AI turn. + +For any interactive loop — whether authored with `prompt` or `command` — Archon +persists the resolved prompt template at the gate. A resumed run prefers that +snapshot, so editing inline YAML or a command source while the run is paused does +not change its prompt; deleting a command source does not break that resume either. +Source edits affect fresh runs, which still require successful command resolution +or compilation. Once loaded, the text behaves identically to an inline `prompt`: all the variable substitution above applies unchanged (including `$LOOP_PREV_OUTPUT` diff --git a/packages/workflows/src/compiled-command.ts b/packages/workflows/src/compiled-command.ts new file mode 100644 index 0000000000..1da48d0d2b --- /dev/null +++ b/packages/workflows/src/compiled-command.ts @@ -0,0 +1,25 @@ +/** Engine-private included-loop compilation metadata. Symbols survive object spreads + * but stay out of YAML, JSON, API payloads, and persisted workflow definitions. */ +export const COMPILED_LOOP_COMMAND = Symbol('archon.compiled-loop-command'); + +export type CompiledLoopCommand = + | { prompt: string; error?: never } + | { prompt?: never; error: string }; + +export interface LoopWithCompiledCommand { + [COMPILED_LOOP_COMMAND]?: CompiledLoopCommand; +} + +export interface IncludeCommandReadError { + path: string; + message: string; + operation: 'inspect' | 'read'; +} + +export type IncludeCommandContent = string | null | IncludeCommandReadError; + +export function isIncludeCommandReadError( + value: IncludeCommandContent | undefined +): value is IncludeCommandReadError { + return typeof value === 'object' && value !== null; +} diff --git a/packages/workflows/src/dag-executor.test.ts b/packages/workflows/src/dag-executor.test.ts index 45eb3620fe..6721a66269 100644 --- a/packages/workflows/src/dag-executor.test.ts +++ b/packages/workflows/src/dag-executor.test.ts @@ -84,6 +84,11 @@ import { dagNodeSchema } from './schemas'; import { discoverWorkflows } from './workflow-discovery'; import { parseWorkflow } from './loader'; import { expandWorkflowIncludes } from './include-expander'; +import { + COMPILED_LOOP_COMMAND, + type CompiledLoopCommand, + type LoopWithCompiledCommand, +} from './compiled-command'; import { OutputRefError } from './output-ref'; import type { WorkflowDeps, IWorkflowPlatform, WorkflowConfig } from './deps'; import type { IWorkflowStore } from './store'; @@ -820,9 +825,9 @@ describe('substituteNodeOutputRefs', () => { it('unknown node ref WITH a field throws (no-silent-drop, unknown-node)', () => { // The whole-text `$missing.output` form stays lenient ('' — see test above), but a - // `.field` ref to an unknown id is a typo the load-time validator can't always see - // (bash/script/approval/cancel + command-file refs aren't scanned). It must fail the - // consuming node loudly, matching known-producer strict-field posture. + // `.field` ref to an unknown id can still reach the executor through a programmatically + // constructed definition that bypasses discovery. It must fail the consuming node + // loudly, matching known-producer strict-field posture. const outputs = new Map([['analyze', makeOutput('completed', '{"type":"BUG"}')]]); let caught: unknown; try { @@ -8469,6 +8474,249 @@ describe('executeDagWorkflow -- resume with priorCompletedNodes', () => { expect(failed).toHaveLength(0); }); + it('reuses the pause-time prompt snapshot after a composed loop prompt is rediscovered', async () => { + mockSendQueryDag.mockImplementation(function* () { + yield { type: 'assistant', content: 'iteration output, no signal yet' }; + yield { type: 'result', sessionId: 'sid-prompt-1' }; + }); + + const platform = createMockPlatform(); + const firstDeps = createMockDeps(); + const blockWorkflow = { + name: 'materialized-loop-block', + description: 'Command-backed loop block', + nodes: [ + { + id: 'gated-loop', + loop: { + command: 'materialized-loop-command', + until: 'COMPLETE', + max_iterations: 5, + interactive: true, + gate_message: 'Review materialized prompt.', + }, + } satisfies DagNode, + ], + } satisfies WorkflowDefinition; + const parentWorkflow = { + name: 'materialized-loop-gated', + description: 'Includes the command-backed loop', + nodes: [{ id: 'included', include: 'materialized-loop-block' } satisfies DagNode], + } satisfies WorkflowDefinition; + const workflowDir = join(testDir, '.archon', 'workflows'); + const commandPath = join(testDir, '.archon', 'commands', 'materialized-loop-command.md'); + await mkdir(workflowDir, { recursive: true }); + await Promise.all([ + writeFile(join(workflowDir, 'materialized-block.yaml'), JSON.stringify(blockWorkflow)), + writeFile(join(workflowDir, 'materialized-parent.yaml'), JSON.stringify(parentWorkflow)), + writeFile(commandPath, 'ORIGINAL materialized command. USER=<<$LOOP_USER_INPUT>>'), + ]); + const firstDiscovery = await discoverWorkflows(testDir, { loadDefaults: false }); + expect(firstDiscovery.errors).toHaveLength(0); + const originalWorkflow = firstDiscovery.workflows.find( + item => item.workflow.name === parentWorkflow.name + )?.workflow; + expect(originalWorkflow).toBeDefined(); + + await executeDagWorkflow( + firstDeps, + platform, + 'conv-dag', + testDir, + originalWorkflow!, + makeWorkflowRun(), + 'claude', + undefined, + join(testDir, 'artifacts'), + join(testDir, 'state'), + join(testDir, 'logs'), + 'main', + 'docs/', + minimalConfig + ); + + const pauseCalls = ( + firstDeps.store.pauseWorkflowRun as Mock< + (id: string, ctx: Record) => Promise + > + ).mock.calls; + const pausedContext = pauseCalls[0]?.[1] as Record; + expect(pausedContext.commandSnapshot).toContain('ORIGINAL materialized command.'); + + mockSendQueryDag.mockClear(); + mockSendQueryDag.mockImplementation(function* () { + yield { type: 'assistant', content: 'refined. COMPLETE' }; + yield { type: 'result', sessionId: 'sid-prompt-2' }; + }); + // Cold filesystem rediscovery after the source command was deleted still returns the + // workflow. Its engine-private compilation error blocks a fresh run, while + // this resumed run can reach and reuse the persisted snapshot. + unlinkSync(commandPath); + const rediscovery = await discoverWorkflows(testDir, { loadDefaults: false }); + expect(rediscovery.errors).toHaveLength(0); + const rediscoveredWorkflow = rediscovery.workflows.find( + item => item.workflow.name === parentWorkflow.name + )?.workflow; + expect(rediscoveredWorkflow).toBeDefined(); + mockSendQueryDag.mockClear(); + const freshStore = createMockStore(); + await executeDagWorkflow( + createMockDeps(freshStore), + platform, + 'conv-dag', + testDir, + rediscoveredWorkflow!, + makeWorkflowRun('fresh-invalid-command-run'), + 'claude', + undefined, + join(testDir, 'artifacts'), + join(testDir, 'state'), + join(testDir, 'logs'), + 'main', + 'docs/', + minimalConfig + ); + expect(mockSendQueryDag).not.toHaveBeenCalled(); + const freshFailures = ( + freshStore.createWorkflowEvent as ReturnType + ).mock.calls.filter( + (call: unknown[]) => (call[0] as Record).event_type === 'node_failed' + ); + expect(freshFailures).toHaveLength(1); + + const resumedRun = makeWorkflowRun('materialized-resume-run', { + metadata: { + approval: { ...pausedContext }, + loop_user_input: 'tighten the summary', + loop_feedback_given: true, + }, + }); + + mockSendQueryDag.mockClear(); + await executeDagWorkflow( + createMockDeps(createMockStore()), + platform, + 'conv-dag', + testDir, + rediscoveredWorkflow!, + resumedRun, + 'claude', + undefined, + join(testDir, 'artifacts'), + join(testDir, 'state'), + join(testDir, 'logs'), + 'main', + 'docs/', + minimalConfig + ); + + const resumedPrompt = mockSendQueryDag.mock.calls[0]?.[0] as string; + expect(resumedPrompt).toContain('ORIGINAL materialized command.'); + expect(resumedPrompt).toContain('USER=<>'); + expect(resumedPrompt).not.toContain('could not be resolved'); + }); + + it('fails before provider invocation when compiled loop metadata is malformed', async () => { + const loop: Extract['loop'] & LoopWithCompiledCommand = { + command: 'malformed-compiled-command', + until: 'DONE', + max_iterations: 1, + }; + loop[COMPILED_LOOP_COMMAND] = {} as CompiledLoopCommand; + const store = createMockStore(); + + await executeDagWorkflow( + createMockDeps(store), + createMockPlatform(), + 'conv-dag', + testDir, + { + name: 'malformed-compiled-command-workflow', + nodes: [{ id: 'repeat', loop }], + }, + makeWorkflowRun('malformed-compiled-command-run'), + 'claude', + undefined, + join(testDir, 'artifacts'), + join(testDir, 'state'), + join(testDir, 'logs'), + 'main', + 'docs/', + minimalConfig + ); + + expect(mockSendQueryDag).not.toHaveBeenCalled(); + const failures = (store.createWorkflowEvent as ReturnType).mock.calls.filter( + (call: unknown[]) => (call[0] as Record).event_type === 'node_failed' + ); + expect(failures).toHaveLength(1); + expect((failures[0]?.[0] as { data: { error: string } }).data.error).toContain( + 'malformed compiled command metadata' + ); + }); + + it('keeps a whitespace-only included loop discoverable but fails a fresh run', async () => { + const workflowDir = join(testDir, '.archon', 'workflows'); + await mkdir(workflowDir, { recursive: true }); + await Promise.all([ + writeFile( + join(workflowDir, 'empty-loop-block.yaml'), + JSON.stringify({ + name: 'empty-loop-block', + description: 'Whitespace loop command block', + nodes: [ + { + id: 'repeat', + loop: { command: 'empty-included-loop', until: 'DONE', max_iterations: 1 }, + }, + ], + }) + ), + writeFile( + join(workflowDir, 'empty-loop-parent.yaml'), + JSON.stringify({ + name: 'empty-loop-parent', + description: 'Includes whitespace loop command block', + nodes: [{ id: 'inc', include: 'empty-loop-block' }], + }) + ), + writeFile(join(testDir, '.archon', 'commands', 'empty-included-loop.md'), ' \n\t'), + ]); + const discovery = await discoverWorkflows(testDir, { loadDefaults: false }); + expect(discovery.errors).toHaveLength(0); + const workflow = discovery.workflows.find( + item => item.workflow.name === 'empty-loop-parent' + )?.workflow; + expect(workflow).toBeDefined(); + const store = createMockStore(); + + await executeDagWorkflow( + createMockDeps(store), + createMockPlatform(), + 'conv-dag', + testDir, + workflow!, + makeWorkflowRun('empty-included-loop-run'), + 'claude', + undefined, + join(testDir, 'artifacts'), + join(testDir, 'state'), + join(testDir, 'logs'), + 'main', + 'docs/', + minimalConfig + ); + + expect(mockSendQueryDag).not.toHaveBeenCalled(); + const failures = (store.createWorkflowEvent as ReturnType).mock.calls.filter( + (call: unknown[]) => (call[0] as Record).event_type === 'node_failed' + ); + expect(failures).toHaveLength(1); + expect((failures[0]?.[0] as { data: { error: string } }).data.error).toContain( + "command 'empty-included-loop' is empty" + ); + }); + it('closes the loop lifecycle with exactly one node_failed on max-iterations exhaustion', async () => { // Failure finalizer contract: every failed exit after node_started goes // through one finalizer — exactly one node_failed row per started loop @@ -16813,7 +17061,7 @@ describe('executeDagWorkflow -- provider-boundary session threading (#1992)', () // top-level nodes for events, terminal-output selection, resume-skip, and always_run. // --------------------------------------------------------------------------- -describe('executeDagWorkflow -- include expansion (zero runtime machinery)', () => { +describe('executeDagWorkflow -- flattened include expansion', () => { let testDir: string; beforeEach(async () => { diff --git a/packages/workflows/src/dag-executor.ts b/packages/workflows/src/dag-executor.ts index 97df87d8ca..1e11a28e12 100644 --- a/packages/workflows/src/dag-executor.ts +++ b/packages/workflows/src/dag-executor.ts @@ -86,6 +86,7 @@ import { } from './output-ref'; import { buildTruncationMarker } from './utils/output-truncation'; import { writeNodeArtifact, readNodeArtifacts } from './artifacts-index'; +import { COMPILED_LOOP_COMMAND, type LoopWithCompiledCommand } from './compiled-command'; import { logNodeStart, logNodeComplete, @@ -760,10 +761,11 @@ function shellQuoteOrFile( * Substitute $node_id.output and $node_id.output.field references in a prompt. * Called AFTER the standard substituteWorkflowVariables pass. * - * KEEP IN SYNC (three ref-surface enumerations must agree): the fields this is called on - * (search call sites below), the loader's validateDagStructure scan (which validates the - * same refs), and rewriteNodeOutputRefs in include-expander.ts (which renames them on - * inline). Adding a substituted field to one means updating all three. + * KEEP IN SYNC: public YAML call sites, the loader's validateDagStructure scan, and + * rewriteNodeOutputRefs must cover the same runtime node-ref surfaces. Included + * loop-command bodies are validated separately during materialization. applyInputsMacro is + * intentionally a superset because some AI configuration strings accept include inputs + * without receiving runtime node-output substitution. * * @param escapedForBash - When true, wraps substituted values in single quotes so * they are safe to embed in bash scripts passed to `bash -c`. Set true only for @@ -780,10 +782,10 @@ export function substituteNodeOutputRefs( (match, nodeId: string, field: string | undefined) => { const nodeOutput = nodeOutputs.get(nodeId); if (!nodeOutput) { - // A `.field` ref that resolves to no output (a typo the load-time validator - // can't always see — refs in bash/script/approval/cancel fields and inside - // command-file content aren't scanned — or a real node that hasn't run before - // this reference) fails the consuming node loudly, matching the strict + // A `.field` ref that resolves to no output (for example, a programmatically + // constructed definition that bypassed discovery, or a real producer whose + // output is unavailable on this execution path) fails the consuming node loudly, + // matching the strict // no-silent-drop posture for known-producer field access below. The whole-text // `$id.output` form stays lenient ('') as a long-documented surface (changing // it is a bigger compatibility break). @@ -4190,23 +4192,36 @@ async function executeLoopNode( return { state: 'completed', output: finalizeOutput, sessionId: currentSessionId }; } - // Resolve the iteration prompt source. `loop.prompt` is used directly; - // `loop.command` is read ONCE per run/node: the first invocation loads the - // command file, and the interactive gate persists the loaded text - // (`commandSnapshot` in the pause context) so a resumed invocation reuses the - // snapshot instead of re-reading — a command file edited or deleted while the - // run sat paused at a gate can neither change nor break the running loop's - // prompt. The schema guarantees exactly one of prompt/command is defined. + // Resolve the iteration prompt source once per run/node. The interactive gate + // persists the resolved template (`commandSnapshot` in the pause context) for + // both inline and command-backed loops. Included loops retain their command identity + // plus a load-time compiled prompt/error; rediscovery after a pause cannot change their + // running prompt because a persisted snapshot takes precedence over that metadata. + // The schema guarantees exactly one of prompt/command is defined. let loopPromptTemplate: string; - if (typeof loop.prompt === 'string') { - loopPromptTemplate = loop.prompt; + if (isLoopResume && typeof loopGateMeta?.commandSnapshot === 'string') { + loopPromptTemplate = loopGateMeta.commandSnapshot; } else if (typeof loop.command === 'string') { - if (isLoopResume && typeof loopGateMeta?.commandSnapshot === 'string') { - loopPromptTemplate = loopGateMeta.commandSnapshot; + const compiled = (loop as typeof loop & LoopWithCompiledCommand)[COMPILED_LOOP_COMMAND]; + const hasCompiledError = compiled !== undefined && typeof compiled.error === 'string'; + const hasCompiledPrompt = compiled !== undefined && typeof compiled.prompt === 'string'; + if (hasCompiledError && !hasCompiledPrompt) { + getLog().error( + { nodeId: node.id, command: loop.command, error: compiled.error }, + 'loop_node.command_compilation_failed' + ); + return failLoopNode(compiled.error, { data: { command: loop.command } }); + } + if (hasCompiledPrompt && !hasCompiledError) { + loopPromptTemplate = compiled.prompt; + } else if (compiled !== undefined) { + const errorMsg = `Loop node '${node.id}' has malformed compiled command metadata for '${loop.command}' — expected exactly one string prompt or error.`; + getLog().error( + { nodeId: node.id, command: loop.command, compiled }, + 'loop_node.command_compilation_metadata_invalid' + ); + return failLoopNode(errorMsg, { data: { command: loop.command } }); } else { - // Fresh execution — or a resume of a run paused under a build that - // predates commandSnapshot: fall back to a fresh read (documented, - // fail-safe) rather than failing an otherwise-valid resume. const promptResult = await loadCommandPrompt( deps, cwd, @@ -4218,12 +4233,12 @@ async function executeLoopNode( { nodeId: node.id, command: loop.command, error: promptResult.message }, 'loop_node.command_load_failed' ); - // The failing command name travels on the node_failed payload so the - // event stream carries the same context as the structured log. return failLoopNode(promptResult.message, { data: { command: loop.command } }); } loopPromptTemplate = promptResult.content; } + } else if (typeof loop.prompt === 'string') { + loopPromptTemplate = loop.prompt; } else { // Unreachable: superRefine on loopNodeConfigSchema enforces exactly-one. throw new Error( @@ -5083,10 +5098,10 @@ async function executeLoopNode( // Usage consumed up to this gate, so a bare approve (finalize, no re-run) // can persist it on node_completed instead of reporting nothing (#2333). signaledTokens: completionDetected ? (loopTotalTokens ?? null) : null, - // Read-once command body for command-backed loops: the resumed invocation - // reuses this snapshot instead of re-reading the file (explicit null for - // prompt-based loops — same json_patch convention as `sessionId`). - commandSnapshot: typeof loop.command === 'string' ? loopPromptTemplate : null, + // Read-once resolved template for both prompt- and command-backed loops. + // Included command-backed loops use their load-time compiled body here, so + // snapshotting both forms preserves resume determinism after source deletion. + commandSnapshot: loopPromptTemplate, }); // Return completed — the between-layer status check sees 'paused' and halts cleanly. // This mirrors the approval-node pattern, preventing false "DAG nodes failed" warnings diff --git a/packages/workflows/src/defaults/bundled-defaults.generated.ts b/packages/workflows/src/defaults/bundled-defaults.generated.ts index 1c86a945a8..4062ebd95b 100644 --- a/packages/workflows/src/defaults/bundled-defaults.generated.ts +++ b/packages/workflows/src/defaults/bundled-defaults.generated.ts @@ -86,7 +86,7 @@ export const BUNDLED_WORKFLOWS: Record = { "archon-smart-pr-review": "name: archon-smart-pr-review\ndescription: |\n Use when: User wants a smart, efficient PR review that adapts to PR complexity.\n Triggers: \"smart review\", \"review this PR\", \"review PR #123\", \"efficient review\",\n \"smart PR review\", \"quick review\".\n Does: Gathers PR scope -> classifies complexity -> routes to only relevant review agents ->\n synthesizes findings -> auto-fixes CRITICAL/HIGH issues.\n NOT for: When you explicitly want ALL review agents (use archon-comprehensive-pr-review instead).\n\n Unlike the comprehensive review, this workflow classifies the PR first and only runs\n the review agents that are relevant. A 3-line typo fix skips test-coverage and docs-impact.\n\nnodes:\n - id: scope\n command: archon-pr-review-scope\n\n - id: sync\n command: archon-sync-pr-with-main\n depends_on: [scope]\n\n - id: classify\n prompt: |\n You are a PR complexity classifier. Analyze the PR scope below and determine\n which review agents should run.\n\n ## PR Scope\n $scope.output\n\n ## Rules\n - **Code review**: Always run unless the diff is empty or only touches non-code files\n (e.g. README-only, config-only, or .yaml-only changes).\n - **Error handling**: Run if the diff touches code with try/catch, error handling,\n async/await, or adds new failure paths.\n - **Test coverage**: Run if the diff touches source code (not just tests, docs, or config).\n - **Comment quality**: Run if the diff adds or modifies comments, docstrings, JSDoc,\n or significant documentation within code files.\n - **Docs impact**: Run if the diff adds/removes/renames public APIs, commands, CLI flags,\n environment variables, or user-facing features.\n\n Classify the PR complexity:\n - **trivial**: Typo fixes, formatting, single-line changes, version bumps\n - **small**: 1-3 files, straightforward logic, no architectural changes\n - **medium**: 4-10 files, moderate logic changes, some cross-cutting concerns\n - **large**: 10+ files, architectural changes, new subsystems, complex refactors\n\n Provide your reasoning for each decision.\n depends_on: [scope]\n model: small\n allowed_tools: []\n output_format:\n type: object\n properties:\n run_code_review:\n type: string\n enum: [\"true\", \"false\"]\n run_error_handling:\n type: string\n enum: [\"true\", \"false\"]\n run_test_coverage:\n type: string\n enum: [\"true\", \"false\"]\n run_comment_quality:\n type: string\n enum: [\"true\", \"false\"]\n run_docs_impact:\n type: string\n enum: [\"true\", \"false\"]\n complexity:\n type: string\n enum: [\"trivial\", \"small\", \"medium\", \"large\"]\n reasoning:\n type: string\n required:\n - run_code_review\n - run_error_handling\n - run_test_coverage\n - run_comment_quality\n - run_docs_impact\n - complexity\n - reasoning\n\n - id: code-review\n command: archon-code-review-agent\n depends_on: [classify, sync]\n when: \"$classify.output.run_code_review == 'true'\"\n\n - id: error-handling\n command: archon-error-handling-agent\n depends_on: [classify, sync]\n when: \"$classify.output.run_error_handling == 'true'\"\n\n - id: test-coverage\n command: archon-test-coverage-agent\n depends_on: [classify, sync]\n when: \"$classify.output.run_test_coverage == 'true'\"\n\n - id: comment-quality\n command: archon-comment-quality-agent\n depends_on: [classify, sync]\n when: \"$classify.output.run_comment_quality == 'true'\"\n\n - id: docs-impact\n command: archon-docs-impact-agent\n depends_on: [classify, sync]\n when: \"$classify.output.run_docs_impact == 'true'\"\n\n - id: synthesize\n command: archon-synthesize-review\n depends_on: [code-review, error-handling, test-coverage, comment-quality, docs-impact]\n trigger_rule: one_success\n\n - id: implement-fixes\n command: archon-implement-review-fixes\n depends_on: [synthesize]\n\n # Optional: push notification when review completes.\n # To enable, create .archon/mcp/ntfy.json — see docs/mcp-servers.md\n - id: check-ntfy\n bash: \"test -f .archon/mcp/ntfy.json && echo 'true' || echo 'false'\"\n depends_on: [implement-fixes]\n\n - id: notify\n depends_on: [check-ntfy, synthesize, implement-fixes]\n when: \"$check-ntfy.output == 'true'\"\n trigger_rule: all_success\n mcp: .archon/mcp/ntfy.json\n allowed_tools: []\n prompt: |\n Send a push notification summarizing the PR review results.\n\n Review synthesis:\n $synthesize.output\n\n Fix results:\n $implement-fixes.output\n\n Send with:\n - title: \"PR Review Complete\"\n - message: 1-2 sentence summary — verdict and issue count. Short enough for a lock screen.\n - priority: 3 if ready to merge, 4 if needs fixes, 5 if critical issues remain\n", "archon-test-loop-dag": "name: archon-test-loop-dag\ndescription: |\n Use when: User explicitly says \"test-loop-dag\" or \"run test-loop-dag\".\n IMPORTANT: This is a DAG workflow with a loop node that iterates until completion.\n NOT for: General testing questions or debugging.\n Does: Initializes a counter, iterates until it reaches 3, then reports completion.\n\nnodes:\n - id: setup\n bash: |\n echo \"0\" > .archon/test-loop-dag-counter.txt\n echo \"Counter initialized to 0\"\n\n - id: loop-counter\n depends_on: [setup]\n loop:\n prompt: |\n You are testing the loop node functionality within a DAG workflow.\n\n ## Your Task\n\n 1. Read the file `.archon/test-loop-dag-counter.txt`\n 2. Parse the current counter value\n 3. Increment it by 1\n 4. Write the new value back to the file\n 5. Report the current iteration\n\n ## User Intent\n\n $USER_MESSAGE\n\n ## Completion Criteria\n\n - If the counter reaches 3 or higher, output: COMPLETE\n - Otherwise, just report your progress and end normally\n\n ## Important\n\n Be concise. Just do the task and report the counter value.\n until: COMPLETE\n max_iterations: 5\n fresh_context: false\n\n - id: report\n depends_on: [loop-counter]\n prompt: |\n The loop counter test has completed. The loop node output was:\n\n $loop-counter.output\n\n Read `.archon/test-loop-dag-counter.txt` and confirm the final counter value.\n Report: \"Test loop DAG completed successfully. Final counter: {value}\"\n", "archon-validate-pr": "name: archon-validate-pr\ndescription: |\n Use when: User wants a thorough PR validation that tests both main (bug present) and feature branch (bug fixed).\n Triggers: \"validate PR\", \"validate pr #123\", \"test this PR\", \"verify PR\", \"full PR validation\",\n \"validate pull request\", \"test PR end-to-end\".\n Does: Fetches PR info -> finds free ports -> parallel code review (main vs feature) ->\n E2E test on main (reproduce bug) -> E2E test on feature (verify fix) -> final verdict report.\n NOT for: Quick code-only reviews (use archon-smart-pr-review), fixing issues, general exploration.\n\n This workflow is designed for running in parallel — each instance finds its own free ports\n to avoid conflicts. Produces artifacts in $ARTIFACTS_DIR/ and posts a validation report.\n\nprovider: claude\nmodel: large\n\nnodes:\n # ═══════════════════════════════════════════════════════════════\n # PHASE 1: SETUP — Fetch PR info and allocate ports\n # ═══════════════════════════════════════════════════════════════\n\n - id: fetch-pr\n bash: |\n # Extract PR number from arguments\n PR_NUMBER=$(echo \"$ARGUMENTS\" | grep -oE '/pull/[0-9]+' | grep -oE '[0-9]+' | head -1)\n # Fallback: extract first number if no URL path found (e.g., \"validate PR 42\")\n if [ -z \"$PR_NUMBER\" ]; then\n PR_NUMBER=$(echo \"$ARGUMENTS\" | grep -oE '[0-9]+' | head -1)\n fi\n if [ -z \"$PR_NUMBER\" ]; then\n # Try getting PR from current branch\n PR_NUMBER=$(gh pr view --json number -q '.number' 2>/dev/null)\n fi\n\n if [ -z \"$PR_NUMBER\" ]; then\n echo \"ERROR: No PR number found in arguments: $ARGUMENTS\"\n exit 1\n fi\n\n echo \"$PR_NUMBER\" > \"$ARTIFACTS_DIR/.pr-number\"\n\n # Fetch full PR details\n gh pr view \"$PR_NUMBER\" --json number,title,body,url,headRefName,baseRefName,files,additions,deletions,changedFiles,state,author,labels,isDraft\n\n - id: find-ports\n bash: |\n # Use Bun to let the OS pick truly free ports (cross-platform: Linux, macOS, Windows)\n BACKEND_PORT=$(bun -e \"const s = Bun.serve({port: 0, fetch: () => new Response('')}); console.log(s.port); s.stop()\")\n FRONTEND_PORT=$(bun -e \"const s = Bun.serve({port: 0, fetch: () => new Response('')}); console.log(s.port); s.stop()\")\n\n echo \"$BACKEND_PORT\" > \"$ARTIFACTS_DIR/.backend-port\"\n echo \"$FRONTEND_PORT\" > \"$ARTIFACTS_DIR/.frontend-port\"\n\n echo \"BACKEND_PORT=$BACKEND_PORT\"\n echo \"FRONTEND_PORT=$FRONTEND_PORT\"\n\n - id: resolve-paths\n bash: |\n # Resolve canonical repo path (main branch) vs worktree path (feature branch)\n CANONICAL_REPO=$(git rev-parse --path-format=absolute --git-common-dir 2>/dev/null | sed 's|/\\.git$||')\n WORKTREE_PATH=$(pwd)\n FEATURE_BRANCH=$(git branch --show-current)\n\n # Get PR branch info\n PR_NUMBER=$(cat \"$ARTIFACTS_DIR/.pr-number\")\n PR_HEAD=$(gh pr view \"$PR_NUMBER\" --json headRefName -q '.headRefName')\n PR_BASE=$(gh pr view \"$PR_NUMBER\" --json baseRefName -q '.baseRefName')\n\n echo \"$CANONICAL_REPO\" > \"$ARTIFACTS_DIR/.canonical-repo\"\n echo \"$WORKTREE_PATH\" > \"$ARTIFACTS_DIR/.worktree-path\"\n echo \"$FEATURE_BRANCH\" > \"$ARTIFACTS_DIR/.feature-branch\"\n echo \"$PR_HEAD\" > \"$ARTIFACTS_DIR/.pr-head\"\n echo \"$PR_BASE\" > \"$ARTIFACTS_DIR/.pr-base\"\n\n echo \"CANONICAL_REPO=$CANONICAL_REPO\"\n echo \"WORKTREE_PATH=$WORKTREE_PATH\"\n echo \"FEATURE_BRANCH=$FEATURE_BRANCH\"\n echo \"PR_HEAD=$PR_HEAD\"\n echo \"PR_BASE=$PR_BASE\"\n depends_on: [fetch-pr]\n\n # ═══════════════════════════════════════════════════════════════\n # PHASE 2: CODE REVIEW — Parallel analysis of main vs feature\n # ═══════════════════════════════════════════════════════════════\n\n - id: code-review-main\n command: archon-validate-pr-code-review-main\n depends_on: [fetch-pr, resolve-paths]\n context: fresh\n\n - id: code-review-feature\n command: archon-validate-pr-code-review-feature\n depends_on: [fetch-pr, resolve-paths, code-review-main]\n context: fresh\n\n # ═══════════════════════════════════════════════════════════════\n # PHASE 3: E2E TESTING — Sequential (after code reviews finish)\n # ═══════════════════════════════════════════════════════════════\n\n - id: classify-testability\n prompt: |\n You are a PR testability classifier. Determine whether this PR's changes can be\n validated via browser E2E testing, or if it requires code-review-only validation.\n\n ## PR Details\n\n $fetch-pr.output\n\n ## Rules\n\n - **e2e_testable**: Changes affect the Web UI (components, hooks, styles, API routes\n that serve the frontend, SSE streaming, layout, user-visible behavior). These can be\n validated by starting Archon and using agent-browser to interact with the UI.\n - **code_review_only**: Changes are purely backend logic, CLI-only, workflow engine,\n database schemas, git operations, build tooling, tests, documentation, or other\n non-UI code. No visual validation possible.\n\n Consider: even if a change is backend, if it affects what the frontend displays\n (e.g., API response format changes, SSE event changes), it IS e2e_testable.\n depends_on: [fetch-pr]\n model: small\n allowed_tools: []\n output_format:\n type: object\n properties:\n testable:\n type: string\n enum: [\"e2e_testable\", \"code_review_only\"]\n reasoning:\n type: string\n test_plan:\n type: string\n required: [testable, reasoning, test_plan]\n\n - id: e2e-test-main\n command: archon-validate-pr-e2e-main\n depends_on: [classify-testability, find-ports, resolve-paths, code-review-main, code-review-feature]\n when: \"$classify-testability.output.testable == 'e2e_testable'\"\n context: fresh\n idle_timeout: 1800000\n\n - id: e2e-test-feature\n command: archon-validate-pr-e2e-feature\n depends_on: [e2e-test-main, find-ports, resolve-paths]\n when: \"$classify-testability.output.testable == 'e2e_testable'\"\n context: fresh\n idle_timeout: 1800000\n\n # ═══════════════════════════════════════════════════════════════\n # PHASE 4: FINAL REPORT — Synthesize all findings\n # ═══════════════════════════════════════════════════════════════\n\n - id: cleanup-processes\n bash: |\n # Safety net: kill any orphaned processes from E2E testing\n # This runs after E2E nodes complete (or timeout/fail) to prevent process accumulation\n BACKEND_PORT=$(cat \"$ARTIFACTS_DIR/.backend-port\" 2>/dev/null | tr -d '\\n')\n FRONTEND_PORT=$(cat \"$ARTIFACTS_DIR/.frontend-port\" 2>/dev/null | tr -d '\\n')\n\n if [ -z \"$BACKEND_PORT\" ] || [ -z \"$FRONTEND_PORT\" ]; then\n echo \"No port files found — skipping cleanup\"\n exit 0\n fi\n\n echo \"Cleaning up ports $BACKEND_PORT and $FRONTEND_PORT...\"\n\n # Kill by all recorded PID files\n for pidfile in \"$ARTIFACTS_DIR\"/.e2e-*-pid; do\n if [ -f \"$pidfile\" ]; then\n PID=$(cat \"$pidfile\" | tr -d '\\n')\n echo \"Killing PID $PID from $pidfile\"\n kill \"$PID\" 2>/dev/null || taskkill //F //T //PID \"$PID\" 2>/dev/null || true\n fi\n done\n\n # Kill by port (cross-platform fallback)\n for PORT in $BACKEND_PORT $FRONTEND_PORT; do\n fuser -k \"$PORT/tcp\" 2>/dev/null || true\n lsof -ti:\"$PORT\" 2>/dev/null | xargs kill -9 2>/dev/null || true\n netstat -ano 2>/dev/null | grep \":$PORT \" | grep LISTENING | awk '{print $5}' | sort -u | while read pid; do\n taskkill //F //T //PID \"$pid\" 2>/dev/null || true\n done\n done\n\n # pkill fallback: catch processes that escaped PID/port cleanup\n pkill -f \"PORT=$BACKEND_PORT.*bun\" 2>/dev/null || true\n pkill -f \"vite.*port.*$FRONTEND_PORT\" 2>/dev/null || true\n\n # Close this workflow's browser session only (scoped by session ID)\n BROWSER_SESSION=$(cat \"$ARTIFACTS_DIR/.browser-session\" 2>/dev/null | tr -d '\\n')\n if [ -n \"$BROWSER_SESSION\" ]; then\n agent-browser --session \"$BROWSER_SESSION\" close 2>/dev/null || true\n fi\n\n # Remove main E2E worktree if it still exists (safety net)\n CANONICAL_REPO=$(cat \"$ARTIFACTS_DIR/.canonical-repo\" 2>/dev/null | tr -d '\\n')\n MAIN_E2E_PATH=$(cat \"$ARTIFACTS_DIR/.e2e-main-worktree\" 2>/dev/null | tr -d '\\n')\n if [ -n \"$MAIN_E2E_PATH\" ] && [ -n \"$CANONICAL_REPO\" ] && [ -d \"$MAIN_E2E_PATH\" ]; then\n echo \"Removing leftover main E2E worktree: $MAIN_E2E_PATH\"\n git -C \"$CANONICAL_REPO\" worktree remove \"$MAIN_E2E_PATH\" --force 2>/dev/null || rm -rf \"$MAIN_E2E_PATH\"\n fi\n\n sleep 1\n echo \"Process cleanup complete\"\n depends_on: [e2e-test-main, e2e-test-feature]\n trigger_rule: all_done\n\n - id: final-report\n command: archon-validate-pr-report\n depends_on: [code-review-main, code-review-feature, e2e-test-main, e2e-test-feature, classify-testability, cleanup-processes]\n trigger_rule: all_done\n context: fresh\n", - "archon-workflow-builder": "name: archon-workflow-builder\ndescription: |\n Use when: User wants to create a new custom workflow for their project.\n Triggers: \"build me a workflow\", \"create a workflow\", \"generate a workflow\",\n \"new workflow\", \"make a workflow for\", \"workflow builder\".\n Does: Scans codebase -> extracts intent (JSON) -> generates YAML -> validates -> saves.\n NOT for: Editing existing workflows or creating non-workflow files.\n\n# Run in the live checkout, not in a fresh sub-worktree. Without this, every\n# archon-workflow-builder invocation creates an isolated sub-worktree and\n# writes the generated YAML there — the file never reaches the caller's\n# .archon/workflows/, so the run reports success while the user's repo gains\n# nothing. Closes #1220.\nworktree:\n enabled: false\n\nnodes:\n - id: scan-codebase\n bash: |\n echo \"=== Existing Commands ===\"\n if [ -d \".archon/commands\" ]; then\n find .archon/commands -type f -name \"*.md\" 2>/dev/null | head -30\n else\n echo \"(no .archon/commands/ directory)\"\n fi\n\n echo \"\"\n echo \"=== Existing Workflows ===\"\n if [ -d \".archon/workflows\" ]; then\n find .archon/workflows -type f \\( -name \"*.yaml\" -o -name \"*.yml\" \\) 2>/dev/null | head -30\n else\n echo \"(no .archon/workflows/ directory)\"\n fi\n\n echo \"\"\n echo \"=== Package Info ===\"\n if [ -f \"package.json\" ]; then\n grep -E '\"name\"|\"scripts\"' package.json | head -10\n else\n echo \"(no package.json)\"\n fi\n\n echo \"\"\n echo \"=== Project Context (CLAUDE.md first 50 lines) ===\"\n if [ -f \"CLAUDE.md\" ]; then\n head -50 CLAUDE.md\n else\n echo \"(no CLAUDE.md)\"\n fi\n\n - id: extract-intent\n prompt: |\n You are a workflow design classifier. Given a user's description of what they want\n a workflow to do, extract structured intent.\n\n ## User's Request\n $ARGUMENTS\n\n ## Codebase Context\n $scan-codebase.output\n\n ## Instructions\n\n Analyze the user's request and the existing codebase to determine:\n 1. A kebab-case workflow name (e.g., \"lint-and-test\", \"deploy-staging\")\n 2. A description following the Archon pattern (Use when / Triggers / Does / NOT for)\n 3. Trigger phrases the router should match\n 4. A list of proposed nodes with their types and purposes\n 5. Whether this should be a simple DAG or include a loop node\n\n Be specific and concrete. Each proposed node should have a clear type\n (bash, prompt, command, script, loop, loop_group, approval, or cancel) and\n a one-line description of what it does.\n model: small\n allowed_tools: []\n output_format:\n type: object\n properties:\n workflow_name:\n type: string\n description:\n type: string\n trigger_phrases:\n type: string\n proposed_nodes:\n type: string\n execution_mode:\n type: string\n enum: [\"dag\", \"loop\"]\n required: [workflow_name, description, trigger_phrases, proposed_nodes, execution_mode]\n depends_on: [scan-codebase]\n\n - id: generate-yaml\n prompt: |\n You are an Archon workflow author. Generate a complete, valid workflow YAML file\n based on the structured intent provided.\n\n ## Intent\n - **Name**: $extract-intent.output.workflow_name\n - **Description**: $extract-intent.output.description\n - **Trigger Phrases**: $extract-intent.output.trigger_phrases\n - **Proposed Nodes**: $extract-intent.output.proposed_nodes\n - **Execution Mode**: $extract-intent.output.execution_mode\n\n ## Original User Request\n $ARGUMENTS\n\n ## Archon Workflow YAML Schema Reference\n\n A workflow YAML file has this structure:\n\n ```yaml\n name: workflow-name\n description: |\n Use when: ...\n Triggers: ...\n Does: ...\n NOT for: ...\n\n # Optional top-level settings:\n # provider: claude (or codex)\n # model: medium (or haiku, opus, etc.)\n # interactive: true (forces foreground execution in web UI)\n\n nodes:\n - id: node-id-kebab-case\n # Choose ONE of: prompt, bash, command, script, loop, loop_group, approval, cancel\n\n # --- prompt node (AI-executed) ---\n prompt: |\n Instructions for the AI...\n # Optional: model, allowed_tools, denied_tools, output_format, context, idle_timeout\n\n # --- bash node (shell script, no AI, stdout = $.output) ---\n bash: |\n #!/bin/bash\n set -e\n echo \"result\"\n\n # --- command node (references a .archon/commands/ file) ---\n command: command-name\n\n # --- script node (TypeScript via bun, or Python via uv — no AI, stdout = $.output) ---\n # Use for deterministic data transforms the shell would mangle (JSON parsing, etc.)\n script: |\n // JSON is valid JS expression syntax — assign directly (String.raw breaks on backticks)\n const data = $other-node.output;\n console.log(JSON.stringify({ count: data.items.length }));\n runtime: bun # required: 'bun' (.ts/.js) or 'uv' (.py)\n # deps: [requests] # uv only\n # Or reference a named script in .archon/scripts/:\n # script: extract-labels # no extension; bun resolves .ts/.js, uv resolves .py\n\n # --- loop node (iterative AI execution) ---\n loop:\n prompt: |\n Instructions repeated each iteration...\n until: COMPLETION_SIGNAL\n max_iterations: 10\n fresh_context: true # optional: reset context each iteration\n\n # --- loop_group node (iterate a multi-node sub-DAG until done) ---\n loop_group:\n until: COMPLETION_SIGNAL\n max_iterations: 5\n nodes: # sealed sub-DAG body, re-run each iteration\n - id: body-step\n prompt: |\n Do one unit of work. Emit COMPLETION_SIGNAL when finished.\n depends_on: []\n\n # --- approval node (human gate — pauses workflow) ---\n approval:\n message: \"Review the plan above. Approve to continue.\"\n # capture_response: true # store reviewer comment as $.output\n\n # --- cancel node (terminate the run with a reason; no AI) ---\n cancel: \"Reason the workflow was terminated\"\n\n # Common options for all node types:\n depends_on: [other-node-id] # DAG edges\n when: \"$.output == 'value'\" # conditional execution\n trigger_rule: all_success # all_success | one_success | all_done\n timeout: 120000 # ms, for bash and script nodes\n ```\n\n ## Variable Reference\n - `$ARGUMENTS` — user's input text\n - `$ARTIFACTS_DIR` — pre-created directory for workflow artifacts\n - `$.output` — stdout from a bash/script node or AI response from a prompt node\n - `$.output.field` — JSON field from a node with output_format\n - `$BASE_BRANCH` — base git branch\n\n ## Rules\n 1. The `name:` field MUST match: $extract-intent.output.workflow_name\n 2. The `description:` MUST follow the \"Use when / Triggers / Does / NOT for\" pattern\n 3. Every node MUST have a unique kebab-case `id`\n 4. Use `depends_on` to define execution order\n 5. Use `bash` nodes for deterministic shell operations (file checks, git commands, installs)\n 6. Use `script` nodes for typed data transforms (TypeScript JSON parsing, Python with deps)\n — stdout is captured as output, stderr is forwarded as a warning.\n `$nodeId.output` is NOT shell-quoted in script bodies.\n - **TypeScript/bun**: assign directly — `const data = $nodeId.output;`\n (JSON is valid JS expression syntax; avoid String.raw — it breaks on backticks)\n - **Python/uv**: use json.loads — `import json; data = json.loads(\"\"\"$nodeId.output\"\"\")`\n Never interpolate into shell syntax.\n 7. Use `prompt` nodes for AI reasoning tasks\n 8. Use `approval` nodes to pause for human review at risky gates (plan→execute boundary, destructive actions)\n 9. Use `output_format` on prompt nodes when downstream nodes need structured data\n 10. Use `allowed_tools: []` on classification/analysis nodes that don't need tools\n 11. Use `denied_tools: [Edit, Bash]` when a node should only use Write (not edit existing files)\n 12. Prefer `model: small` for simple classification tasks to save cost\n\n ## Output\n\n Write the complete workflow YAML to: `$ARTIFACTS_DIR/generated-workflow.yaml`\n\n Use the Write tool. Do NOT use Edit or Bash. The file must be valid YAML and follow\n all the patterns above.\n denied_tools: [Edit, Bash]\n depends_on: [extract-intent]\n\n - id: validate-yaml\n bash: |\n FILE=\"$ARTIFACTS_DIR/generated-workflow.yaml\"\n\n if [ ! -f \"$FILE\" ]; then\n echo \"ERROR: generated-workflow.yaml not found at $FILE\"\n exit 1\n fi\n\n if [ ! -s \"$FILE\" ]; then\n echo \"ERROR: generated-workflow.yaml is empty\"\n exit 1\n fi\n\n if ! grep -q \"^name:\" \"$FILE\"; then\n echo \"ERROR: missing 'name:' field\"\n exit 1\n fi\n\n if ! grep -q \"^nodes:\" \"$FILE\"; then\n echo \"ERROR: missing 'nodes:' field\"\n exit 1\n fi\n\n echo \"VALID\"\n depends_on: [generate-yaml]\n\n - id: save-or-report\n prompt: |\n You are a workflow installer. Save the generated workflow and report to the user.\n\n ## Workflow Details\n - **Name**: $extract-intent.output.workflow_name\n - **Trigger Phrases**: $extract-intent.output.trigger_phrases\n\n ## Instructions\n\n 1. Read the generated workflow from `$ARTIFACTS_DIR/generated-workflow.yaml`\n 2. Create the directory `.archon/workflows/` if it doesn't exist (use Bash: `mkdir -p .archon/workflows/`)\n 3. Save the workflow to `.archon/workflows/$extract-intent.output.workflow_name.yaml`\n Use the Write tool to write the file.\n 4. Report to the user:\n - Workflow name and file location\n - Trigger phrases that will invoke it\n - How to run it: `bun run cli workflow run $extract-intent.output.workflow_name \"your input\"`\n - How to test it: `bun run cli validate workflows $extract-intent.output.workflow_name`\n depends_on: [validate-yaml]\n", + "archon-workflow-builder": "name: archon-workflow-builder\ndescription: |\n Use when: User wants to create a new custom workflow for their project.\n Triggers: \"build me a workflow\", \"create a workflow\", \"generate a workflow\",\n \"new workflow\", \"make a workflow for\", \"workflow builder\".\n Does: Scans codebase -> extracts intent (JSON) -> generates YAML -> validates -> saves.\n NOT for: Editing existing workflows or creating non-workflow files.\n\n# Run in the live checkout, not in a fresh sub-worktree. Without this, every\n# archon-workflow-builder invocation creates an isolated sub-worktree and\n# writes the generated YAML there — the file never reaches the caller's\n# .archon/workflows/, so the run reports success while the user's repo gains\n# nothing. Closes #1220.\nworktree:\n enabled: false\n\nnodes:\n - id: scan-codebase\n bash: |\n echo \"=== Existing Commands ===\"\n if [ -d \".archon/commands\" ]; then\n find .archon/commands -type f -name \"*.md\" 2>/dev/null | head -30\n else\n echo \"(no .archon/commands/ directory)\"\n fi\n\n echo \"\"\n echo \"=== Existing Workflows ===\"\n if [ -d \".archon/workflows\" ]; then\n find .archon/workflows -type f \\( -name \"*.yaml\" -o -name \"*.yml\" \\) 2>/dev/null | head -30\n else\n echo \"(no .archon/workflows/ directory)\"\n fi\n\n echo \"\"\n echo \"=== Package Info ===\"\n if [ -f \"package.json\" ]; then\n grep -E '\"name\"|\"scripts\"' package.json | head -10\n else\n echo \"(no package.json)\"\n fi\n\n echo \"\"\n echo \"=== Project Context (CLAUDE.md first 50 lines) ===\"\n if [ -f \"CLAUDE.md\" ]; then\n head -50 CLAUDE.md\n else\n echo \"(no CLAUDE.md)\"\n fi\n\n - id: extract-intent\n prompt: |\n You are a workflow design classifier. Given a user's description of what they want\n a workflow to do, extract structured intent.\n\n ## User's Request\n $ARGUMENTS\n\n ## Codebase Context\n $scan-codebase.output\n\n ## Instructions\n\n Analyze the user's request and the existing codebase to determine:\n 1. A kebab-case workflow name (e.g., \"lint-and-test\", \"deploy-staging\")\n 2. A description following the Archon pattern (Use when / Triggers / Does / NOT for)\n 3. Trigger phrases the router should match\n 4. A list of proposed nodes with their types and purposes\n 5. Whether this should be a simple DAG or include a loop node\n\n Be specific and concrete. Each proposed node should have a clear type\n (bash, prompt, command, script, loop, loop_group, approval, or cancel) and\n a one-line description of what it does.\n model: small\n allowed_tools: []\n output_format:\n type: object\n properties:\n workflow_name:\n type: string\n description:\n type: string\n trigger_phrases:\n type: string\n proposed_nodes:\n type: string\n execution_mode:\n type: string\n enum: [\"dag\", \"loop\"]\n required: [workflow_name, description, trigger_phrases, proposed_nodes, execution_mode]\n depends_on: [scan-codebase]\n\n - id: generate-yaml\n prompt: |\n You are an Archon workflow author. Generate a complete, valid workflow YAML file\n based on the structured intent provided.\n\n ## Intent\n - **Name**: $extract-intent.output.workflow_name\n - **Description**: $extract-intent.output.description\n - **Trigger Phrases**: $extract-intent.output.trigger_phrases\n - **Proposed Nodes**: $extract-intent.output.proposed_nodes\n - **Execution Mode**: $extract-intent.output.execution_mode\n\n ## Original User Request\n $ARGUMENTS\n\n ## Archon Workflow YAML Schema Reference\n\n A workflow YAML file has this structure:\n\n ```yaml\n name: workflow-name\n description: |\n Use when: ...\n Triggers: ...\n Does: ...\n NOT for: ...\n\n # Optional top-level settings:\n # provider: claude (or codex)\n # model: medium (or haiku, opus, etc.)\n # interactive: true (forces foreground execution in web UI)\n\n nodes:\n - id: node-id-kebab-case\n # Choose ONE of: prompt, bash, command, script, loop, loop_group, approval, cancel\n\n # --- prompt node (AI-executed) ---\n prompt: |\n Instructions for the AI...\n # Optional: model, allowed_tools, denied_tools, output_format, context, idle_timeout\n\n # --- bash node (shell script, no AI, stdout = $.output) ---\n bash: |\n #!/bin/bash\n set -e\n echo \"result\"\n\n # --- command node (references a .archon/commands/ file) ---\n command: command-name\n\n # --- script node (TypeScript via bun, or Python via uv — no AI, stdout = $.output) ---\n # Use for deterministic data transforms the shell would mangle (JSON parsing, etc.)\n script: |\n // JSON is valid JS expression syntax — assign directly (String.raw breaks on backticks)\n const data = $.output;\n console.log(JSON.stringify({ count: data.items.length }));\n runtime: bun # required: 'bun' (.ts/.js) or 'uv' (.py)\n # deps: [requests] # uv only\n # Or reference a named script in .archon/scripts/:\n # script: extract-labels # no extension; bun resolves .ts/.js, uv resolves .py\n\n # --- loop node (iterative AI execution) ---\n loop:\n prompt: |\n Instructions repeated each iteration...\n until: COMPLETION_SIGNAL\n max_iterations: 10\n fresh_context: true # optional: reset context each iteration\n\n # --- loop_group node (iterate a multi-node sub-DAG until done) ---\n loop_group:\n until: COMPLETION_SIGNAL\n max_iterations: 5\n nodes: # sealed sub-DAG body, re-run each iteration\n - id: body-step\n prompt: |\n Do one unit of work. Emit COMPLETION_SIGNAL when finished.\n depends_on: []\n\n # --- approval node (human gate — pauses workflow) ---\n approval:\n message: \"Review the plan above. Approve to continue.\"\n # capture_response: true # store reviewer comment as $.output\n\n # --- cancel node (terminate the run with a reason; no AI) ---\n cancel: \"Reason the workflow was terminated\"\n\n # Common options for all node types:\n depends_on: [other-node-id] # DAG edges\n when: \"$.output == 'value'\" # conditional execution\n trigger_rule: all_success # all_success | one_success | all_done\n timeout: 120000 # ms, for bash and script nodes\n ```\n\n ## Variable Reference\n - `$ARGUMENTS` — user's input text\n - `$ARTIFACTS_DIR` — pre-created directory for workflow artifacts\n - `$.output` — stdout from a bash/script node or AI response from a prompt node\n - `$.output.field` — JSON field from a node with output_format\n - `$BASE_BRANCH` — base git branch\n\n ## Rules\n 1. The `name:` field MUST match: $extract-intent.output.workflow_name\n 2. The `description:` MUST follow the \"Use when / Triggers / Does / NOT for\" pattern\n 3. Every node MUST have a unique kebab-case `id`\n 4. Use `depends_on` to define execution order\n 5. Use `bash` nodes for deterministic shell operations (file checks, git commands, installs)\n 6. Use `script` nodes for typed data transforms (TypeScript JSON parsing, Python with deps)\n — stdout is captured as output, stderr is forwarded as a warning.\n `$.output` is NOT shell-quoted in script bodies.\n - **TypeScript/bun**: assign directly — `const data = $.output;`\n (JSON is valid JS expression syntax; avoid String.raw — it breaks on backticks)\n - **Python/uv**: use json.loads — `import json; data = json.loads(\"\"\"$.output\"\"\")`\n Never interpolate into shell syntax.\n 7. Use `prompt` nodes for AI reasoning tasks\n 8. Use `approval` nodes to pause for human review at risky gates (plan→execute boundary, destructive actions)\n 9. Use `output_format` on prompt nodes when downstream nodes need structured data\n 10. Use `allowed_tools: []` on classification/analysis nodes that don't need tools\n 11. Use `denied_tools: [Edit, Bash]` when a node should only use Write (not edit existing files)\n 12. Prefer `model: small` for simple classification tasks to save cost\n\n ## Output\n\n Write the complete workflow YAML to: `$ARTIFACTS_DIR/generated-workflow.yaml`\n\n Use the Write tool. Do NOT use Edit or Bash. The file must be valid YAML and follow\n all the patterns above.\n denied_tools: [Edit, Bash]\n depends_on: [extract-intent]\n\n - id: validate-yaml\n bash: |\n FILE=\"$ARTIFACTS_DIR/generated-workflow.yaml\"\n\n if [ ! -f \"$FILE\" ]; then\n echo \"ERROR: generated-workflow.yaml not found at $FILE\"\n exit 1\n fi\n\n if [ ! -s \"$FILE\" ]; then\n echo \"ERROR: generated-workflow.yaml is empty\"\n exit 1\n fi\n\n if ! grep -q \"^name:\" \"$FILE\"; then\n echo \"ERROR: missing 'name:' field\"\n exit 1\n fi\n\n if ! grep -q \"^nodes:\" \"$FILE\"; then\n echo \"ERROR: missing 'nodes:' field\"\n exit 1\n fi\n\n echo \"VALID\"\n depends_on: [generate-yaml]\n\n - id: save-or-report\n prompt: |\n You are a workflow installer. Save the generated workflow and report to the user.\n\n ## Workflow Details\n - **Name**: $extract-intent.output.workflow_name\n - **Trigger Phrases**: $extract-intent.output.trigger_phrases\n\n ## Instructions\n\n 1. Read the generated workflow from `$ARTIFACTS_DIR/generated-workflow.yaml`\n 2. Create the directory `.archon/workflows/` if it doesn't exist (use Bash: `mkdir -p .archon/workflows/`)\n 3. Save the workflow to `.archon/workflows/$extract-intent.output.workflow_name.yaml`\n Use the Write tool to write the file.\n 4. Report to the user:\n - Workflow name and file location\n - Trigger phrases that will invoke it\n - How to run it: `bun run cli workflow run $extract-intent.output.workflow_name \"your input\"`\n - How to test it: `bun run cli validate workflows $extract-intent.output.workflow_name`\n depends_on: [validate-yaml]\n", }; // Packaged workflow owners (0 total) diff --git a/packages/workflows/src/include-expander.test.ts b/packages/workflows/src/include-expander.test.ts index eedd50d856..73744d07bf 100644 --- a/packages/workflows/src/include-expander.test.ts +++ b/packages/workflows/src/include-expander.test.ts @@ -2,6 +2,7 @@ import { describe, test, expect } from 'bun:test'; import { expandWorkflowIncludes, INCLUDE_MAX_DEPTH } from './include-expander'; import { dagNodeSchema } from './schemas'; import type { WorkflowDefinition, DagNode } from './schemas'; +import { COMPILED_LOOP_COMMAND, type LoopWithCompiledCommand } from './compiled-command'; // --------------------------------------------------------------------------- // Helpers — build WorkflowDefinitions in-memory (pure: no parseWorkflow, no @@ -24,6 +25,11 @@ function nodeById(w: WorkflowDefinition, id: string): DagNode | undefined { return w.nodes.find(n => n.id === id); } +function compiledLoopPrompt(node: DagNode | undefined): string | undefined { + if (!node || !('loop' in node)) return undefined; + return (node.loop as typeof node.loop & LoopWithCompiledCommand)[COMPILED_LOOP_COMMAND]?.prompt; +} + /** A 3-node review-like block: verify -> scope -> impl (sole sink = impl). */ function blockWorkflow(): WorkflowDefinition { return wf('blk', [ @@ -176,7 +182,7 @@ describe('expandWorkflowIncludes — with input mapping', () => { const { workflows, errors } = expandWorkflowIncludes(mapOf(block, parent)); expect(workflows.has('parent')).toBe(false); expect(errors.find(error => error.filename === 'parent')?.error).toContain( - "Node 'review__judge' references unknown node '$nosuch.output'" + "Node 'review__judge' field 'prompt' references unknown node '$nosuch.output'" ); }); @@ -461,21 +467,25 @@ describe('expandWorkflowIncludes — with input mapping', () => { // --------------------------------------------------------------------------- describe('expandWorkflowIncludes — when gate combination', () => { - // Parent with a `gate` node; the include references it via when. The block's entry - // node carries its OWN when (referencing the same parent node, left intact because - // `gate` is not a block-internal id). function parentWith(includeWhen: string, entryWhen: string): Map { const block = wf('gated-blk', [{ id: 'e', prompt: 'e', when: entryWhen }]); + block.inputs = { gate: { required: true } }; const parent = wf('parent', [ { id: 'gate', bash: 'echo gate' }, - { id: 'review', include: 'gated-blk', depends_on: ['gate'], when: includeWhen }, + { + id: 'review', + include: 'gated-blk', + depends_on: ['gate'], + when: includeWhen, + with: { gate: '$gate.output' }, + }, ]); return mapOf(block, parent); } test('combines the include gate with the entry node own when (both plain, no ||)', () => { const { workflows, errors } = expandWorkflowIncludes( - parentWith("$gate.output == 'go'", "$gate.output == 'yes'") + parentWith("$gate.output == 'go'", "$INPUTS.gate == 'yes'") ); expect(errors).toHaveLength(0); expect(nodeById(workflows.get('parent')!, 'review__e')?.when).toBe( @@ -485,7 +495,7 @@ describe('expandWorkflowIncludes — when gate combination', () => { test('fails the expansion when the ENTRY own when uses || (precedence would change)', () => { const { workflows, errors } = expandWorkflowIncludes( - parentWith("$gate.output == 'go'", "$gate.output == 'a' || $gate.output == 'b'") + parentWith("$gate.output == 'go'", "$INPUTS.gate == 'a' || $INPUTS.gate == 'b'") ); expect(workflows.has('parent')).toBe(false); const err = errors.find(e => e.filename === 'parent'); @@ -495,17 +505,23 @@ describe('expandWorkflowIncludes — when gate combination', () => { test('fails the expansion when the INCLUDE gate uses || (precedence would change)', () => { const { workflows, errors } = expandWorkflowIncludes( - parentWith("$gate.output == 'go' || $gate.output == 'stop'", "$gate.output == 'yes'") + parentWith("$gate.output == 'go' || $gate.output == 'stop'", "$INPUTS.gate == 'yes'") ); expect(workflows.has('parent')).toBe(false); expect(errors.find(e => e.filename === 'parent')?.error).toContain('cannot combine'); }); test('entry-only when is preserved unchanged when the include has no gate', () => { - const block = wf('gated-blk', [{ id: 'e', prompt: 'e', when: "$gate.output == 'yes'" }]); + const block = wf('gated-blk', [{ id: 'e', prompt: 'e', when: "$INPUTS.gate == 'yes'" }]); + block.inputs = { gate: { required: true } }; const parent = wf('parent', [ { id: 'gate', bash: 'echo gate' }, - { id: 'review', include: 'gated-blk', depends_on: ['gate'] }, + { + id: 'review', + include: 'gated-blk', + depends_on: ['gate'], + with: { gate: '$gate.output' }, + }, ]); const { workflows, errors } = expandWorkflowIncludes(mapOf(block, parent)); expect(errors).toHaveLength(0); @@ -544,6 +560,43 @@ describe('expandWorkflowIncludes — nested', () => { expect(ids).toContain('outer__inner__x'); expect(workflows.get('parent')!.nodes.some(n => 'include' in n)).toBe(false); }); + + test('preserves a nested compiled loop command across three-level composition', () => { + const leaf = wf('leaf-loop', [ + { id: 'seed', bash: 'echo seed' }, + { + id: 'group', + depends_on: ['seed'], + loop_group: { + until: 'DONE', + max_iterations: 1, + nodes: [ + { + id: 'repeat', + loop: { command: 'leaf-loop-command', until: 'DONE', max_iterations: 1 }, + }, + ], + }, + }, + ]); + leaf.inputs = { context: { required: true } }; + const middle = wf('middle-loop', [ + { id: 'inner', include: 'leaf-loop', with: { context: 'bound value' } }, + ]); + const parent = wf('parent', [{ id: 'outer', include: 'middle-loop' }]); + + const { workflows, errors } = expandWorkflowIncludes( + mapOf(leaf, middle, parent), + new Map([['leaf-loop-command', 'Use $seed.output with $INPUTS.context and continue.']]) + ); + + expect(errors).toHaveLength(0); + const group = nodeById(workflows.get('parent')!, 'outer__inner__group'); + const repeat = group && 'loop_group' in group ? group.loop_group.nodes[0] : undefined; + expect(compiledLoopPrompt(repeat)).toBe( + 'Use $outer__inner__seed.output with bound value and continue.' + ); + }); }); // --------------------------------------------------------------------------- @@ -584,14 +637,27 @@ describe('expandWorkflowIncludes — shorthand when: refs', () => { const { workflows } = expandWorkflowIncludes(mapOf(block, parent)); expect(nodeById(workflows.get('parent')!, 'inc__b')?.when).toBe("$inc__a.output == 'x'"); }); + + test('rejects an external shorthand ref even when the parent has a colliding id', () => { + const block = wf('blk3', [{ id: 'task', prompt: 'work', when: "$caller.status == 'ok'" }]); + const parent = wf('parent', [ + { id: 'caller', bash: 'echo parent' }, + { id: 'inc', include: 'blk3', depends_on: ['caller'] }, + ]); + const { workflows, errors } = expandWorkflowIncludes(mapOf(block, parent)); + expect(workflows.has('parent')).toBe(false); + expect(errors.find(error => error.filename === 'parent')?.error).toContain( + "field 'when' references unknown node '$caller.status'" + ); + }); }); // --------------------------------------------------------------------------- -// Fence-aware prose: documentation examples inside prompts must NOT be rewritten +// Markdown code spans are live because runtime substitution is syntax-agnostic // --------------------------------------------------------------------------- -describe('expandWorkflowIncludes — fence-aware prose', () => { - test('rewrites a live prompt ref but leaves a fenced example untouched', () => { +describe('expandWorkflowIncludes — refs in Markdown code spans', () => { + test('rewrites refs both outside and inside a fenced example', () => { const block = wf('blk', [ { id: 'helper', bash: 'echo hi' }, { @@ -605,10 +671,8 @@ describe('expandWorkflowIncludes — fence-aware prose', () => { expect(errors).toHaveLength(0); const writer = nodeById(workflows.get('parent')!, 'inc__writer'); const prompt = writer && 'prompt' in writer ? writer.prompt : ''; - // Live ref (outside the fence) renamed… expect(prompt).toContain('Live: $inc__helper.output'); - // …fenced example left verbatim. - expect(prompt).toContain('```\nexample: $helper.output\n```'); + expect(prompt).toContain('```\nexample: $inc__helper.output\n```'); }); test('bash refs are rewritten verbatim (code fields are not fence-protected)', () => { @@ -622,6 +686,33 @@ describe('expandWorkflowIncludes — fence-aware prose', () => { expect(b && 'bash' in b ? b.bash : '').toBe('echo $inc__a.output'); }); + test('rewrites approval rejection prompts to the included sibling namespace', () => { + const block = wf('approval-block', [ + { id: 'plan', prompt: 'plan' }, + { + id: 'gate', + approval: { + message: 'Approve $plan.output', + on_reject: { prompt: 'Revise $plan.output' }, + }, + depends_on: ['plan'], + }, + ]); + const parent = wf('parent', [ + { id: 'plan', prompt: 'parent plan' }, + { id: 'inc', include: 'approval-block', depends_on: ['plan'] }, + ]); + const { workflows, errors } = expandWorkflowIncludes(mapOf(block, parent)); + expect(errors).toHaveLength(0); + const gate = nodeById(workflows.get('parent')!, 'inc__gate'); + expect(gate && 'approval' in gate ? gate.approval.message : '').toBe( + 'Approve $inc__plan.output' + ); + expect(gate && 'approval' in gate ? gate.approval.on_reject?.prompt : '').toBe( + 'Revise $inc__plan.output' + ); + }); + // #2121 Phase 2: a `workflow:` (sub-run) node inside an included block is a live // ref surface — its node id must namespace and its input: refs must rewrite so // executeWorkflowNode's re-entry (keyed on the namespaced parent_node_id) and @@ -667,10 +758,10 @@ describe('expandWorkflowIncludes — fence-aware prose', () => { }); // --------------------------------------------------------------------------- -// Command-file ref scan (contents can't be rewritten → fail-fast at load time) +// Included command compilation (resolved bodies become namespaced inline prompts) // --------------------------------------------------------------------------- -describe('expandWorkflowIncludes — command-file ref scan', () => { +describe('expandWorkflowIncludes — included command compilation', () => { function blockWithCommand(): [WorkflowDefinition, WorkflowDefinition] { const block = wf('cmdblk', [ { id: 'sib', bash: 'echo hi' }, @@ -680,99 +771,205 @@ describe('expandWorkflowIncludes — command-file ref scan', () => { return [block, parent]; } - test('fails when a block command file references a renamed sibling id', () => { + test('materializes a block command and namespaces its local sibling ref', () => { const [block, parent] = blockWithCommand(); const commandContents = new Map([ ['my-cmd', 'Process the results from $sib.output and summarize.'], ]); const { workflows, errors } = expandWorkflowIncludes(mapOf(block, parent), commandContents); - expect(workflows.has('parent')).toBe(false); - const err = errors.find(e => e.filename === 'parent'); - expect(err?.error).toContain("command file 'my-cmd.md'"); - expect(err?.error).toContain("sibling node '$sib'"); + expect(errors).toHaveLength(0); + const runner = nodeById(workflows.get('parent')!, 'inc__runner'); + expect(runner && 'prompt' in runner ? runner.prompt : '').toBe( + 'Process the results from $inc__sib.output and summarize.' + ); + expect(runner && 'command' in runner).toBe(false); }); - test('fails when a block command file references an include input', () => { + test('materializes a command body and binds its declared include input', () => { const [block, parent] = blockWithCommand(); + block.inputs = { scope: { required: true } }; + const includeNode = parent.nodes[0]; + if (includeNode && 'include' in includeNode) includeNode.with = { scope: 'prod' }; const commandContents = new Map([ ['my-cmd', 'Review scope $INPUTS.scope.'], ]); const { workflows, errors } = expandWorkflowIncludes(mapOf(block, parent), commandContents); - expect(workflows.has('parent')).toBe(false); - const message = errors.find(error => error.filename === 'parent')?.error; - expect(message).toContain("Node 'inc'"); - expect(message).toContain("command file 'my-cmd.md'"); - expect(message).toContain("included block 'cmdblk'"); - expect(message).toContain("parameter '$INPUTS.scope'"); - expect(message).toContain('inline the prompt'); + expect(errors).toHaveLength(0); + const runner = nodeById(workflows.get('parent')!, 'inc__runner'); + expect(runner && 'prompt' in runner ? runner.prompt : '').toBe('Review scope prod.'); }); - test('passes when the command file has no cross-node reference', () => { + test('binds a declared include input named output in an ordinary command', () => { + const [block, parent] = blockWithCommand(); + block.inputs = { output: { required: true } }; + const includeNode = parent.nodes[0]; + if (includeNode && 'include' in includeNode) includeNode.with = { output: 'bound value' }; + const { workflows, errors } = expandWorkflowIncludes( + mapOf(block, parent), + new Map([['my-cmd', 'Review $INPUTS.output.']]) + ); + expect(errors).toHaveLength(0); + const runner = nodeById(workflows.get('parent')!, 'inc__runner'); + expect(runner && 'prompt' in runner ? runner.prompt : '').toBe('Review bound value.'); + }); + + test('keeps a caller ref passed through a command input parent-scoped on id collision', () => { + const block = wf('collision-command-block', [ + { id: 'gather', prompt: 'local gather' }, + { id: 'runner', command: 'collision-command', depends_on: ['gather'] }, + ]); + block.inputs = { context: { required: true } }; + const parent = wf('parent', [ + { id: 'gather', prompt: 'parent gather' }, + { + id: 'inc', + include: 'collision-command-block', + depends_on: ['gather'], + with: { context: '$gather.output' }, + }, + ]); + const { workflows, errors } = expandWorkflowIncludes( + mapOf(block, parent), + new Map([['collision-command', 'Review $INPUTS.context.']]) + ); + expect(errors).toHaveLength(0); + const runner = nodeById(workflows.get('parent')!, 'inc__runner'); + expect(runner && 'prompt' in runner ? runner.prompt : '').toBe('Review $gather.output.'); + }); + + test('treats canonical refs inside Markdown code as live and namespaces them', () => { const [block, parent] = blockWithCommand(); const commandContents = new Map([ ['my-cmd', 'Work from $ARTIFACTS_DIR only. See `$sib.output` in fenced docs.'], ]); const { workflows, errors } = expandWorkflowIncludes(mapOf(block, parent), commandContents); - // The only $sib.output is inside inline code (stripped), so no live ref → clean. expect(errors).toHaveLength(0); - expect(workflows.has('parent')).toBe(true); + const runner = nodeById(workflows.get('parent')!, 'inc__runner'); + expect(runner && 'prompt' in runner ? runner.prompt : '').toContain('`$inc__sib.output`'); }); - // A command body can never have inputs applied — it is read at execution time, after - // expansion. So `$INPUTS.` there is an unkeepable promise wherever it appears, - // and unlike the sibling-ref scan the fence has no bearing on it: the macro itself - // substitutes inside code spans, because `$INPUTS` has no documentation-only meaning. - test('fails when a command file references an include input inside a fenced block', () => { + test('binds a declared include input inside a fenced block', () => { const [block, parent] = blockWithCommand(); + block.inputs = { scope: { required: true } }; + const includeNode = parent.nodes[0]; + if (includeNode && 'include' in includeNode) includeNode.with = { scope: 'prod' }; const commandContents = new Map([ ['my-cmd', 'Run this:\n\n```bash\necho "$INPUTS.scope"\n```\n'], ]); const { workflows, errors } = expandWorkflowIncludes(mapOf(block, parent), commandContents); - expect(workflows.has('parent')).toBe(false); - expect(errors.find(error => error.filename === 'parent')?.error).toContain( - "parameter '$INPUTS.scope'" - ); + expect(errors).toHaveLength(0); + const runner = nodeById(workflows.get('parent')!, 'inc__runner'); + expect(runner && 'prompt' in runner ? runner.prompt : '').toContain('echo "prod"'); }); - test('fails when a command file references an include input inside inline code', () => { + test('rejects a command ref outside the included workflow namespace', () => { const [block, parent] = blockWithCommand(); const commandContents = new Map([ - ['my-cmd', 'The scope is `$INPUTS.scope` — use it.'], + ['my-cmd', 'Use $caller.output directly.'], ]); const { workflows, errors } = expandWorkflowIncludes(mapOf(block, parent), commandContents); expect(workflows.has('parent')).toBe(false); - expect(errors.find(error => error.filename === 'parent')?.error).toContain( - "parameter '$INPUTS.scope'" + const message = errors.find(error => error.filename === 'parent')?.error; + expect(message).toContain("included workflow 'cmdblk'"); + expect(message).toContain("node 'runner'"); + expect(message).toContain("command 'my-cmd'"); + expect(message).toContain("'$caller.output'"); + expect(message).toContain('inputs:'); + expect(message).toContain('with:'); + }); + + test('rejects the same external command ref when the parent has a colliding id', () => { + const [block] = blockWithCommand(); + const parent = wf('parent', [ + { id: 'caller', bash: 'echo parent' }, + { id: 'inc', include: 'cmdblk', depends_on: ['caller'] }, + ]); + const result = expandWorkflowIncludes( + mapOf(block, parent), + new Map([['my-cmd', 'Use $caller.output directly.']]) + ); + expect(result.workflows.has('parent')).toBe(false); + expect(result.errors.find(error => error.filename === 'parent')?.error).toContain( + "'$caller.output'" ); }); - // An unresolvable command file is an incomplete-information state, not an unsafe one. - // Failing it would drop workflows that never opted into this feature — including ones - // with no `with:` and no `$INPUTS` anywhere. - test('warns (not fails) when the command file cannot be resolved for scanning', () => { + test('fails closed when the command body cannot be resolved', () => { const [block, parent] = blockWithCommand(); const commandContents = new Map([['my-cmd', null]]); const { workflows, errors } = expandWorkflowIncludes(mapOf(block, parent), commandContents); - expect(errors).toHaveLength(0); - expect(workflows.has('parent')).toBe(true); + expect(workflows.has('parent')).toBe(false); + expect(errors.find(error => error.filename === 'parent')?.error).toContain("command 'my-cmd'"); + expect(errors.find(error => error.filename === 'parent')?.error).toContain( + 'could not be resolved during composition' + ); + }); + + test('rejects an empty command body during composition', () => { + const [block, parent] = blockWithCommand(); + const { workflows, errors } = expandWorkflowIncludes( + mapOf(block, parent), + new Map([['my-cmd', ' \n\t']]) + ); + expect(workflows.has('parent')).toBe(false); + const message = errors.find(error => error.filename === 'parent')?.error; + expect(message).toContain("command 'my-cmd' is empty"); + expect(message).toContain('non-whitespace prompt body'); }); - test('fails when an included loop command file references an include input', () => { + test('materializes loop.command and binds its declared include input', () => { const block = wf('loopblk', [ { id: 'repeat', loop: { command: 'loop-cmd', until: 'DONE', max_iterations: 1 } }, ]); + block.inputs = { scope: { required: true } }; const parent = wf('parent', [{ id: 'inc', include: 'loopblk', with: { scope: 'prod' } }]); const { workflows, errors } = expandWorkflowIncludes( mapOf(block, parent), new Map([['loop-cmd', 'Review $INPUTS.scope.']]) ); - expect(workflows.has('parent')).toBe(false); - expect(errors.find(error => error.filename === 'parent')?.error).toContain( - "command file 'loop-cmd.md'" + expect(errors).toHaveLength(0); + const repeat = nodeById(workflows.get('parent')!, 'inc__repeat'); + expect(compiledLoopPrompt(repeat)).toBe('Review prod.'); + expect(repeat && 'loop' in repeat ? repeat.loop.command : undefined).toBe('loop-cmd'); + }); + + test('binds a declared include input named output in a loop command', () => { + const block = wf('loop-output-block', [ + { id: 'repeat', loop: { command: 'loop-output-cmd', until: 'DONE', max_iterations: 1 } }, + ]); + block.inputs = { output: { required: true } }; + const parent = wf('parent', [ + { id: 'inc', include: 'loop-output-block', with: { output: 'bound value' } }, + ]); + const { workflows, errors } = expandWorkflowIncludes( + mapOf(block, parent), + new Map([['loop-output-cmd', 'Review $INPUTS.output.']]) + ); + expect(errors).toHaveLength(0); + expect(compiledLoopPrompt(nodeById(workflows.get('parent')!, 'inc__repeat'))).toBe( + 'Review bound value.' ); }); - test('fails when a nested loop command file references a renamed top-level node', () => { + test('keeps a whitespace-only loop command as an actionable compiled error', () => { + const block = wf('empty-loop-block', [ + { id: 'repeat', loop: { command: 'empty-loop-cmd', until: 'DONE', max_iterations: 1 } }, + ]); + const parent = wf('parent', [{ id: 'inc', include: 'empty-loop-block' }]); + const { workflows, errors } = expandWorkflowIncludes( + mapOf(block, parent), + new Map([['empty-loop-cmd', ' \n\t']]) + ); + expect(errors).toHaveLength(0); + const repeat = nodeById(workflows.get('parent')!, 'inc__repeat'); + const compiled = + repeat && 'loop' in repeat + ? (repeat.loop as typeof repeat.loop & LoopWithCompiledCommand)[COMPILED_LOOP_COMMAND] + : undefined; + expect(compiled?.error).toContain("command 'empty-loop-cmd' is empty"); + }); + + test('materializes a nested loop command and namespaces an enclosing top-level ref', () => { const block = wf('nested-loopblk', [ { id: 'seed', bash: 'echo seed' }, { @@ -794,13 +991,13 @@ describe('expandWorkflowIncludes — command-file ref scan', () => { mapOf(block, parent), new Map([['nested-loop-cmd', 'Read $seed.output and continue.']]) ); - expect(workflows.has('parent')).toBe(false); - const message = errors.find(error => error.filename === 'parent')?.error; - expect(message).toContain("command file 'nested-loop-cmd.md'"); - expect(message).toContain("sibling node '$seed'"); + expect(errors).toHaveLength(0); + const group = nodeById(workflows.get('parent')!, 'inc__group'); + const repeat = group && 'loop_group' in group ? group.loop_group.nodes[0] : undefined; + expect(compiledLoopPrompt(repeat)).toBe('Read $inc__seed.output and continue.'); }); - test('fails for a command node inside a second-level nested loop group', () => { + test('materializes a command inside a second-level nested loop group', () => { const block = wf('deep-command-block', [ { id: 'seed', bash: 'echo seed' }, { @@ -827,13 +1024,16 @@ describe('expandWorkflowIncludes — command-file ref scan', () => { new Map([['deep-command', 'Read $seed.output and continue.']]) ); - expect(workflows.has('parent')).toBe(false); - expect(errors.find(error => error.filename === 'parent')?.error).toContain( - "command file 'deep-command.md'" + expect(errors).toHaveLength(0); + const outer = nodeById(workflows.get('parent')!, 'inc__outer'); + const inner = outer && 'loop_group' in outer ? outer.loop_group.nodes[0] : undefined; + const review = inner && 'loop_group' in inner ? inner.loop_group.nodes[0] : undefined; + expect(review && 'prompt' in review ? review.prompt : '').toBe( + 'Read $inc__seed.output and continue.' ); }); - test('fails when a nested loop command file references an include input', () => { + test('binds an include input in a nested loop command', () => { const block = wf('nested-input-loopblk', [ { id: 'group', @@ -852,14 +1052,15 @@ describe('expandWorkflowIncludes — command-file ref scan', () => { const parent = wf('parent', [ { id: 'inc', include: 'nested-input-loopblk', with: { scope: 'prod' } }, ]); + block.inputs = { scope: { required: true } }; const { workflows, errors } = expandWorkflowIncludes( mapOf(block, parent), new Map([['nested-input-cmd', 'Review $INPUTS.scope.']]) ); - expect(workflows.has('parent')).toBe(false); - expect(errors.find(error => error.filename === 'parent')?.error).toContain( - "parameter '$INPUTS.scope'" - ); + expect(errors).toHaveLength(0); + const group = nodeById(workflows.get('parent')!, 'inc__group'); + const repeat = group && 'loop_group' in group ? group.loop_group.nodes[0] : undefined; + expect(compiledLoopPrompt(repeat)).toBe('Review prod.'); }); test('passes when a nested loop command file references a local body node', () => { @@ -888,11 +1089,11 @@ describe('expandWorkflowIncludes — command-file ref scan', () => { expect(workflows.has('parent')).toBe(true); }); - test('skips the scan entirely when no commandContents map is supplied', () => { + test('fails closed when no commandContents map is supplied', () => { const [block, parent] = blockWithCommand(); const { workflows, errors } = expandWorkflowIncludes(mapOf(block, parent)); - expect(errors).toHaveLength(0); - expect(workflows.has('parent')).toBe(true); + expect(workflows.has('parent')).toBe(false); + expect(errors).toHaveLength(1); }); }); diff --git a/packages/workflows/src/include-expander.ts b/packages/workflows/src/include-expander.ts index b2e4609838..336ba0c098 100644 --- a/packages/workflows/src/include-expander.ts +++ b/packages/workflows/src/include-expander.ts @@ -20,10 +20,11 @@ * * Targets are resolved recursively (a target may itself `include:` others), * depth-capped and cycle-detected. Because expansion runs BEFORE any - * WorkflowDefinition reaches the executor, the inlined nodes are indistinguishable - * from hand-written nodes — there is zero new runtime machinery. Every execution - * path re-discovers → re-expands deterministically, so resume matches the persisted - * namespaced step names byte-for-byte. + * WorkflowDefinition reaches the executor, the executor receives a flat DAG with no + * include nodes. Included command-backed loops additionally carry symbol-keyed compiled + * prompt/error metadata so a resumed run can prefer its persisted prompt snapshot even + * when the source command has disappeared. Every execution path re-discovers → re-expands + * deterministically, so resume matches the persisted namespaced step names byte-for-byte. * * Delimiter note: the namespace joiner is `__` (double underscore), NOT `.`. The * output-ref substitution regex forbids dots in a node id, so a dotted id would @@ -33,6 +34,7 @@ import type { WorkflowDefinition, WorkflowLoadError, DagNode, IncludeNode } from './schemas'; import { isIncludeNode, + isCommandNode, isLoopNode, isLoopGroupNode, isApprovalNode, @@ -43,9 +45,15 @@ import { INPUT_NAME_SOURCE, } from './schemas'; import { createLogger } from '@archon/paths'; -import { collectFileBackedCommandNames } from './command-file'; import { validateDagStructure } from './loader'; import { resolveDeclaredInputs } from './workflow-inputs'; +import { + COMPILED_LOOP_COMMAND, + isIncludeCommandReadError, + type CompiledLoopCommand, + type IncludeCommandContent, + type LoopWithCompiledCommand, +} from './compiled-command'; /** * Resolve the logger on every call rather than caching it at module scope. @@ -58,9 +66,9 @@ import { resolveDeclaredInputs } from './workflow-inputs'; * (#2458 — it cost three red tests in `loader.test.ts` whenever that file shared a * `bun test` process with `include-expander.test.ts`). * - * Resolving per call costs one `rootLogger.child()`, and both call sites are warn-only - * discovery paths: the first fires at most once per include node, the second once per - * unresolved command node. Neither is a hot loop. + * Resolving per call costs one `rootLogger.child()` on a warn-only discovery path that + * fires at most once per include node whose workflow-level fields are dropped. It is not + * a hot loop. */ function getLog(): ReturnType { return createLogger('workflow.include-expander'); @@ -102,9 +110,6 @@ const WHEN_REF_PATTERN = /\$([a-zA-Z_][a-zA-Z0-9_-]*)(?=\.[a-zA-Z_])/g; */ const INPUTS_REF = new RegExp(String.raw`\$INPUTS\.(${INPUT_NAME_SOURCE})`, 'g'); -/** Fenced (``` ```) and inline (` `` `) markdown code spans — documentation, not live refs. */ -const CODE_SPAN_PATTERN = /```[\s\S]*?```|`[^`\n]*`/g; - function applyOutputRefRename(text: string, rename: (id: string) => string): string { return text.replace(OUTPUT_REF_PATTERN, (match, id: string) => { const renamed = rename(id); @@ -119,30 +124,6 @@ function applyWhenRefRename(text: string, rename: (id: string) => string): strin }); } -/** - * Apply `fn` only to the text OUTSIDE markdown code spans, leaving fenced/inline code - * verbatim. Used for prose fields (prompt/loop.prompt/approval.message) where a - * `$other.output` inside a fenced example is documentation for the LLM, not a live ref — - * mirroring the loader's fence-stripping in validateDagStructure so validation and - * rewriting agree. - */ -function rewriteOutsideCode(text: string, fn: (chunk: string) => string): string { - let result = ''; - let last = 0; - CODE_SPAN_PATTERN.lastIndex = 0; - let m: RegExpExecArray | null; - while ((m = CODE_SPAN_PATTERN.exec(text)) !== null) { - result += fn(text.slice(last, m.index)) + m[0]; - last = m.index + m[0].length; - } - return result + fn(text.slice(last)); -} - -/** Escape a node id for use inside a dynamically-built RegExp. */ -function escapeRegExp(s: string): string { - return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); -} - /** Internal signal for a per-workflow expansion failure (resilient: drop one, keep the rest). */ class IncludeExpansionError extends Error {} @@ -156,34 +137,34 @@ class IncludeExpansionError extends Error {} * - `when:` — dual grammar (`$id.output[.field]` AND shorthand `$id.field`), never * markdown → `applyWhenRefRename`. Missing the shorthand would leave e.g. * `$verify.exit_code` pointing at a renamed sibling (silent fail-closed skip). - * - Prose (prompt / loop.prompt / approval.message) — canonical `.output` refs, but may - * embed fenced/inline code examples that must NOT be rewritten → fence-aware. + * - Prompt text (prompt / loop.prompt / approval fields) — canonical `.output` refs are + * live everywhere, including Markdown code spans, because runtime substitution is + * syntax-agnostic. * - Code/expression (bash / script / loop.until_bash / loop_group.until_bash / cancel / * workflow.input / workflow.fan_out.items) — canonical `.output` refs are LIVE (never * documentation) → rewritten verbatim. * - * KEEP IN SYNC (FOUR ref-surface enumerations must agree): this rewrite, applyInputsMacro - * below, the loader's validateDagStructure scan, and the substituteNodeOutputRefs call - * sites in dag-executor.ts. Adding a substituted field to one means updating all four. - * (The count read "three" while applyInputsMacro already existed and had already drifted — - * it was missing workflow.fan_out.items, which shipped literal `$INPUTS` text to the model.) + * Public runtime node-ref surfaces stay aligned across this rewrite, the loader's + * validateDagStructure scan, and the substituteNodeOutputRefs call sites in + * dag-executor.ts. Included loop-command bodies are validated separately during command + * materialization, then their compiled prompts pass through this rewrite. * - * applyInputsMacro is a SUPERSET of this function, not a mirror: it additionally walks the - * AI-turn surfaces below (systemPrompt / agents / approval.on_reject.prompt) that this - * rewrite skips. That asymmetry is deliberate — see the note on applyInputsMacro. + * applyInputsMacro is a SUPERSET of these node-ref surfaces, not a mirror: it additionally walks + * systemPrompt and agents fields. Those fields accept include inputs but do not receive + * node-output substitution at runtime, so they are not node-reference surfaces. */ function rewriteNodeOutputRefs(node: DagNode, rename: (id: string) => string): void { const code = (text: string): string => applyOutputRefRename(text, rename); - const prose = (text: string): string => - rewriteOutsideCode(text, chunk => applyOutputRefRename(chunk, rename)); const whenExpr = (text: string): string => applyWhenRefRename(text, rename); if (node.when !== undefined) node.when = whenExpr(node.when); if (isLoopNode(node)) { - // A command-backed loop has no inline prompt; its `command` is a NAME, not a ref - // (same rule as `command:` nodes above), so there is nothing to rewrite. - if (node.loop.prompt !== undefined) node.loop.prompt = prose(node.loop.prompt); + if (node.loop.prompt !== undefined) node.loop.prompt = code(node.loop.prompt); + const compiled = (node.loop as typeof node.loop & LoopWithCompiledCommand)[ + COMPILED_LOOP_COMMAND + ]; + if (compiled?.prompt !== undefined) compiled.prompt = code(compiled.prompt); if (node.loop.until_bash !== undefined) node.loop.until_bash = code(node.loop.until_bash); } else if (isLoopGroupNode(node)) { if (node.loop_group.until_bash !== undefined) { @@ -191,7 +172,10 @@ function rewriteNodeOutputRefs(node: DagNode, rename: (id: string) => string): v } for (const body of node.loop_group.nodes) rewriteNodeOutputRefs(body, rename); } else if (isApprovalNode(node)) { - node.approval.message = prose(node.approval.message); + node.approval.message = code(node.approval.message); + if (node.approval.on_reject !== undefined) { + node.approval.on_reject.prompt = code(node.approval.on_reject.prompt); + } } else if (isBashNode(node)) { node.bash = code(node.bash); } else if (isScriptNode(node)) { @@ -208,7 +192,7 @@ function rewriteNodeOutputRefs(node: DagNode, rename: (id: string) => string): v } else if (isCancelNode(node)) { node.cancel = code(node.cancel); } else if ('prompt' in node && typeof node.prompt === 'string') { - node.prompt = prose(node.prompt); + node.prompt = code(node.prompt); } } @@ -230,10 +214,9 @@ function rewriteNodeOutputRefs(node: DagNode, rename: (id: string) => string): v * the model as text, and because the field was never visited the name never reaches * `missing` either — so a caller who forgot to supply it gets no load error. * - * That is why systemPrompt / agents.*.prompt / agents.*.description / - * approval.on_reject.prompt are walked here despite being blind spots in the rewrite (a - * separate, lower-severity gap tracked on its own). Every model-facing string field must - * be walked here, whether or not the rewrite walks it. + * That is why systemPrompt / agents.*.prompt / agents.*.description are walked here even + * though they are not node-output reference surfaces. Every model-facing string field must + * be walked for include inputs, whether or not runtime also resolves node outputs there. */ function applyInputsMacro(node: DagNode, args: Record, missing: Set): void { const substitute = (text: string): string => @@ -266,6 +249,10 @@ function applyInputsMacro(node: DagNode, args: Record, missing: if (isLoopNode(node)) { if (node.loop.prompt !== undefined) node.loop.prompt = substitute(node.loop.prompt); + const compiled = (node.loop as typeof node.loop & LoopWithCompiledCommand)[ + COMPILED_LOOP_COMMAND + ]; + if (compiled?.prompt !== undefined) compiled.prompt = substitute(compiled.prompt); if (node.loop.until_bash !== undefined) { node.loop.until_bash = substitute(node.loop.until_bash); } @@ -331,12 +318,50 @@ function resolveIncludeInputs( } } +/** structuredClone intentionally drops symbol keys; retain the engine-private compiled + * loop metadata while cloning a reusable child for another include level. */ +function cloneNodeForInclude(node: DagNode): DagNode { + const clone = structuredClone(node); + const preserveCompiledLoops = (source: DagNode, target: DagNode): void => { + if (isLoopNode(source) && isLoopNode(target)) { + const compiled = (source.loop as typeof source.loop & LoopWithCompiledCommand)[ + COMPILED_LOOP_COMMAND + ]; + if (compiled !== undefined) { + (target.loop as typeof target.loop & LoopWithCompiledCommand)[COMPILED_LOOP_COMMAND] = + structuredClone(compiled); + } + } + if (isLoopGroupNode(source) && isLoopGroupNode(target)) { + for (const [index, sourceChild] of source.loop_group.nodes.entries()) { + const targetChild = target.loop_group.nodes[index]; + if (targetChild !== undefined) preserveCompiledLoops(sourceChild, targetChild); + } + } + }; + preserveCompiledLoops(node, clone); + return clone; +} + /** * Inline one include node's fully-expanded child into namespaced parent nodes. * 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, child: WorkflowDefinition): ExpandedInclude { +function inlineInclude( + includeNode: IncludeNode, + child: WorkflowDefinition, + commandContents: ReadonlyMap +): ExpandedInclude { + // Prove the child's lexical boundary before its nodes share the parent's flat id/output + // maps. Discovery already parsed each file independently; this repeat is intentional so + // direct/programmatic callers of the pure expander cannot bypass the same invariant. + const childStructureError = validateDagStructure(child.nodes); + if (childStructureError !== null) { + throw new IncludeExpansionError( + `Node '${includeNode.id}': included workflow '${child.name}' is not hermetic: ${childStructureError}` + ); + } const childNodes = child.nodes; const prefix = `${includeNode.id}__`; const childTopLevelIds = new Set(childNodes.map(n => n.id)); @@ -351,7 +376,15 @@ function inlineInclude(includeNode: IncludeNode, child: WorkflowDefinition): Exp const resolvedInputs = resolveIncludeInputs(includeNode, child); const namespaced = childNodes.map(cn => { - const clone = structuredClone(cn); + const clone = materializeBlockCommandPrompts( + cloneNodeForInclude(cn), + includeNode, + child, + commandContents, + childTopLevelIds, + new Set(), + cn.id + ); const wasEntry = (cn.depends_on ?? []).length === 0; // Rewrite child-internal refs before inserting caller values. This ordering is @@ -478,66 +511,108 @@ function warnDroppedWorkflowLevelFields(includeNode: IncludeNode, child: Workflo } /** - * A `command:` node's file remains external to the flattened DAG and becomes the node's - * prompt at execution time. Discovery may pre-read it for validation, but the expander cannot - * rewrite `$sibling.output` refs inside it the way it rewrites inline node text. If a - * block's command file references a sibling node id that namespacing renames, the ref - * would silently substitute to '' at run time. This applies equally to a loop's deferred - * `loop.command` prompt. Scan resolved command content for refs to any renamed id, and for - * `$INPUTS.` parameters that can never be applied, and FAIL the expansion on a hit. - * - * BEST-EFFORT BY CONSTRUCTION. This scan sees only what discovery could resolve. So a - * clean scan is "nothing found in what we could read", never a proof of - * safety. That is why an UNRESOLVABLE file warns and continues instead of failing: it is - * an incomplete-information state, not an unsafe one, and the difference matters because - * failing it would drop workflows that never opted into inputs at all (no `with:`, no - * `$INPUTS` anywhere) — breaking the "undeclared includes keep working byte-for-byte" - * guarantee. Only a file we actually READ and found a problem in is a hard error. - * - * Skipped entirely when no `commandContents` is supplied (e.g. unit tests that don't - * exercise command files). + * Compile an included workflow's named AI command bodies into the flat DAG. Composition + * must prove the child's lexical boundary before parent nodes share one output map, so + * every canonical ref in a resolved body must name a node in the command node's current or + * enclosing workflow scope. Ordinary command nodes become prompt nodes. Loop commands keep + * their authored identity plus symbol-keyed compiled prompt/error metadata so cold resume can + * reach a persisted prompt snapshot even after source deletion. The ordinary namespacing and + * `$INPUTS` passes transform compiled bodies without a second grammar. + * Named script files are deliberately outside this function: their source is opaque. An + * included block can bind inputs in the YAML `script:` selector, but the flattened include + * does not add `INPUTS_*` environment variables inside the selected script program. */ -function scanBlockCommandRefs( +function materializeBlockCommandPrompts( + node: DagNode, includeNode: IncludeNode, child: WorkflowDefinition, - commandContents: ReadonlyMap -): void { - const renamedIds = child.nodes.map(n => n.id); // every child top-level id gets a prefix - for (const commandName of collectFileBackedCommandNames(child.nodes)) { + commandContents: ReadonlyMap, + currentIds: ReadonlySet, + enclosingIds: ReadonlySet, + nodePath: string +): DagNode { + const compile = (commandName: string): CompiledLoopCommand => { const content = commandContents.get(commandName); + if (isIncludeCommandReadError(content)) { + const failure = + content.operation === 'inspect' + ? `could not inspect higher-precedence command scope '${content.path}'` + : `matched '${content.path}' but could not be read`; + return { + error: `Node '${includeNode.id}': included workflow '${child.name}' node '${nodePath}' command '${commandName}' ${failure}: ${content.message}. Archon will not fall through to a lower-precedence command when a higher-precedence scope cannot be inspected or its matched file cannot be read.`, + }; + } if (content === undefined || content === null) { - getLog().warn( - { include: includeNode.id, target: child.name, command: commandName, renamedIds }, - 'include.command_file_unresolved_for_ref_scan' - ); - continue; + return { + error: `Node '${includeNode.id}': included workflow '${child.name}' node '${nodePath}' uses command '${commandName}', but its body could not be resolved during composition through the package-owned, project/configured, user, or enabled bundled command scopes. Included commands must resolve before a fresh execution so their references and declared inputs can be compiled safely.`, + }; } - const stripped = content.replace(/```[\s\S]*?```/g, '').replace(/`[^`\n]*`/g, ''); - for (const id of renamedIds) { - // `$id.output` or the shorthand `$id.field` — either points at the pre-rename id. - const refRe = new RegExp(`\\$${escapeRegExp(id)}(?=\\.[a-zA-Z_])`); - if (refRe.test(stripped)) { - throw new IncludeExpansionError( - `Node '${includeNode.id}': command file '${commandName}.md' in included block '${child.name}' references sibling node '$${id}', which include namespacing renames to '${includeNode.id}__${id}'. Command-file contents are read at execution time and cannot be rewritten — inline the prompt, or restructure so the command has no cross-node reference.` - ); - } + if (content.trim().length === 0) { + return { + error: `Node '${includeNode.id}': included workflow '${child.name}' node '${nodePath}' command '${commandName}' is empty. Included commands must contain a non-whitespace prompt body.`, + }; } - // Scanned against RAW content, not the fence-stripped copy the sibling scan uses. - // The sibling scan strips because a fenced `$other.output` can plausibly be an example - // the author wants rendered literally to the model. `$INPUTS` has no such reading — - // applyInputsMacro deliberately substitutes inside code spans, so a fenced - // `$INPUTS.` in an INLINE prompt is a live parameter. A command body can never - // have inputs applied at all, which makes writing one there an unkeepable promise - // wherever it appears. Stripping here would let exactly that promise through. - INPUTS_REF.lastIndex = 0; - const inputMatch = INPUTS_REF.exec(content); - INPUTS_REF.lastIndex = 0; - if (inputMatch?.[1] !== undefined) { - throw new IncludeExpansionError( - `Node '${includeNode.id}': command file '${commandName}.md' in included block '${child.name}' references parameter '$INPUTS.${inputMatch[1]}'. Command-file contents are read at execution time and cannot apply include inputs — inline the prompt instead.` - ); + + const outputRefPattern = new RegExp(OUTPUT_REF_PATTERN.source, 'g'); + let match: RegExpExecArray | null; + while ((match = outputRefPattern.exec(content)) !== null) { + const referencedId = match[1]; + if (referencedId === 'INPUTS') continue; + if ( + referencedId !== undefined && + !currentIds.has(referencedId) && + !enclosingIds.has(referencedId) + ) { + const offendingRef = match[0]; + return { + error: `Node '${includeNode.id}': included workflow '${child.name}' node '${nodePath}' command '${commandName}' references '${offendingRef}' outside its workflow namespace. Declare it under '${child.name}' inputs:, pass the caller value through '${includeNode.id}' with:, and read it as '$INPUTS.' instead.`, + }; + } } + return { prompt: content }; + }; + + if (isCommandNode(node)) { + const compiled = compile(node.command); + if (compiled.error !== undefined) throw new IncludeExpansionError(compiled.error); + const { command, ...base } = node; + void command; + return { ...base, prompt: compiled.prompt }; + } + + if (isLoopNode(node) && node.loop.command !== undefined) { + const existing = (node.loop as typeof node.loop & LoopWithCompiledCommand)[ + COMPILED_LOOP_COMMAND + ]; + if (existing !== undefined) return node; + const loop = { ...node.loop } as typeof node.loop & LoopWithCompiledCommand; + loop[COMPILED_LOOP_COMMAND] = compile(node.loop.command); + return { ...node, loop }; } + + if (isLoopGroupNode(node)) { + const bodyIds = new Set(node.loop_group.nodes.map(body => body.id)); + const bodyEnclosingIds = new Set([...enclosingIds, ...currentIds]); + return { + ...node, + loop_group: { + ...node.loop_group, + nodes: node.loop_group.nodes.map(body => + materializeBlockCommandPrompts( + body, + includeNode, + child, + commandContents, + bodyIds, + bodyEnclosingIds, + `${nodePath} → ${body.id}` + ) + ), + }, + }; + } + + return node; } /** @@ -549,14 +624,17 @@ function scanBlockCommandRefs( * depth, id collision, invalid flattened structure, command-file cross-ref) is dropped * from the output and an error is recorded — other workflows still expand. * - * `commandContents` maps command NAME → file content (or null when unresolvable). When - * provided (discovery pre-resolves it for include-target command nodes) the expander - * scans block command files for sibling refs that namespacing would break and `$INPUTS` - * parameters that cannot be applied to external command bodies; omit it to skip that scan. + * `commandContents` maps command NAME → file content, null when no candidate resolves, or a + * path-bearing error when a higher-precedence scope cannot be inspected or a matched file + * cannot be read. Discovery pre-resolves every include-target command with + * execution-equivalent precedence and never falls through after either error. A caller that + * omits the map may still expand workflows without commands. Included command nodes fail + * composition; included loop commands fail before a fresh AI turn but remain discoverable + * so an already-paused loop can resume from its persisted read-once snapshot. */ export function expandWorkflowIncludes( rawByName: Map, - commandContents?: ReadonlyMap + commandContents?: ReadonlyMap ): { workflows: Map; errors: WorkflowLoadError[]; @@ -611,8 +689,7 @@ export function expandWorkflowIncludes( throw e; } warnDroppedWorkflowLevelFields(node, child); - if (commandContents) scanBlockCommandRefs(node, child, commandContents); - const inlined = inlineInclude(node, child); + const inlined = inlineInclude(node, child, commandContents ?? new Map()); 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 2d8976eb09..6e0ba40b40 100644 --- a/packages/workflows/src/loader.test.ts +++ b/packages/workflows/src/loader.test.ts @@ -36,6 +36,7 @@ registerBuiltinProviders(); import { discoverWorkflows, discoverWorkflowsWithConfig } from './workflow-discovery'; import { isBashNode, isCancelNode, isLoopNode } from './schemas'; import { parseWorkflow } from './loader'; +import { COMPILED_LOOP_COMMAND, type LoopWithCompiledCommand } from './compiled-command'; import { workflowDefinitionSchema } from './schemas/workflow'; import type { WorkflowDefinition } from './schemas/workflow'; import * as bundledDefaults from './defaults/bundled-defaults'; @@ -104,11 +105,12 @@ describe('Workflow Loader', () => { }); }); - it('retains an included workflow own packaged resource owner', async () => { + it('resolves an included workflow command from its own package before compiling it', async () => { const parentDir = join(testDir, '.archon', 'workflows', 'product', 'parent'); const blockDir = join(testDir, '.archon', 'workflows', 'shared', 'review-block'); + const blockCommandsDir = join(blockDir, 'commands'); await mkdir(parentDir, { recursive: true }); - await mkdir(blockDir, { recursive: true }); + await mkdir(blockCommandsDir, { recursive: true }); await writeFile( join(parentDir, 'parent.yaml'), `name: parent\ndescription: parent\nnodes:\n - id: review\n include: review-block\n` @@ -117,16 +119,14 @@ describe('Workflow Loader', () => { join(blockDir, 'block.yaml'), `name: review-block\ndescription: block\nnodes:\n - id: run\n command: inspect\n` ); + await writeFile(join(blockCommandsDir, 'inspect.md'), 'Package-owned review prompt.'); const result = await discoverWorkflows(testDir, { loadDefaults: false }); const parent = result.workflows.find(entry => entry.workflow.name === 'parent')?.workflow; - const included = parent?.nodes.find(node => node.id === 'review__run') as - | { command: string } - | undefined; - expect(parsePackagedResourceReference(included?.command ?? '')).toEqual({ - owner: { source: 'project', pack: 'shared', workflow: 'review-block' }, - name: 'inspect', - }); + const included = parent?.nodes.find(node => node.id === 'review__run'); + expect(included && 'prompt' in included ? included.prompt : '').toBe( + 'Package-owned review prompt.' + ); }); it('uses the identical authored structure in home scope', async () => { @@ -2275,6 +2275,46 @@ nodes: expect(result.workflows).toHaveLength(0); }); + it('treats $INPUTS.output as a declared input macro before include expansion', async () => { + const workflowDir = join(testDir, '.archon', 'workflows'); + await mkdir(workflowDir, { recursive: true }); + await Promise.all([ + writeFile( + join(workflowDir, 'input-output-block.yaml'), + ` +name: input-output-block +description: Block with an input named output +inputs: + output: + required: true +nodes: + - id: review + prompt: "Review $INPUTS.output" +` + ), + writeFile( + join(workflowDir, 'input-output-parent.yaml'), + ` +name: input-output-parent +description: Includes the input output block +nodes: + - id: inc + include: input-output-block + with: + output: bound-value +` + ), + ]); + + const result = await discoverWorkflows(testDir, { loadDefaults: false }); + expect(result.errors).toHaveLength(0); + const parent = result.workflows.find( + item => item.workflow.name === 'input-output-parent' + )?.workflow; + const review = parent?.nodes.find(node => node.id === 'inc__review'); + expect(review && 'prompt' in review ? review.prompt : undefined).toBe('Review bound-value'); + }); + it('should validate script/cancel/approval.message/until_bash refs at load time', async () => { const workflowDir = join(testDir, '.archon', 'workflows'); await mkdir(workflowDir, { recursive: true }); @@ -2301,10 +2341,7 @@ nodes: expect(result.errors[0].error).toContain('$missing.output'); }); - it('should ignore $nodeId.output inside fenced code blocks in prompt: bodies', async () => { - // Prompt bodies often embed fenced documentation examples for the LLM - // (e.g. workflow-builder shows how to author a script node). The literal - // $other-node.output in such a fence is documentation, not a real ref. + it('should validate $nodeId.output inside fenced code blocks in prompt: bodies', async () => { const workflowDir = join(testDir, '.archon', 'workflows'); await mkdir(workflowDir, { recursive: true }); @@ -2327,12 +2364,12 @@ nodes: ); const result = await discoverWorkflows(testDir, { loadDefaults: false }); - expect(result.errors).toHaveLength(0); - expect(result.workflows).toHaveLength(1); + expect(result.errors).toHaveLength(1); + expect(result.errors[0].error).toContain('$other-node.output'); + expect(result.workflows).toHaveLength(0); }); - it('should ignore $nodeId.output inside inline backtick code in prompt: bodies', async () => { - // Inline `code` mentions like \`$nodeId.output\` are also documentation. + it('should validate $nodeId.output inside inline backtick code in prompt: bodies', async () => { const workflowDir = join(testDir, '.archon', 'workflows'); await mkdir(workflowDir, { recursive: true }); @@ -2350,8 +2387,51 @@ nodes: ); const result = await discoverWorkflows(testDir, { loadDefaults: false }); - expect(result.errors).toHaveLength(0); - expect(result.workflows).toHaveLength(1); + expect(result.errors).toHaveLength(1); + expect(result.errors[0].error).toContain('$nodeId.output'); + expect(result.workflows).toHaveLength(0); + }); + + it('should validate shorthand when refs at load time', async () => { + const workflowDir = join(testDir, '.archon', 'workflows'); + await mkdir(workflowDir, { recursive: true }); + await writeFile( + join(workflowDir, 'bad-when-shorthand.yaml'), + `name: bad-when-shorthand +description: dangling shorthand condition +nodes: + - id: task + prompt: work + when: "$caller.status == 'ok'" +` + ); + + const result = await discoverWorkflows(testDir, { loadDefaults: false }); + expect(result.errors).toHaveLength(1); + expect(result.errors[0].error).toContain("field 'when'"); + expect(result.errors[0].error).toContain('$caller.status'); + }); + + it('should validate approval.on_reject.prompt refs at load time', async () => { + const workflowDir = join(testDir, '.archon', 'workflows'); + await mkdir(workflowDir, { recursive: true }); + await writeFile( + join(workflowDir, 'bad-rejection-ref.yaml'), + `name: bad-rejection-ref +description: dangling rejection prompt ref +nodes: + - id: gate + approval: + message: Approve? + on_reject: + prompt: "Revise $caller.output" +` + ); + + const result = await discoverWorkflows(testDir, { loadDefaults: false }); + expect(result.errors).toHaveLength(1); + expect(result.errors[0].error).toContain("field 'approval.on_reject.prompt'"); + expect(result.errors[0].error).toContain('$caller.output'); }); it('should still reject unknown $nodeId.output refs outside code', async () => { @@ -2383,8 +2463,7 @@ nodes: expect(result.errors[0].error).toContain('missing-node'); }); - it('should ignore $nodeId.output inside fenced code in loop.prompt', async () => { - // Loop prompts get the same documentation-stripping treatment as node prompts. + it('should validate $nodeId.output inside fenced code in loop.prompt', async () => { const workflowDir = join(testDir, '.archon', 'workflows'); await mkdir(workflowDir, { recursive: true }); @@ -2408,8 +2487,9 @@ nodes: ); const result = await discoverWorkflows(testDir, { loadDefaults: false }); - expect(result.errors).toHaveLength(0); - expect(result.workflows).toHaveLength(1); + expect(result.errors).toHaveLength(1); + expect(result.errors[0].error).toContain('$other-node.output'); + expect(result.workflows).toHaveLength(0); }); }); @@ -4078,13 +4158,12 @@ nodes: expect(payload.safetyNote).toContain('sandbox'); }); - it('should fail expansion when a block command file references a renamed sibling', async () => { + it('should compile a block command file and namespace a local sibling ref', async () => { const workflowDir = join(testDir, '.archon', 'workflows'); const commandsDir = join(testDir, '.archon', 'commands'); await mkdir(workflowDir, { recursive: true }); await mkdir(commandsDir, { recursive: true }); - // Command file references a SIBLING node id that namespacing will rename. await writeFile(join(commandsDir, 'blk-runner.md'), 'Summarize $sib.output for the report.'); await writeFile( join(workflowDir, 'cmd-block.yaml'), @@ -4111,13 +4190,15 @@ nodes: ); const result = await discoverWorkflows(testDir, { loadDefaults: false }); - expect(result.workflows.some(w => w.workflow.name === 'cmd-parent')).toBe(false); - const err = result.errors.find(e => e.filename === 'cmd-parent.yaml'); - expect(err?.error).toContain("command file 'blk-runner.md'"); - expect(err?.error).toContain("sibling node '$sib'"); + expect(result.errors.filter(error => error.filename === 'cmd-parent.yaml')).toHaveLength(0); + const parent = result.workflows.find(w => w.workflow.name === 'cmd-parent')?.workflow; + const runner = parent?.nodes.find(node => node.id === 'rev__runner'); + expect(runner && 'prompt' in runner ? runner.prompt : '').toBe( + 'Summarize $rev__sib.output for the report.' + ); }); - it('should fail expansion when a resolved block command file references an include input', async () => { + it('should compile a resolved block command file with a declared include input', async () => { const workflowDir = join(testDir, '.archon', 'workflows'); const commandsDir = join(testDir, '.archon', 'commands'); await mkdir(workflowDir, { recursive: true }); @@ -4129,6 +4210,8 @@ nodes: ` name: parameterized-block description: Block whose command references an include input +inputs: + scope: { required: true } nodes: - id: runner command: parameterized-runner @@ -4148,24 +4231,22 @@ nodes: ); const result = await discoverWorkflows(testDir, { loadDefaults: false }); - expect(result.workflows.some(w => w.workflow.name === 'parameterized-parent')).toBe(false); - const message = result.errors.find( - error => error.filename === 'parameterized-parent.yaml' - )?.error; - expect(message).toContain("Node 'review'"); - expect(message).toContain("included block 'parameterized-block'"); - expect(message).toContain("command file 'parameterized-runner.md'"); - expect(message).toContain("parameter '$INPUTS.scope'"); - expect(message).toContain('inline the prompt'); + expect( + result.errors.filter(error => error.filename === 'parameterized-parent.yaml') + ).toHaveLength(0); + const parent = result.workflows.find( + workflow => workflow.workflow.name === 'parameterized-parent' + )?.workflow; + const runner = parent?.nodes.find(node => node.id === 'review__runner'); + expect(runner && 'prompt' in runner ? runner.prompt : '').toBe('Review main.'); }); - it('should scan block command files in a configured custom command folder (config parity)', async () => { + it('should compile block commands from a configured custom command folder', async () => { const workflowDir = join(testDir, '.archon', 'workflows'); const customCmds = join(testDir, 'my-cmds'); await mkdir(workflowDir, { recursive: true }); await mkdir(customCmds, { recursive: true }); - // The command file lives ONLY in the configured custom folder, referencing a sibling. await writeFile( join(customCmds, 'custom-runner.md'), 'Summarize $sib.output for the report.' @@ -4194,20 +4275,23 @@ nodes: ` ); - // Through discoverWorkflowsWithConfig with the custom command folder configured, the - // scan resolves the command (config parity) and catches the sibling ref. const result = await discoverWorkflowsWithConfig(testDir, () => Promise.resolve({ defaults: { loadDefaultWorkflows: false }, commands: { folder: 'my-cmds' }, }) ); - expect(result.workflows.some(w => w.workflow.name === 'cc-parent')).toBe(false); - const err = result.errors.find(e => e.filename === 'cc-parent.yaml'); - expect(err?.error).toContain("sibling node '$sib'"); + expect(result.errors.filter(error => error.filename === 'cc-parent.yaml')).toHaveLength(0); + const parent = result.workflows.find( + workflow => workflow.workflow.name === 'cc-parent' + )?.workflow; + const runner = parent?.nodes.find(node => node.id === 'rev__runner'); + expect(runner && 'prompt' in runner ? runner.prompt : '').toBe( + 'Summarize $rev__sib.output for the report.' + ); }); - it('should warn (not fail) when a block command file cannot be resolved for scanning', async () => { + it('should fail closed when a block command file cannot be resolved', async () => { const workflowDir = join(testDir, '.archon', 'workflows'); await mkdir(workflowDir, { recursive: true }); @@ -4233,19 +4317,14 @@ nodes: ); const result = await discoverWorkflows(testDir, { loadDefaults: false }); - // Unresolvable command → WARN, never a hard expansion error. The scan is - // best-effort by construction; a file it cannot read is unverified, not unsafe, - // and dropping the workflow would break includes that never used this feature. const parentErrors = result.errors.filter(e => e.filename === 'ghost-parent.yaml'); - expect(parentErrors).toHaveLength(0); - expect(result.workflows.some(w => w.workflow.name === 'ghost-parent')).toBe(true); - expect(mockLogger.warn).toHaveBeenCalledWith( - expect.objectContaining({ include: 'g', command: 'ghost-cmd-does-not-exist-xyz' }), - 'include.command_file_unresolved_for_ref_scan' - ); + expect(parentErrors).toHaveLength(1); + expect(result.workflows.some(w => w.workflow.name === 'ghost-parent')).toBe(false); + expect(parentErrors[0].error).toContain("command 'ghost-cmd-does-not-exist-xyz'"); + expect(parentErrors[0].error).toContain('could not be resolved during composition'); }); - it('should scan an included loop.command file for include inputs', async () => { + it('should compile an included loop.command file with declared inputs', async () => { const workflowDir = join(testDir, '.archon', 'workflows'); const commandDir = join(testDir, '.archon', 'commands'); await mkdir(workflowDir, { recursive: true }); @@ -4256,6 +4335,8 @@ nodes: ` name: loop-block description: Block with a deferred loop prompt +inputs: + scope: { required: true } nodes: - id: repeat loop: @@ -4278,10 +4359,17 @@ nodes: ); const result = await discoverWorkflows(testDir, { loadDefaults: false }); - expect(result.workflows.some(w => w.workflow.name === 'loop-parent')).toBe(false); - expect(result.errors.find(error => error.filename === 'loop-parent.yaml')?.error).toContain( - "command file 'loop-review.md'" - ); + expect(result.errors.filter(error => error.filename === 'loop-parent.yaml')).toHaveLength(0); + const parent = result.workflows.find( + workflow => workflow.workflow.name === 'loop-parent' + )?.workflow; + const repeat = parent?.nodes.find(node => node.id === 'review__repeat'); + const compiled = + repeat && 'loop' in repeat + ? (repeat.loop as typeof repeat.loop & LoopWithCompiledCommand)[COMPILED_LOOP_COMMAND] + : undefined; + expect(compiled?.prompt).toBe('Review production.'); + expect(repeat && 'loop' in repeat ? repeat.loop.command : undefined).toBe('loop-review'); }); }); diff --git a/packages/workflows/src/loader.ts b/packages/workflows/src/loader.ts index 4b5ad05487..c9acdb7e04 100644 --- a/packages/workflows/src/loader.ts +++ b/packages/workflows/src/loader.ts @@ -115,6 +115,9 @@ function formatNodeIssue(id: string, issue: z.ZodIssue): string { */ const OUTPUT_REF_SOURCE = String.raw`\$([a-zA-Z_][a-zA-Z0-9_-]*)\.output`; +/** `when:` also accepts `$nodeId.field` as shorthand for `$nodeId.output.field`. */ +const WHEN_REF_SOURCE = String.raw`\$([a-zA-Z_][a-zA-Z0-9_-]*)\.([a-zA-Z_][a-zA-Z0-9_]*)`; + /** * The node's `id` for messages, falling back to its 1-based position when the * id is missing or blank (the schema reports that separately as an error). @@ -333,7 +336,8 @@ function parseDagNode( /** * Validate DAG structure: unique IDs, depends_on references exist, no cycles, - * and $nodeId.output refs in when:/prompt: fields point to known nodes. + * and every runtime-substituted node-output reference points to a known node in its + * current or enclosing loop scope. * Returns error message or null if valid. * * Exported so the include-expander can re-run the same structural checks on the @@ -399,75 +403,98 @@ export function validateDagStructure( return `Cycle detected among nodes: ${cycleNodes.join(', ')}`; } - // Check $nodeId.output references across EVERY field the executor substitutes at + // Check $nodeId.output references across every public YAML field the executor substitutes at // runtime: when:, and the text surfaces that flow through substituteNodeOutputRefs - // (prompt, bash, script, approval.message, cancel, loop.prompt, loop.until_bash, - // 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. + // (prompt, bash, script, approval.message/on_reject.prompt, cancel, loop.prompt, + // loop.until_bash, loop_group.until_bash, workflow.input/with/fan_out.items). A dangling + // ref in any of them can bind the wrong flat-DAG output or fail at run time, so all must + // be validated here. // - // KEEP IN SYNC (four ref-surface enumerations must agree): + // KEEP IN SYNC (public runtime node-ref surfaces): // 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, - // 4. applyInputsMacro (include-expander.ts) — inlines include `with:` values (#2470). - // Adding a substituted field to one means updating all four. + // Adding a substituted field to one means updating all three. Included loop-command + // bodies are validated separately while materialized, then rewritten by (2). + // applyInputsMacro is intentionally a superset because some AI configuration strings + // accept include inputs without being runtime node-ref surfaces. // - // Prose fields (prompt / loop.prompt) may contain triple-backtick fenced blocks or - // single-backtick inline code that are documentation meant to render literally to - // the LLM (e.g. the workflow-builder shows authors how to write `$.output` - // inside a script-node example); strip those before scanning so they don't false-match. - // The code/expression fields (bash / script / until_bash / cancel) and when: clauses - // carry live refs (not documentation), so they are scanned verbatim. + // Runtime substitution is syntax-agnostic: canonical refs inside Markdown fences and + // inline code are live too. Validation therefore scans every surface verbatim. const outputRefPattern = new RegExp(OUTPUT_REF_SOURCE, 'g'); - const stripMarkdownCode = (s: string): string => - s.replace(/```[\s\S]*?```/g, '').replace(/`[^`\n]*`/g, ''); + const whenRefPattern = new RegExp(WHEN_REF_SOURCE, 'g'); for (const node of nodes) { - const sources: string[] = []; - if (node.when) sources.push(node.when); + const sources: { field: string; text: string }[] = []; if ('prompt' in node && typeof node.prompt === 'string') { - sources.push(stripMarkdownCode(node.prompt)); + sources.push({ field: 'prompt', text: node.prompt }); } - if (isBashNode(node)) sources.push(node.bash); - if (isScriptNode(node)) sources.push(node.script); + if (isBashNode(node)) sources.push({ field: 'bash', text: node.bash }); + if (isScriptNode(node)) sources.push({ field: 'script', text: node.script }); // workflow.input is a live ref surface (a data string), scanned verbatim like // bash/script — not prose, so no markdown stripping. workflow.fan_out.items (slice // 2, PR-C) is a live `$node.output` ref to a JSON array — scanned the same way. if (isWorkflowNode(node)) { - if (node.input) sources.push(node.input); - if (node.fan_out) sources.push(node.fan_out.items); + if (node.input) sources.push({ field: 'input', text: node.input }); + if (node.fan_out) sources.push({ field: 'fan_out.items', text: 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); + for (const [name, value] of Object.entries(node.with)) { + sources.push({ field: `with.${name}`, text: value }); + } + } + } + if (isCancelNode(node)) sources.push({ field: 'cancel', text: node.cancel }); + if (isApprovalNode(node)) { + sources.push({ field: 'approval.message', text: node.approval.message }); + if (node.approval.on_reject !== undefined) { + sources.push({ + field: 'approval.on_reject.prompt', + text: node.approval.on_reject.prompt, + }); } } - if (isCancelNode(node)) sources.push(node.cancel); - if (isApprovalNode(node)) sources.push(node.approval.message); if (isLoopNode(node)) { - // Only inline `loop.prompt` is scanned for `$nodeId.output` refs. A - // command-backed loop (`loop.command`) loads its prompt text from a file - // at runtime; that file's contents are the author's responsibility, the - // same way a `command:` node's body is not scanned at parse time. if (typeof node.loop.prompt === 'string') { - sources.push(stripMarkdownCode(node.loop.prompt)); + sources.push({ field: 'loop.prompt', text: node.loop.prompt }); + } + if (node.loop.until_bash) { + sources.push({ field: 'loop.until_bash', text: node.loop.until_bash }); } - if (node.loop.until_bash) sources.push(node.loop.until_bash); } if (isLoopGroupNode(node) && node.loop_group.until_bash) { - sources.push(node.loop_group.until_bash); + sources.push({ field: 'loop_group.until_bash', text: node.loop_group.until_bash }); } for (const source of sources) { let m: RegExpExecArray | null; outputRefPattern.lastIndex = 0; // reset stateful g-flag regex before each new source string - while ((m = outputRefPattern.exec(source)) !== null) { + while ((m = outputRefPattern.exec(source.text)) !== null) { const refNodeId = m[1]; + // `$INPUTS.name` is an input macro. In particular, `$INPUTS.output` also + // matches the canonical node-ref grammar, so the macro must take precedence. + if (refNodeId === 'INPUTS') continue; // Output refs (unlike depends_on) may also reach ENCLOSING-scope nodes: the // executor seeds a loop_group iteration's scoped output map with the outer // DAG's outputs, so `$outerNode.output` inside a body prompt is valid. if (refNodeId !== undefined && !ids.has(refNodeId) && !enclosingIds?.has(refNodeId)) { - return `Node '${node.id}' references unknown node '$${refNodeId}.output'`; + return `Node '${node.id}' field '${source.field}' references unknown node '$${refNodeId}.output'. In a composed workflow, pass caller data through declared 'inputs:' and caller 'with:' instead of referencing a caller node directly`; + } + } + } + + if (node.when !== undefined) { + let m: RegExpExecArray | null; + whenRefPattern.lastIndex = 0; + while ((m = whenRefPattern.exec(node.when)) !== null) { + const refNodeId = m[1]; + const field = m[2]; + // `$INPUTS.name` is an include-time macro, not a node reference. It is + // resolved (or rejected as missing) before the composed graph is revalidated. + if (refNodeId === 'INPUTS') continue; + if (refNodeId !== undefined && !ids.has(refNodeId) && !enclosingIds?.has(refNodeId)) { + return `Node '${node.id}' field 'when' references unknown node '$${refNodeId}.${field ?? ''}'. In a composed workflow, pass caller data through declared 'inputs:' and caller 'with:' instead of referencing a caller node directly`; } } } diff --git a/packages/workflows/src/schemas/workflow-run.ts b/packages/workflows/src/schemas/workflow-run.ts index 17985dd489..40c9bfef3d 100644 --- a/packages/workflows/src/schemas/workflow-run.ts +++ b/packages/workflows/src/schemas/workflow-run.ts @@ -302,13 +302,12 @@ export interface ApprovalContext { */ signaledTokens?: { input: number; output: number } | null; /** - * Interactive-loop only. Read-once snapshot of a command-backed loop's - * (`loop.command`) loaded prompt body, persisted at gate pause so the resumed - * invocation reuses the exact text the run started with — a command file - * edited or deleted while the run sat paused cannot change or break the - * running loop's prompt. Null for prompt-based loops (explicit-null pause - * convention, same as `sessionId`). Absent on runs paused by builds that - * predate this field — the resume path then falls back to re-reading the file. + * Interactive-loop only. Read-once snapshot of the resolved loop prompt + * template, whether authored as `loop.prompt` or loaded from `loop.command`, + * persisted at gate pause so the resumed invocation reuses the exact text the + * run started with. This also takes precedence over an included loop command's + * load-time compiled prompt/error after rediscovery. Absent on runs paused by builds + * that predate this field; those resume from the current prompt or command source. */ commandSnapshot?: string | null; } diff --git a/packages/workflows/src/workflow-discovery-command-scan.test.ts b/packages/workflows/src/workflow-discovery-command-scan.test.ts index 4fb000873f..9efb858507 100644 --- a/packages/workflows/src/workflow-discovery-command-scan.test.ts +++ b/packages/workflows/src/workflow-discovery-command-scan.test.ts @@ -1,8 +1,9 @@ -import { mkdtemp, mkdir, rm, writeFile } from 'fs/promises'; +import { chmod, mkdtemp, mkdir, rm, writeFile } from 'fs/promises'; import { tmpdir } from 'os'; import { join } from 'path'; import { afterEach, describe, expect, test } from 'bun:test'; import { discoverWorkflows } from './workflow-discovery'; +import { COMPILED_LOOP_COMMAND, type LoopWithCompiledCommand } from './compiled-command'; const tempDirectories: string[] = []; @@ -12,8 +13,8 @@ afterEach(async () => { ); }); -describe('discoverWorkflows — nested command-file scan', () => { - test('pre-resolves loop_group command files before include expansion', async () => { +describe('discoverWorkflows — nested included command compilation', () => { + test('pre-resolves and compiles loop_group command files before include expansion', async () => { const cwd = await mkdtemp(join(tmpdir(), 'archon-workflow-discovery-')); tempDirectories.push(cwd); const workflowDir = join(cwd, '.archon', 'workflows'); @@ -58,8 +59,171 @@ describe('discoverWorkflows — nested command-file scan', () => { const result = await discoverWorkflows(cwd, { loadDefaults: false }); + expect(result.errors.filter(error => error.filename === 'parent.yaml')).toHaveLength(0); + const parent = result.workflows.find(item => item.workflow.name === 'parent')?.workflow; + const group = parent?.nodes.find(node => node.id === 'inc__group'); + const repeat = group && 'loop_group' in group ? group.loop_group.nodes[0] : undefined; + const compiled = + repeat && 'loop' in repeat + ? (repeat.loop as typeof repeat.loop & LoopWithCompiledCommand)[COMPILED_LOOP_COMMAND] + : undefined; + expect(compiled?.prompt).toBe('Read $inc__seed.output and continue.'); + expect(repeat && 'loop' in repeat ? repeat.loop.command : undefined).toBe('nested-command'); + }); + + test('rejects a command-body caller ref even when the parent has the same node id', async () => { + const cwd = await mkdtemp(join(tmpdir(), 'archon-workflow-discovery-')); + tempDirectories.push(cwd); + const workflowDir = join(cwd, '.archon', 'workflows'); + const commandDir = join(cwd, '.archon', 'commands'); + await Promise.all([ + mkdir(workflowDir, { recursive: true }), + mkdir(commandDir, { recursive: true }), + ]); + await writeFile( + join(workflowDir, 'block.yaml'), + JSON.stringify({ + name: 'leaky-block', + description: 'Must not bind parent state', + nodes: [{ id: 'review', command: 'leaky-command' }], + }) + ); + await writeFile( + join(workflowDir, 'parent.yaml'), + JSON.stringify({ + name: 'parent', + description: 'Has a colliding caller id', + nodes: [ + { id: 'caller', bash: 'echo parent' }, + { id: 'inc', include: 'leaky-block', depends_on: ['caller'] }, + ], + }) + ); + await writeFile(join(commandDir, 'leaky-command.md'), 'Use $caller.output directly.'); + + const result = await discoverWorkflows(cwd, { loadDefaults: false }); + expect(result.workflows.map(item => item.workflow.name)).not.toContain('parent'); - expect(result.errors.some(error => error.filename === 'parent.yaml')).toBe(true); - expect(result.errors.some(error => error.error.includes("sibling node '$seed'"))).toBe(true); + const message = result.errors.find(error => error.filename === 'parent.yaml')?.error; + expect(message).toContain("command 'leaky-command'"); + expect(message).toContain("'$caller.output'"); + expect(message).toContain('inputs:'); + expect(message).toContain('with:'); }); + + test.skipIf(process.platform === 'win32')( + 'fails closed when a matched project command cannot be read', + async () => { + const cwd = await mkdtemp(join(tmpdir(), 'archon-workflow-discovery-')); + tempDirectories.push(cwd); + const workflowDir = join(cwd, '.archon', 'workflows'); + const commandDir = join(cwd, '.archon', 'commands'); + const archonHome = join(cwd, 'home'); + const homeCommandDir = join(archonHome, 'commands'); + await Promise.all([ + mkdir(workflowDir, { recursive: true }), + mkdir(commandDir, { recursive: true }), + mkdir(homeCommandDir, { recursive: true }), + ]); + await writeFile( + join(workflowDir, 'block.yaml'), + JSON.stringify({ + name: 'read-error-block', + description: 'Matched command read errors must not fall through', + nodes: [{ id: 'review', command: 'read-error-command' }], + }) + ); + await writeFile( + join(workflowDir, 'parent.yaml'), + JSON.stringify({ + name: 'parent', + description: 'Includes a command whose project file is unreadable', + nodes: [{ id: 'inc', include: 'read-error-block' }], + }) + ); + const projectCommandPath = join(commandDir, 'read-error-command.md'); + await writeFile(projectCommandPath, 'PROJECT body must not fall through.'); + await chmod(projectCommandPath, 0o000); + await writeFile( + join(homeCommandDir, 'read-error-command.md'), + 'HOME fallback must never execute.' + ); + + const originalArchonHome = process.env.ARCHON_HOME; + process.env.ARCHON_HOME = archonHome; + try { + const result = await discoverWorkflows(cwd, { loadDefaults: false }); + + expect(result.workflows.map(item => item.workflow.name)).not.toContain('parent'); + const message = result.errors.find(error => error.filename === 'parent.yaml')?.error; + expect(message).toContain("included workflow 'read-error-block'"); + expect(message).toContain("node 'review'"); + expect(message).toContain("command 'read-error-command'"); + expect(message).toContain(projectCommandPath); + expect(message).toContain('could not be read'); + expect(message).toContain('will not fall through'); + } finally { + if (originalArchonHome === undefined) delete process.env.ARCHON_HOME; + else process.env.ARCHON_HOME = originalArchonHome; + await chmod(projectCommandPath, 0o600); + } + } + ); + + test.skipIf(process.platform === 'win32')( + 'fails closed when a higher-precedence command scope cannot be inspected', + async () => { + const cwd = await mkdtemp(join(tmpdir(), 'archon-workflow-discovery-')); + tempDirectories.push(cwd); + const workflowDir = join(cwd, '.archon', 'workflows'); + const commandDir = join(cwd, '.archon', 'commands'); + const archonHome = join(cwd, 'home'); + const homeCommandDir = join(archonHome, 'commands'); + await Promise.all([ + mkdir(workflowDir, { recursive: true }), + mkdir(commandDir, { recursive: true }), + mkdir(homeCommandDir, { recursive: true }), + ]); + await writeFile( + join(workflowDir, 'block.yaml'), + JSON.stringify({ + name: 'scope-error-block', + description: 'Scope inspection errors must not fall through', + nodes: [{ id: 'review', command: 'scope-error-command' }], + }) + ); + await writeFile( + join(workflowDir, 'parent.yaml'), + JSON.stringify({ + name: 'parent', + description: 'Includes a command while the project scope is unreadable', + nodes: [{ id: 'inc', include: 'scope-error-block' }], + }) + ); + await writeFile( + join(homeCommandDir, 'scope-error-command.md'), + 'HOME fallback must never execute.' + ); + await chmod(commandDir, 0o000); + + const originalArchonHome = process.env.ARCHON_HOME; + process.env.ARCHON_HOME = archonHome; + try { + const result = await discoverWorkflows(cwd, { loadDefaults: false }); + + expect(result.workflows.map(item => item.workflow.name)).not.toContain('parent'); + const message = result.errors.find(error => error.filename === 'parent.yaml')?.error; + expect(message).toContain("included workflow 'scope-error-block'"); + expect(message).toContain("node 'review'"); + expect(message).toContain("command 'scope-error-command'"); + expect(message).toContain(commandDir); + expect(message).toContain('could not inspect higher-precedence command scope'); + expect(message).toContain('will not fall through'); + } finally { + if (originalArchonHome === undefined) delete process.env.ARCHON_HOME; + else process.env.ARCHON_HOME = originalArchonHome; + await chmod(commandDir, 0o700); + } + } + ); }); diff --git a/packages/workflows/src/workflow-discovery.ts b/packages/workflows/src/workflow-discovery.ts index 59d6c4d308..e1c50c6235 100644 --- a/packages/workflows/src/workflow-discovery.ts +++ b/packages/workflows/src/workflow-discovery.ts @@ -44,6 +44,7 @@ import { parsePackagedResourceReference, qualifyWorkflowResources, } from './packaged-workflow'; +import type { IncludeCommandContent } from './compiled-command'; export { isValidWorkflowFolderSegment } from './packaged-workflow'; @@ -398,15 +399,16 @@ interface CommandScanConfig { /** * Resolve a command name to its file CONTENT, mirroring the runtime/validator search * order (repo `.archon/commands/` + configured `commandFolder` → `~/.archon/commands/` → - * bundled defaults, unless `loadDefaultCommands` is false). Returns `null` when the command - * cannot be resolved. Read-only; used solely so the include expander can scan a block's - * command files for sibling refs that namespacing renames. + * bundled defaults, unless `loadDefaultCommands` is false). Returns `null` when no candidate + * resolves, and a path-bearing error when a higher-precedence scope cannot be inspected or a + * matched candidate cannot be read. Read-only; used so the include expander can compile a + * block's command body while proving its lexical reference boundary. */ async function resolveCommandContentForScan( cwd: string | null, commandName: string, config: CommandScanConfig -): Promise { +): Promise { if (!isValidCommandName(commandName)) return null; const packaged = parsePackagedResourceReference(commandName); @@ -425,21 +427,16 @@ async function resolveCommandContentForScan( } else { workflowsRoot = dirname(archonPaths.getDefaultWorkflowsPath()); } + const commandPath = join( + getPackagedResourceDirectory(workflowsRoot, packaged.owner, 'commands'), + `${packaged.name}.md` + ); try { - return await readFile( - join( - getPackagedResourceDirectory(workflowsRoot, packaged.owner, 'commands'), - `${packaged.name}.md` - ), - 'utf-8' - ); + return await readFile(commandPath, 'utf-8'); } catch (error) { const err = error as NodeJS.ErrnoException; if (err.code === 'ENOENT') return null; - throw new Error( - `Failed to read packaged command "${commandName}" during include validation: ${err.message}`, - { cause: err } - ); + return { path: commandPath, message: err.message, operation: 'read' }; } } @@ -455,12 +452,22 @@ async function resolveCommandContentForScan( dirs.push(archonPaths.getHomeCommandsPath()); for (const dir of dirs) { + let entries: Awaited>; + try { + entries = await archonPaths.findMarkdownFilesRecursive(dir, '', { maxDepth: 1 }); + } catch (error) { + const err = error as NodeJS.ErrnoException; + if (err.code === 'ENOENT') continue; + return { path: dir, message: err.message, operation: 'inspect' }; + } + const match = entries.find(e => e.commandName === commandName); + if (!match) continue; + const commandPath = join(dir, match.relativePath); try { - const entries = await archonPaths.findMarkdownFilesRecursive(dir, '', { maxDepth: 1 }); - const match = entries.find(e => e.commandName === commandName); - if (match) return await readFile(join(dir, match.relativePath), 'utf-8'); - } catch { - // ENOENT / unreadable scope → try the next one. + return await readFile(commandPath, 'utf-8'); + } catch (error) { + const err = error as NodeJS.ErrnoException; + return { path: commandPath, message: err.message, operation: 'read' }; } } @@ -471,15 +478,24 @@ async function resolveCommandContentForScan( if (isBinaryBuild()) { return BUNDLED_COMMANDS[commandName] ?? null; } + const defaultsDir = archonPaths.getDefaultCommandsPath(); + let entries: Awaited>; try { - const defaultsDir = archonPaths.getDefaultCommandsPath(); - const entries = await archonPaths.findMarkdownFilesRecursive(defaultsDir, '', { maxDepth: 1 }); - const match = entries.find(e => e.commandName === commandName); - if (match) return await readFile(join(defaultsDir, match.relativePath), 'utf-8'); - } catch { - // no app defaults dir + entries = await archonPaths.findMarkdownFilesRecursive(defaultsDir, '', { maxDepth: 1 }); + } catch (error) { + const err = error as NodeJS.ErrnoException; + if (err.code === 'ENOENT') return null; + return { path: defaultsDir, message: err.message, operation: 'inspect' }; + } + const match = entries.find(e => e.commandName === commandName); + if (!match) return null; + const commandPath = join(defaultsDir, match.relativePath); + try { + return await readFile(commandPath, 'utf-8'); + } catch (error) { + const err = error as NodeJS.ErrnoException; + return { path: commandPath, message: err.message, operation: 'read' }; } - return null; } /** @@ -492,7 +508,7 @@ async function resolveIncludeBlockCommandContents( cwd: string | null, byName: ReadonlyMap, config: CommandScanConfig -): Promise> { +): Promise> { const targetNames = new Set(); const visit = (workflow: WorkflowDefinition): void => { for (const node of workflow.nodes) { @@ -505,7 +521,7 @@ async function resolveIncludeBlockCommandContents( }; for (const workflow of byName.values()) visit(workflow); - const contents = new Map(); + const contents = new Map(); if (targetNames.size === 0) return contents; // no includes → nothing to scan for (const name of targetNames) { const workflow = byName.get(name);