diff --git a/.changeset/fix-ce-workflow-skill-loading.md b/.changeset/fix-ce-workflow-skill-loading.md new file mode 100644 index 0000000000..2ced999a39 --- /dev/null +++ b/.changeset/fix-ce-workflow-skill-loading.md @@ -0,0 +1,5 @@ +--- +"@runfusion/fusion": patch +--- + +Make the compound-engineering built-in workflow actually load skills and run the full CE flow. Previously the workflow named CE skills at each node but the graph-node execution path (`runGraphCustomNode`) never loaded them: the named skill was only injected as prompt text, the plugin-injected `FUSION_CE_*` runtime env never reached the step session, and `fn_spawn_agent` was never registered for workflow steps, so persona fan-out and skill loading silently no-op'd. Now skill-executor graph steps thread the injected env, load the named skill (discovery + selection via `additionalSkillPaths`), register the spawn tool in coding mode, and receive an engine-injected Fusion workflow-step conventions preamble (await-input for questions, `FUSION_HEADLESS` degrade path, persona fan-out via `systemPromptOverride`). Adds an explicit `unattended` opt-in for `FUSION_HEADLESS`, reconciles the preamble with the gate verdict-JSON contract, and carries `skillName` through the `WorkflowStep` round-trip. diff --git a/docs/plans/2026-06-20-001-fix-compound-engineering-workflow-skill-loading-plan.md b/docs/plans/2026-06-20-001-fix-compound-engineering-workflow-skill-loading-plan.md new file mode 100644 index 0000000000..f7699fa942 --- /dev/null +++ b/docs/plans/2026-06-20-001-fix-compound-engineering-workflow-skill-loading-plan.md @@ -0,0 +1,332 @@ +--- +title: "fix: Make the compound-engineering workflow actually load skills and run the full CE flow" +type: fix +status: active +date: 2026-06-20 +plan_depth: deep +origin: docs/plans/2026-06-13-002-feat-compound-engineering-workflow-integration-plan.md +--- + +# fix: Make the compound-engineering workflow actually load skills and run the full CE flow + +## Summary + +The `builtin:compound-engineering` workflow *looks* fully wired — every node names the right CE skill (`ce-plan`, `ce-work`, `ce-code-review`, `ce-commit-push-pr`, `ce-resolve-pr-feedback`, `ce-compound`), the engine ships the enabling primitives (`systemPromptOverride` on `fn_spawn_agent`, `FUSION_WORKFLOW_STEP=1`, `===FUSION_AWAIT_INPUT===` sentinel parsing, `FUSION_CE_AGENTS_DIR`), and the dashboard has a working "Answer questions" card button + resume banner. Most of the 2026-06-13 integration plan shipped. + +But a deep dive into the live execution path shows the workflow does **not** actually deliver compound engineering on a board run. Four load-bearing gaps remain: + +1. **The named skill is never loaded into the step session (critical).** `executeWorkflowStep` builds skill selection from the assigned agent / role fallback (`executor → fusion`) only — it never forwards the step's `skillName` as a `requestedSkillName`, and never adds the CE install dir to the session's skill-discovery paths. The skill name is injected only as prompt *text* ("Invoke the `compound-engineering:ce-work` skill…") pointing at a skill the session cannot load. The June-3 skill-loading fix was applied to the interactive path, not the workflow-step path. +2. **9 of 11 bundled CE skills are verbatim upstream (critical).** Only `ce-debug` and `ce-resolve-pr-feedback` were adapted for Fusion. The rest — including `ce-plan`, `ce-work`, `ce-code-review`, `ce-commit-push-pr`, `ce-compound` — still call `AskUserQuestion` (no listener → questions lost) and fan out via raw `Task ce-*(...)` with no `systemPromptOverride` / `FUSION_CE_AGENTS_DIR` wiring, so the `ce-*` personas never resolve. +3. **Fan-out / writing steps have the wrong tool mode (high).** `plan` and `code-review` run readonly, which strips `fn_spawn_agent` — so `ce-plan`'s research agents and `ce-code-review`'s reviewer panel cannot spawn at all. `document` runs readonly, so `ce-compound` cannot write to `docs/solutions`. +4. **No genuinely-unattended signal (medium).** `FUSION_HEADLESS` is reserved but never set, so an LFG/pipeline run with no human would park on `awaiting-user-input` forever instead of recording assumptions and proceeding. +5. **The real execution seam is missing the plumbing entirely (critical, root cause).** The built-in workflow runs through the **graph-node path** (`runGraphCustomNode`, `executor.ts:5998`), which synthesizes an ephemeral step at `~6230` and calls `executeWorkflowStep` with `nodeEnv = undefined` for skill nodes (the injected `FUSION_CE_*` runtime env is only threaded on the *legacy* caller at `:12116`). And `fn_spawn_agent` is registered **only** in the main executor session (`:8076`), never in `executeWorkflowStep` — so coding mode grants write/edit but **not** spawn in any workflow step. Without fixing this seam, the skill-loading and fan-out fixes below silently no-op on the real workflow. + +This plan closes all five so the workflow genuinely loads and runs the CE skills end-to-end. The foundational fix is the graph-node seam (U8); the rest build on it. Per the scope decision, the fix targets **all 11 bundled skills** via an **engine-injected conventions preamble** (skills stay byte-for-byte upstream; one maintenance point) — even though only 6 are currently workflow nodes (see U5). + +**Target repo:** this repo (kb / Fusion). All paths repo-relative. + +--- + +## Problem Frame + +The CE skills were authored for an *interactive* Claude Code session: a human answers blocking questions, and the `Agent`/`Task` primitive resolves a rich `ce-*` registry. Fusion runs them in the opposite environment — an autonomous, ephemeral, readonly-by-default workflow-step session with no human attached and no `ce-*` registry. The 2026-06-13 plan added the *primitives* to bridge that gap (`systemPromptOverride`, the await-input sentinel, the bundled persona defs + `FUSION_CE_AGENTS_DIR`, coding tool mode), and the dashboard pause/resume surface. What never landed is the *wiring that makes the primitives fire*: + +- The step session never **loads** the skill it names (so the model improvises from a one-line instruction instead of following the skill). +- The skills never **use** the primitives (they still call `AskUserQuestion` and raw `Task ce-*`), because no per-skill adaptation or engine preamble tells them to. +- Three of the steps that must spawn or write are locked **readonly**. +- There is no signal to **degrade honestly** when truly nobody can answer. + +The result is a workflow that name-drops compound engineering at each node but executes a degraded, skill-less version of it. The fix has three threads — **load the skill**, **teach every skill the Fusion conventions once (engine preamble)**, and **give each node the capability (tool mode / headless signal) its skill needs** — plus the end-to-end test that would have caught the loading gap. + +--- + +## Requirements + +- R1. For every `executor: "skill"` workflow step, the named skill is actually **loaded** into the step session (discovered on a skill path AND selected by name), not merely referenced in the prompt text. +- R2. Every CE skill step receives the Fusion workflow-step conventions — emit `===FUSION_AWAIT_INPUT===` for user questions instead of `AskUserQuestion`; fan out to a `ce-*` persona by reading its def from `FUSION_CE_AGENTS_DIR` and passing the body as `systemPromptOverride` to `fn_spawn_agent` — **without** forking each skill's markdown. +- R3. A genuinely-unattended run (LFG / pipeline / `disable-model-invocation`) sets `FUSION_HEADLESS=1`; skills then record assumptions and proceed instead of parking. A normal board run (human reachable asynchronously) does **not** set it and still pauses for answers via the existing await-input + card-button surface. +- R4. CE steps whose skills fan out can spawn subagents (`ce-plan`, `ce-code-review`), and the `document` step can write to `docs/solutions` (`ce-compound`). +- R5. All 11 bundled skills behave correctly under the preamble: the two already hand-adapted skills (`ce-debug`, `ce-resolve-pr-feedback`) do not double-instruct, and `ce-resolve-pr-feedback`'s resolver fan-out resolves a persona via `systemPromptOverride`. +- R6. An end-to-end test exercises an `executor: "skill"` step **on the graph-node path** and asserts skill loading, coding-mode spawn availability, preamble presence, and the await-input pause; all existing `builtin-workflows` and executor tests stay green. +- R7. On the graph-node execution path (`runGraphCustomNode`), a skill step's session receives the injected CE runtime env (`FUSION_CE_SKILLS_DIR`, `FUSION_CE_AGENTS_DIR`) and — in coding mode — the `fn_spawn_agent` tool, so R1/R2/R3 actually fire on the workflow the plan targets (not only on the legacy `executeWorkflowStep` caller). + +--- + +## Key Technical Decisions + +- KTD-1 — **Load the step skill via `requestedSkillNames` + `additionalSkillPaths`, mirroring the interactive fix.** The resolver already works when fed both (proven by `packages/engine/src/__tests__/compound-engineering-skill-resolution.test.ts`); the gap is that `executeWorkflowStep` (`packages/engine/src/executor.ts:12409`, with the skill-context build at `~12532` and `createResolvedAgentSession` at `~12564`) never feeds them. Carry the node's `skillName` onto the `WorkflowStep` and, in the step session, merge it into the resolved `skillSelection`'s requested names and pass the CE skills install root as `additionalSkillPaths`, read from the step env (`FUSION_CE_SKILLS_DIR`, which the plugin exposes via `executorRuntimeEnv`). **Critical ordering:** on the graph-node path that the built-in workflow actually runs, that env key is absent until U8 threads the injected runtime env into `runGraphCustomNode` — so KTD-1 depends on U8, and U6 must assert the dir is present at runtime on the *graph* path, not on a hand-fed `stepEnv`. *Rationale:* the resolver and the discovery seam (`pi.ts` `additionalSkillPaths`) already exist — once the env is threaded (U8), this is the one missing hand-off, and it is exactly the "workflow execution loads skills" requirement. + +- KTD-2 — **Engine-injected Fusion-conventions preamble, not per-skill markdown forks.** At the skill-prompt build path (`executor.ts` ~6172, the `executorKind === "skill"` branch) prepend a stable conventions block to the step prompt whenever a skill step runs in a workflow step. The block teaches any skill: (a) you are a Fusion workflow step — `AskUserQuestion` has no listener; surface user questions via a single `===FUSION_AWAIT_INPUT===` block and stop; (b) to fan out to a `ce-`, read `$FUSION_CE_AGENTS_DIR/.md`, strip frontmatter, and pass the body as `systemPromptOverride` to `fn_spawn_agent`; when spawn is unavailable, do the work inline. *Rationale:* the chosen approach — skills stay byte-for-byte upstream (trivial re-vendoring on CE updates), and the conventions live in one place that applies to all 11 skills at once. *Idempotency:* `ce-debug` (JSON protocol) and `ce-resolve-pr-feedback` (already emit the sentinel) must not double-instruct — see KTD-5 / U5. *Output-contract coexistence:* `executeWorkflowStep` already injects a system prompt requiring a trailing `{"verdict":...}` JSON object (`executor.ts:12453-12484`); the preamble's "emit a sentinel and stop" / skill-native output must reconcile with that — see KTD-6. *Override correctness:* the preamble must reliably win over 9 unforked skill bodies whose own text still says "call `AskUserQuestion`"; this is asserted behaviorally for at least one skill in U5/U6, not assumed. + +- KTD-3 — **`FUSION_HEADLESS` is an explicit opt-in flag threaded through the run, not an inferred origin heuristic.** "Headless" is *not* "running in a workflow step" — a board step is unattended *now* but a human can answer later via the card button. `FUSION_HEADLESS=1` means *no human will ever answer* — LFG / pipeline / `disable-model-invocation` runs. **No such origin signal exists in the executor path today** (verified: no `lfg`/`pipeline`/`disableModelInvocation`/`unattended`/`runOrigin` marker reaches the workflow-step walk — only the existing `FUSION_HEADLESS` reservation comment at `executor.ts:12552`). So this is a threading change, not a lookup: add an explicit `unattended` boolean on the workflow-run options, set it only at the LFG/pipeline/`disable-model-invocation` entrypoints, and thread it through `runGraphCustomNode` → `executeWorkflowStep` → the `stepEnv` build at `executor.ts:12553` (the same threading seam as U8). **Default is unset = board run** — absence of the explicit flag must *always* yield no `FUSION_HEADLESS`, regardless of any other run attribute, so a misdetection can only ever park a task a human can answer (safe direction), never silently skip approval. The preamble's question branch checks `FUSION_HEADLESS`: set → record assumptions and proceed; unset → emit the await-input block. *Rationale:* satisfies "degrade honestly" (R3) without an inference that could regress the board's human-in-the-loop pause. + +- KTD-4 — **Bump tool mode on the fan-out / writing nodes — but coding mode alone does not deliver spawn.** `plan` and `code-review` move to `toolMode: "coding"`, and `document` moves to `coding` so `ce-compound` can write `docs/solutions`. **Correction (verified):** coding mode gives write/edit but **not** `fn_spawn_agent` in a workflow step — that tool is registered only in the main executor session (`executor.ts:8076`), never in `executeWorkflowStep`, which passes an empty custom-tool set in coding mode (`:12554-12556, :12581`). The persona fan-out therefore cannot fire in *any* CE step (including the already-coding `execute`/`commit-pr`/`resolve-feedback`) until U8 registers the spawn tool for coding-mode skill steps. So coding mode is necessary but not sufficient; KTD-4 pairs with U8. *Write-capability posture (decision):* coding mode exposes write/edit on `plan`/`code-review`, which those skills are not supposed to use. We accept this for now with **no engine-level guard** — the only protection is skill discipline plus a U6 detection assertion (non-empty diff on plan/code-review is flagged, not blocked). The proper guard, a dedicated readonly-plus-spawn tool mode (spawn without write), stays **deferred** (see Scope Boundaries); the residual risk is documented in Risk-1. *Rationale:* the tool policy is binary today (`workflow-step-tool-policy.ts`), so coding is the only mode that can carry write or (post-U8) spawn. + +- KTD-5 — **Persona fan-out stays skill-driven through the preamble + the shipped `systemPromptOverride` primitive.** No new engine surface for personas: the preamble instructs the skill to read the def and pass it as `systemPromptOverride` (the primitive added in the prior plan, `executor.ts` ~958 / ~15217). For `ce-resolve-pr-feedback`, whose await-input branch is already in-place, keep its bespoke question handling but let the preamble supply the persona-fan-out convention for its `ce-pr-comment-resolver` spawn (R5). *Rationale:* reuse the existing primitive; avoid a persona-registry API (still deferred). + +- KTD-6 — **Reconcile the preamble with the step's existing verdict-JSON output contract.** `executeWorkflowStep` injects a system prompt requiring every step agent to end with a single trailing `{"verdict":"APPROVE|APPROVE_WITH_NOTES|REVISE",...}` JSON object (`executor.ts:12453-12484`). That collides with the preamble telling a skill to "emit a `===FUSION_AWAIT_INPUT===` block and stop" and with non-gate skills (`ce-plan`, `ce-work`, `ce-compound`) whose output isn't verdict-shaped. Define precedence: the await-input sentinel, when present, takes priority and the verdict is not required that turn; **gate** steps (`code-review`) still emit the verdict after their skill work; **non-gate** skill steps relax the verdict requirement (the executor already runs `parseAwaitInputSentinel` on output regardless). Implement by making the verdict instruction conditional on the step being a gate (or skill-less) step. *Rationale:* without this, the model is told to end two different ways and the gate parsing or the skill output breaks. *Note:* the preamble rides on the node prompt (`~6172`) while the verdict instruction rides on the step system prompt (`~12453`); U2/U6 must confirm both reach the same session and don't contradict. + +- KTD-7 — **Confine and trust-bound the persona-def read before injecting it as `systemPromptOverride`.** The preamble has the skill read `$FUSION_CE_AGENTS_DIR/.md` and inject its body verbatim as a child's system prompt. Guard it: resolve the persona-def path and assert it stays within the install dir (reject `../` traversal in the persona name); require valid frontmatter and a body-size cap before use; and keep `FUSION_CE_AGENTS_DIR` plugin-installer-owned and **outside the repo worktree** — important now that coding-mode `plan`/`code-review` steps can write files and could otherwise reach an in-tree agents dir. *Rationale:* the defs are trusted today (vendored by the plugin installer), but injecting a file body verbatim into a spawned agent's system prompt is a filesystem prompt-injection surface if that dir is ever writable by anything else; the confinement check is cheap insurance. + +--- + +## High-Level Technical Design + +### Where the flow breaks today vs. the target + +```mermaid +flowchart LR + subgraph Today["Today (degraded)"] + A1[Node names ce-work] --> A2["prompt text only:
'Invoke the ce-work skill'"] + A2 --> A3[Step session loads
role fallback 'fusion' ONLY] + A3 --> A4[Skill body never loaded;
model improvises] + A4 --> A5["raw Task ce-* → no persona
AskUserQuestion → lost"] + end + subgraph Target["Target (full CE)"] + B1[Node names ce-work] --> B2["skillName → requestedSkillNames
+ CE dir → additionalSkillPaths"] + B2 --> B3[Step session LOADS ce-work] + B3 --> B4[Engine preamble adds
Fusion conventions] + B4 --> B5["fan-out via systemPromptOverride
questions via await-input"] + end +``` + +### Skill-loading data flow (KTD-1) + +```mermaid +flowchart TD + N["WorkflowStep.skillName
= compound-engineering:ce-work"] --> M[executeWorkflowStep] + M --> SC[buildSessionSkillContext
role/agent skills] + M --> RN["merge skillName into
requestedSkillNames (bare + namespaced)"] + E["stepEnv.FUSION_CE_SKILLS_DIR"] --> AP[additionalSkillPaths] + RN --> CR[createResolvedAgentSession] + AP --> CR + SC --> CR + CR --> RL["DefaultResourceLoader scans CE dir
+ resolver keeps requested name"] + RL --> OK[ce-work SKILL.md loaded into session] +``` + +### Question handling: board vs. headless (KTD-3) + +```mermaid +flowchart TD + Q[Skill has a user question] --> H{FUSION_HEADLESS set?} + H -->|yes: LFG/pipeline| AS[Record assumption,
proceed] + H -->|no: board run| AI["emit ===FUSION_AWAIT_INPUT===
→ task awaiting-user-input"] + AI --> CB[Card button + banner
→ steering answer → resume] +``` + +--- + +## Changed Surfaces + +```text +packages/ + engine/src/executor.ts # U8 runGraphCustomNode (~5998/6247/6252): thread injected + # env into skill steps + register fn_spawn_agent (~8076/15148); + # U1 skill loading in executeWorkflowStep (~12409/12532/12564); + # U2 conventions preamble at skill-prompt build (~6172); + # KTD-6 verdict-contract reconciliation (~12453-12484); + # U3 FUSION_HEADLESS on stepEnv (~12553) via explicit flag; + # U9 systemPromptOverride path-confinement (~15217) + engine/src/__tests__/ # NEW e2e graph-path skill-step test (U6) + core/src/types.ts # add skillName to WorkflowStep (~610) + WorkflowStepInput (~734) + core/src/workflow-compiler.ts # carry skillName in nodeToStepInput (~240) + comment (~233) + core/src/workflow-steps-to-ir.ts # INVERSION CONTRACT: stepInputToNode + parity test (U1) + core/src/builtin-workflows.ts # U4 toolMode on plan/code-review/document + core/src/__tests__/builtin-workflows.test.ts # updated assertions (U7) +plugins/fusion-plugin-compound-engineering/ + .fusion-ce-skills/ce-debug/SKILL.md # reconcile vs preamble (U5) + .fusion-ce-skills/ce-resolve-pr-feedback/... # reconcile + persona override (U5) + src/index.ts # FUSION_CE_SKILLS_DIR / FUSION_CE_AGENTS_DIR exposure +``` + +--- + +## Implementation Units + +**Sequencing:** U8 is foundational — it threads the CE runtime env and registers the spawn tool on the graph-node path the workflow actually runs, and U1/U3/U4/U6 depend on it. Build U8 first, then U1 (skill loading) and U2 (preamble), then U3/U4/U5/U9, with U6/U7 locking the behavior in. Units are listed below in their original numeric order, not build order. + +### U1. Load the named skill into the workflow-step session +- **Goal:** The skill a step names is actually discovered and selected in that step's session — the core "workflow execution loads skills" fix. +- **Requirements:** R1 +- **Dependencies:** U8 (the graph-node path must thread `FUSION_CE_SKILLS_DIR` into the step env first, or this fix no-ops on the real workflow) +- **Files:** `packages/core/src/types.ts` (add `skillName?: string` to `WorkflowStep` ~610-655 **and** to `WorkflowStepInput` ~734); `packages/engine/src/executor.ts` **`~6230`** (the graph-node step-literal synthesis seam — copy `cfg.skillName` onto the synthesized `WorkflowStep`; this is the seam the built-in workflow runs, and it currently drops the field) and `executeWorkflowStep` (`~12409`, skill-context build `~12532`: merge the step's skill name into the resolved `skillSelection.requestedSkillNames`; pass `additionalSkillPaths: [stepEnv.FUSION_CE_SKILLS_DIR]` into `createResolvedAgentSession` `~12564`); `packages/core/src/workflow-compiler.ts` (`nodeToStepInput` ~240 + the compiler-visible-fields comment ~233 — carry `skillName`) and `packages/core/src/workflow-steps-to-ir.ts` (`stepInputToNode` + the round-trip **parity test** — the INVERSION CONTRACT at `workflow-compiler.ts:237` requires both sides updated together) for the persisted-step path. +- **Approach:** Carry `skillName` onto the `WorkflowStep` at **both** materialization sites — the graph-node literal at `~6230` (where `cfg.skillName` is already in scope from the `~6172` prompt build but not copied onto the step) and the legacy compiler `nodeToStepInput`. In `executeWorkflowStep`, after `buildSessionSkillContext`, if `workflowStep.skillName` is set, add both the namespaced and bare forms (the resolver's `bareSkillName` strips `compound-engineering:`) to the requested-names set, and pass the CE install root (read from `stepEnv.FUSION_CE_SKILLS_DIR`) as `additionalSkillPaths`. Mirror the interactive fix documented in `docs/solutions/integration-issues/plugin-bundled-skills-not-loading-in-interactive-sessions.md` — discovery (path) AND selection (name) must both be satisfied. Honor the INVERSION CONTRACT: any field added to the compiler must be reflected in `stepInputToNode` and the parity test, or persisted-step round-tripping breaks. +- **Patterns to follow:** `compound-engineering-skill-resolution.test.ts` (the proven resolver contract); `pi.ts` `additionalSkillPaths` discovery seam; `index.ts:323` forwarding. +- **Test scenarios:** + - Happy path: a step with `skillName: "compound-engineering:ce-work"` produces a session whose resolved skills include `ce-work`. + - Bare-vs-namespaced: requesting `compound-engineering:ce-plan` resolves the bundled `ce-plan` dir. + - Discovery without selection: CE dir on the path but name not requested → skill not retained (asserts both halves are required). + - Edge: `FUSION_CE_SKILLS_DIR` unset → no throw; step still runs with role-fallback skills. + - Non-skill step (prompt executor) is unchanged (no requested name added). +- **Verification:** the named skill appears in the step session's resolved skill set; a real `DefaultResourceLoader` test (not a scripted session) proves it. + +### U2. Inject the Fusion workflow-step conventions preamble for skill steps +- **Goal:** Teach every skill the Fusion conventions (await-input for questions; persona fan-out via `FUSION_CE_AGENTS_DIR` + `systemPromptOverride`) in one engine-side place. +- **Requirements:** R2 +- **Dependencies:** none (composes with U1) +- **Files:** `packages/engine/src/executor.ts` (skill-prompt build, ~6172-6173). +- **Approach:** When `executorKind === "skill"` and the session is a workflow step, prepend a stable, skill-agnostic conventions block before the existing "Invoke the `` skill…" line. The block states: (1) you are a Fusion autonomous workflow step; `AskUserQuestion`/`request_user_input` have no listener — to ask the user, emit exactly one `===FUSION_AWAIT_INPUT=== … ===END_FUSION_AWAIT_INPUT===` block and stop (the executor already parses this, `executor.ts` ~974/6254); (2) when `FUSION_HEADLESS=1`, do not ask — record an assumption and proceed; (3) to dispatch a `ce-` subagent, read `$FUSION_CE_AGENTS_DIR/.md`, strip frontmatter, and pass the body as `systemPromptOverride` to `fn_spawn_agent`; if `fn_spawn_agent` is absent (readonly), do the persona's work inline. Keep the text short and declarative; it is read by every CE skill step. +- **Patterns to follow:** the existing prompt-prefix construction at 6172-6173; the sentinel grammar in `parseAwaitInputSentinel`; the `systemPromptOverride` param shape (`executor.ts` ~958). +- **Test scenarios:** + - A compiled skill step's prompt contains the conventions block ahead of the "Invoke the skill" line. + - A non-skill (prompt/gate) step does not get the block. + - The block references the await-input sentinel and `FUSION_CE_AGENTS_DIR` / `systemPromptOverride` verbatim (contract assertion so skills and engine agree on the grammar). +- **Verification:** golden-string assertion on the composed step prompt for a skill node. + +### U3. Set `FUSION_HEADLESS` for genuinely-unattended runs +- **Goal:** Distinguish "no human now, but reachable async" (board) from "no human ever" (LFG/pipeline) so skills degrade honestly. +- **Requirements:** R3 +- **Dependencies:** U2 (the preamble reads the var), U8 (shares the run-options → `runGraphCustomNode` → `stepEnv` threading seam) +- **Files:** `packages/engine/src/executor.ts` (`stepEnv` build `~12553`; thread a new `unattended` boolean from the workflow-run options through `runGraphCustomNode`/`executeWorkflowStep`); the LFG/pipeline run entrypoints that must set the flag (enumerate them — at minimum the `disable-model-invocation`/LFG pipeline caller). +- **Approach:** No origin signal exists to look up (verified — see KTD-3), so add an **explicit `unattended` boolean** on the workflow-run options and set it only at the LFG/pipeline/`disable-model-invocation` entrypoints. Thread it through the same seam U8 adds, and at the `stepEnv` build set `FUSION_HEADLESS=1` **iff** the flag is explicitly true. Default/absent → no `FUSION_HEADLESS` (board run). Leave `FUSION_WORKFLOW_STEP=1` always-on as today. Enumerate the entrypoints in the implementation rather than assuming a single marker. +- **Patterns to follow:** the existing `FUSION_WORKFLOW_STEP` injection at `~12553`; U8's env-threading change. +- **Test scenarios:** + - Board run (flag unset): `FUSION_HEADLESS` is absent on the step env. + - LFG/pipeline run (flag explicitly set): `FUSION_HEADLESS=1` on the step env. + - **Default-safe invariant:** absence of the explicit flag yields no `FUSION_HEADLESS` regardless of any other run-context attribute (guards against a heuristic creeping in). + - The variable never leaks into interactive (non-workflow) sessions. +- **Verification:** step env carries the var only when the explicit flag is set; combined with U2, a headless skill records assumptions instead of emitting an await-input block, and a board skill still parks. + +### U4. Give the fan-out / writing CE nodes the right tool mode +- **Goal:** `ce-plan` and `ce-code-review` can spawn their subagents (paired with U8); `ce-compound` can write `docs/solutions`. +- **Requirements:** R4 +- **Dependencies:** U8 (coding mode is necessary but not sufficient for spawn — U8 registers `fn_spawn_agent` for coding-mode skill steps) +- **Files:** `packages/core/src/builtin-workflows.ts` (the `plan`, `code-review`, and `document` nodes in `builtin:compound-engineering` ~186-258). +- **Approach:** Add `toolMode: "coding"` to the `plan`, `code-review`, and `document` node configs (execute/commit-pr/resolve-feedback already coding; merge stays the generic Fusion boundary per the prior plan's KTD-6). Add a short comment on each noting *why* coding is required (spawn for plan/review — once U8 lands; write for document) and that, per the documented write-capability posture (KTD-4), plan/review are not supposed to edit code and U6 asserts they produce no diff. +- **Patterns to follow:** the existing `toolMode: "coding"` execute/commit-pr nodes in the same file; `workflow-step-tool-policy.ts` allow/deny lists. +- **Test scenarios:** + - `compileWorkflowToSteps` yields `plan`, `code-review`, `document` steps with compiled `toolMode === "coding"`. + - `execute`, `commit-pr`, `resolve-feedback` remain coding; `merge` remains the generic boundary. + - Readonly-stripping does not fire for these three steps (no `[readonly-violation]` log). +- **Verification:** updated `builtin-workflows.test.ts` asserts the tool modes; combined with U8, a coding-mode step exposes `fn_spawn_agent` and write tools (before U8, spawn is absent in both modes — see U6). + +### U5. Reconcile the two adapted skills and verify all 11 against the preamble +- **Goal:** Cover all 11 bundled skills under the preamble without double-instruction, and close `ce-resolve-pr-feedback`'s persona-override gap. +- **Requirements:** R2, R5 +- **Dependencies:** U2 +- **Files:** `plugins/fusion-plugin-compound-engineering/.fusion-ce-skills/ce-debug/SKILL.md`, `.fusion-ce-skills/ce-resolve-pr-feedback/SKILL.md` (+ its `references/full-mode.md`), and an audit pass over the remaining nine (`ce-brainstorm`, `ce-code-review`, `ce-commit`, `ce-commit-push-pr`, `ce-compound`, `ce-ideate`, `ce-plan`, `ce-strategy`, `ce-work`). +- **Scope note (all 11, with caveat):** Per the scope decision the audit covers all 11 bundled skills, but only **6 are current `builtin:compound-engineering` nodes** (`ce-plan`, `ce-work`, `ce-code-review`, `ce-commit-push-pr`, `ce-resolve-pr-feedback`, `ce-compound`). The other 5 (`ce-brainstorm`, `ce-ideate`, `ce-strategy`, `ce-commit`, `ce-debug`) are **not** workflow nodes today — the preamble fires only when a skill runs as a workflow step, so R2/R5 coverage for them is **forward-looking** (it pays off if/when they become nodes). State this explicitly rather than implying an active consumer. `ce-debug`'s JSON protocol in particular only matters for standalone/interactive debug invocation, not the workflow — do not put it on the workflow-critical path. +- **Approach:** With the preamble supplying the generic conventions, remove or slim the *duplicated* in-place instructions in `ce-debug` and `ce-resolve-pr-feedback` so the same guidance isn't stated twice (keep only genuinely bespoke behavior — e.g. `ce-debug`'s JSON protocol, which is structurally incompatible with the plaintext sentinel and is a standalone-invocation caveat, not a workflow reconciliation). For `ce-resolve-pr-feedback`, ensure its `ce-pr-comment-resolver` dispatch follows the preamble's persona-override pattern. For the rest, confirm the preamble is sufficient by **enumerating** each skill's persona list and question sites and mapping them onto await-input + `systemPromptOverride`; add a brief per-skill note only where the generic preamble can't express what the skill needs. Do not re-fork skills the preamble already covers. +- **Patterns to follow:** the existing `FUSION_WORKFLOW_STEP` branch in `ce-resolve-pr-feedback/references/full-mode.md`; `ce-debug`'s JSON-protocol section; upstream skill structure (keep diffs minimal for re-vendoring). +- **Test scenarios:** + - **Override-correctness (behavioral):** for at least one verbatim skill (`ce-plan`), assert at runtime that under the preamble the skill emits a `===FUSION_AWAIT_INPUT===` block (or, headless, records an assumption) rather than calling `AskUserQuestion` — i.e. the preamble wins over the skill's own "call `AskUserQuestion`" text. (Covered in U6.) + - **Persona override (behavioral):** a `ce-resolve-pr-feedback` step spawns its resolver child with a non-empty `systemPromptOverride` sourced from the persona def. (Covered in U6.) + - **No double-instruction:** the composed `ce-debug` / `ce-resolve-pr-feedback` step prompt contains the convention guidance exactly once (engine preamble), not duplicated by the skill body. + - Manifest/install assertions: all 11 skills still install (no regression from edits). +- **Verification:** the U6 behavioral assertions above pass; `ce-debug`/`ce-resolve` do not emit duplicated convention text; the per-skill persona/question mapping is recorded in the implementation notes. + +### U6. End-to-end `executor: "skill"` workflow-step test +- **Goal:** Lock in the behavior that silently regressed — prove a skill step loads the skill, can spawn, carries the preamble, and pauses on a question. +- **Requirements:** R6, R7 +- **Dependencies:** U1, U2, U4, U8 (the test drives the graph-node path, which only works once U8 threads env + spawn) +- **Files:** `packages/engine/src/__tests__/` (new test mirroring executor/step-session test harnesses), using a real `DefaultResourceLoader` and the bundled CE skill fixtures. +- **Approach:** Drive the **graph-node path** (`runGraphCustomNode` → `executeWorkflowStep`) for a CE skill node — not only the legacy caller — so the test exercises the seam the built-in workflow actually uses. Assert: (1) `FUSION_CE_SKILLS_DIR`/`FUSION_CE_AGENTS_DIR` are present in the step session env at runtime (R7/U8); (2) the resolved session skills include the named skill (U1); (3) the composed step prompt contains the conventions preamble exactly once (U2); (4) a coding-mode skill step exposes `fn_spawn_agent` after U8 (and a readonly step does not) — note this assertion **fails against pre-U8 code where spawn is absent in both modes**, so U8 must land first; (5) emitting `===FUSION_AWAIT_INPUT===` parks the task `awaiting-user-input` with a parseable marker and a steering answer resumes it; (6) the verdict-JSON contract and the sentinel coexist per KTD-6 (a non-gate skill step does not fail for lacking a verdict; a gate step still emits one); (7) a `ce-resolve-pr-feedback` step spawns its resolver child with a non-empty `systemPromptOverride`; (8) `plan`/`code-review` steps produce no working-tree diff (write-capability detection per KTD-4). Reuse the resolver fixtures from `compound-engineering-skill-resolution.test.ts`. +- **Patterns to follow:** `compound-engineering-skill-resolution.test.ts`, the step-session executor tests, the await-input pause/resume assertions. +- **Test scenarios:** + - Env present at runtime on the graph path: `FUSION_CE_SKILLS_DIR`/`FUSION_CE_AGENTS_DIR` reach the step session. + - Skill loaded: named CE skill present in resolved session skills. + - Preamble present exactly once: conventions block in the step prompt, not duplicated. + - Spawn gating (post-U8): `fn_spawn_agent` present in coding, absent in readonly. + - Verdict/sentinel coexistence: non-gate skill step OK without verdict; gate step emits verdict. + - Persona override: `ce-resolve-pr-feedback` resolver child spawned with non-empty `systemPromptOverride`. + - Override-correctness: `ce-plan` emits the await-input sentinel (board) / records an assumption (headless) instead of calling `AskUserQuestion`. + - Write detection: `plan`/`code-review` steps leave the tree unmodified. + - Await-input: sentinel → `awaiting-user-input` + marker; steering reply → resume. +- **Verification:** `pnpm --filter @fusion/engine test` green including the new file. + +### U7. Update built-in workflow tests +- **Goal:** Pin the node-level wiring and tool modes. +- **Requirements:** R6 +- **Dependencies:** U1, U4 +- **Files:** `packages/core/src/__tests__/builtin-workflows.test.ts`. +- **Approach:** Extend the compound-engineering assertions to cover: each skill node carries its `skillName` onto the compiled step; `plan`/`code-review`/`document` compile to `toolMode: "coding"`; step ordering (plan → execute → review → code-review → commit-pr → resolve-feedback → merge → document) is intact; plugin gating still holds. +- **Patterns to follow:** existing assertions in the same file. +- **Test scenarios:** + - `skillName` present on each skill step. + - Tool modes for plan/code-review/document/execute/commit-pr/resolve-feedback. + - Workflow hidden without the plugin, shown with it. +- **Verification:** `pnpm --filter @fusion/core test builtin-workflows` green. + +### U8. Thread the injected runtime env and register `fn_spawn_agent` on the graph-node step path +- **Goal:** The seam the built-in workflow actually runs (`runGraphCustomNode`) must hand the skill-step session the injected CE env **and** the spawn tool — without this, U1 (skill loading), U3 (headless), and persona fan-out all silently no-op. **Foundational — U1, U3, U4, U6 depend on it.** +- **Requirements:** R7 (and unblocks R1, R2, R3) +- **Dependencies:** none +- **Files:** `packages/engine/src/executor.ts` — `runGraphCustomNode` (`~5998`; its call site `~4158` and the `executeWorkflowStep` call at `~6252` pass `nodeEnv`, which is `undefined` for skill nodes — `~6247`); the injected `taskEnv` build at `~7445` (`collectExecutorRuntimeEnv`); the spawn-tool registration `createSpawnAgentTool` (`~15148`, today wired only into the main session at `~8076`); the `executeWorkflowStep` custom-tools assembly (`~12554-12581`). +- **Approach:** Two coupled changes on the graph path. **(a) Env:** make the injected runtime env (carrying `FUSION_CE_SKILLS_DIR`/`FUSION_CE_AGENTS_DIR`, built from `collectExecutorRuntimeEnv`) reachable inside `runGraphCustomNode`, and pass it as the `taskEnv` argument to `executeWorkflowStep` for skill nodes (today only CLI nodes get a non-`undefined` env). The legacy caller at `~12116` already passes `taskEnv` — this closes the graph-vs-legacy asymmetry. **(b) Spawn:** register `createSpawnAgentTool` into the custom-tool set for coding-mode skill steps in `executeWorkflowStep`, so `fn_spawn_agent` is actually present where the CE skills fan out (today it's never registered there; coding mode passes an empty custom-tool set). Gate it to coding-mode steps; readonly steps keep no spawn. Account for the per-child git worktree cost the spawn tool incurs (see Deferred: lighter read-only spawn path). +- **Patterns to follow:** the main-session tool assembly at `~8052-8082` (how `createSpawnAgentTool` is constructed and passed); the legacy `executeWorkflowStep` caller at `~12116` that already threads `taskEnv`; the `FUSION_WORKFLOW_STEP` injection at `~12553`. +- **Test scenarios:** + - Graph-path skill step session env contains the injected `FUSION_CE_*` keys (was empty/`process.env` before). + - A coding-mode skill step on the graph path has `fn_spawn_agent` available; a readonly one does not. + - The legacy `executeWorkflowStep` caller behavior is unchanged (no regression). + - A spawned child still runs with `systemPromptOverride` honored (composes with the existing primitive). +- **Verification:** the U6 end-to-end test (driving the graph path) sees the env keys and the spawn tool; a manual CE board run logs the resolved skills and a successful persona spawn. + +### U9. Confine the persona-def read and document the write-capability posture +- **Goal:** Close the filesystem prompt-injection surface KTD-7 names, and make the accepted write-capability risk (KTD-4) explicit in the plan and code comments. +- **Requirements:** R2, R5 (safety of the fan-out path) +- **Dependencies:** U2 (preamble defines the read), U8 (spawn must exist for the override to be used) +- **Files:** `packages/engine/src/executor.ts` (the `systemPromptOverride` consumption path `~15217` — add path-confinement/size validation if the engine ever resolves the def; otherwise the guidance lives in the preamble text from U2); the preamble text (U2) instructing the path-confined read; plugin docs noting `FUSION_CE_AGENTS_DIR` must be installer-owned and outside the worktree. +- **Approach:** Have the preamble (U2) instruct a path-confined read: resolve `.md` and require it stays within `FUSION_CE_AGENTS_DIR` (reject `../` in the persona name), require valid frontmatter, and cap body size before passing as `systemPromptOverride`. Confirm `FUSION_CE_AGENTS_DIR` resolves outside the repo worktree (so coding-mode `plan`/`code-review` steps can't write into it). Add a code comment + a Risk-1 note documenting that plan/review write capability is model-discipline-only until the readonly-plus-spawn mode ships (the chosen posture). +- **Patterns to follow:** the `assertPluginLocalAgentsTarget` guard already in `plugins/fusion-plugin-compound-engineering/src/agent-installation.ts` (mirror its path-confinement style). +- **Test scenarios:** + - A persona name containing `../` is rejected before any read. + - `FUSION_CE_AGENTS_DIR` resolves outside the task worktree. + - `Test expectation:` the write-posture documentation is prose/comments — no behavioral test beyond U6's plan/code-review no-diff assertion. +- **Verification:** traversal attempt rejected; agents dir confirmed out-of-tree; Risk-1 posture documented. + +--- + +## Scope Boundaries + +**In scope:** loading the named skill into step sessions (R1); the engine conventions preamble for all 11 bundled skills (R2); the `FUSION_HEADLESS` unattended signal (R3); tool-mode fixes for plan/code-review/document (R4); reconciling the two already-adapted skills + the resolver persona override (R5); the end-to-end and node-level tests (R6). + +### Deferred to Follow-Up Work +- A dedicated **readonly-plus-spawn** tool mode (spawn without write) so `plan`/`code-review` can fan out without exposing edit/write — today coding is the only mode carrying `fn_spawn_agent`. +- A **lighter spawn path** that avoids a full git worktree per child for read-only reviewer personas (the `ce-code-review` panel can fan out wide; each `fn_spawn_agent` child currently gets its own worktree). +- A general **plugin-provided agent-definition registry** API (this plan keeps the file-based `FUSION_CE_AGENTS_DIR` + `systemPromptOverride` approach). +- Applying the preamble / headless signal to the **other** built-in workflows (`builtin:coding`, `builtin:stepwise-coding`, `builtin:marketing`, etc.). +- Evidence-capture / demo-reel integration in the PR flow. + +### Out of scope +- Redesigning the workflow engine, the await-input mechanism, or the workflow-owned-merge system. +- Changing the CE skills' interactive (Claude Code) behavior — the preamble only fires in workflow-step sessions. +- The dashboard "Answer questions" button/banner — already shipped and tested. + +--- + +## Risks & Dependencies + +- Risk-1 (high, **accepted + documented**) — **Coding mode on `plan`/`code-review` exposes write/edit with no engine guard.** Per the chosen posture (KTD-4), the only protection is skill discipline plus a U6 **detection** assertion (a non-empty diff on plan/code-review is flagged, not blocked). An autonomous run — especially headless, where the skill is told to proceed without asking — could mutate the tree before code is written and corrupt the artifact the next step consumes. *Mitigation:* documented model-discipline-only posture until the proper guard (readonly-plus-spawn tool mode) ships (Deferred); U6 detects violations; re-evaluate before enabling the CE workflow for LFG/pipeline. *This is a knowingly-accepted gap, not a closed one.* +- Risk-2 (high) — **The preamble must override 9 unforked upstream skill bodies that still say "call `AskUserQuestion`" / "`Task ce-*`".** The maintenance win of the preamble approach holds only if a generic block reliably wins over each skill's own contradicting text; the real exposure is these 9, not `ce-debug` (which isn't a workflow node). *Mitigation:* U5/U6 assert override-correctness behaviorally for at least `ce-plan`; each upstream re-vendor needs a re-check that no skill body defeats the preamble. *`ce-debug`'s JSON protocol is a standalone-invocation caveat, not a workflow collision.* +- Risk-3 (med) — **`FUSION_HEADLESS` misdetection.** A wrong signal could make a board run skip questions (proceed on assumptions when a human was available) or an LFG run park forever. *Mitigation:* KTD-3 makes it an **explicit opt-in** flag with a default-safe invariant (absent → board), threaded through U8's seam; both directions covered in U3 tests. The safe default means a miss can only park a recoverable task, never silently skip approval. +- Risk-4 (med) — **The graph-node path doesn't thread the injected env (root cause).** `FUSION_CE_SKILLS_DIR`/`FUSION_CE_AGENTS_DIR` are exported by the plugin (`index.ts`, both confirmed present) but never reach the graph-node step session, so without U8 the skill-loading and fan-out fixes no-op on the real workflow. *Mitigation:* U8 closes the graph-vs-legacy asymmetry; U6 asserts the env at runtime on the graph path (not a hand-fed `stepEnv`). +- Risk-5 (med) — **Persona-def injection surface.** The preamble reads `.md` and injects its body verbatim as a child `systemPromptOverride`. If `FUSION_CE_AGENTS_DIR` is ever writable by something other than the plugin installer (compromised update, in-tree dir reachable by a coding-mode step, `../` traversal), arbitrary instructions enter a spawned agent's system prompt. *Mitigation:* U9 path-confinement + out-of-tree dir + the existing `assertPluginLocalAgentsTarget` install guard. +- Risk-6 (med) — **Verdict-contract vs. skill output collision.** The step system prompt requires a trailing verdict JSON; the preamble tells skills to emit a sentinel / skill-native output. Unreconciled, gate parsing or skill output breaks. *Mitigation:* KTD-6 defines precedence (sentinel wins; gate steps still emit verdict; non-gate skill steps relax it); U6 asserts coexistence. +- Dependency: U6's e2e test depends on the bundled CE skill fixtures being installable in the test harness (same fixtures `compound-engineering-skill-resolution.test.ts` already uses). + +--- + +## Verification Strategy + +- Unit/integration tests per unit, centered on a new engine e2e skill-step test (U6) and `builtin-workflows.test.ts` (U7), plus the existing resolver and await-input tests staying green. +- A manual autonomous board run of `builtin:compound-engineering` on a real task, confirming: (1) each step's named CE skill is actually loaded (log the resolved session skills); (2) `ce-plan` / `ce-code-review` fan out to `ce-*` personas via `systemPromptOverride`; (3) a planning question parks the task and the card button → banner → steering answer resumes it; (4) `ce-compound` writes a `docs/solutions` entry; (5) an LFG run of the same workflow records assumptions instead of parking. +- `pnpm` typecheck + affected package suites (`@fusion/engine`, `@fusion/core`) green before PR. + +--- + +## Sources & Research + +- Node wiring + skill nodes: `packages/core/src/builtin-workflows.ts:178-260`; tool policy `packages/engine/src/workflow-step-tool-policy.ts`. +- Skill-prompt build (text-only injection today): `packages/engine/src/executor.ts:6172-6173`. +- Step session + skill selection (no `skillName` / `additionalSkillPaths` forwarded): `executeWorkflowStep` at `packages/engine/src/executor.ts:12409` (skill-context build `~12532`, `createResolvedAgentSession` `~12564`); `session-skill-context.ts` (role fallback `executor → fusion`); resolver `skill-resolver.ts:211-321`. +- **Verified during doc review (root-cause seams):** the built-in workflow runs via the graph-node path `runGraphCustomNode` (`executor.ts:5998`, called `~4158`), which synthesizes the step at `~6230` (no `skillName` copied) and calls `executeWorkflowStep` with `nodeEnv = undefined` for skill nodes (`~6247/6252`) — while the legacy caller at `~12116` passes `taskEnv`. `fn_spawn_agent` (`createSpawnAgentTool` `~15148`) is registered only in the main session (`~8076`), never in `executeWorkflowStep` (coding passes an empty custom-tool set, `~12554-12581`). The step system prompt mandates a trailing `{"verdict":...}` JSON object (`~12453-12484`). The `fusion` role-fallback skill carries no CE behavior (confirmed — no partial-credit fallback). `WorkflowStep`/`WorkflowStepInput` lack `skillName` (`types.ts:610, 734`); compiler INVERSION CONTRACT at `workflow-compiler.ts:237`. `FUSION_HEADLESS` is only a reservation comment (`~12552`); no LFG/pipeline origin marker reaches the step walk. +- `additionalSkillPaths` discovery seam: `packages/engine/src/pi.ts:993, 2077-2078`; `packages/engine/src/index.ts:323`; core seam `packages/core/src/plugin-types.ts:179`. +- `systemPromptOverride` primitive: `packages/engine/src/executor.ts:958-963, 15162-15239` (child gets its own worktree ~15204-15207). +- Await-input sentinel + pause/resume: `packages/engine/src/executor.ts:974-979, 6254-6273` and `runAwaitInputNode` ~5674-5728. +- `FUSION_WORKFLOW_STEP` set / `FUSION_HEADLESS` reserved: `packages/engine/src/executor.ts:12551-12553`. +- Plugin env exposure (`FUSION_CE_AGENTS_DIR`, runtime env hook): `plugins/fusion-plugin-compound-engineering/src/index.ts:148-181`; `agent-installation.ts`; runtime-env merge `executor.ts:7434-7452`. +- Bundled skills (2 adapted, 9 verbatim): `plugins/fusion-plugin-compound-engineering/.fusion-ce-skills/` — `ce-debug` (JSON protocol), `ce-resolve-pr-feedback/references/full-mode.md` (`FUSION_WORKFLOW_STEP` await-input branch); the rest call `AskUserQuestion` + raw `Task ce-*`. +- Dashboard pause/resume (shipped): `packages/dashboard/app/components/TaskCard.tsx:2048-2061`, `WorkflowResultsTab.tsx:68-76, 922-951`, `app/api/legacy.ts:5376-5381`. +- Prior plan + skill-loading learning: `docs/plans/2026-06-13-002-feat-compound-engineering-workflow-integration-plan.md`; `docs/solutions/integration-issues/plugin-bundled-skills-not-loading-in-interactive-sessions.md`. +- Tests: `packages/engine/src/__tests__/compound-engineering-skill-resolution.test.ts`; `packages/core/src/__tests__/builtin-workflows.test.ts`. diff --git a/packages/core/src/__tests__/builtin-workflows.test.ts b/packages/core/src/__tests__/builtin-workflows.test.ts index ecc49eda94..14bbbfb57f 100644 --- a/packages/core/src/__tests__/builtin-workflows.test.ts +++ b/packages/core/src/__tests__/builtin-workflows.test.ts @@ -393,6 +393,26 @@ describe("built-in workflows", () => { expect(ids.indexOf("merge")).toBeLessThan(ids.indexOf("document")); }); + it("compound-engineering runs plan/code-review/document in coding mode and carries skillName onto compiled steps (U1/U4)", () => { + const ce = getBuiltinWorkflow("builtin:compound-engineering")!; + const byId = (id: string) => ce.ir.nodes.find((n) => n.id === id); + // U4: fan-out steps (plan, code-review) need coding so fn_spawn_agent is + // available for persona fan-out; document needs coding to WRITE docs/solutions. + expect(byId("plan")?.config?.toolMode).toBe("coding"); + expect(byId("code-review")?.config?.toolMode).toBe("coding"); + expect(byId("document")?.config?.toolMode).toBe("coding"); + // U1: the compiler carries each node's skillName onto the materialized step so + // the step session can actually LOAD the skill (not just name it in prompt text). + const steps = compileWorkflowToSteps(ce.ir); + const plan = steps.find((s) => s.name === "Plan"); + expect(plan?.skillName).toBe("compound-engineering:ce-plan"); + expect(plan?.toolMode).toBe("coding"); + const codeReview = steps.find((s) => s.skillName === "compound-engineering:ce-code-review"); + expect(codeReview?.toolMode).toBe("coding"); + const document = steps.find((s) => s.skillName === "compound-engineering:ce-compound"); + expect(document?.toolMode).toBe("coding"); + }); + describe("store integration", () => { const harness = createTaskStoreTestHarness(); let store: ReturnType; diff --git a/packages/core/src/__tests__/workflow-steps-to-ir.test.ts b/packages/core/src/__tests__/workflow-steps-to-ir.test.ts index de041be2d3..d98f664ee3 100644 --- a/packages/core/src/__tests__/workflow-steps-to-ir.test.ts +++ b/packages/core/src/__tests__/workflow-steps-to-ir.test.ts @@ -16,6 +16,7 @@ function step(overrides: Partial): WorkflowStep { gateMode: overrides.gateMode ?? "advisory", prompt: overrides.prompt ?? "", toolMode: overrides.toolMode, + skillName: overrides.skillName, scriptName: overrides.scriptName, enabled: overrides.enabled ?? true, defaultOn: overrides.defaultOn, @@ -38,6 +39,7 @@ function visible(input: WorkflowStepInput) { prompt: input.mode === "script" ? undefined : (input.prompt ?? ""), scriptName: input.scriptName, toolMode: input.mode === "script" ? undefined : input.toolMode, + skillName: input.mode === "script" ? undefined : input.skillName, modelProvider: input.modelProvider, modelId: input.modelId, }; @@ -52,6 +54,7 @@ function visibleStep(s: WorkflowStep) { prompt: s.mode === "script" ? undefined : (s.prompt ?? ""), scriptName: s.mode === "script" ? s.scriptName : undefined, toolMode: s.mode === "script" ? undefined : (s.toolMode ?? "readonly"), + skillName: s.mode === "script" ? undefined : s.skillName, modelProvider: s.mode === "prompt" ? s.modelProvider : undefined, modelId: s.mode === "prompt" ? s.modelId : undefined, }; @@ -89,6 +92,18 @@ describe("stepsToWorkflowIr — round-trip parity (R4/KTD-2)", () => { modelId: "claude-sonnet-4-5", phase: "pre-merge", }), + // U1 / INVERSION CONTRACT: a skill-executor step (pre-merge, grouped with + // the other pre-merge steps so declaration order matches compiled order) + // must round-trip its skillName through stepInputToNode → nodeToStepInput. + step({ + id: "WS-6", + name: "CE skill step", + mode: "prompt", + gateMode: "advisory", + prompt: "Invoke the skill", + skillName: "compound-engineering:ce-work", + phase: "pre-merge", + }), step({ id: "WS-4", name: "Document", diff --git a/packages/core/src/builtin-workflows.ts b/packages/core/src/builtin-workflows.ts index 26b0562cfd..936c33f403 100644 --- a/packages/core/src/builtin-workflows.ts +++ b/packages/core/src/builtin-workflows.ts @@ -187,6 +187,10 @@ export const BUILTIN_WORKFLOWS: WorkflowDefinition[] = [ name: "Plan", executor: "skill", skillName: "compound-engineering:ce-plan", + // Coding mode so ce-plan can fan out to its research subagents via + // fn_spawn_agent (registered only for coding-mode steps). It is not + // meant to write — see the accepted write-capability posture (Risk-1). + toolMode: "coding", prompt: "Produce a short implementation plan for this task before any code is written.", }, }, @@ -213,6 +217,10 @@ export const BUILTIN_WORKFLOWS: WorkflowDefinition[] = [ executor: "skill", skillName: "compound-engineering:ce-code-review", gateMode: "gate", + // Coding mode so ce-code-review can fan out to its reviewer-persona + // subagents via fn_spawn_agent. As a gate step it still emits the + // verdict JSON (KTD-6); it is not meant to write the tree (Risk-1). + toolMode: "coding", prompt: "Run a structured code review of the changes. Block merge on P0/P1 findings.", }, }, @@ -253,6 +261,9 @@ export const BUILTIN_WORKFLOWS: WorkflowDefinition[] = [ name: "Document learnings", executor: "skill", skillName: "compound-engineering:ce-compound", + // Coding mode so ce-compound can WRITE the learning doc into + // docs/solutions (readonly would strip write tools). + toolMode: "coding", prompt: "Capture any reusable learnings from this task into docs/solutions.", }, }, diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 5bca54843c..a064210265 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -626,6 +626,11 @@ export interface WorkflowStep { prompt: string; /** Tool set available to prompt-mode workflow agents. Defaults to readonly. */ toolMode?: WorkflowStepToolMode; + /** Name of a skill to load into this step's session (e.g. + * "compound-engineering:ce-work"). When set, the step session loads the named + * skill (discovery + selection) and the engine injects the Fusion workflow-step + * conventions preamble. Only meaningful for skill-executor graph nodes. */ + skillName?: string; /** Name of a script from project settings `scripts` map to execute (required when mode is "script") */ scriptName?: string; /** Whether this step is available for selection on new tasks */ @@ -746,6 +751,9 @@ export interface WorkflowStepInput { prompt?: string; /** Tool set available to prompt-mode workflow agents. Defaults to readonly. */ toolMode?: WorkflowStepToolMode; + /** Name of a skill to load into this step's session (e.g. + * "compound-engineering:ce-work"). See `WorkflowStep.skillName`. */ + skillName?: string; /** Script name from project settings (required when mode is "script"). * Must reference a named script in `settings.scripts` — no raw commands. */ scriptName?: string; diff --git a/packages/core/src/workflow-compiler.ts b/packages/core/src/workflow-compiler.ts index 5d617d0a7b..9d13370793 100644 --- a/packages/core/src/workflow-compiler.ts +++ b/packages/core/src/workflow-compiler.ts @@ -231,8 +231,9 @@ function defaultGateMode(node: WorkflowIrNode, mode: "prompt" | "script"): Workf * its exact inverse is `stepInputToNode` in `workflow-steps-to-ir.ts`. Parity is * pinned by `__tests__/workflow-steps-to-ir.test.ts` over exactly the * compiler-visible fields: name / mode / phase / gateMode / prompt / scriptName / - * toolMode / modelProvider / modelId. `enabled` / `defaultOn` / `templateId` are - * NOT compiler-visible and are handled by migration policy, not the converter. + * toolMode / skillName / modelProvider / modelId. `enabled` / `defaultOn` / + * `templateId` are NOT compiler-visible and are handled by migration policy, not + * the converter. * * INVERSION CONTRACT: when you add a field here, extend `stepInputToNode` (and * the parity test) in `workflow-steps-to-ir.ts` to keep the round-trip exact. @@ -255,6 +256,10 @@ function nodeToStepInput(node: WorkflowIrNode, phase: "pre-merge" | "post-merge" } else { input.prompt = configString(node, "prompt") ?? ""; input.toolMode = node.config?.toolMode === "coding" ? "coding" : "readonly"; + // Carry the node's skill name so the step session can load it (U1). Only + // present on skill-executor nodes; omitted otherwise to keep round-trip exact. + const skillName = configString(node, "skillName"); + if (skillName) input.skillName = skillName; const provider = configString(node, "modelProvider"); const modelId = configString(node, "modelId"); if (provider && modelId) { diff --git a/packages/core/src/workflow-steps-to-ir.ts b/packages/core/src/workflow-steps-to-ir.ts index 5d4f3cbe96..196a942aa4 100644 --- a/packages/core/src/workflow-steps-to-ir.ts +++ b/packages/core/src/workflow-steps-to-ir.ts @@ -12,10 +12,10 @@ import { parseWorkflowIr } from "./workflow-ir.js"; * compileWorkflowToSteps(stepsToWorkflowIr(steps, name)) ≡ steps * * over exactly the compiler-visible fields: name / mode / phase / gateMode / - * prompt / scriptName / toolMode / modelProvider / modelId. `enabled` / - * `defaultOn` / `templateId` / `migratedFragmentId` are NOT compiler-visible and - * are handled by migration policy (KTD-3), not by this converter. Parity is - * pinned by `__tests__/workflow-steps-to-ir.test.ts`. + * prompt / scriptName / toolMode / skillName / modelProvider / modelId. + * `enabled` / `defaultOn` / `templateId` / `migratedFragmentId` are NOT + * compiler-visible and are handled by migration policy (KTD-3), not by this + * converter. Parity is pinned by `__tests__/workflow-steps-to-ir.test.ts`. * * INVERSION CONTRACT: when a compiler-visible field is added to `nodeToStepInput` * (see the contract comment there), extend `stepInputToNode` below and the parity @@ -68,6 +68,9 @@ function stepInputToNode(step: WorkflowStep, id: string): WorkflowIrNode { // prompt mode config.prompt = step.prompt ?? ""; config.toolMode = step.toolMode === "coding" ? "coding" : "readonly"; + // Skill name round-trips when set (U1). The compiler reads it back via + // configString(node, "skillName"), so this keeps the inverse exact. + if (step.skillName) config.skillName = step.skillName; // Model overrides only round-trip when BOTH are present (compiler requirement). if (step.modelProvider && step.modelId) { config.modelProvider = step.modelProvider; diff --git a/packages/engine/src/__tests__/ce-workflow-step-conventions.test.ts b/packages/engine/src/__tests__/ce-workflow-step-conventions.test.ts new file mode 100644 index 0000000000..7679580823 --- /dev/null +++ b/packages/engine/src/__tests__/ce-workflow-step-conventions.test.ts @@ -0,0 +1,187 @@ +/** + * Compound-Engineering workflow-step skill-loading — focused unit coverage for + * the plan units U8/U1/U2/U3/U9 engine surface that does NOT require a full + * executor e2e: + * + * 1. The exported `FUSION_WORKFLOW_STEP_CONVENTIONS_PREAMBLE` constant carries + * the await-input sentinel grammar, the FUSION_HEADLESS degrade + * instruction, and the persona-fan-out / systemPromptOverride / + * path-confinement instruction (always feasible — pure constant assertion). + * + * 2. Skill resolution: a skill step requesting BOTH the namespaced + * `compound-engineering:ce-X` and bare `ce-X` forms (exactly what + * executeWorkflowStep merges into requestedSkillNames) resolves the named + * CE skill once the install dir is fed as a discovery path — the bare name + * matches case-insensitively against the discovered SKILL.md. This mirrors + * and extends compound-engineering-skill-resolution.test.ts, asserting the + * DUAL (namespaced + bare) request form U1 now produces. + * + * The full runGraphCustomNode -> executeWorkflowStep session path (skillName on + * the synthesized step, spawn gating, FUSION_HEADLESS, verdict conditional) is + * covered separately in ce-workflow-step-executor.test.ts (driving the real + * executor with a mocked agent session). + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { loadSkills } from "@earendil-works/pi-coding-agent"; +import { FUSION_WORKFLOW_STEP_CONVENTIONS_PREAMBLE } from "../executor.js"; +import { + createSkillsOverrideFromSelection, + resolveSessionSkills, +} from "../skill-resolver.js"; + +vi.mock("../logger.js", () => { + const mk = () => ({ log: vi.fn(), warn: vi.fn(), error: vi.fn() }); + return { + createLogger: vi.fn(() => mk()), + piLog: mk(), + schedulerLog: mk(), + executorLog: mk(), + planLog: mk(), + mergerLog: mk(), + worktreePoolLog: mk(), + reviewerLog: mk(), + prMonitorLog: mk(), + runtimeLog: mk(), + ipcLog: mk(), + projectManagerLog: mk(), + hybridExecutorLog: mk(), + formatError: (err: unknown) => + err instanceof Error ? { message: err.message, detail: err.stack ?? err.message } : { message: String(err), detail: String(err) }, + }; +}); + +describe("FUSION_WORKFLOW_STEP_CONVENTIONS_PREAMBLE (U2/U9 constant)", () => { + it("documents the await-input sentinel grammar verbatim", () => { + // These exact tokens are the cross-module contract with parseAwaitInputSentinel. + expect(FUSION_WORKFLOW_STEP_CONVENTIONS_PREAMBLE).toContain("===FUSION_AWAIT_INPUT==="); + expect(FUSION_WORKFLOW_STEP_CONVENTIONS_PREAMBLE).toContain("===END_FUSION_AWAIT_INPUT==="); + // It must tell the skill to emit exactly one block and STOP. + expect(FUSION_WORKFLOW_STEP_CONVENTIONS_PREAMBLE).toMatch(/emit EXACTLY ONE block/); + }); + + it("carries the FUSION_HEADLESS degrade-to-assumption instruction (U3)", () => { + expect(FUSION_WORKFLOW_STEP_CONVENTIONS_PREAMBLE).toContain("FUSION_HEADLESS=1"); + // In headless mode the skill must NOT ask and must record an assumption. + expect(FUSION_WORKFLOW_STEP_CONVENTIONS_PREAMBLE).toMatch(/do NOT ask the user/i); + expect(FUSION_WORKFLOW_STEP_CONVENTIONS_PREAMBLE).toMatch(/record a reasonable assumption/i); + expect(FUSION_WORKFLOW_STEP_CONVENTIONS_PREAMBLE).toMatch(/never emit the await-input block in this mode/i); + }); + + it("carries the persona fan-out / systemPromptOverride / path-confinement instruction (U8/U9)", () => { + // Persona def is read from the agents dir env var… + expect(FUSION_WORKFLOW_STEP_CONVENTIONS_PREAMBLE).toContain("$FUSION_CE_AGENTS_DIR/.md"); + // …and passed to fn_spawn_agent as systemPromptOverride (the U8 fan-out contract). + expect(FUSION_WORKFLOW_STEP_CONVENTIONS_PREAMBLE).toContain("systemPromptOverride"); + expect(FUSION_WORKFLOW_STEP_CONVENTIONS_PREAMBLE).toContain("fn_spawn_agent"); + // Path confinement (the U9 filesystem prompt-injection guard): reject ../ traversal. + expect(FUSION_WORKFLOW_STEP_CONVENTIONS_PREAMBLE).toMatch(/reject any[\s\S]*path traversal/i); + expect(FUSION_WORKFLOW_STEP_CONVENTIONS_PREAMBLE).toMatch(/Resolve the path strictly inside/i); + expect(FUSION_WORKFLOW_STEP_CONVENTIONS_PREAMBLE).toContain("$FUSION_CE_AGENTS_DIR"); + // Readonly fallback: if spawn is unavailable, do the persona's work inline. + expect(FUSION_WORKFLOW_STEP_CONVENTIONS_PREAMBLE).toMatch(/readonly step.*inline/i); + }); + + it("overrides contrary skill-body instructions (the conventions win)", () => { + expect(FUSION_WORKFLOW_STEP_CONVENTIONS_PREAMBLE).toMatch(/override any contrary instruction in the skill body/i); + }); +}); + +/** + * U1 dual-form resolution. executeWorkflowStep merges BOTH the namespaced + * `compound-engineering:ce-work` and the bare `ce-work` into requestedSkillNames + * before resolution. + * + * REAL FINDING (asserted below): the resolver's name match is `bareSkillName`, + * which only strips a trailing `/SKILL.md` — it does NOT strip a `namespace:` + * prefix. So the NAMESPACED form alone does NOT match the on-disk bare + * `ce-work`; only the BARE form does. This is precisely why executeWorkflowStep + * must merge both forms — the bare half is what actually selects the skill, and + * the namespaced half is a (currently non-matching) belt-and-suspenders entry. + */ +describe("U1: dual-form (namespaced + bare) CE skill resolution", () => { + let tmp: string; + let projectRootDir: string; + let agentDir: string; + let installRoot: string; + + function materialize(id: string): void { + const dir = join(installRoot, id); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, "SKILL.md"), + `---\nname: ${id}\ndescription: ${id} pipeline stage\n---\n\n# ${id}\n`, + ); + } + + function resolveFor(requestedSkillNames: string[]): string[] { + const discovered = loadSkills({ + cwd: projectRootDir, + agentDir, + skillPaths: [installRoot], + includeDefaults: false, + }); + const selection = resolveSessionSkills({ + projectRootDir, + requestedSkillNames, + sessionPurpose: "executor", + }); + const override = createSkillsOverrideFromSelection(selection, { + requestedSkillNames, + sessionPurpose: "executor", + }); + const result = override({ skills: discovered.skills, diagnostics: discovered.diagnostics }); + return result.skills.map((s) => s.name); + } + + beforeEach(() => { + tmp = mkdtempSync(join(tmpdir(), "ce-conv-")); + projectRootDir = join(tmp, "project"); + agentDir = join(tmp, "agent"); + installRoot = join(tmp, ".fusion-ce-skills"); + mkdirSync(projectRootDir, { recursive: true }); + mkdirSync(agentDir, { recursive: true }); + materialize("ce-work"); + }); + + afterEach(() => { + rmSync(tmp, { recursive: true, force: true }); + }); + + it("bare name `ce-work` resolves the installed skill", () => { + expect(resolveFor(["ce-work"])).toContain("ce-work"); + }); + + it("namespaced name `compound-engineering:ce-work` ALONE does NOT resolve it (no namespace stripping)", () => { + // bareSkillName only strips `/SKILL.md`, not a `namespace:` prefix — so the + // namespaced tail never matches the on-disk bare `ce-work`. This is the gap + // that makes the bare half of the executor's dual merge load-bearing. + expect(resolveFor(["compound-engineering:ce-work"])).not.toContain("ce-work"); + }); + + it("the dual request (both forms together, as executeWorkflowStep merges them) resolves it once via the bare half", () => { + const resolved = resolveFor(["compound-engineering:ce-work", "ce-work"]); + expect(resolved).toContain("ce-work"); + // No duplicate skill entries from the two request forms. + expect(resolved.filter((n) => n === "ce-work")).toHaveLength(1); + }); + + it("without the install dir on the discovery path, the request does NOT resolve (both halves required)", () => { + // Repoint discovery away from the install root: name alone is insufficient. + const discovered = loadSkills({ cwd: projectRootDir, agentDir, skillPaths: [], includeDefaults: false }); + const selection = resolveSessionSkills({ + projectRootDir, + requestedSkillNames: ["compound-engineering:ce-work", "ce-work"], + sessionPurpose: "executor", + }); + const override = createSkillsOverrideFromSelection(selection, { + requestedSkillNames: ["compound-engineering:ce-work", "ce-work"], + sessionPurpose: "executor", + }); + const result = override({ skills: discovered.skills, diagnostics: discovered.diagnostics }); + expect(result.skills.map((s) => s.name)).not.toContain("ce-work"); + }); +}); diff --git a/packages/engine/src/__tests__/ce-workflow-step-executor.test.ts b/packages/engine/src/__tests__/ce-workflow-step-executor.test.ts new file mode 100644 index 0000000000..3c3ff5cc13 --- /dev/null +++ b/packages/engine/src/__tests__/ce-workflow-step-executor.test.ts @@ -0,0 +1,409 @@ +/** + * Compound-Engineering workflow-step skill-loading — executor integration + * coverage. Drives the REAL TaskExecutor (over a mock store + mocked agent + * session, the established executor harness) through: + * + * - runGraphCustomNode (skill graph node) → asserts the synthesized + * WorkflowStep carries `skillName` and that the U2 conventions preamble is + * prepended to the prompt (item 3). + * + * - executeWorkflowStep directly → asserts, by capturing the exact + * session-creation args reaching createFnAgent: + * * spawn gating: fn_spawn_agent present in coding, absent in readonly (item 4) + * * FUSION_HEADLESS: on stepEnv only when unattended=true (item 5) + * * the step's named skill is merged into requestedSkillNames as BOTH the + * namespaced and bare form, and FUSION_CE_SKILLS_DIR is threaded as + * additionalSkillPaths (item 2, integration half) + * * verdict conditional: gate / skill-less step gets the verdict-JSON + * Feedback Format; a non-gate skill step gets the relaxed Output Format (item 6) + * + * HARNESS NOTE: createFnAgent is mocked (executor-test-helpers) so no real model + * runs. We assert on the arguments the executor hands the session layer — the + * engine-owned wiring — not on model behavior. The mock session emits a verdict + * line on prompt so the parse path completes cleanly. + */ + +import { beforeEach, describe, expect, it, vi } from "vitest"; +import "./executor-test-helpers.js"; +import { TaskExecutor } from "../executor.js"; +import { + createMockStore, + mockedCreateFnAgent, + mockedExecSync, + resetExecutorMocks, +} from "./executor-test-helpers.js"; + +type CapturedSession = { + customTools?: Array<{ name?: string }>; + systemPrompt?: string; + taskEnv?: NodeJS.ProcessEnv; + skillSelection?: { requestedSkillNames?: string[] }; + additionalSkillPaths?: string[]; +}; + +/** + * Make createFnAgent capture its session-creation args and return a mock session + * that emits the given output line, then resolves. Returns the capture holder. + */ +function captureSession(output = '{"verdict":"APPROVE","notes":""}'): { last?: CapturedSession; all: CapturedSession[] } { + const holder: { last?: CapturedSession; all: CapturedSession[] } = { all: [] }; + mockedCreateFnAgent.mockImplementation(async (opts: any) => { + const captured: CapturedSession = { + customTools: opts.customTools, + systemPrompt: opts.systemPrompt, + taskEnv: opts.taskEnv, + skillSelection: opts.skillSelection, + additionalSkillPaths: opts.additionalSkillPaths, + }; + holder.last = captured; + holder.all.push(captured); + + const listeners: Array<(e: any) => void> = []; + const session: any = { + state: {}, + subscribe: (fn: (e: any) => void) => { + listeners.push(fn); + return () => {}; + }, + prompt: vi.fn(async () => { + for (const fn of listeners) { + fn({ + type: "message_update", + assistantMessageEvent: { + type: "text_delta", + partial: output, + contentIndex: 0, + delta: output, + }, + }); + } + }), + dispose: vi.fn(), + }; + return { session }; + }); + return holder; +} + +function makeExecutor(store: ReturnType) { + const agentStore = { getAgent: vi.fn().mockResolvedValue(null), createAgent: vi.fn() }; + const executor = new TaskExecutor(store as any, "/tmp/test", { agentStore } as any); + return { executor, agentStore }; +} + +function baseStepTask(overrides: Record = {}) { + return { + id: "FN-CE-1", + title: "CE", + description: "do the thing", + column: "in-progress" as const, + worktree: "/tmp/wt", + branch: "fusion/fn-ce-1", + baseCommitSha: "abc123", + dependencies: [], + steps: [{ name: "s", status: "in-progress" as const }], + currentStep: 0, + log: [], + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + ...overrides, + }; +} + +function makeStep(overrides: Record = {}) { + const now = new Date().toISOString(); + return { + id: "graph:ce-plan", + name: "Plan", + description: "", + mode: "prompt" as const, + phase: "pre-merge" as const, + gateMode: "advisory" as const, + prompt: "Plan the work.", + toolMode: "readonly" as const, + enabled: true, + createdAt: now, + updatedAt: now, + ...overrides, + }; +} + +/** captureModifiedFiles / git diff calls go through the mocked execSync→exec. */ +function quietGit() { + mockedExecSync.mockImplementation(() => Buffer.from("")); +} + +describe("CE workflow-step executor integration", () => { + beforeEach(() => { + resetExecutorMocks(); + quietGit(); + }); + + // ── Item 3: synthesized WorkflowStep from a skill graph node ──────────────── + describe("runGraphCustomNode skill node (U1/U2)", () => { + it("carries skillName onto the synthesized step AND prepends the conventions preamble", async () => { + const store = createMockStore(); + store.getTask.mockResolvedValue(baseStepTask() as any); + const { executor } = makeExecutor(store); + + const captured: { step?: any } = {}; + vi.spyOn(executor as any, "executeWorkflowStep").mockImplementation(async (...args: any[]) => { + captured.step = args[1]; + return { success: true, output: "ok" }; + }); + + const node = { + id: "ce-plan", + kind: "prompt", + column: "review", + config: { executor: "skill", skillName: "compound-engineering:ce-plan", prompt: "Plan the work." }, + }; + + const result = await (executor as any).runGraphCustomNode(node, { id: "FN-CE-1" }, {}, undefined); + + expect(result.outcome).toBe("success"); + // (U1) skillName threaded onto the step so the session can LOAD it. + expect(captured.step.skillName).toBe("compound-engineering:ce-plan"); + // (U2) conventions preamble prepended before the "Invoke the skill" line. + expect(captured.step.prompt).toContain("## Fusion workflow-step conventions"); + expect(captured.step.prompt).toContain("===FUSION_AWAIT_INPUT==="); + expect(captured.step.prompt).toContain('Invoke the "compound-engineering:ce-plan" skill'); + // Original node prompt still present after the preamble. + expect(captured.step.prompt).toContain("Plan the work."); + }); + + it("a non-skill (model) node synthesizes NO skillName and NO preamble", async () => { + const store = createMockStore(); + store.getTask.mockResolvedValue(baseStepTask() as any); + const { executor } = makeExecutor(store); + + const captured: { step?: any } = {}; + vi.spyOn(executor as any, "executeWorkflowStep").mockImplementation(async (...args: any[]) => { + captured.step = args[1]; + return { success: true, output: "ok" }; + }); + + const node = { id: "review", kind: "prompt", column: "review", config: { prompt: "Just review." } }; + await (executor as any).runGraphCustomNode(node, { id: "FN-CE-1" }, {}, undefined); + + expect(captured.step.skillName).toBeUndefined(); + expect(captured.step.prompt).not.toContain("## Fusion workflow-step conventions"); + expect(captured.step.prompt).toContain("Just review."); + }); + }); + + // ── Item 5: FUSION_HEADLESS gating on stepEnv ─────────────────────────────── + describe("executeWorkflowStep FUSION_HEADLESS (U3)", () => { + it("sets FUSION_HEADLESS=1 only when unattended=true; always sets FUSION_WORKFLOW_STEP", async () => { + const store = createMockStore(); + const { executor } = makeExecutor(store); + const cap = captureSession(); + + // unattended → headless present. + await (executor as any).executeWorkflowStep( + baseStepTask(), + makeStep({ skillName: "compound-engineering:ce-plan" }), + "/tmp/wt", + {}, + undefined, + { unattended: true }, + ); + expect(cap.last?.taskEnv?.FUSION_HEADLESS).toBe("1"); + expect(cap.last?.taskEnv?.FUSION_WORKFLOW_STEP).toBe("1"); + + // board run (default / explicit false) → headless absent, workflow-step still set. + await (executor as any).executeWorkflowStep( + baseStepTask(), + makeStep({ skillName: "compound-engineering:ce-plan" }), + "/tmp/wt", + {}, + undefined, + { unattended: false }, + ); + expect(cap.last?.taskEnv?.FUSION_HEADLESS).toBeUndefined(); + expect(cap.last?.taskEnv?.FUSION_WORKFLOW_STEP).toBe("1"); + + // no stepOptions at all → headless absent. + await (executor as any).executeWorkflowStep( + baseStepTask(), + makeStep({ skillName: "compound-engineering:ce-plan" }), + "/tmp/wt", + {}, + undefined, + ); + expect(cap.last?.taskEnv?.FUSION_HEADLESS).toBeUndefined(); + }); + + it("strips an INHERITED FUSION_HEADLESS on a board run (default-safe invariant)", async () => { + const store = createMockStore(); + const { executor } = makeExecutor(store); + const cap = captureSession(); + + // An outer pipeline exported FUSION_HEADLESS=1 into the inherited env. A board + // run (unattended=false) must NOT inherit it — otherwise the step silently + // skips user questions instead of parking. (PR #1696 review fix.) + await (executor as any).executeWorkflowStep( + baseStepTask(), + makeStep({ skillName: "compound-engineering:ce-plan" }), + "/tmp/wt", + {}, + { FUSION_HEADLESS: "1" }, + { unattended: false }, + ); + expect(cap.last?.taskEnv?.FUSION_HEADLESS).toBeUndefined(); + + // An explicit unattended opt-in still sets it. + await (executor as any).executeWorkflowStep( + baseStepTask(), + makeStep({ skillName: "compound-engineering:ce-plan" }), + "/tmp/wt", + {}, + { FUSION_HEADLESS: "1" }, + { unattended: true }, + ); + expect(cap.last?.taskEnv?.FUSION_HEADLESS).toBe("1"); + }); + }); + + // ── Item 2 (integration half): skillName → requestedSkillNames + paths ─────── + describe("executeWorkflowStep skill merge (U1)", () => { + it("merges the step skillName as BOTH namespaced and bare into requestedSkillNames, and threads FUSION_CE_SKILLS_DIR as additionalSkillPaths", async () => { + const store = createMockStore(); + const { executor } = makeExecutor(store); + const cap = captureSession(); + + await (executor as any).executeWorkflowStep( + baseStepTask(), + makeStep({ skillName: "compound-engineering:ce-work" }), + "/tmp/wt", + {}, + { FUSION_CE_SKILLS_DIR: "/opt/ce/.fusion-ce-skills" }, + undefined, + ); + + const requested = cap.last?.skillSelection?.requestedSkillNames ?? []; + expect(requested).toContain("compound-engineering:ce-work"); + expect(requested).toContain("ce-work"); + // The install root from the injected env becomes the discovery path. + expect(cap.last?.additionalSkillPaths).toEqual(["/opt/ce/.fusion-ce-skills"]); + }); + + it("a skill-less step contributes no skillName merge and no additionalSkillPaths", async () => { + const store = createMockStore(); + const { executor } = makeExecutor(store); + const cap = captureSession(); + + await (executor as any).executeWorkflowStep( + baseStepTask(), + makeStep({ gateMode: "gate" }), // no skillName + "/tmp/wt", + {}, + undefined, + undefined, + ); + + // No CE skills dir injected → no additionalSkillPaths. + expect(cap.last?.additionalSkillPaths).toBeUndefined(); + }); + }); + + // ── Item 4: spawn-tool gating by toolMode ─────────────────────────────────── + describe("executeWorkflowStep spawn gating (U8b)", () => { + function toolNames(cap: ReturnType): string[] { + return (cap.last?.customTools ?? []).map((t) => t.name ?? ""); + } + + it("coding-mode step registers fn_spawn_agent", async () => { + const store = createMockStore(); + const { executor } = makeExecutor(store); + const cap = captureSession(); + + await (executor as any).executeWorkflowStep( + baseStepTask(), + makeStep({ skillName: "compound-engineering:ce-code-review", toolMode: "coding" }), + "/tmp/wt", + {}, + undefined, + undefined, + ); + + expect(toolNames(cap)).toContain("fn_spawn_agent"); + }); + + it("readonly-mode step does NOT register fn_spawn_agent", async () => { + const store = createMockStore(); + const { executor } = makeExecutor(store); + const cap = captureSession(); + + await (executor as any).executeWorkflowStep( + baseStepTask(), + makeStep({ skillName: "compound-engineering:ce-code-review", toolMode: "readonly" }), + "/tmp/wt", + {}, + undefined, + undefined, + ); + + expect(toolNames(cap)).not.toContain("fn_spawn_agent"); + }); + }); + + // ── Item 6: verdict-contract conditional ──────────────────────────────────── + describe("executeWorkflowStep verdict conditional (KTD-6)", () => { + it("a GATE skill step still requires the trailing verdict JSON (Feedback Format)", async () => { + const store = createMockStore(); + const { executor } = makeExecutor(store); + const cap = captureSession(); + + await (executor as any).executeWorkflowStep( + baseStepTask(), + makeStep({ skillName: "compound-engineering:ce-code-review", gateMode: "gate" }), + "/tmp/wt", + {}, + undefined, + undefined, + ); + + expect(cap.last?.systemPrompt).toContain("## Feedback Format"); + expect(cap.last?.systemPrompt).toContain('{"verdict":"APPROVE|APPROVE_WITH_NOTES|REVISE"'); + expect(cap.last?.systemPrompt).not.toContain("## Output Format"); + }); + + it("a skill-LESS prompt step requires the verdict JSON (legacy reviewer contract)", async () => { + const store = createMockStore(); + const { executor } = makeExecutor(store); + const cap = captureSession(); + + await (executor as any).executeWorkflowStep( + baseStepTask(), + makeStep({ gateMode: "advisory" }), // no skillName, advisory + "/tmp/wt", + {}, + undefined, + undefined, + ); + + expect(cap.last?.systemPrompt).toContain("## Feedback Format"); + expect(cap.last?.systemPrompt).toContain('{"verdict":"APPROVE|APPROVE_WITH_NOTES|REVISE"'); + }); + + it("a NON-GATE skill step is RELAXED — Output Format, no required verdict JSON", async () => { + const store = createMockStore(); + const { executor } = makeExecutor(store); + const cap = captureSession(); + + await (executor as any).executeWorkflowStep( + baseStepTask(), + makeStep({ skillName: "compound-engineering:ce-plan", gateMode: "advisory" }), + "/tmp/wt", + {}, + undefined, + undefined, + ); + + expect(cap.last?.systemPrompt).toContain("## Output Format"); + expect(cap.last?.systemPrompt).toContain("NOT required to end with a"); + expect(cap.last?.systemPrompt).not.toContain("## Feedback Format"); + }); + }); +}); diff --git a/packages/engine/src/agent-runtime.ts b/packages/engine/src/agent-runtime.ts index c60afdd89e..ad8ba417a3 100644 --- a/packages/engine/src/agent-runtime.ts +++ b/packages/engine/src/agent-runtime.ts @@ -88,6 +88,12 @@ export interface AgentRuntimeOptions { skillSelection?: SkillSelectionContext; /** Convenience: skill names to include in the session */ skills?: string[]; + /** Extra directories to scan for skills (each holding `/SKILL.md`), in + * addition to the default cwd/agent-dir roots. Forwarded to the resource + * loader so caller-requested `skills`/`skillSelection` names installed to a + * private dir (e.g. a plugin's bundled-skill root) are discoverable in the + * live session. Mirrors `AgentOptions.additionalSkillPaths` in pi.ts. */ + additionalSkillPaths?: string[]; /** Runtime-facing context for non-pi runtimes that cannot consume JS ToolDefinition objects directly. */ runtimeContext?: AgentRuntimeContext; /** diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 090a2cd870..328f641f5d 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -978,6 +978,37 @@ export function parseAwaitInputSentinel(output: string | undefined): string | nu return question ? question : null; } +/** + * (U2 / KTD-2) Fusion workflow-step conventions preamble, prepended to a skill + * step's prompt at the skill-prompt build path (runGraphCustomNode). It teaches + * any bundled skill the conventions Fusion needs — in ONE engine-side place, so + * the skills stay byte-for-byte upstream. The block is skill-agnostic and rides + * on the node prompt; it deliberately overrides the upstream skill bodies that + * still say "call AskUserQuestion" / "Task ce-*". Stable text — the await-input + * grammar here must match `parseAwaitInputSentinel` and the persona-override + * contract (fn_spawn_agent's `systemPromptOverride` param) verbatim. + * + * (U9 / KTD-7) The persona-fan-out instruction is path-confined: the skill must + * resolve `.md` strictly within `$FUSION_CE_AGENTS_DIR` and reject any + * `../` traversal before reading, since the file body is injected verbatim into a + * child's system prompt (a filesystem prompt-injection surface otherwise). + */ +export const FUSION_WORKFLOW_STEP_CONVENTIONS_PREAMBLE = `## Fusion workflow-step conventions + +You are running as a Fusion autonomous workflow step — NOT an interactive Claude Code session. Follow these conventions; they override any contrary instruction in the skill body below. + +1. Asking the user: there is no interactive listener here. \`AskUserQuestion\` / \`request_user_input\` go into the void. When you must ask the user a question, emit EXACTLY ONE block of the form: + ===FUSION_AWAIT_INPUT=== + + ===END_FUSION_AWAIT_INPUT=== + and then STOP. Fusion parks the task awaiting the user's answer and re-runs this step with their reply. + +2. Headless runs: when the environment variable \`FUSION_HEADLESS=1\` is set, do NOT ask the user anything. Record a reasonable assumption explicitly in your output and proceed — never emit the await-input block in this mode. + +3. Dispatching a \`ce-\` subagent: do NOT use a raw \`Task ce-*(...)\` call. Instead, read the persona definition from \`$FUSION_CE_AGENTS_DIR/.md\`, strip its YAML frontmatter, and pass the remaining body as the \`systemPromptOverride\` argument to the \`fn_spawn_agent\` tool. Resolve the path strictly inside \`$FUSION_CE_AGENTS_DIR\` — reject any \`\` containing \`/\` or \`..\` (path traversal), and skip a def whose body is empty or implausibly large. If \`fn_spawn_agent\` is not available (a readonly step), do the persona's work inline yourself instead of spawning. + +`; + /** Result returned from fn_spawn_agent tool */ interface SpawnAgentResult { agentId: string; @@ -4021,6 +4052,13 @@ export class TaskExecutor { * coding/step session runs as a column agent. Cleared in the run's finally. */ private graphColumnAgentResolver = new Map WorkflowColumnAgent | undefined>(); + /** (U3) Task ids whose current graph run is genuinely unattended (LFG / + * pipeline / disable-model-invocation — no human will ever answer). Set only + * by an explicit `unattended` workflow-run option; default-absent means a + * board run. runGraphCustomNode reads this to set FUSION_HEADLESS on skill + * steps. Cleared in maybeExecuteWorkflowGraph's finally alongside the resolver. */ + private graphUnattendedRuns = new Set(); + /** Column-agent seam wiring (column-agent plan U4). The governing graph node id * for the implementation pass currently in flight for a task — the execute-seam * prompt node's id (execute seam), or the foreach instance node id (step-execute @@ -4142,6 +4180,21 @@ export class TaskExecutor { this.graphColumnAgentResolver.set(task.id, resolveBindingForNode); } + // (U3) Genuinely-unattended run signal. This is an EXPLICIT opt-in, not an + // inferred heuristic: a run is unattended only when an entrypoint that + // knows no human will ever answer (LFG / pipeline / disable-model-invocation) + // marks it so. No such marker reaches this executor path today (verified — + // KTD-3), so this resolves to false (board run) for every current run, and + // the safe default is preserved: absence of the explicit flag ALWAYS yields + // no FUSION_HEADLESS, so a board task can only ever park (a human can answer + // via the await-input card button), never silently skip approval. When such + // an entrypoint is added, it sets `unattended` here. + // No entrypoint sets this today, so clear any stale entry; a board run never + // sets FUSION_HEADLESS. When an LFG/pipeline/disable-model-invocation + // entrypoint is added, call `this.graphUnattendedRuns.add(task.id)` here and + // the finally below clears it. + this.graphUnattendedRuns.delete(task.id); + const runner = new WorkflowGraphTaskRunner({ store: { ...this.store, @@ -4245,6 +4298,20 @@ export class TaskExecutor { } return true; } finally { + // FNXC:WorkflowGraph 2026-06-20-23:35: + // Terminate child agents spawned by this graph run's coding-mode skill steps. + // U8 registered fn_spawn_agent for coding-mode steps, but the graph path + // returns from execute() at the graphOwned early-return — BEFORE execute()'s + // outer finally that calls terminateAllChildren. Without this, graph-step + // children orphan their sessions/worktrees, and their ids accumulate in the + // per-parent spawn budget (spawnedAgents[taskId]), starving later steps' + // fan-out (e.g. ce-code-review's reviewer panel). Mirror the non-graph + // cleanup; run it before the per-run graph bookkeeping below. + try { + await this.terminateAllChildren(task.id); + } catch (err) { + executorLog.warn(`terminateAllChildren failed for graph task ${task.id}: ${err instanceof Error ? err.message : String(err)}`); + } this.graphRouting.delete(task.id); // Clear per-run step-inversion pins (KTD-8: pinned only for the run's life). this.graphStepSessionPinned.delete(task.id); @@ -4252,6 +4319,7 @@ export class TaskExecutor { // Clear per-run column-agent seam wiring (U4): the resolver and any dangling // governing-node-id are scoped to this run only. this.graphColumnAgentResolver.delete(task.id); + this.graphUnattendedRuns.delete(task.id); this.graphSeamGoverningNodeId.delete(task.id); // Per-instance keys: clear every instance slot owned by this task. const ctxPrefix = `${task.id}:`; @@ -5988,6 +6056,37 @@ export class TaskExecutor { return false; } + /** Build the task-scoped runtime env that carries plugin-injected keys + * (e.g. compound-engineering `FUSION_CE_SKILLS_DIR` / `FUSION_CE_AGENTS_DIR`) + * plus the plugin PATH contribution. Shared by the legacy single-session path + * (agentWork, ~7434) and the graph-node skill-step path (runGraphCustomNode, + * U8) so both deliver the same injected env to their sessions. We never mutate + * process.env globally — this scoped env is threaded through taskEnv so session + * subprocesses inherit it without leaking across concurrent tasks. */ + private async buildInjectedRuntimeEnv( + taskId: string, + worktreePath: string, + branch: string | undefined, + ): Promise<{ env: NodeJS.ProcessEnv; injectedKeyCount: number; pathEntryCount: number }> { + const runtimeEnvContribution = await this.options.pluginRunner?.collectExecutorRuntimeEnv({ + taskId, + worktreePath, + rootDir: this.rootDir, + branch, + }); + const pathPrepend = runtimeEnvContribution?.pathPrepend ?? []; + const injectedEnv = runtimeEnvContribution?.env ?? {}; + return { + env: { + ...process.env, + ...injectedEnv, + PATH: [...pathPrepend, process.env.PATH ?? ""].filter(Boolean).join(delimiter), + }, + injectedKeyCount: Object.keys(injectedEnv).length, + pathEntryCount: pathPrepend.length, + }; + } + /** Run a custom (non-seam) graph node on the proven WorkflowStep machinery. * * `columnBinding` (plan U3) is the agent binding governing this node's @@ -6170,7 +6269,10 @@ export class TaskExecutor { // Agent lookup is best-effort; fall back to the default model. } } else if (executorKind === "skill" && typeof cfg.skillName === "string" && cfg.skillName.trim()) { - prompt = `Invoke the "${cfg.skillName}" skill with the following input, following the skill's instructions exactly:\n\n${prompt}`; + // (U2) Prepend the Fusion workflow-step conventions preamble BEFORE the + // "Invoke the skill" line. A skill node always runs as a workflow step here + // (graph path → executeWorkflowStep), so the conventions always apply. + prompt = `${FUSION_WORKFLOW_STEP_CONVENTIONS_PREAMBLE}Invoke the "${cfg.skillName}" skill with the following input, following the skill's instructions exactly:\n\n${prompt}`; } else if (executorKind === "cli") { const rawCommand = rawCliCommand; if (rawCommand) { @@ -6227,6 +6329,13 @@ export class TaskExecutor { const mode: "prompt" | "script" = executorKind === "cli" || node.kind === "script" || (node.kind === "gate" && scriptName) ? "script" : "prompt"; const now = new Date().toISOString(); + // (U1) Carry the node's skill name onto the synthesized step so the step + // session can actually LOAD it (executeWorkflowStep merges it into the + // resolved skillSelection). Without this, the named skill was only injected + // as prompt text pointing at a skill the session never discovered. + const stepSkillName = executorKind === "skill" && typeof cfg.skillName === "string" && cfg.skillName.trim() + ? cfg.skillName.trim() + : undefined; const step: WorkflowStep = { id: `graph:${node.id}`, name: typeof cfg.name === "string" && cfg.name.trim() ? cfg.name : node.id, @@ -6240,16 +6349,36 @@ export class TaskExecutor { enabled: true, createdAt: now, updatedAt: now, + ...(stepSkillName ? { skillName: stepSkillName } : {}), ...(modelProvider && modelId ? { modelProvider, modelId } : {}), }; - // CLI executor passes the node prompt to the named script via env. - const nodeEnv: NodeJS.ProcessEnv | undefined = - executorKind === "cli" && prompt ? { ...process.env, FUSION_NODE_PROMPT: prompt } : undefined; + // (U8a) Thread the plugin-injected runtime env (FUSION_CE_SKILLS_DIR / + // FUSION_CE_AGENTS_DIR + PATH contribution) into prompt-mode skill/model + // steps on the GRAPH path. The legacy single-session caller builds this in + // agentWork; the graph path never did, so skill loading and persona fan-out + // silently no-op'd here. CLI executor keeps its own FUSION_NODE_PROMPT env. + let nodeEnv: NodeJS.ProcessEnv | undefined; + if (executorKind === "cli" && prompt) { + nodeEnv = { ...process.env, FUSION_NODE_PROMPT: prompt }; + } else if (mode === "prompt") { + const injected = await this.buildInjectedRuntimeEnv(live.id, worktreePath, live.branch ?? undefined); + nodeEnv = injected.env; + executorLog.log( + `${live.id}: graph node '${node.id}' runtime env injected (${injected.pathEntryCount} PATH entries, ${injected.injectedKeyCount} env keys)`, + ); + } + + // (U3) Genuinely-unattended signal. `unattended` is an explicit opt-in + // threaded from the workflow-run options (default false = board run, where a + // human can still answer asynchronously via the await-input card button). + // No origin heuristic — absence always yields a board run. executeWorkflowStep + // sets FUSION_HEADLESS=1 only when this is explicitly true. + const unattended = this.graphUnattendedRuns.has(live.id); const outcome = mode === "script" ? await this.executeScriptWorkflowStep(live, step, worktreePath, settings, nodeEnv) - : await this.executeWorkflowStep(live, step, worktreePath, settings, nodeEnv); + : await this.executeWorkflowStep(live, step, worktreePath, settings, nodeEnv, { unattended }); // Skill-emitted await-input (U6): if the skill asked the user a blocking // question via the ===FUSION_AWAIT_INPUT=== sentinel, park the task @@ -7431,24 +7560,10 @@ export class TaskExecutor { this.activeWorktrees.set(task.id, worktreePath); executorLog.log(`${task.id}: worktree ready at ${worktreePath}`); - const runtimeEnvContribution = await this.options.pluginRunner?.collectExecutorRuntimeEnv({ - taskId: task.id, - worktreePath, - rootDir: this.rootDir, - branch: acquisition.branch ?? undefined, - }); - const pathPrepend = runtimeEnvContribution?.pathPrepend ?? []; - const injectedEnv = runtimeEnvContribution?.env ?? {}; - // We intentionally do NOT mutate process.env globally. This task-scoped env is - // passed through AgentRuntimeOptions so executor session subprocesses inherit it - // without leaking across concurrent tasks. - taskEnv = { - ...process.env, - ...injectedEnv, - PATH: [...pathPrepend, process.env.PATH ?? ""].filter(Boolean).join(delimiter), - }; + const injected = await this.buildInjectedRuntimeEnv(task.id, worktreePath, acquisition.branch ?? undefined); + taskEnv = injected.env; executorLog.log( - `${task.id}: executor runtime env injected (${pathPrepend.length} PATH entries, ${Object.keys(injectedEnv).length} env keys)`, + `${task.id}: executor runtime env injected (${injected.pathEntryCount} PATH entries, ${injected.injectedKeyCount} env keys)`, ); this.options.onStart?.(task, worktreePath); @@ -12412,8 +12527,13 @@ ${failureFeedback} worktreePath: string, settings: Settings, taskEnv?: NodeJS.ProcessEnv, + stepOptions?: { unattended?: boolean }, ): Promise { const toolMode: "coding" | "readonly" = workflowStep.toolMode || "readonly"; + // (U3) Genuinely-unattended run — set FUSION_HEADLESS=1 below so skills record + // assumptions and proceed instead of parking on a question. Explicit opt-in + // only (default false = board run); see runGraphCustomNode / KTD-3. + const unattended = stepOptions?.unattended === true; // Compute the diff scope so the workflow step agent reviews only what THIS // task changed — not unrelated files it might wander into. Without this, @@ -12450,6 +12570,42 @@ CRITICAL SCOPING RULES — read before doing anything else: - If NONE of the files in the diff scope are relevant to your review category (e.g. a UX/design reviewer with no UI/CSS/component files in scope, a security reviewer with no auth/network code in scope, an a11y reviewer with no markup changes), respond IMMEDIATELY with a single short approval line such as "No relevant changes in scope — approved." and STOP. Do not start exploring the codebase. - Your wall-clock budget is short. Spending it browsing unmodified files will cause this step to time out and block merge.`; + // (KTD-6) Verdict-contract reconciliation. The trailing-verdict JSON is the + // gate-parsing contract — it only matters for steps that gate merge. A skill + // step that isn't a gate (e.g. ce-plan / ce-work / ce-compound) produces + // skill-native output (and may emit a ===FUSION_AWAIT_INPUT=== sentinel and + // stop), so forcing a verdict would contradict the U2 preamble. Require the + // verdict only for gate steps (and skill-less prompt steps, which keep the + // legacy reviewer contract); relax it for non-gate skill steps. The executor + // runs parseAwaitInputSentinel on output regardless, so the await-input + // sentinel always takes priority when present. + const isSkillStep = typeof workflowStep.skillName === "string" && workflowStep.skillName.trim().length > 0; + const requireVerdict = workflowStep.gateMode === "gate" || !isSkillStep; + const verdictBlock = requireVerdict + ? ` + +## Feedback Format + +When your review is complete, your final line MUST be a single JSON object (no markdown fences): + +{"verdict":"APPROVE|APPROVE_WITH_NOTES|REVISE","notes":"..."} + +Rules: +- Output exactly one trailing JSON object and stop. +- verdict must be exactly APPROVE, APPROVE_WITH_NOTES, or REVISE. +- notes should be concise and actionable. Use an empty string when there are no notes. +- For out-of-scope fast-bail responses, use: {"verdict":"APPROVE","notes":"out of scope: no UI files changed"} + +Backward compat fallback: if JSON is unavailable, you may still begin output with REQUEST REVISION to request changes.` + : ` + +## Output Format + +Follow the skill's own output conventions. You are NOT required to end with a +verdict JSON object — this step does not gate merge. If you need to ask the user +a question, emit a single ===FUSION_AWAIT_INPUT=== block and stop (see the +workflow-step conventions in your instructions).`; + const systemPrompt = `You are a workflow step agent executing: ${workflowStep.name} Task Context: @@ -12467,21 +12623,7 @@ Your role: Your Instructions: ${workflowStep.prompt} -You have access to the file system to review changes. - -## Feedback Format - -When your review is complete, your final line MUST be a single JSON object (no markdown fences): - -{"verdict":"APPROVE|APPROVE_WITH_NOTES|REVISE","notes":"..."} - -Rules: -- Output exactly one trailing JSON object and stop. -- verdict must be exactly APPROVE, APPROVE_WITH_NOTES, or REVISE. -- notes should be concise and actionable. Use an empty string when there are no notes. -- For out-of-scope fast-bail responses, use: {"verdict":"APPROVE","notes":"out of scope: no UI files changed"} - -Backward compat fallback: if JSON is unavailable, you may still begin output with REQUEST REVISION to request changes.`; +You have access to the file system to review changes.${verdictBlock}`; const agentLogger = new AgentLogger({ store: this.store, @@ -12548,12 +12690,86 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit // convention (which the dashboard / task card renders) instead of calling // AskUserQuestion into the void. Scoped to the step session — the main // executor session deliberately does not carry it. - // (FUSION_HEADLESS is reserved for a future genuinely-unattended run signal — - // LFG/pipeline — where no human can answer even asynchronously.) - const stepEnv: NodeJS.ProcessEnv = { ...(taskEnv ?? process.env), FUSION_WORKFLOW_STEP: "1" }; + // (U3) FUSION_HEADLESS=1 marks a genuinely-unattended run (LFG/pipeline) so + // skills record assumptions and proceed instead of parking. Set ONLY when + // the explicit `unattended` flag is true; absent on a board run. + const stepEnv: NodeJS.ProcessEnv = { + ...(taskEnv ?? process.env), + FUSION_WORKFLOW_STEP: "1", + }; + // FNXC:WorkflowSteps 2026-06-21-06:30: + // Default-safe invariant (KTD-3): a board run must NEVER be headless. Since + // stepEnv spreads taskEnv/process.env, an inherited FUSION_HEADLESS (e.g. an + // outer pipeline exported it) would otherwise leak in and silently skip user + // questions. Set it ONLY on an explicit opt-in; strip any inherited value + // otherwise so absence of the flag always yields a board run. + if (unattended) { + stepEnv.FUSION_HEADLESS = "1"; + } else { + delete stepEnv.FUSION_HEADLESS; + } + + // (U1) Load the step's named skill into THIS session. The interactive fix + // proved the resolver works when fed BOTH a requested name AND a discovery + // path (compound-engineering-skill-resolution.test.ts). Here we mirror it: + // merge the step's skillName (both namespaced `compound-engineering:ce-work` + // and bare `ce-work` — the resolver matches bare names case-insensitively) + // into the resolved requestedSkillNames, and pass the CE install root (from + // the injected FUSION_CE_SKILLS_DIR env) as additionalSkillPaths so the + // loader can actually discover the bundled SKILL.md. Without both halves the + // named skill was only prompt text pointing at a skill the session never had. + let effectiveSkillSelection = skillContext.skillSelectionContext; + const ceSkillsDir = typeof stepEnv.FUSION_CE_SKILLS_DIR === "string" && stepEnv.FUSION_CE_SKILLS_DIR.trim() + ? stepEnv.FUSION_CE_SKILLS_DIR.trim() + : undefined; + if (workflowStep.skillName && workflowStep.skillName.trim()) { + const namespaced = workflowStep.skillName.trim(); + const bare = namespaced.includes(":") ? namespaced.slice(namespaced.lastIndexOf(":") + 1) : namespaced; + const existing = effectiveSkillSelection?.requestedSkillNames ?? []; + const mergedNames = [...new Set([...existing, namespaced, bare])]; + effectiveSkillSelection = { + projectRootDir: effectiveSkillSelection?.projectRootDir ?? this.rootDir, + ...(effectiveSkillSelection?.sessionPurpose ? { sessionPurpose: effectiveSkillSelection.sessionPurpose } : { sessionPurpose: "executor" }), + requestedSkillNames: mergedNames, + }; + } + // FNXC:WorkflowSteps 2026-06-20-23:35: + // A named skill with no discovery path silently degrades to the role-fallback + // skill (the exact pre-fix bug this change exists to kill). If the injected + // FUSION_CE_SKILLS_DIR never arrived (degraded/throwing plugin, missing install + // dir), warn loudly so an env-threading regression is visible on a board run + // instead of failing silent with a green hand-fed test. + if (workflowStep.skillName && workflowStep.skillName.trim() && !ceSkillsDir) { + await this.store.logEntry( + task.id, + `[skill-load] Workflow step '${workflowStep.name}' requests skill '${workflowStep.skillName}' but FUSION_CE_SKILLS_DIR is unset — the skill cannot be discovered; the step runs with role-fallback skills only.`, + ); + } + const additionalSkillPaths = ceSkillsDir ? [ceSkillsDir] : undefined; + + // (U8b) Coding-mode skill steps fan out to ce- subagents via + // fn_spawn_agent (read the persona def, pass its body as systemPromptOverride). + // That tool is registered only in the main executor session — never here — + // so coding mode granted write/edit but NOT spawn. Register it for + // coding-mode steps now; readonly steps keep no spawn (filterCustomToolsForReadonly + // strips it). The spawn tool inherits the injected env so children also see + // FUSION_CE_AGENTS_DIR. + // + // (U9 / KTD-4, Risk-1) ACCEPTED WRITE-CAPABILITY POSTURE: coding mode also + // exposes write/edit. The CE plan/code-review steps run coding ONLY to gain + // spawn (they are not supposed to mutate the tree), but the tool policy is + // binary today — coding is the only mode that carries fn_spawn_agent. There + // is NO engine guard preventing those steps from writing; the only protection + // is skill discipline plus the U6 no-diff detection assertion. The proper fix + // (a dedicated readonly-plus-spawn tool mode) is deferred; this is a + // knowingly-accepted gap, not a closed one — re-evaluate before enabling the + // CE workflow for genuinely-unattended (FUSION_HEADLESS) LFG/pipeline runs. + const codingCustomTools: ToolDefinition[] = toolMode === "coding" + ? [this.createSpawnAgentTool(task.id, worktreePath, settings, stepEnv)] + : []; const readonlyCustomTools = toolMode === "readonly" - ? filterCustomToolsForReadonly([]) - : { allowed: [] as ToolDefinition[], denied: [] as string[] }; + ? filterCustomToolsForReadonly(codingCustomTools) + : { allowed: codingCustomTools, denied: [] as string[] }; if (toolMode === "readonly" && readonlyCustomTools.denied.length > 0) { await this.store.logEntry( task.id, @@ -12576,8 +12792,10 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit runAuditor: createRunAuditor(this.store, this.getRunContextFor(task.id)), settings, taskEnv: stepEnv, - // Skill selection: use assigned agent skills if available, otherwise role fallback - ...(skillContext.skillSelectionContext ? { skillSelection: skillContext.skillSelectionContext } : {}), + // Skill selection: assigned-agent / role-fallback skills, plus the step's + // own named skill (U1) made discoverable via additionalSkillPaths. + ...(effectiveSkillSelection ? { skillSelection: effectiveSkillSelection } : {}), + ...(additionalSkillPaths ? { additionalSkillPaths } : {}), ...(readonlyCustomTools.allowed.length > 0 ? { customTools: readonlyCustomTools.allowed } : {}), }); @@ -15214,6 +15432,16 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit // A non-empty systemPromptOverride lets the caller run the child as a // specific persona (e.g. a compound-engineering reviewer) instead of the // generic child executor. Executor instructions are still appended below. + // + // (U9 / KTD-7) The engine does NOT itself resolve the persona def file — + // the calling skill reads `$FUSION_CE_AGENTS_DIR/.md` (the + // FUSION_WORKFLOW_STEP_CONVENTIONS_PREAMBLE instructs a path-confined + // read: confined to the install dir, `../` rejected, body-size sanity + // checked) and passes the stripped body here. The override body is + // therefore trusted only to the extent that read was confined; the + // agents dir is plugin-installer-owned and lives OUTSIDE the task + // worktree (so coding-mode plan/code-review steps can't write into it — + // see assertPluginLocalAgentsTarget in the CE plugin installer). const personaOverride = systemPromptOverride?.trim(); const childBasePrompt = personaOverride ? `${personaOverride}