From 1df67ae3da700369b3e0dd75f23e3c3379927eb0 Mon Sep 17 00:00:00 2001 From: Valera Brizhatiuk <19537764+plombeer31@users.noreply.github.com> Date: Mon, 14 Sep 2026 23:34:11 +0300 Subject: [PATCH 1/2] feat(fusion): F7 fusion.delegate takes a contract between the parts of a fan-out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Workers never see each other, and in the benchmark that cost two turns: one worker wrote HD.Ship as a class while its sibling called it as an object, and main.js looked for launch-btn against a btn-launch in the markup. Each brief was consistent on its own; the disagreement lived between them, where nothing was written down. `fusion.delegate` gains an optional call-level `contract`: { owners?: {path: taskId}, provides?: [{task, kind: symbol|file|id|endpoint|env|flag|other, name, in?}], requires?: [{task, name}], checks?: [{task?, ...verify.run args}] } - delegate-args.ts validates it against the task ids; every error names the field. A `requires` name must match a `provides` name (the launch-btn / btn-launch mismatch is refused before any worker runs), and a non-file provide must have somewhere to be looked for (`in`, an owned path or a declared file). Limits: 64 provides, 16 checks, 8,000 rendered chars. A contract sent as JSON text is parsed like `tasks`. - worker-prompt.ts renders the shared CONTRACT block (owners, provides, requires, checks) above every TASK, followed by "You own / You provide / You may rely on" for that task, and a RULE to end the reply with a PROVIDED: list. Briefs without a contract are byte-identical. - contract-checks.ts checks presence after the fan-out by language-agnostic means: a file exists; a symbol appears as a whole word in `in` or the owned/declared files; an id appears as id="…"/id='…'; anything else as a literal. Missing provides land as a note on the owner's row and on a `contract:` line under the head line of the status table. `checks` run through an injected `deps.runChecks` (verify's runner; undefined-safe — declared checks with no runner are reported as not run, never as passed); a task whose declared check fails becomes `failed` with `checks: …` as its error. The report also rides on `details.contract`. - The descriptor's argsSchema and the native-tools JSON schema carry the new field (the descriptor is mounted only in fusion mode). --- AGENTS.md | 2 +- .../openai/openai-strict-tools.test.ts | 11 +- src/prompt/default-tool-args-schemas.ts | 44 +++ src/prompt/default-tool-descriptors-b.ts | 2 +- src/tools/fusion/contract-checks.test.ts | 295 +++++++++++++++ src/tools/fusion/contract-checks.ts | 353 ++++++++++++++++++ src/tools/fusion/contract.test.ts | 95 +++++ src/tools/fusion/contract.ts | 167 +++++++++ src/tools/fusion/delegate-args.test.ts | 186 +++++++++ src/tools/fusion/delegate-args.ts | 190 +++++++++- src/tools/fusion/fusion-delegate.test.ts | 125 +++++++ src/tools/fusion/fusion-delegate.ts | 53 ++- src/tools/fusion/index.ts | 34 ++ src/tools/fusion/worker-prompt.test.ts | 42 +++ src/tools/fusion/worker-prompt.ts | 33 +- src/tools/fusion/worker-result.test.ts | 49 +++ src/tools/fusion/worker-result.ts | 51 ++- src/tools/fusion/worker-runner.test.ts | 24 ++ src/tools/fusion/worker-runner.ts | 7 + 19 files changed, 1752 insertions(+), 11 deletions(-) create mode 100644 src/tools/fusion/contract-checks.test.ts create mode 100644 src/tools/fusion/contract-checks.ts create mode 100644 src/tools/fusion/contract.test.ts create mode 100644 src/tools/fusion/contract.ts diff --git a/AGENTS.md b/AGENTS.md index ff0d78ef..4bd45bc5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2497,7 +2497,7 @@ On the fusion route the strip walks **four** controls, not three: `backend ⇄ p The fourth control, `workers` ([src/tui/composer-switch/composer-switch-worker-rows.ts](src/tui/composer-switch/composer-switch-worker-rows.ts)), is the **local** half: the downloaded models the workers can run, then `1..8 workers`. A model row goes through `LocalModelsOrchestrator.setActive`, which restarts the managed daemon and writes only `localModels.*` — never `activeTextProvider` — so fusion survives the pick; `triggerLlmPrimary` is deliberately not used, because its local branch also makes `local-llama` the active text provider. Row `active` is read from `localModelsPanel.rows[].active` (which model the daemon serves), not from the LLM pane's row, whose `active` additionally requires `local-llama` to be the chat route — under fusion it never is. A count row goes through `setFusionWorkersInConfig`, which moves `llm.runMode.fusion.workers` and `localModels.managed.parallel` in one write: they are one fact seen from two sides, and a running daemon keeps its old slot count until restarted, which the orchestrator says in a notice. `/runmode workers N` is the same call. ### The orchestrator and its workers -`fusion.delegate` ([src/tools/fusion/](src/tools/fusion/)) is the tool that makes the mode more than a label. The orchestrator plans, then hands independent parts down in one call: `{ tasks: [{ id, title, instructions, deliverable?, files? }] (1..8), maxWorkers? }`. `maxWorkers` has no parser ceiling — an over-ambitious number runs as wide as the machine allows rather than coming back as a validation error the model has to notice and retry; only a value below one is invalid. Each task becomes one ephemeral worker session running a turn pinned to the local leg — `origin: "fusion"`, `maxSteps: workerMaxSteps`, `taskMaxDurationMs: workerTimeoutMs`, `toolFilter: isWorkerVisibleTool`, an approval policy of `refuse` — and the call returns every reply plus a per-task status (`ok` / `failed` / `cancelled` / `max_steps` / `needs_orchestrator`). The brief ([worker-prompt.ts](src/tools/fusion/worker-prompt.ts)) tells the worker it has no memory of the parent conversation, that it must never ask a question, and that a refused approval is handed back up rather than retried. It also quotes the operator's **original request** above the task, labelled "context only; your task is below" and clipped at `ORIGINAL_REQUEST_CHAR_BUDGET` (16,000 chars) with an explicit note: orchestrator briefs are summaries, and thin ones made workers build the wrong thing or scavenge the disk for the spec. `executeTurn` records the request per session for the running turn (`pickOriginalRequest` — the turn's message, plus the message before it when the turn started with a short follow-up like "continue") and the tool reads it through `resolveOriginalRequest`. With the spec carried that way, `instructions` is capped at 32,000 chars, and the rejection names the limit and the exact overage. Statuses are ground-truthed rather than taken from the reply: a `reply` on the loop's forced finalization step comes back with `RunTurnResult.stopCause` and is reported `max_steps`, as is a worker stopped by its own `workerTimeoutMs` (which is not a cancellation); an `ok` task whose `files` entry does not exist afterwards is downgraded to `failed` ([declared-files.ts](src/tools/fusion/declared-files.ts)), and an existing but untouched entry is only a note, since `files` may be inputs. A failed worker's loop/provider error lands on the task's head line with a remediation hint for the recognised classes (context exceeded, first-token/idle timeout, 402/429). A worker's filesystem **reads** are confined to its working directory plus the fan-out's write scope ([worker-read-scope.ts](src/tools/fusion/worker-read-scope.ts), installed on the registry by `confineWorkerReads` at boot); other sessions are unaffected. `### fusion` in the stable prefix ([src/prompt/fusion-guidance.ts](src/prompt/fusion-guidance.ts)) is what makes a cloud model reach for the tool at all — it is present only while the descriptor is mounted, so a non-fusion install's prefix is byte-identical to a build without the feature. It carries two things the orchestrator cannot get anywhere else: a push to delegate whenever the work splits into independent, self-contained parts (with the honest limit — a part that only makes sense with this conversation in front of it stays the orchestrator's), and one line of **machine facts** ([fusion-machine-facts.ts](src/prompt/fusion-machine-facts.ts)) so the width it picks is informed rather than guessed: the llama-server request slots, the local model behind them, and therefore how many workers run at once before the rest queue. Those facts are read from the already-loaded config by `buildPrompt` — no probe at prompt-build time — and a fact the runtime does not own (an `external` server's `--parallel`) is left unsaid rather than guessed, because a guessed number is one the model will plan against. They are config values, so they move only when the operator writes the config file, which is the same event that already flips the descriptor gate; nothing per-turn goes in that line or the KV cache would drop on every step. +`fusion.delegate` ([src/tools/fusion/](src/tools/fusion/)) is the tool that makes the mode more than a label. The orchestrator plans, then hands independent parts down in one call: `{ tasks: [{ id, title, instructions, deliverable?, files? }] (1..8), maxWorkers?, contract? }`. The optional `contract` ([contract.ts](src/tools/fusion/contract.ts): `owners` path → task, `provides` — a `symbol` / `file` / `id` / `endpoint` / `env` / `flag` / `other` a task must produce, `requires`, and `checks` as `verify.run` specs) is the interface between the parts: it is validated against the task ids (a `requires` name must match a `provides` name; ≤ 64 provides, ≤ 16 checks, ≤ 8,000 rendered chars), prepended to every brief with a per-task "You own / You provide / You may rely on", and after the fan-out each provide is checked for presence by language-agnostic means and the checks run through the injected `runChecks` seam ([contract-checks.ts](src/tools/fusion/contract-checks.ts)) — a missing provide is a note on its owner's row and a `contract:` line in the status table; a failing declared check makes its task `failed`; checks with no runner wired are reported as not run, never as passed. `maxWorkers` has no parser ceiling — an over-ambitious number runs as wide as the machine allows rather than coming back as a validation error the model has to notice and retry; only a value below one is invalid. Each task becomes one ephemeral worker session running a turn pinned to the local leg — `origin: "fusion"`, `maxSteps: workerMaxSteps`, `taskMaxDurationMs: workerTimeoutMs`, `toolFilter: isWorkerVisibleTool`, an approval policy of `refuse` — and the call returns every reply plus a per-task status (`ok` / `failed` / `cancelled` / `max_steps` / `needs_orchestrator`). The brief ([worker-prompt.ts](src/tools/fusion/worker-prompt.ts)) tells the worker it has no memory of the parent conversation, that it must never ask a question, and that a refused approval is handed back up rather than retried. It also quotes the operator's **original request** above the task, labelled "context only; your task is below" and clipped at `ORIGINAL_REQUEST_CHAR_BUDGET` (16,000 chars) with an explicit note: orchestrator briefs are summaries, and thin ones made workers build the wrong thing or scavenge the disk for the spec. `executeTurn` records the request per session for the running turn (`pickOriginalRequest` — the turn's message, plus the message before it when the turn started with a short follow-up like "continue") and the tool reads it through `resolveOriginalRequest`. With the spec carried that way, `instructions` is capped at 32,000 chars, and the rejection names the limit and the exact overage. Statuses are ground-truthed rather than taken from the reply: a `reply` on the loop's forced finalization step comes back with `RunTurnResult.stopCause` and is reported `max_steps`, as is a worker stopped by its own `workerTimeoutMs` (which is not a cancellation); an `ok` task whose `files` entry does not exist afterwards is downgraded to `failed` ([declared-files.ts](src/tools/fusion/declared-files.ts)), and an existing but untouched entry is only a note, since `files` may be inputs. A failed worker's loop/provider error lands on the task's head line with a remediation hint for the recognised classes (context exceeded, first-token/idle timeout, 402/429). A worker's filesystem **reads** are confined to its working directory plus the fan-out's write scope ([worker-read-scope.ts](src/tools/fusion/worker-read-scope.ts), installed on the registry by `confineWorkerReads` at boot); other sessions are unaffected. `### fusion` in the stable prefix ([src/prompt/fusion-guidance.ts](src/prompt/fusion-guidance.ts)) is what makes a cloud model reach for the tool at all — it is present only while the descriptor is mounted, so a non-fusion install's prefix is byte-identical to a build without the feature. It carries two things the orchestrator cannot get anywhere else: a push to delegate whenever the work splits into independent, self-contained parts (with the honest limit — a part that only makes sense with this conversation in front of it stays the orchestrator's), and one line of **machine facts** ([fusion-machine-facts.ts](src/prompt/fusion-machine-facts.ts)) so the width it picks is informed rather than guessed: the llama-server request slots, the local model behind them, and therefore how many workers run at once before the rest queue. Those facts are read from the already-loaded config by `buildPrompt` — no probe at prompt-build time — and a fact the runtime does not own (an `external` server's `--parallel`) is left unsaid rather than guessed, because a guessed number is one the model will plan against. They are config values, so they move only when the operator writes the config file, which is the same event that already flips the descriptor gate; nothing per-turn goes in that line or the KV cache would drop on every step. **Nothing about the mode is decided at boot.** The tool is registered unconditionally, because its own live `resolveRunMode()` refusal is the correct and only gate it needs; and `bootstrap.ts` resolves the descriptor gate (`fusion: { enabled: … }`) on *every* read of `effectiveToolDescriptors()`, memoised on the gate's own value so the array identity — and therefore the prefix bytes — only changes when the mode does. Both were boot-time `if`s once, and the pair made a mid-session switch inert: the operator got the chip, the tint and the config, no `fusion.delegate`, no `### fusion`, and a fusion mode that silently did nothing until a restart. When the gate does flip, that session's KV cache drops once — the same cost, for the same reason, as installing a skill or live-adding an MCP server (`refreshMcp`): the tool catalog changed, so the prefix must. The GBNF grammar's `tool-name` rule lists `fusion.delegate` unconditionally (a local orchestrator has to be able to emit it — see the comment in `grammars/tool-call.gbnf`); a worker's per-request grammar drops it again through `toolFilter`, and the tool refuses from a worker session in any case. diff --git a/src/llm/provider/openai/openai-strict-tools.test.ts b/src/llm/provider/openai/openai-strict-tools.test.ts index cbb71af6..51ce5934 100644 --- a/src/llm/provider/openai/openai-strict-tools.test.ts +++ b/src/llm/provider/openai/openai-strict-tools.test.ts @@ -484,7 +484,16 @@ describe("strict-tool conformance over every registered tool", () => { // `os.http.request` carries a free-form header map and a free-form // JSON body; `mcp.prompt.get` forwards a server-defined argument // map. Both are the tool's actual payload, so neither can be closed. - expect(refused).toEqual(["os__http__request", "mcp__prompt__get"]); + // `fusion.delegate` joined them with its `contract`: `owners` is a + // path → task map, and each `checks` entry is a `verify.run` spec + // whose keys belong to that tool — closing it would leave the model + // unable to write a check at all. `parseDelegateArgs` validates the + // shape at run time, as it always did. + expect(refused).toEqual([ + "os__http__request", + "mcp__prompt__get", + "fusion__delegate", + ]); }); it("is idempotent across the whole catalog", () => { diff --git a/src/prompt/default-tool-args-schemas.ts b/src/prompt/default-tool-args-schemas.ts index a466e5a9..570942e4 100644 --- a/src/prompt/default-tool-args-schemas.ts +++ b/src/prompt/default-tool-args-schemas.ts @@ -721,6 +721,50 @@ const DEFAULT_TOOL_ARGS_SCHEMAS: ReadonlyMap = new Map< // the tool bounds the number by the task count and the server's // request slots. See `delegate-args.ts`. maxWorkers: { type: "integer", minimum: 1 }, + // The interface between the parts (`contract.ts`). `checks` + // items are `verify.run` specs plus a `task`, so they stay open. + contract: obj({ + owners: { + type: "object", + additionalProperties: { type: "string" }, + }, + provides: { + type: "array", + maxItems: 64, + items: obj( + { + task: stringSchema, + kind: { + type: "string", + enum: [ + "symbol", + "file", + "id", + "endpoint", + "env", + "flag", + "other", + ], + }, + name: stringSchema, + in: stringSchema, + }, + ["task", "kind", "name"], + ), + }, + requires: { + type: "array", + items: obj({ task: stringSchema, name: stringSchema }, [ + "task", + "name", + ]), + }, + checks: { + type: "array", + maxItems: 16, + items: objOpen({ task: stringSchema }), + }, + }), }, ["tasks"], ), diff --git a/src/prompt/default-tool-descriptors-b.ts b/src/prompt/default-tool-descriptors-b.ts index 121829b3..a6434557 100644 --- a/src/prompt/default-tool-descriptors-b.ts +++ b/src/prompt/default-tool-descriptors-b.ts @@ -235,7 +235,7 @@ export const DEFAULT_TOOL_DESCRIPTORS_B: readonly ToolDescriptor[] = [ summary: "Delegate independent parts of the work to local worker agents that run concurrently and report back. Each task becomes one throwaway worker turn that sees the operator's original request and your `instructions`, but nothing else from this conversation, so `instructions` must carry what the request does not (exact paths, the contract between parts, acceptance criteria, the answer format you want). Returns every worker's reply plus a per-task status. You choose `maxWorkers`; it is bounded only by the task count and the machine. Call it on its own, never alongside other tool calls.", argsSchema: - "{ tasks: [{ id: string, title: string, instructions: string, deliverable?: string, files?: string[] }] /* 1..8 */, maxWorkers?: number /* how many run at once; you decide */ }", + '{ tasks: [{ id: string, title: string, instructions: string, deliverable?: string, files?: string[] }] /* 1..8 */, maxWorkers?: number /* how many run at once; you decide */, contract?: { owners?: { [path]: taskId }, provides?: [{ task, kind: "symbol"|"file"|"id"|"endpoint"|"env"|"flag"|"other", name, in?: path }], requires?: [{ task, name /* a provides name */ }], checks?: [{ task?, ...verify.run args }] } /* the interface between the parts: shown to every worker, checked after the fan-out */ }', examples: [ '{"tasks":[{"id":"t1","title":"Map the auth routes","instructions":"List every route under src/http/ that touches auth. For each: path, method, and the middleware it runs.","deliverable":"one bullet per route"},{"id":"t2","title":"Summarise the session store","instructions":"Read src/session/session-store.ts and describe its public API and persistence model.","files":["src/session/session-store.ts"]}]}', ], diff --git a/src/tools/fusion/contract-checks.test.ts b/src/tools/fusion/contract-checks.test.ts new file mode 100644 index 00000000..5b69c01a --- /dev/null +++ b/src/tools/fusion/contract-checks.test.ts @@ -0,0 +1,295 @@ +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 { + applyCheckOutcomes, + applyContractFindings, + contentProvides, + describeMissing, + inspectContractProvides, + renderContractLine, + runContractChecks, + type ContractCheckRunner, + type ContractFinding, +} from "./contract-checks.js"; +import type { DelegateContract } from "./contract.js"; +import type { DelegateTask } from "./delegate-args.js"; +import type { WorkerTaskResult } from "./worker-result.js"; + +function row(over: Partial = {}): WorkerTaskResult { + return { + id: "t1", + title: "T", + status: "ok", + reply: "done", + stepCount: 1, + durationMs: 1, + tools: { calls: 0, errors: 0, byTool: {} }, + ...over, + }; +} + +describe("contentProvides", () => { + it("matches a symbol as a whole word, dots included", () => { + expect(contentProvides("const x = HD.Ship.reset();", { kind: "symbol", name: "HD.Ship.reset" })).toBe(true); + // `HD.Shipyard` must not pass for `HD.Ship`. + expect(contentProvides("HD.Shipyard = {}", { kind: "symbol", name: "HD.Ship" })).toBe(false); + expect(contentProvides("myHD.Ship = 1", { kind: "symbol", name: "HD.Ship" })).toBe(false); + // `$` is an identifier character; `\b` alone would misread it. + expect(contentProvides("function $init() {}", { kind: "symbol", name: "$init" })).toBe(true); + expect(contentProvides("function x$init() {}", { kind: "symbol", name: "$init" })).toBe(false); + }); + + it("matches an id attribute in either quote and nothing else", () => { + expect(contentProvides(''); + writeFileSync(join(dir, "js", "main.js"), "fetch('/api/score')"); + }); + afterEach(() => rmSync(dir, { recursive: true, force: true })); + + const tasks: DelegateTask[] = [ + { id: "ship", title: "Ship", instructions: "x" }, + { id: "html", title: "Html", instructions: "x", files: ["index.html"] }, + { id: "main", title: "Main", instructions: "x" }, + { id: "lost", title: "Lost", instructions: "x" }, + ]; + + it("checks each provide where the contract says to look, and says where it looked", async () => { + const contract: DelegateContract = { + owners: { "js/main.js": "main" }, + provides: [ + { task: "ship", kind: "symbol", name: "HD.Ship.reset", in: "js/ship.js" }, + { task: "ship", kind: "symbol", name: "HD.Ship.fire", in: "js/ship.js" }, + // No `in`: the task's declared files. + { task: "html", kind: "id", name: "btn-launch" }, + // No `in`: the owned path. + { task: "main", kind: "endpoint", name: "/api/score" }, + { task: "ship", kind: "file", name: "js/ship.js" }, + { task: "ship", kind: "file", name: "js/hud.js" }, + { task: "ship", kind: "symbol", name: "X", in: "js/nope.js" }, + { task: "lost", kind: "symbol", name: "Y" }, + ], + }; + const findings = await inspectContractProvides(contract, tasks, dir); + expect(findings.map((f) => [f.name, f.present, f.where])).toEqual([ + ["HD.Ship.reset", true, ["js/ship.js"]], + ["HD.Ship.fire", false, ["js/ship.js"]], + ["btn-launch", false, ["index.html"]], + ["/api/score", true, ["js/main.js"]], + ["js/ship.js", true, ["js/ship.js"]], + ["js/hud.js", false, ["js/hud.js"]], + ["X", false, ["js/nope.js"]], + ["Y", false, []], + ]); + expect(findings[6]!.detail).toBe("file missing or unreadable"); + expect(findings[7]!.detail).toContain("no file to look in"); + expect(describeMissing(findings[1]!)).toBe( + "[ship] symbol HD.Ship.fire not in js/ship.js", + ); + expect(describeMissing(findings[2]!)).toBe("[html] id btn-launch not in index.html"); + expect(describeMissing(findings[5]!)).toBe("[ship] file js/hud.js does not exist"); + expect(describeMissing(findings[7]!)).toContain("nowhere (no file to look in"); + }); + + it("is satisfied by any one of several owned paths", async () => { + const contract: DelegateContract = { + owners: { "index.html": "ship", "js/ship.js": "ship" }, + provides: [{ task: "ship", kind: "symbol", name: "HD.Ship" }], + }; + const [finding] = await inspectContractProvides(contract, tasks, dir); + expect(finding).toMatchObject({ present: true, where: ["index.html", "js/ship.js"] }); + }); +}); + +describe("applyContractFindings", () => { + it("notes each missing provide on its owner's row without changing the status", () => { + const findings: ContractFinding[] = [ + { task: "t1", kind: "symbol", name: "A", where: ["a.js"], present: false }, + { task: "t1", kind: "id", name: "b", where: ["i.html"], present: true }, + { task: "t2", kind: "file", name: "c.js", where: ["c.js"], present: false }, + ]; + const out = applyContractFindings( + [row(), row({ id: "t2", notes: ["earlier"] }), row({ id: "t3" })], + findings, + ); + expect(out[0]).toMatchObject({ status: "ok", notes: ["contract: symbol A not in a.js"] }); + expect(out[1]!.notes).toEqual(["earlier", "contract: file c.js does not exist"]); + expect(out[2]).not.toHaveProperty("notes"); + }); +}); + +describe("runContractChecks", () => { + const ctx = { workingDir: "/repo", signal: new AbortController().signal }; + + it("reports declared checks as NOT RUN when no runner is wired — never as passed", async () => { + const out = await runContractChecks([{ kind: "command", cmd: "x" }], undefined, ctx); + expect(out.outcomes).toEqual([]); + expect(out.checksSkipped).toBe("1 check not run — no check runner is wired"); + }); + + it("does nothing for an empty list", async () => { + const runner = vi.fn(); + expect(await runContractChecks([], runner, ctx)).toEqual({ outcomes: [] }); + expect(runner).not.toHaveBeenCalled(); + }); + + it("hands the runner the specs WITHOUT the task key and pairs results back by index", async () => { + const runner = vi.fn(async (specs) => ({ + ok: false, + results: specs.map((s, i) => + i === 0 + ? { ok: true, summary: "exit 0" } + : { ok: false, summary: `${s.cmd as string}: exit code 1\nline two` }, + ), + })); + const out = await runContractChecks( + [ + { task: "a", kind: "command", cmd: "node" }, + { kind: "command", cmd: "npm" }, + ], + runner, + ctx, + ); + expect(runner).toHaveBeenCalledWith( + [{ kind: "command", cmd: "node" }, { kind: "command", cmd: "npm" }], + ctx, + ); + expect(out.outcomes).toEqual([ + { task: "a", ok: true, detail: "exit 0" }, + { ok: false, detail: "npm: exit code 1 line two" }, + ]); + }); + + it("marks a check the runner returned nothing for as failed, and survives a runner that throws", async () => { + const short = await runContractChecks( + [{ task: "a", cmd: "x" }, { task: "b", cmd: "y" }], + async () => ({ ok: true, results: [{ ok: true }] }), + ctx, + ); + expect(short.outcomes).toEqual([ + { task: "a", ok: true, detail: "passed" }, + { task: "b", ok: false, detail: "the runner returned no result" }, + ]); + const thrown = await runContractChecks( + [{ cmd: "x" }], + async () => { + throw new Error("no browser available"); + }, + ctx, + ); + expect(thrown.outcomes).toEqual([]); + expect(thrown.checksSkipped).toBe( + "1 check not run — the check runner failed: no browser available", + ); + }); + + it("skips the checks when the turn is already cancelled", async () => { + const controller = new AbortController(); + controller.abort(); + const runner = vi.fn(); + const out = await runContractChecks([{ cmd: "x" }, { cmd: "y" }], runner, { + workingDir: "/repo", + signal: controller.signal, + }); + expect(runner).not.toHaveBeenCalled(); + expect(out.checksSkipped).toBe("2 checks not run — the turn was cancelled"); + }); +}); + +describe("applyCheckOutcomes", () => { + it("fails a task whose declared check failed, with `checks:` as the error", () => { + const out = applyCheckOutcomes( + [row(), row({ id: "t2" })], + [ + { task: "t1", ok: true, detail: "exit 0" }, + { task: "t1", ok: false, detail: "no errors: 1 pageerror" }, + { task: "t2", ok: true, detail: "exit 0" }, + ], + ); + expect(out[0]).toMatchObject({ + status: "failed", + error: "checks: no errors: 1 pageerror", + checks: { total: 2, failed: 1, detail: "no errors: 1 pageerror" }, + }); + expect(out[1]).toMatchObject({ status: "ok", checks: { total: 1, failed: 0 } }); + expect(out[1]!.checks).not.toHaveProperty("detail"); + }); + + it("keeps an existing error behind the checks verdict, and leaves a cancelled task cancelled", () => { + const out = applyCheckOutcomes( + [ + row({ status: "max_steps", error: "boom" }), + row({ id: "t2", status: "cancelled" }), + row({ id: "t3" }), + ], + [ + { task: "t1", ok: false, detail: "exit code 1" }, + { task: "t2", ok: false, detail: "exit code 1" }, + // A call-level check touches no row. + { ok: false, detail: "status 500" }, + ], + ); + expect(out[0]).toMatchObject({ status: "failed", error: "checks: exit code 1; boom" }); + expect(out[1]).toMatchObject({ status: "cancelled", checks: { failed: 1 } }); + expect(out[1]).not.toHaveProperty("error"); + expect(out[2]).not.toHaveProperty("checks"); + }); +}); + +describe("renderContractLine", () => { + const present: ContractFinding = { task: "a", kind: "file", name: "a.js", where: ["a.js"], present: true }; + const missing: ContractFinding = { task: "b", kind: "symbol", name: "HD.Ship.reset", where: ["js/ship.js"], present: false }; + + it("leads with the missing provides, owner first", () => { + expect(renderContractLine({ findings: [present, missing], checks: [] })).toBe( + "contract: 1 missing — [b] symbol HD.Ship.reset not in js/ship.js", + ); + expect(renderContractLine({ findings: [present], checks: [] })).toBe( + "contract: all 1 provide present", + ); + }); + + it("carries call-level check failures and the reason checks did not run", () => { + expect( + renderContractLine({ + findings: [], + checks: [{ ok: false, detail: "status 500" }, { task: "a", ok: true, detail: "ok" }], + }), + ).toBe("contract: call-level checks: 1 of 1 failed — status 500"); + expect( + renderContractLine({ + findings: [], + checks: [{ task: "a", ok: false, detail: "x" }, { task: "a", ok: true, detail: "y" }], + }), + ).toBe("contract: checks: 1 of 2 failed (see the task rows)"); + expect( + renderContractLine({ findings: [], checks: [], checksSkipped: "2 checks not run — no check runner is wired" }), + ).toBe("contract: 2 checks not run — no check runner is wired"); + }); + + it("is absent when there was nothing to report", () => { + expect(renderContractLine({ findings: [], checks: [] })).toBeUndefined(); + }); +}); diff --git a/src/tools/fusion/contract-checks.ts b/src/tools/fusion/contract-checks.ts new file mode 100644 index 00000000..1387c7cb --- /dev/null +++ b/src/tools/fusion/contract-checks.ts @@ -0,0 +1,353 @@ +import { readFile, stat } from "node:fs/promises"; + +import { resolveUserPath } from "../os/expand-home.js"; +import { + describeProvide, + ownedPaths, + type ContractCheck, + type ContractProvide, + type DelegateContract, +} from "./contract.js"; +import type { DelegateTask } from "./delegate-args.js"; +import type { WorkerTaskResult } from "./worker-result.js"; + +/** + * What the disk says about the contract once the workers are done. + * + * Every check here is language-agnostic on purpose: a file exists, a + * literal appears in a file, an `id="…"` attribute appears in the + * markup. That is enough to catch the failures the contract was written + * for — a symbol nobody exported, an id spelled two ways — and it is + * all a grep can honestly claim. Whether the symbol has the right shape + * is what `checks` (a runtime, through `runChecks`) are for. + */ +export interface ContractFinding { + task: string; + kind: ContractProvide["kind"]; + name: string; + /** The paths that were searched, as the contract named them. */ + where: string[]; + present: boolean; + /** Why nothing could be said, when `present` is false for a reason other than absence. */ + detail?: string; +} + +/** + * One check's verdict as the wired runner reports it. The runner is + * `verify`'s `runChecks`; this is the least it has to return. + */ +export interface ContractCheckResult { + ok: boolean; + summary?: string; + error?: string; +} + +/** + * The seam to `verify.run`. Injected because the runner lives in + * another module family; absent, the checks are reported as not run — + * never as passed. + */ +export type ContractCheckRunner = ( + specs: readonly Record[], + ctx: { workingDir: string; signal: AbortSignal }, +) => Promise<{ ok: boolean; results: readonly ContractCheckResult[] }>; + +export interface ContractCheckOutcome { + /** The task the check was attributed to; absent for a call-level check. */ + task?: string; + ok: boolean; + /** The runner's summary or error, head only. */ + detail: string; +} + +export interface ContractReport { + findings: ContractFinding[]; + checks: ContractCheckOutcome[]; + /** Why the checks did not run, when they did not. */ + checksSkipped?: string; +} + +/** Files above this are not searched; a provide is not that big. */ +const MAX_SEARCHED_FILE_BYTES = 8 * 1024 * 1024; +/** How much of a check's verdict a row carries. */ +const CHECK_DETAIL_CHARS = 400; +/** Bound on the `contract:` line of the status table. */ +const CONTRACT_LINE_CHARS = 1200; + +const GLOB_CHARS = /[*?[\]{}]/; + +function escapeRegExp(text: string): string { + return text.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + +/** + * Whether `content` carries the provide. A symbol is matched as a whole + * word so `HD.Ship` does not pass on `HD.Shipyard`; the boundaries are + * lookarounds because `\b` misreads names that start or end in `$`. An + * id is the attribute, in either quote. Everything else is the literal. + */ +export function contentProvides( + content: string, + provide: Pick, +): boolean { + const name = escapeRegExp(provide.name); + if (provide.kind === "symbol") { + return new RegExp(`(? !GLOB_CHARS.test(p), + ); + if (owned.length > 0) return owned; + return (task?.files ?? []).filter((f) => !GLOB_CHARS.test(f)); +} + +/** + * Check every provide against the working directory. Never throws: a + * path that cannot be read counts as not providing, with the reason on + * the finding. + */ +export async function inspectContractProvides( + contract: DelegateContract, + tasks: readonly DelegateTask[], + workingDir: string, +): Promise { + const findings: ContractFinding[] = []; + const cache = new Map>(); + const read = (path: string): Promise => { + let pending = cache.get(path); + if (pending === undefined) { + pending = (async () => { + try { + const absolute = resolveUserPath(path, workingDir); + const info = await stat(absolute); + if (!info.isFile() || info.size > MAX_SEARCHED_FILE_BYTES) { + return null; + } + return await readFile(absolute, "utf8"); + } catch { + return null; + } + })(); + cache.set(path, pending); + } + return pending; + }; + + for (const provide of contract.provides ?? []) { + const base = { task: provide.task, kind: provide.kind, name: provide.name }; + if (provide.kind === "file") { + let present = false; + try { + present = (await stat(resolveUserPath(provide.name, workingDir))).isFile(); + } catch { + present = false; + } + findings.push({ ...base, where: [provide.name], present }); + continue; + } + const task = tasks.find((t) => t.id === provide.task); + const where = searchPaths(provide, contract, task); + if (where.length === 0) { + findings.push({ + ...base, + where, + present: false, + detail: "no file to look in (give `in` or an owners entry)", + }); + continue; + } + let present = false; + let unreadable = 0; + for (const path of where) { + const content = await read(path); + if (content === null) { + unreadable += 1; + continue; + } + if (contentProvides(content, provide)) { + present = true; + break; + } + } + findings.push({ + ...base, + where, + present, + ...(!present && unreadable === where.length + ? { detail: "file missing or unreadable" } + : {}), + }); + } + return findings; +} + +/** `[ship_js] symbol HD.Ship.reset not in js/ship.js` */ +export function describeMissing(finding: ContractFinding): string { + if (finding.kind === "file") { + return `[${finding.task}] file ${finding.name} does not exist`; + } + const what = describeProvide({ + task: finding.task, + kind: finding.kind, + name: finding.name, + }); + const where = + finding.where.length > 0 ? `not in ${finding.where.join(", ")}` : "nowhere"; + const detail = finding.detail === undefined ? "" : ` (${finding.detail})`; + return `[${finding.task}] ${what} ${where}${detail}`; +} + +/** + * Put each missing provide on its owner's row as a note. The status is + * not changed: presence by grep is evidence for the orchestrator's + * review, not a verdict on the task — that is what `checks` decide. + */ +export function applyContractFindings( + results: readonly WorkerTaskResult[], + findings: readonly ContractFinding[], +): WorkerTaskResult[] { + return results.map((result) => { + const missing = findings.filter((f) => !f.present && f.task === result.id); + if (missing.length === 0) return result; + const notes = [ + ...(result.notes ?? []), + `contract: ${missing.map((f) => describeMissing(f).replace(/^\[[^\]]*\] /, "")).join("; ")}`, + ]; + return { ...result, notes }; + }); +} + +function head(text: string, cap: number): string { + const flat = text.replace(/\s+/g, " ").trim(); + return flat.length > cap ? `${flat.slice(0, cap)}…` : flat; +} + +/** + * Run the contract's checks through the wired runner. Never throws and + * never invents a pass: no runner, an aborted turn or a runner that + * threw all come back as `checksSkipped` with the reason. + */ +export async function runContractChecks( + checks: readonly ContractCheck[], + runner: ContractCheckRunner | undefined, + ctx: { workingDir: string; signal: AbortSignal }, +): Promise<{ outcomes: ContractCheckOutcome[]; checksSkipped?: string }> { + if (checks.length === 0) return { outcomes: [] }; + const plural = `${checks.length} check${checks.length === 1 ? "" : "s"}`; + if (runner === undefined) { + return { + outcomes: [], + checksSkipped: `${plural} not run — no check runner is wired`, + }; + } + if (ctx.signal.aborted) { + return { outcomes: [], checksSkipped: `${plural} not run — the turn was cancelled` }; + } + const specs = checks.map(({ task: _task, ...spec }) => spec); + let results: readonly ContractCheckResult[]; + try { + results = (await runner(specs, ctx)).results; + } catch (error) { + return { + outcomes: [], + checksSkipped: `${plural} not run — the check runner failed: ${error instanceof Error ? error.message : String(error)}`, + }; + } + const outcomes = checks.map((check, i): ContractCheckOutcome => { + const result = results[i]; + const task = check.task === undefined ? {} : { task: check.task }; + if (result === undefined) { + return { ...task, ok: false, detail: "the runner returned no result" }; + } + const detail = head( + result.error ?? result.summary ?? (result.ok ? "passed" : "failed"), + CHECK_DETAIL_CHARS, + ); + return { ...task, ok: result.ok, detail }; + }); + return { outcomes }; +} + +/** + * Fold the checks into the rows. A task whose declared check failed + * becomes `failed` with `checks: …` as its error — unless it was + * cancelled, which says the operator ended it and outranks a check that + * could not have passed. Call-level checks (no task) stay on the + * contract line. + */ +export function applyCheckOutcomes( + results: readonly WorkerTaskResult[], + outcomes: readonly ContractCheckOutcome[], +): WorkerTaskResult[] { + return results.map((result) => { + const own = outcomes.filter((o) => o.task === result.id); + if (own.length === 0) return result; + const failed = own.filter((o) => !o.ok); + const detail = + failed.length > 0 ? failed.map((o) => o.detail).join("; ") : undefined; + const checks = { + total: own.length, + failed: failed.length, + ...(detail === undefined ? {} : { detail }), + }; + if (failed.length === 0 || result.status === "cancelled") { + return { ...result, checks }; + } + const error = `checks: ${detail}`; + return { + ...result, + status: "failed", + checks, + error: result.error === undefined ? error : `${error}; ${result.error}`, + }; + }); +} + +/** + * The `contract:` line of the status table — presence first, then the + * checks that belong to no task, then why the checks did not run. + * Nothing when the contract declared nothing checkable. + */ +export function renderContractLine(report: ContractReport): string | undefined { + const parts: string[] = []; + if (report.findings.length > 0) { + const missing = report.findings.filter((f) => !f.present); + parts.push( + missing.length === 0 + ? `all ${report.findings.length} provide${report.findings.length === 1 ? "" : "s"} present` + : `${missing.length} missing — ${missing.map(describeMissing).join("; ")}`, + ); + } + const callLevel = report.checks.filter((o) => o.task === undefined); + if (callLevel.length > 0) { + const failed = callLevel.filter((o) => !o.ok); + parts.push( + failed.length === 0 + ? `call-level checks: ${callLevel.length} of ${callLevel.length} passed` + : `call-level checks: ${failed.length} of ${callLevel.length} failed — ${failed.map((o) => o.detail).join("; ")}`, + ); + } else if (report.checks.length > 0) { + const failed = report.checks.filter((o) => !o.ok).length; + parts.push( + failed === 0 + ? `checks: ${report.checks.length} of ${report.checks.length} passed` + : `checks: ${failed} of ${report.checks.length} failed (see the task rows)`, + ); + } + if (report.checksSkipped !== undefined) parts.push(report.checksSkipped); + if (parts.length === 0) return undefined; + return `contract: ${head(parts.join("; "), CONTRACT_LINE_CHARS)}`; +} diff --git a/src/tools/fusion/contract.test.ts b/src/tools/fusion/contract.test.ts new file mode 100644 index 00000000..edd603c7 --- /dev/null +++ b/src/tools/fusion/contract.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, it } from "vitest"; + +import { + MAX_CONTRACT_RENDERED_CHARS, + ownedPaths, + renderContractBlock, + renderContractForTask, + type DelegateContract, +} from "./contract.js"; + +const CONTRACT: DelegateContract = { + owners: { "js/ship.js": "ship", "index.html": "html", "js/main.js": "main" }, + provides: [ + { task: "ship", kind: "symbol", name: "HD.Ship", in: "js/ship.js" }, + { task: "html", kind: "id", name: "btn-launch", in: "index.html" }, + { task: "ship", kind: "file", name: "js/hud.js" }, + ], + requires: [ + { task: "main", name: "HD.Ship" }, + { task: "main", name: "btn-launch" }, + ], + checks: [ + { task: "main", kind: "page", path: "index.html", checks: ["no errors"] }, + { kind: "command", cmd: "node", args: ["--check", "js/main.js"] }, + ], +}; + +describe("renderContractBlock", () => { + it("renders every section, each entry tagged with its task", () => { + const block = renderContractBlock(CONTRACT); + expect(block.split("\n")[0]).toMatch(/^CONTRACT — the interface between the parts/); + expect(block).toContain("OWNERS (path → task; write only in paths you own):"); + expect(block).toContain("- js/ship.js → ship"); + expect(block).toContain("PROVIDES:"); + expect(block).toContain("- [ship] symbol HD.Ship in js/ship.js"); + expect(block).toContain("- [html] id btn-launch in index.html"); + expect(block).toContain("- [ship] file js/hud.js"); + expect(block).toContain("REQUIRES:"); + // Grouped per task, so a worker reads one line for its dependencies. + expect(block).toContain("- [main] HD.Ship, btn-launch"); + expect(block).toContain("CHECKS (run after the fan-out; a failing check fails its task):"); + expect(block).toContain( + '- [main] {"kind":"page","path":"index.html","checks":["no errors"]}', + ); + // A call-level check has no tag; its `task` key is not part of the spec. + expect(block).toContain('- {"kind":"command","cmd":"node","args":["--check","js/main.js"]}'); + expect(block).not.toContain('"task"'); + }); + + it("omits sections the contract does not declare", () => { + const block = renderContractBlock({ + provides: [{ task: "a", kind: "file", name: "x.txt" }], + }); + expect(block).toContain("PROVIDES:"); + expect(block).not.toContain("OWNERS"); + expect(block).not.toContain("REQUIRES"); + expect(block).not.toContain("CHECKS"); + }); + + it("clips one check's JSON so a huge spec cannot eat the block", () => { + const block = renderContractBlock({ + checks: [{ kind: "command", cmd: "x".repeat(2000) }], + }); + expect(block.length).toBeLessThan(MAX_CONTRACT_RENDERED_CHARS); + expect(block).toContain("…"); + }); +}); + +describe("renderContractForTask", () => { + it("says what this task owns, provides and may rely on, resolving each require to its source", () => { + const main = renderContractForTask(CONTRACT, "main"); + expect(main.split("\n")).toEqual([ + "You own: js/main.js", + "You provide: nothing listed", + "You may rely on: HD.Ship (symbol from ship in js/ship.js); btn-launch (id from html in index.html)", + ]); + const ship = renderContractForTask(CONTRACT, "ship"); + expect(ship).toContain("You own: js/ship.js"); + expect(ship).toContain("You provide: symbol HD.Ship in js/ship.js; file js/hud.js"); + expect(ship).toContain("You may rely on: nothing from the other parts"); + }); + + it("tells a task that owns nothing to stay inside its TASK's files", () => { + const lines = renderContractForTask(CONTRACT, "nobody"); + expect(lines).toContain( + "You own: no path in this contract — write only the files your TASK names", + ); + }); + + it("lists owned paths in declaration order", () => { + expect( + ownedPaths({ owners: { b: "t", a: "t", c: "u" } }, "t"), + ).toEqual(["b", "a"]); + }); +}); diff --git a/src/tools/fusion/contract.ts b/src/tools/fusion/contract.ts new file mode 100644 index 00000000..a7ee3195 --- /dev/null +++ b/src/tools/fusion/contract.ts @@ -0,0 +1,167 @@ +/** + * The contract between the parts of one fan-out. + * + * Workers never see each other. In one benchmark that cost two whole + * turns: one worker wrote `HD.Ship` as a class while its sibling called + * it as an object, and a `launch-btn` in `main.js` looked for a + * `btn-launch` in the markup. Each brief was internally consistent; the + * disagreement lived between them, where nothing was written down. + * + * A contract writes it down once, in language-agnostic terms: who owns + * which path, what each task must produce (a symbol in a file, a file, + * an id in the markup, an endpoint, an env var, a CLI flag), what a task + * may rely on from the others, and which `verify.run` checks the whole + * must pass. It is prepended to every brief (`worker-prompt.ts`) and + * checked for presence after the fan-out (`contract-checks.ts`). + * + * Presence is all this module can promise. A grep sees that `HD.Ship` + * appears in `js/ship.js`; whether it is a class or an object is what + * `checks` — a runtime — is for. + */ + +/** What a `provides` entry can name. `file` uses `name` as the path. */ +export const CONTRACT_PROVIDE_KINDS = [ + "symbol", + "file", + "id", + "endpoint", + "env", + "flag", + "other", +] as const; + +export type ContractProvideKind = (typeof CONTRACT_PROVIDE_KINDS)[number]; + +export interface ContractProvide { + /** The task that must produce it. */ + task: string; + kind: ContractProvideKind; + /** The literal name: `HD.Ship.reset`, `btn-launch`, `/api/score`, `js/hud.js`. */ + name: string; + /** Where it must appear. Absent: any path the task owns (or declares). */ + in?: string; +} + +export interface ContractRequire { + /** The task that relies on it. */ + task: string; + /** Matches a `provides[].name` exactly. */ + name: string; +} + +/** + * One `verify.run` spec, plus the task it is attributed to. The spec's + * own keys are owned by the verify tool and pass through untouched. + */ +export type ContractCheck = { task?: string } & Record; + +export interface DelegateContract { + /** Path → task id. Nobody else writes there. */ + owners?: Record; + provides?: ContractProvide[]; + requires?: ContractRequire[]; + checks?: ContractCheck[]; +} + +export const MAX_CONTRACT_PROVIDES = 64; +export const MAX_CONTRACT_CHECKS = 16; +/** Bound on the shared block every worker pays for in its context. */ +export const MAX_CONTRACT_RENDERED_CHARS = 8000; + +/** How much of one check's JSON the brief shows. */ +const CHECK_RENDER_CHARS = 300; + +/** `symbol HD.Ship in js/ship.js` — the same phrase everywhere. */ +export function describeProvide(provide: ContractProvide): string { + const where = provide.in === undefined ? "" : ` in ${provide.in}`; + return `${provide.kind} ${provide.name}${where}`; +} + +/** Paths a task owns, in declaration order. */ +export function ownedPaths( + contract: DelegateContract, + taskId: string, +): string[] { + return Object.entries(contract.owners ?? {}) + .filter(([, owner]) => owner === taskId) + .map(([path]) => path); +} + +function renderCheck(check: ContractCheck): string { + const { task, ...spec } = check; + const json = JSON.stringify(spec); + const clipped = + json.length > CHECK_RENDER_CHARS + ? `${json.slice(0, CHECK_RENDER_CHARS)}…` + : json; + return task === undefined ? `- ${clipped}` : `- [${task}] ${clipped}`; +} + +/** + * The block shared by every worker: all owners, provides, requires and + * checks. Measured against `MAX_CONTRACT_RENDERED_CHARS` at parse time. + */ +export function renderContractBlock(contract: DelegateContract): string { + const lines = [ + `CONTRACT — the interface between the parts of this fan-out. Every worker sees this same block. Produce exactly what it says you provide, under exactly these names, and reach the other parts only through what they provide.`, + ]; + const owners = Object.entries(contract.owners ?? {}); + if (owners.length > 0) { + lines.push( + `OWNERS (path → task; write only in paths you own):`, + ...owners.map(([path, task]) => `- ${path} → ${task}`), + ); + } + const provides = contract.provides ?? []; + if (provides.length > 0) { + lines.push( + `PROVIDES:`, + ...provides.map((p) => `- [${p.task}] ${describeProvide(p)}`), + ); + } + const requires = contract.requires ?? []; + if (requires.length > 0) { + const byTask = new Map(); + for (const r of requires) { + byTask.set(r.task, [...(byTask.get(r.task) ?? []), r.name]); + } + lines.push( + `REQUIRES:`, + ...[...byTask].map(([task, names]) => `- [${task}] ${names.join(", ")}`), + ); + } + const checks = contract.checks ?? []; + if (checks.length > 0) { + lines.push( + `CHECKS (run after the fan-out; a failing check fails its task):`, + ...checks.map(renderCheck), + ); + } + return lines.join("\n"); +} + +/** + * The three lines that turn the shared block into this worker's own + * obligations. A require is resolved to the provide it names so the + * worker knows who produces it and where to find it. + */ +export function renderContractForTask( + contract: DelegateContract, + taskId: string, +): string { + const owned = ownedPaths(contract, taskId); + const provides = (contract.provides ?? []).filter((p) => p.task === taskId); + const relies = (contract.requires ?? []) + .filter((r) => r.task === taskId) + .map((r) => { + const source = (contract.provides ?? []).find((p) => p.name === r.name); + return source === undefined + ? r.name + : `${r.name} (${source.kind} from ${source.task}${source.in === undefined ? "" : ` in ${source.in}`})`; + }); + return [ + `You own: ${owned.length > 0 ? owned.join(", ") : "no path in this contract — write only the files your TASK names"}`, + `You provide: ${provides.length > 0 ? provides.map(describeProvide).join("; ") : "nothing listed"}`, + `You may rely on: ${relies.length > 0 ? relies.join("; ") : "nothing from the other parts"}`, + ].join("\n"); +} diff --git a/src/tools/fusion/delegate-args.test.ts b/src/tools/fusion/delegate-args.test.ts index fc00779b..63cbeb96 100644 --- a/src/tools/fusion/delegate-args.test.ts +++ b/src/tools/fusion/delegate-args.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it } from "vitest"; +import { + MAX_CONTRACT_CHECKS, + MAX_CONTRACT_PROVIDES, + MAX_CONTRACT_RENDERED_CHARS, +} from "./contract.js"; import { MAX_DELEGATE_TASKS, MAX_INSTRUCTIONS_CHARS, @@ -225,3 +230,184 @@ describe("parseDelegateArgs", () => { ); }); }); + +describe("parseDelegateArgs — contract", () => { + const TASKS = [ + task({ id: "ship", files: ["js/ship.js"] }), + task({ id: "html" }), + task({ id: "main" }), + ]; + const CONTRACT = { + owners: { "index.html": "html", " js/main.js ": " main " }, + provides: [ + { task: "ship", kind: "symbol", name: "HD.Ship", in: "js/ship.js" }, + { task: "html", kind: "id", name: "btn-launch", in: "index.html" }, + { task: "ship", kind: "file", name: "js/hud.js" }, + ], + requires: [{ task: "main", name: "HD.Ship" }], + checks: [ + { task: "main", kind: "page", path: "index.html" }, + { kind: "command", cmd: "node", args: ["--check", "js/main.js"] }, + ], + }; + + it("carries a valid contract through, trimmed", () => { + const parsed = parseDelegateArgs({ tasks: TASKS, contract: CONTRACT }); + expect(parsed.ok).toBe(true); + expect(parsed.ok && parsed.contract).toEqual({ + owners: { "index.html": "html", "js/main.js": "main" }, + provides: CONTRACT.provides, + requires: CONTRACT.requires, + checks: CONTRACT.checks, + }); + }); + + it("accepts a contract that arrived as JSON text, like the task list", () => { + const parsed = parseDelegateArgs({ + tasks: TASKS, + contract: JSON.stringify(CONTRACT), + }); + expect(parsed.ok && parsed.contract?.provides).toHaveLength(3); + }); + + it("omits an empty or null contract rather than carrying it", () => { + for (const contract of [undefined, null, {}, { owners: {}, provides: [] }]) { + const parsed = parseDelegateArgs({ tasks: TASKS, contract }); + expect(parsed.ok).toBe(true); + expect(parsed).not.toHaveProperty("contract"); + } + }); + + it("names the field on every shape error", () => { + const bad = (contract: unknown): string => + expectError(parseDelegateArgs({ tasks: TASKS, contract })); + expect(bad("nonsense")).toContain("contract must be an object"); + expect(bad({ owners: [] })).toContain("contract.owners must be an object"); + expect(bad({ owners: { "": "ship" } })).toContain("contract.owners has an empty path"); + expect(bad({ owners: { "a.js": "nobody" } })).toBe( + 'validation: contract.owners["a.js"] names unknown task "nobody"', + ); + expect(bad({ provides: {} })).toContain("contract.provides must be an array"); + expect(bad({ provides: ["x"] })).toContain("contract.provides[0] must be an object"); + expect(bad({ provides: [{ task: "ghost", kind: "file", name: "a" }] })).toContain( + 'contract.provides[0].task names unknown task "ghost"', + ); + expect(bad({ provides: [{ task: "ship", kind: "class", name: "a" }] })).toContain( + "contract.provides[0].kind must be one of symbol, file, id, endpoint, env, flag, other", + ); + expect(bad({ provides: [{ task: "ship", kind: "file", name: " " }] })).toContain( + "contract.provides[0].name must be a non-empty string", + ); + expect(bad({ provides: [{ task: "ship", kind: "symbol", name: "a", in: 3 }] })).toContain( + "contract.provides[0].in must be a non-empty path", + ); + expect(bad({ requires: "x" })).toContain("contract.requires must be an array"); + expect(bad({ requires: [{ task: "ghost", name: "a" }] })).toContain( + 'contract.requires[0].task names unknown task "ghost"', + ); + expect(bad({ checks: {} })).toContain("contract.checks must be an array"); + expect(bad({ checks: [{ task: "ghost", cmd: "x" }] })).toContain( + 'contract.checks[0].task names unknown task "ghost"', + ); + expect(bad({ checks: [{ task: "main" }] })).toContain( + "contract.checks[0] carries no verify.run arguments", + ); + }); + + it("rejects a require that no provide satisfies — the launch-btn / btn-launch mismatch, caught before any worker runs", () => { + const error = expectError( + parseDelegateArgs({ + tasks: TASKS, + contract: { + provides: [{ task: "html", kind: "id", name: "btn-launch", in: "index.html" }], + requires: [{ task: "main", name: "launch-btn" }], + }, + }), + ); + expect(error).toBe( + 'validation: contract.requires[0].name "launch-btn" matches no provides entry (provided: btn-launch)', + ); + }); + + it("requires a place to look for a non-file provide", () => { + // `html` owns nothing and declares no files: a symbol it "provides" + // could only ever be reported unknown, so the brief is wrong now. + expect( + expectError( + parseDelegateArgs({ + tasks: TASKS, + contract: { provides: [{ task: "html", kind: "id", name: "x" }] }, + }), + ), + ).toContain( + 'contract.provides[0].in is required: task "html" owns no path and declares no files to look in', + ); + // An owned path, a declared file, or `in` each satisfy it; a glob does not. + for (const contract of [ + { owners: { "index.html": "html" }, provides: [{ task: "html", kind: "id", name: "x" }] }, + { provides: [{ task: "ship", kind: "symbol", name: "x" }] }, + { provides: [{ task: "html", kind: "id", name: "x", in: "index.html" }] }, + { provides: [{ task: "html", kind: "file", name: "index.html" }] }, + ]) { + expect(parseDelegateArgs({ tasks: TASKS, contract }).ok).toBe(true); + } + expect( + parseDelegateArgs({ + tasks: [task({ id: "g", files: ["js/**/*.js"] })], + contract: { provides: [{ task: "g", kind: "symbol", name: "x" }] }, + }).ok, + ).toBe(false); + }); + + it(`caps provides at ${MAX_CONTRACT_PROVIDES}, checks at ${MAX_CONTRACT_CHECKS} and the rendered block at ${MAX_CONTRACT_RENDERED_CHARS} chars`, () => { + expect( + expectError( + parseDelegateArgs({ + tasks: TASKS, + contract: { + provides: Array.from({ length: MAX_CONTRACT_PROVIDES + 1 }, (_, i) => ({ + task: "ship", + kind: "file", + name: `f${i}`, + })), + }, + }), + ), + ).toContain(`contract.provides has ${MAX_CONTRACT_PROVIDES + 1} entries; at most ${MAX_CONTRACT_PROVIDES}`); + expect( + expectError( + parseDelegateArgs({ + tasks: TASKS, + contract: { + checks: Array.from({ length: MAX_CONTRACT_CHECKS + 1 }, () => ({ cmd: "x" })), + }, + }), + ), + ).toContain(`contract.checks has ${MAX_CONTRACT_CHECKS + 1} entries; at most ${MAX_CONTRACT_CHECKS}`); + const error = expectError( + parseDelegateArgs({ + tasks: TASKS, + contract: { + provides: Array.from({ length: 40 }, (_, i) => ({ + task: "ship", + kind: "file", + name: `${"p".repeat(240)}${i}`, + })), + }, + }), + ); + expect(error).toMatch(/contract renders to [\d,]+ chars; the limit is 8,000 — shorten it by at least [\d,]+ chars/); + }); + + it("never throws on hostile contract input", () => { + for (const contract of [ + { owners: null, provides: null, requires: null, checks: null }, + { provides: [null] }, + { provides: [{ task: {}, kind: [], name: 7 }] }, + { checks: [[]] }, + "{not json", + ]) { + expect(() => parseDelegateArgs({ tasks: TASKS, contract })).not.toThrow(); + } + }); +}); diff --git a/src/tools/fusion/delegate-args.ts b/src/tools/fusion/delegate-args.ts index 7bf8d8bb..12280302 100644 --- a/src/tools/fusion/delegate-args.ts +++ b/src/tools/fusion/delegate-args.ts @@ -33,6 +33,20 @@ * still a validation error. */ +import { + CONTRACT_PROVIDE_KINDS, + MAX_CONTRACT_CHECKS, + MAX_CONTRACT_PROVIDES, + MAX_CONTRACT_RENDERED_CHARS, + ownedPaths, + renderContractBlock, + type ContractCheck, + type ContractProvide, + type ContractProvideKind, + type ContractRequire, + type DelegateContract, +} from "./contract.js"; + /** One unit of delegated work; becomes exactly one worker turn. */ export interface DelegateTask { /** Orchestrator-chosen id, unique within the call. Echoed in the output. */ @@ -51,13 +65,21 @@ export interface DelegateTask { } export type ParsedDelegateArgs = - | { ok: true; tasks: DelegateTask[]; maxWorkers?: number } + | { + ok: true; + tasks: DelegateTask[]; + maxWorkers?: number; + contract?: DelegateContract; + } | { ok: false; error: string }; export const MAX_DELEGATE_TASKS = 8; export const MAX_INSTRUCTIONS_CHARS = 32_000; export const MAX_TASK_FILES = 32; +/** Globs are patterns, not paths — never somewhere a provide can be looked for. */ +const GLOB_CHARS = /[*?[\]{}]/; + /** `8436` → `"8,436"`: the number the orchestrator has to act on, readable. */ function formatCount(n: number): string { return n.toLocaleString("en-US"); @@ -130,7 +152,7 @@ function readMaxWorkers(value: unknown): number | null | string { * string costs one `JSON.parse`; anything that does not parse falls * through unchanged and gets the same error it got before. */ -function readTaskList(value: unknown): unknown { +function readJsonArg(value: unknown): unknown { if (typeof value !== "string") return value; try { return JSON.parse(value); @@ -139,10 +161,169 @@ function readTaskList(value: unknown): unknown { } } +function isRecord(value: unknown): value is Record { + return value !== null && typeof value === "object" && !Array.isArray(value); +} + +/** + * Validate `contract` against the tasks it binds. Every error names the + * field, because the orchestrator fixes exactly one thing per retry. + * + * Two checks go beyond shape. A `requires` name must match a `provides` + * name exactly — a require nobody provides is the `launch-btn` / + * `btn-launch` mismatch the contract exists to catch, and the cheapest + * moment to catch it is before any worker runs. And a non-file provide + * needs somewhere to be looked for: `in`, an owned path, or the task's + * declared files — otherwise the presence check after the fan-out + * would have nothing to open and could only report "unknown". + */ +function readContract( + raw: unknown, + tasks: readonly DelegateTask[], +): DelegateContract | undefined | string { + const value = readJsonArg(raw); + if (value === undefined || value === null) return undefined; + if (!isRecord(value)) return "contract must be an object"; + const ids = new Set(tasks.map((t) => t.id)); + const known = (task: unknown, field: string): string | null => { + const id = readString(task); + if (id === null) return `${field} must be a task id`; + if (!ids.has(id)) return `${field} names unknown task "${id}"`; + return null; + }; + const contract: DelegateContract = {}; + + if (value.owners !== undefined && value.owners !== null) { + if (!isRecord(value.owners)) { + return "contract.owners must be an object of { path: taskId }"; + } + const owners: Record = {}; + for (const [path, task] of Object.entries(value.owners)) { + const key = path.trim(); + if (key.length === 0) return "contract.owners has an empty path"; + const bad = known(task, `contract.owners["${key}"]`); + if (bad !== null) return bad; + owners[key] = (task as string).trim(); + } + if (Object.keys(owners).length > 0) contract.owners = owners; + } + + if (value.provides !== undefined && value.provides !== null) { + if (!Array.isArray(value.provides)) { + return "contract.provides must be an array of { task, kind, name, in? }"; + } + if (value.provides.length > MAX_CONTRACT_PROVIDES) { + return `contract.provides has ${value.provides.length} entries; at most ${MAX_CONTRACT_PROVIDES}`; + } + const provides: ContractProvide[] = []; + for (let i = 0; i < value.provides.length; i += 1) { + const entry: unknown = value.provides[i]; + const label = `contract.provides[${i}]`; + if (!isRecord(entry)) return `${label} must be an object`; + const bad = known(entry.task, `${label}.task`); + if (bad !== null) return bad; + const kind = readString(entry.kind); + if ( + kind === null || + !(CONTRACT_PROVIDE_KINDS as readonly string[]).includes(kind) + ) { + return `${label}.kind must be one of ${CONTRACT_PROVIDE_KINDS.join(", ")}`; + } + const name = readString(entry.name); + if (name === null) return `${label}.name must be a non-empty string`; + const inPath = + entry.in === undefined || entry.in === null + ? null + : readString(entry.in); + if (entry.in !== undefined && entry.in !== null && inPath === null) { + return `${label}.in must be a non-empty path`; + } + provides.push({ + task: (entry.task as string).trim(), + kind: kind as ContractProvideKind, + name, + ...(inPath === null ? {} : { in: inPath }), + }); + } + if (provides.length > 0) contract.provides = provides; + } + + // A provide that is not a file must have somewhere to be looked for. + for (const [i, p] of (contract.provides ?? []).entries()) { + if (p.kind === "file" || p.in !== undefined) continue; + const task = tasks.find((t) => t.id === p.task); + const declared = (task?.files ?? []).filter((f) => !GLOB_CHARS.test(f)); + if (ownedPaths(contract, p.task).length === 0 && declared.length === 0) { + return `contract.provides[${i}].in is required: task "${p.task}" owns no path and declares no files to look in`; + } + } + + if (value.requires !== undefined && value.requires !== null) { + if (!Array.isArray(value.requires)) { + return "contract.requires must be an array of { task, name }"; + } + const provided = new Set((contract.provides ?? []).map((p) => p.name)); + const requires: ContractRequire[] = []; + for (let i = 0; i < value.requires.length; i += 1) { + const entry: unknown = value.requires[i]; + const label = `contract.requires[${i}]`; + if (!isRecord(entry)) return `${label} must be an object`; + const bad = known(entry.task, `${label}.task`); + if (bad !== null) return bad; + const name = readString(entry.name); + if (name === null) return `${label}.name must be a non-empty string`; + if (!provided.has(name)) { + const names = [...provided].slice(0, 10).join(", "); + return ( + `${label}.name "${name}" matches no provides entry` + + (names.length > 0 ? ` (provided: ${names})` : "") + ); + } + requires.push({ task: (entry.task as string).trim(), name }); + } + if (requires.length > 0) contract.requires = requires; + } + + if (value.checks !== undefined && value.checks !== null) { + if (!Array.isArray(value.checks)) { + return "contract.checks must be an array of verify.run specs"; + } + if (value.checks.length > MAX_CONTRACT_CHECKS) { + return `contract.checks has ${value.checks.length} entries; at most ${MAX_CONTRACT_CHECKS}`; + } + const checks: ContractCheck[] = []; + for (let i = 0; i < value.checks.length; i += 1) { + const entry: unknown = value.checks[i]; + const label = `contract.checks[${i}]`; + if (!isRecord(entry)) return `${label} must be an object`; + const { task, ...spec } = entry; + if (Object.keys(spec).length === 0) { + return `${label} carries no verify.run arguments`; + } + if (task === undefined || task === null) { + checks.push(spec); + continue; + } + const bad = known(task, `${label}.task`); + if (bad !== null) return bad; + checks.push({ task: (task as string).trim(), ...spec }); + } + if (checks.length > 0) contract.checks = checks; + } + + if (Object.keys(contract).length === 0) return undefined; + const rendered = renderContractBlock(contract).length; + if (rendered > MAX_CONTRACT_RENDERED_CHARS) { + const over = rendered - MAX_CONTRACT_RENDERED_CHARS; + return `contract renders to ${formatCount(rendered)} chars; the limit is ${formatCount(MAX_CONTRACT_RENDERED_CHARS)} — shorten it by at least ${formatCount(over)} chars`; + } + return contract; +} + export function parseDelegateArgs( raw: Record, ): ParsedDelegateArgs { - const rawTasks = readTaskList(raw.tasks); + const rawTasks = readJsonArg(raw.tasks); if (!Array.isArray(rawTasks)) { return fail("tasks must be an array of { id, title, instructions }"); } @@ -198,9 +379,12 @@ export function parseDelegateArgs( const maxWorkers = readMaxWorkers(raw.maxWorkers); if (typeof maxWorkers === "string") return fail(maxWorkers); + const contract = readContract(raw.contract, tasks); + if (typeof contract === "string") return fail(contract); return { ok: true, tasks, ...(maxWorkers === null ? {} : { maxWorkers }), + ...(contract === undefined ? {} : { contract }), }; } diff --git a/src/tools/fusion/fusion-delegate.test.ts b/src/tools/fusion/fusion-delegate.test.ts index f2525e55..77860a96 100644 --- a/src/tools/fusion/fusion-delegate.test.ts +++ b/src/tools/fusion/fusion-delegate.test.ts @@ -1,5 +1,8 @@ import { FanoutScopeRegistry } from "../../approval/fanout-scope.js"; import { 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 type { RunTurnResult } from "../../agent/agent-loop.js"; import type { ResolvedRunMode } from "../../llm/run-mode/index.js"; @@ -495,6 +498,128 @@ describe("fusion.delegate", () => { expect(asked).toHaveLength(1); }); + describe("with a contract", () => { + const CONTRACT = { + owners: { "js/ship.js": "t1", "index.html": "t2" }, + provides: [ + { task: "t1", kind: "symbol", name: "HD.Ship", in: "js/ship.js" }, + { task: "t1", kind: "symbol", name: "HD.Ship.reset", in: "js/ship.js" }, + { task: "t2", kind: "id", name: "btn-launch", in: "index.html" }, + ], + requires: [{ task: "t2", name: "HD.Ship" }], + checks: [ + { task: "t1", kind: "command", cmd: "node", args: ["--check", "js/ship.js"] }, + { task: "t2", kind: "page", path: "index.html", checks: ["no errors"] }, + { kind: "command", cmd: "npm", args: ["test"] }, + ], + }; + + function fixture(): string { + const dir = mkdtempSync(join(tmpdir(), "fusion-delegate-contract-")); + mkdirSync(join(dir, "js")); + writeFileSync(join(dir, "js", "ship.js"), "HD.Ship = class {};"); + writeFileSync(join(dir, "index.html"), ''); + return dir; + } + + it("briefs every worker with it, checks presence on disk and runs the checks through the injected runner", async () => { + const dir = fixture(); + try { + const briefs: string[] = []; + const runChecks = vi.fn( + async (specs: readonly Record[], runCtx: { workingDir: string }) => ({ + ok: false, + results: specs.map((spec) => + spec.kind === "page" + ? { ok: false, summary: "no errors: 1 pageerror — ReferenceError: p is not defined" } + : { ok: true, summary: `${runCtx.workingDir}: exit 0` }, + ), + }), + ); + const tool = buildFusionDelegateTool( + deps({ + workingDir: dir, + runChecks, + runTurn: async (_session, userMessage) => { + briefs.push(userMessage); + return turnResult(); + }, + }), + ); + const result = await tool.run( + { tasks: TASKS, contract: CONTRACT }, + ctx({ workingDir: dir }), + ); + expect(briefs).toHaveLength(2); + expect(briefs[0]).toContain("CONTRACT — the interface between the parts"); + expect(briefs[0]).toContain("You provide: symbol HD.Ship in js/ship.js; symbol HD.Ship.reset in js/ship.js"); + expect(briefs[1]).toContain("You may rely on: HD.Ship (symbol from t1 in js/ship.js)"); + + // The runner sees the specs without their `task` key, and the call's cwd. + expect(runChecks).toHaveBeenCalledTimes(1); + expect(runChecks.mock.calls[0]![0]).toEqual([ + { kind: "command", cmd: "node", args: ["--check", "js/ship.js"] }, + { kind: "page", path: "index.html", checks: ["no errors"] }, + { kind: "command", cmd: "npm", args: ["test"] }, + ]); + expect(runChecks.mock.calls[0]![1].workingDir).toBe(dir); + + // Presence: `HD.Ship.reset` was never written; the id is spelled the other way. + const lines = result.summary.split("\n"); + expect(lines[0]).toBe("2 tasks: 1 ok, 1 failed"); + expect(lines[1]).toBe( + "contract: 2 missing — [t1] symbol HD.Ship.reset not in js/ship.js; [t2] id btn-launch not in index.html; call-level checks: 1 of 1 passed", + ); + expect(lines[2]).toBe( + "- [t1] ok — One — checks: 1 of 1 passed — contract: symbol HD.Ship.reset not in js/ship.js", + ); + // The task whose declared check failed is `failed`, with the verdict as its error. + expect(lines[3]).toBe( + "- [t2] failed — Two — error: checks: no errors: 1 pageerror — ReferenceError: p is not defined — checks: 1 of 1 failed — contract: id btn-launch not in index.html", + ); + const rows = result.details.tasks as WorkerTaskResult[]; + expect(rows.map((r) => r.status)).toEqual(["ok", "failed"]); + const report = result.details.contract as { findings: unknown[]; checks: unknown[] }; + expect(report.findings).toHaveLength(3); + expect(report.checks).toHaveLength(3); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("reports declared checks as not run when no runner is wired, and never fails a task on them", async () => { + const dir = fixture(); + try { + const tool = buildFusionDelegateTool(deps({ workingDir: dir })); + const result = await tool.run( + { tasks: TASKS, contract: CONTRACT }, + ctx({ workingDir: dir }), + ); + expect(result.summary.split("\n")[1]).toContain("3 checks not run — no check runner is wired"); + const rows = result.details.tasks as WorkerTaskResult[]; + expect(rows.map((r) => r.status)).toEqual(["ok", "ok"]); + expect(rows[0]).not.toHaveProperty("checks"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("rejects a contract that does not bind the tasks, before any worker runs", async () => { + const runTurn = vi.fn(async () => turnResult()); + const tool = buildFusionDelegateTool(deps({ runTurn })); + const result = await tool.run( + { + tasks: TASKS, + contract: { provides: [{ task: "ghost", kind: "file", name: "a" }] }, + }, + ctx(), + ); + expect(result.status).toBe("error"); + expect(result.summary).toContain('contract.provides[0].task names unknown task "ghost"'); + expect(runTurn).not.toHaveBeenCalled(); + }); + }); + it("asks again when a later fan-out reaches outside what was approved", async () => { const asked: string[] = []; const scopes = new FanoutScopeRegistry(); diff --git a/src/tools/fusion/fusion-delegate.ts b/src/tools/fusion/fusion-delegate.ts index fc28b9c7..4ca16eea 100644 --- a/src/tools/fusion/fusion-delegate.ts +++ b/src/tools/fusion/fusion-delegate.ts @@ -9,6 +9,15 @@ import type { StructuredLogger } from "../../tracing/index.js"; import { isFusionWorkerSessionId } from "../../session/fusion-worker-session.js"; import type { ToolDefinition } from "../tool-registry.js"; import { parseDelegateArgs } from "./delegate-args.js"; +import { + applyCheckOutcomes, + applyContractFindings, + inspectContractProvides, + renderContractLine, + runContractChecks, + type ContractCheckRunner, + type ContractReport, +} from "./contract-checks.js"; import { runWorkerTasks, type WorkerRunnerDeps } from "./worker-runner.js"; import { formatDelegateOutput, @@ -41,6 +50,13 @@ export interface FusionDelegateDeps extends WorkerRunnerDeps { * only the orchestrator's instructions, as they did before. */ resolveOriginalRequest?: (sessionId: string) => string | undefined; + /** + * Runs a contract's `checks` (`verify.run` specs) after the fan-out — + * the verify tool family's `runChecks`, wired by the runtime. Absent, + * declared checks are reported as not run; they are never assumed to + * have passed. + */ + runChecks?: ContractCheckRunner; } function error( @@ -123,7 +139,7 @@ export function buildFusionDelegateTool( return { name: FUSION_DELEGATE_TOOL, description: - "Delegate independent parts of the work to local worker agents that run concurrently. You choose how many run at once with `maxWorkers`. Args: { tasks: [{ id, title, instructions, deliverable?, files? }], maxWorkers? }.", + "Delegate independent parts of the work to local worker agents that run concurrently. You choose how many run at once with `maxWorkers`. An optional `contract` (owners, provides, requires, checks) is prepended to every brief and checked after the fan-out. Args: { tasks: [{ id, title, instructions, deliverable?, files? }], maxWorkers?, contract? }.", readonly: false, async run(rawArgs, ctx): Promise { if (isFusionWorkerSessionId(ctx.sessionId)) { @@ -266,6 +282,7 @@ export function buildFusionDelegateTool( try { results = await runWorkerTasks(deps, { ...(originalRequest === undefined ? {} : { originalRequest }), + ...(parsed.contract === undefined ? {} : { contract: parsed.contract }), parentSessionId: ctx.sessionId, tasks: parsed.tasks, maxWorkers, @@ -285,6 +302,35 @@ export function buildFusionDelegateTool( ); } + // The contract's verdict, from the disk and the check runner, folded + // into the rows BEFORE the head line counts them: a task whose + // declared check failed is `failed` in the table the orchestrator + // reads, not `ok` with a footnote. + let contract: ContractReport | undefined; + if (parsed.contract !== undefined) { + const findings = await inspectContractProvides( + parsed.contract, + parsed.tasks, + ctx.workingDir, + ); + results = applyContractFindings(results, findings); + const checks = await runContractChecks( + parsed.contract.checks ?? [], + deps.runChecks, + { workingDir: ctx.workingDir, signal: ctx.signal }, + ); + results = applyCheckOutcomes(results, checks.outcomes); + contract = { + findings, + checks: checks.outcomes, + ...(checks.checksSkipped === undefined + ? {} + : { checksSkipped: checks.checksSkipped }), + }; + } + const contractLine = + contract === undefined ? undefined : renderContractLine(contract); + // …and takes the turn back. One line, so the operator can see the // spend return to the cloud leg instead of guessing which of the // lines above was the last worker. @@ -311,12 +357,15 @@ export function buildFusionDelegateTool( { tool: FUSION_DELEGATE_TOOL, status: "ok", - output: `${formatDelegateOutput(results, deps.outputCharCap)}${hint}`, + output: `${formatDelegateOutput(results, deps.outputCharCap, { + ...(contractLine === undefined ? {} : { contractLine }), + })}${hint}`, details: { tasks: results, maxWorkers, requestedWorkers: requested, ...(Number.isFinite(poolSize) ? { slotPoolSize: poolSize } : {}), + ...(contract === undefined ? {} : { contract }), }, }, { maxSummaryLength: deps.outputCharCap + 400, maxTailLines: 2000 }, diff --git a/src/tools/fusion/index.ts b/src/tools/fusion/index.ts index 06a0efb3..a396fffc 100644 --- a/src/tools/fusion/index.ts +++ b/src/tools/fusion/index.ts @@ -11,6 +11,39 @@ export { MAX_TASK_FILES, } from "./delegate-args.js"; export type { DelegateTask, ParsedDelegateArgs } from "./delegate-args.js"; +export { + CONTRACT_PROVIDE_KINDS, + MAX_CONTRACT_CHECKS, + MAX_CONTRACT_PROVIDES, + MAX_CONTRACT_RENDERED_CHARS, + describeProvide, + ownedPaths, + renderContractBlock, + renderContractForTask, +} from "./contract.js"; +export type { + ContractCheck, + ContractProvide, + ContractProvideKind, + ContractRequire, + DelegateContract, +} from "./contract.js"; +export { + applyCheckOutcomes, + applyContractFindings, + contentProvides, + describeMissing, + inspectContractProvides, + renderContractLine, + runContractChecks, +} from "./contract-checks.js"; +export type { + ContractCheckOutcome, + ContractCheckResult, + ContractCheckRunner, + ContractFinding, + ContractReport, +} from "./contract-checks.js"; export { renderWorkerBrief, pickOriginalRequest, @@ -29,6 +62,7 @@ export { WORKER_HINT_QUOTA, } from "./worker-result.js"; export type { + TaskCheckSummary, WorkerStopCause, WorkerTaskResult, WorkerTaskStatus, diff --git a/src/tools/fusion/worker-prompt.test.ts b/src/tools/fusion/worker-prompt.test.ts index 5a75d14a..fd226122 100644 --- a/src/tools/fusion/worker-prompt.test.ts +++ b/src/tools/fusion/worker-prompt.test.ts @@ -121,6 +121,48 @@ describe("renderWorkerBrief — the original request", () => { }); }); +describe("renderWorkerBrief — the contract", () => { + const CONTRACT = { + owners: { "js/ship.js": "t1", "index.html": "html" }, + provides: [ + { task: "t1", kind: "symbol" as const, name: "HD.Ship", in: "js/ship.js" }, + { task: "html", kind: "id" as const, name: "btn-launch", in: "index.html" }, + ], + requires: [{ task: "t1", name: "btn-launch" }], + }; + + it("prepends the shared block and this task's three lines, between the request and the TASK", () => { + const brief = renderWorkerBrief(TASK, { + workingDir: "/repo", + originalRequest: "Build the game", + contract: CONTRACT, + }); + const requestEnd = brief.indexOf("----- END ORIGINAL REQUEST -----"); + const contractAt = brief.indexOf("CONTRACT — the interface between the parts"); + const forTaskAt = brief.indexOf("For TASK t1:"); + const taskAt = brief.indexOf("TASK t1: Map the auth routes"); + expect(requestEnd).toBeGreaterThan(0); + expect(contractAt).toBeGreaterThan(requestEnd); + expect(forTaskAt).toBeGreaterThan(contractAt); + expect(taskAt).toBeGreaterThan(forTaskAt); + // The whole contract, then what it means for this worker. + expect(brief).toContain("- [html] id btn-launch in index.html"); + expect(brief).toContain("You own: js/ship.js"); + expect(brief).toContain("You provide: symbol HD.Ship in js/ship.js"); + expect(brief).toContain("You may rely on: btn-launch (id from html in index.html)"); + // The frame line is still first. + expect(brief.split("\n")[0]).toContain("You are a worker agent"); + }); + + it("adds the PROVIDED rule only when there is a contract", () => { + const withContract = renderWorkerBrief(TASK, { workingDir: "/repo", contract: CONTRACT }); + expect(withContract).toMatch(/- End the reply with a `PROVIDED:` list/); + const without = renderWorkerBrief(TASK, { workingDir: "/repo" }); + expect(without).not.toContain("PROVIDED:"); + expect(without).not.toContain("CONTRACT"); + }); +}); + describe("pickOriginalRequest", () => { const LONG = "Build the thing. ".repeat(30); diff --git a/src/tools/fusion/worker-prompt.ts b/src/tools/fusion/worker-prompt.ts index c87eab4a..f1d4c279 100644 --- a/src/tools/fusion/worker-prompt.ts +++ b/src/tools/fusion/worker-prompt.ts @@ -1,5 +1,10 @@ import type { ConversationTurn } from "../../session/conversation-turn.js"; import type { DelegateTask } from "./delegate-args.js"; +import { + renderContractBlock, + renderContractForTask, + type DelegateContract, +} from "./contract.js"; import { FUSION_WORKER_APPROVAL_MARKER } from "./worker-tool-policy.js"; /** @@ -98,15 +103,36 @@ function quoteOriginalRequest(request: string): string[] { return lines; } +/** + * The contract, when the fan-out has one, sits between the request + * (what the whole job is) and the task (what this worker does): the + * shared block first, then the three lines that say what it means for + * this task. Every worker of the fan-out reads the same shared block, + * which is the point — the names they must agree on are written once, + * not paraphrased eight times. + */ +function quoteContract(contract: DelegateContract, taskId: string): string[] { + return [ + renderContractBlock(contract), + `For TASK ${taskId}:`, + renderContractForTask(contract, taskId), + ]; +} + export function renderWorkerBrief( task: DelegateTask, - options: { workingDir: string; originalRequest?: string }, + options: { + workingDir: string; + originalRequest?: string; + contract?: DelegateContract; + }, ): string { const request = options.originalRequest?.trim() ?? ""; const lines: string[] = [ `You are a worker agent executing one delegated task inside ${options.workingDir}; you have no memory of the parent conversation.`, ``, ...(request.length > 0 ? [...quoteOriginalRequest(request), ``] : []), + ...(options.contract ? [...quoteContract(options.contract, task.id), ``] : []), `TASK ${task.id}: ${task.title}`, ``, task.instructions, @@ -125,6 +151,11 @@ export function renderWorkerBrief( `- The operator authorised this fan-out to write files AND run commands in the directories the task names, so working there needs no permission: write the files, run the build, run the tests, read the output. Anything outside them is refused, not queued: a tool result carrying "${FUSION_WORKER_APPROVAL_MARKER}" means nobody can approve it here. Stop retrying it and say in your reply exactly what was blocked and where, so the orchestrator can re-send the task with that path named — it cannot run the action for you.`, `- Read files only inside ${options.workingDir} and the directories this task writes in; a read anywhere else is refused. Do not search other projects for context — ${request.length > 0 ? "your task, its FILES and the original request are" : "your task and its FILES are"} the context you have.`, `- Finish with \`reply\` carrying the concise result of this task (about ${WORKER_REPLY_CHAR_BUDGET} characters at most). That reply is the ONLY thing the orchestrator receives — findings, file paths, decisions and anything it needs to merge your part must be inside it.`, + ...(options.contract + ? [ + `- End the reply with a \`PROVIDED:\` list of what you produced, one line per item, using the CONTRACT's exact names (kind, name, path). Name anything you provide differently from the contract, or could not provide, on its own line.`, + ] + : []), ); return lines.join("\n"); } diff --git a/src/tools/fusion/worker-result.test.ts b/src/tools/fusion/worker-result.test.ts index fe80f93b..2a3921ad 100644 --- a/src/tools/fusion/worker-result.test.ts +++ b/src/tools/fusion/worker-result.test.ts @@ -320,6 +320,55 @@ describe("formatDelegateOutput", () => { }); }); +describe("formatDelegateOutput — the contract", () => { + it("puts the contract line right under the head line, and each task's checks on its row and block", () => { + const out = formatDelegateOutput( + [ + row({ checks: { total: 2, failed: 0 } }), + row({ + id: "t2", + status: "failed", + error: "checks: no errors: 1 pageerror", + checks: { total: 2, failed: 1, detail: "no errors: 1 pageerror" }, + }), + ], + 8000, + { contractLine: "contract: 1 missing — [t1] symbol HD.Ship not in js/ship.js" }, + ); + const lines = out.split("\n"); + expect(lines[0]).toBe("2 tasks: 1 ok, 1 failed"); + expect(lines[1]).toBe("contract: 1 missing — [t1] symbol HD.Ship not in js/ship.js"); + expect(lines[2]).toBe("- [t1] ok — Map — checks: 2 of 2 passed"); + // The error already carries the verdict, so the count stands alone. + expect(lines[3]).toBe( + "- [t2] failed — Map — error: checks: no errors: 1 pageerror — checks: 1 of 2 failed", + ); + const block = out.split("\n\n")[2]!.split("\n"); + expect(block[0]).toContain("[t2] failed — Map (2 steps, 3s, 1 tool calls, 0 errors) — error: checks: no errors: 1 pageerror"); + expect(block[1]).toBe("checks: 1 of 2 failed"); + }); + + it("keeps the checks detail on a row whose error is something else", () => { + const out = formatDelegateOutput( + [ + row({ + status: "failed", + error: "declared file a.js does not exist after the task", + checks: { total: 1, failed: 1, detail: "exit code 1" }, + }), + ], + 4000, + ); + expect(out.split("\n")[1]).toBe( + "- [t1] failed — Map — error: declared file a.js does not exist after the task — checks: 1 of 1 failed — exit code 1", + ); + }); + + it("renders no contract line when none was given", () => { + expect(formatDelegateOutput([row()], 4000).split("\n")[1]).toBe("- [t1] ok — Map"); + }); +}); + describe("classifyWorkerStatus — a ceiling that ended the task", () => { it("reports max_steps for a reply written on the forced final step", () => { // A worker at 40/40 replied "the step limit was reached before the diff --git a/src/tools/fusion/worker-result.ts b/src/tools/fusion/worker-result.ts index 64da354e..22dc4d72 100644 --- a/src/tools/fusion/worker-result.ts +++ b/src/tools/fusion/worker-result.ts @@ -51,6 +51,38 @@ export interface WorkerTaskResult { * never touched. */ notes?: string[]; + /** + * The contract's `checks` attributed to this task, once they ran + * (`contract-checks.ts`). A failure is also the row's `error`. + */ + checks?: TaskCheckSummary; +} + +export interface TaskCheckSummary { + total: number; + failed: number; + /** The failing checks' verdicts, joined; absent when all passed. */ + detail?: string; +} + +/** + * `checks: 1 of 2 failed — …` / `checks: 2 of 2 passed`. The detail is + * left off when the row's error already carries it (a task failed BY + * its checks has `checks: …` as its error), so the verdict reads once. + */ +export function describeChecks( + checks: TaskCheckSummary, + cap: number, + error?: string, +): string { + if (checks.failed === 0) { + return `checks: ${checks.total} of ${checks.total} passed`; + } + const detail = + checks.detail === undefined || error?.startsWith("checks: ") + ? "" + : ` — ${oneLine(checks.detail, cap)}`; + return `checks: ${checks.failed} of ${checks.total} failed${detail}`; } /** @@ -288,9 +320,10 @@ const ERROR_HEAD_CHARS = 400; export function formatDelegateOutput( results: readonly WorkerTaskResult[], charCap: number, + extra: { contractLine?: string } = {}, ): string { if (results.length === 0) return "(no tasks were run)"; - const table = renderStatusTable(results); + const table = renderStatusTable(results, extra.contractLine); const room = Math.max(0, charCap - table.length - 4); const perTask = Math.max(200, Math.floor(room / results.length)); const blocks = results.map((r) => renderBlock(r, perTask)); @@ -302,7 +335,16 @@ export function formatDelegateOutput( /** How much of an error or a note one status-table line carries. */ const TABLE_DETAIL_CHARS = 160; -function renderStatusTable(results: readonly WorkerTaskResult[]): string { +/** + * The head line, the contract's verdict when there is one, then one + * line per task. The contract line sits second because it is the one + * cross-task fact: a missing provide is a hole between parts, not a + * property of any single row. + */ +function renderStatusTable( + results: readonly WorkerTaskResult[], + contractLine: string | undefined, +): string { const counts = new Map(); for (const r of results) { counts.set(r.status, (counts.get(r.status) ?? 0) + 1); @@ -314,11 +356,13 @@ function renderStatusTable(results: readonly WorkerTaskResult[]): string { [ `- [${r.id}] ${r.status} — ${r.title}`, ...(r.error ? [`error: ${oneLine(r.error, TABLE_DETAIL_CHARS)}`] : []), + ...(r.checks ? [describeChecks(r.checks, TABLE_DETAIL_CHARS, r.error)] : []), ...(r.notes ?? []).map((note) => oneLine(note, TABLE_DETAIL_CHARS)), ].join(" — "), ); return [ `${results.length} task${results.length === 1 ? "" : "s"}: ${tally}`, + ...(contractLine === undefined ? [] : [contractLine]), ...lines, ].join("\n"); } @@ -342,6 +386,9 @@ function renderBlock(result: WorkerTaskResult, perTaskCap: number): string { (result.error ? ` — error: ${oneLine(result.error, ERROR_HEAD_CHARS)}` : ""); const diagnosis = [ ...(result.hint ? [`hint: ${result.hint}`] : []), + ...(result.checks + ? [describeChecks(result.checks, ERROR_HEAD_CHARS, result.error)] + : []), ...(result.notes ?? []).map((note) => `note: ${note}`), ]; const used = [head, ...diagnosis].join("\n").length; diff --git a/src/tools/fusion/worker-runner.test.ts b/src/tools/fusion/worker-runner.test.ts index 09f9e39b..c7716309 100644 --- a/src/tools/fusion/worker-runner.test.ts +++ b/src/tools/fusion/worker-runner.test.ts @@ -411,6 +411,30 @@ describe("runWorkerTasks", () => { expect(calls[1]!.userMessage).toContain("Do part 1"); }); + it("renders the fan-out's contract into every worker's brief", async () => { + const { deps, calls } = harness(async () => turnResult()); + await runWorkerTasks(deps, { + ...BASE, + tasks: tasks(2), + maxWorkers: 2, + contract: { + owners: { "a.js": "t0", "b.js": "t1" }, + provides: [{ task: "t0", kind: "symbol", name: "A", in: "a.js" }], + requires: [{ task: "t1", name: "A" }], + }, + signal: new AbortController().signal, + }); + expect(calls).toHaveLength(2); + for (const call of calls) { + expect(call.userMessage).toContain("CONTRACT — the interface between the parts"); + expect(call.userMessage).toContain("- [t0] symbol A in a.js"); + } + expect(calls[0]!.userMessage).toContain("You own: a.js"); + expect(calls[0]!.userMessage).toContain("You provide: symbol A in a.js"); + expect(calls[1]!.userMessage).toContain("You own: b.js"); + expect(calls[1]!.userMessage).toContain("You may rely on: A (symbol from t0 in a.js)"); + }); + it("reports max_steps, not ok, when the worker replied on its forced final step", async () => { const { deps } = harness(async ({ options }) => { options.eventHook?.({ diff --git a/src/tools/fusion/worker-runner.ts b/src/tools/fusion/worker-runner.ts index 8a3f1595..78bc5010 100644 --- a/src/tools/fusion/worker-runner.ts +++ b/src/tools/fusion/worker-runner.ts @@ -4,6 +4,7 @@ import type { SessionState } from "../../session/session-state.js"; import type { FusionWorkerMeta } from "../../session/fusion-worker-session.js"; import type { TurnOrigin } from "../../runtime/turn-controller.js"; import type { DelegateTask } from "./delegate-args.js"; +import type { DelegateContract } from "./contract.js"; import { applyDeclaredFileReport, inspectDeclaredFiles, @@ -84,6 +85,11 @@ export interface RunWorkerTasksOptions { * the parent turn has none to give. */ originalRequest?: string; + /** + * The fan-out's contract, rendered into every brief above the task + * (`renderWorkerBrief`). Checked after the fan-out by the caller. + */ + contract?: DelegateContract; signal: AbortSignal; } @@ -235,6 +241,7 @@ async function runOneTask( ...(options.originalRequest === undefined ? {} : { originalRequest: options.originalRequest }), + ...(options.contract === undefined ? {} : { contract: options.contract }), }), { origin: "fusion", From e0e2ec0d3e982e6af14f708026e8c5596b9b74b2 Mon Sep 17 00:00:00 2001 From: Valera Brizhatiuk <19537764+plombeer31@users.noreply.github.com> Date: Mon, 14 Sep 2026 23:39:41 +0300 Subject: [PATCH 2/2] fix(fusion): F2 no_changes task status and a truthful call outcome for fusion.delegate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two lies the fan-out result could still tell after patch 1: - A worker that declared `files`, read them, and replied "I'm done!" without writing anything came back `ok` (run 14, repair 1). New task status `no_changes`: an `ok` task that declared `files`, made no successful write / edit / patch call (the collector now counts them as `tools.writes`) and changed none of its declared files on disk (`DeclaredFileReport.modified`). Both halves are needed — the call count alone misreads a worker that wrote through the shell, the disk alone misreads declared globs. Tasks without `files` never get it; statuses that already say why the work is incomplete are left alone. The feed line reads "no changes — …" and the row carries the reason. - A fan-out where every worker failed returned `status: "ok"`. The call's status now summarises its tasks (`details.outcome`: `all_ok` / `partial` / `all_failed`) and is `error` only when every task failed or was cancelled — partial results stay `ok` because they are the value of a fan-out. The head line counts every status in a fixed order: `7 tasks: 5 ok, 1 no_changes, 1 failed`. AGENTS.md's fusion section is updated to match. --- AGENTS.md | 4 +- src/tools/fusion/contract-checks.test.ts | 2 +- src/tools/fusion/declared-files.test.ts | 48 ++++++++++- src/tools/fusion/declared-files.ts | 36 +++++++- src/tools/fusion/fusion-delegate.test.ts | 31 ++++++- src/tools/fusion/fusion-delegate.ts | 19 +++-- src/tools/fusion/index.ts | 5 ++ src/tools/fusion/worker-result.test.ts | 53 +++++++++++- src/tools/fusion/worker-result.ts | 61 ++++++++++++- src/tools/fusion/worker-runner.test.ts | 104 ++++++++++++++++++++++- src/tools/fusion/worker-runner.ts | 20 +++-- 11 files changed, 354 insertions(+), 29 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 4bd45bc5..1f38c9df 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2497,13 +2497,13 @@ On the fusion route the strip walks **four** controls, not three: `backend ⇄ p The fourth control, `workers` ([src/tui/composer-switch/composer-switch-worker-rows.ts](src/tui/composer-switch/composer-switch-worker-rows.ts)), is the **local** half: the downloaded models the workers can run, then `1..8 workers`. A model row goes through `LocalModelsOrchestrator.setActive`, which restarts the managed daemon and writes only `localModels.*` — never `activeTextProvider` — so fusion survives the pick; `triggerLlmPrimary` is deliberately not used, because its local branch also makes `local-llama` the active text provider. Row `active` is read from `localModelsPanel.rows[].active` (which model the daemon serves), not from the LLM pane's row, whose `active` additionally requires `local-llama` to be the chat route — under fusion it never is. A count row goes through `setFusionWorkersInConfig`, which moves `llm.runMode.fusion.workers` and `localModels.managed.parallel` in one write: they are one fact seen from two sides, and a running daemon keeps its old slot count until restarted, which the orchestrator says in a notice. `/runmode workers N` is the same call. ### The orchestrator and its workers -`fusion.delegate` ([src/tools/fusion/](src/tools/fusion/)) is the tool that makes the mode more than a label. The orchestrator plans, then hands independent parts down in one call: `{ tasks: [{ id, title, instructions, deliverable?, files? }] (1..8), maxWorkers?, contract? }`. The optional `contract` ([contract.ts](src/tools/fusion/contract.ts): `owners` path → task, `provides` — a `symbol` / `file` / `id` / `endpoint` / `env` / `flag` / `other` a task must produce, `requires`, and `checks` as `verify.run` specs) is the interface between the parts: it is validated against the task ids (a `requires` name must match a `provides` name; ≤ 64 provides, ≤ 16 checks, ≤ 8,000 rendered chars), prepended to every brief with a per-task "You own / You provide / You may rely on", and after the fan-out each provide is checked for presence by language-agnostic means and the checks run through the injected `runChecks` seam ([contract-checks.ts](src/tools/fusion/contract-checks.ts)) — a missing provide is a note on its owner's row and a `contract:` line in the status table; a failing declared check makes its task `failed`; checks with no runner wired are reported as not run, never as passed. `maxWorkers` has no parser ceiling — an over-ambitious number runs as wide as the machine allows rather than coming back as a validation error the model has to notice and retry; only a value below one is invalid. Each task becomes one ephemeral worker session running a turn pinned to the local leg — `origin: "fusion"`, `maxSteps: workerMaxSteps`, `taskMaxDurationMs: workerTimeoutMs`, `toolFilter: isWorkerVisibleTool`, an approval policy of `refuse` — and the call returns every reply plus a per-task status (`ok` / `failed` / `cancelled` / `max_steps` / `needs_orchestrator`). The brief ([worker-prompt.ts](src/tools/fusion/worker-prompt.ts)) tells the worker it has no memory of the parent conversation, that it must never ask a question, and that a refused approval is handed back up rather than retried. It also quotes the operator's **original request** above the task, labelled "context only; your task is below" and clipped at `ORIGINAL_REQUEST_CHAR_BUDGET` (16,000 chars) with an explicit note: orchestrator briefs are summaries, and thin ones made workers build the wrong thing or scavenge the disk for the spec. `executeTurn` records the request per session for the running turn (`pickOriginalRequest` — the turn's message, plus the message before it when the turn started with a short follow-up like "continue") and the tool reads it through `resolveOriginalRequest`. With the spec carried that way, `instructions` is capped at 32,000 chars, and the rejection names the limit and the exact overage. Statuses are ground-truthed rather than taken from the reply: a `reply` on the loop's forced finalization step comes back with `RunTurnResult.stopCause` and is reported `max_steps`, as is a worker stopped by its own `workerTimeoutMs` (which is not a cancellation); an `ok` task whose `files` entry does not exist afterwards is downgraded to `failed` ([declared-files.ts](src/tools/fusion/declared-files.ts)), and an existing but untouched entry is only a note, since `files` may be inputs. A failed worker's loop/provider error lands on the task's head line with a remediation hint for the recognised classes (context exceeded, first-token/idle timeout, 402/429). A worker's filesystem **reads** are confined to its working directory plus the fan-out's write scope ([worker-read-scope.ts](src/tools/fusion/worker-read-scope.ts), installed on the registry by `confineWorkerReads` at boot); other sessions are unaffected. `### fusion` in the stable prefix ([src/prompt/fusion-guidance.ts](src/prompt/fusion-guidance.ts)) is what makes a cloud model reach for the tool at all — it is present only while the descriptor is mounted, so a non-fusion install's prefix is byte-identical to a build without the feature. It carries two things the orchestrator cannot get anywhere else: a push to delegate whenever the work splits into independent, self-contained parts (with the honest limit — a part that only makes sense with this conversation in front of it stays the orchestrator's), and one line of **machine facts** ([fusion-machine-facts.ts](src/prompt/fusion-machine-facts.ts)) so the width it picks is informed rather than guessed: the llama-server request slots, the local model behind them, and therefore how many workers run at once before the rest queue. Those facts are read from the already-loaded config by `buildPrompt` — no probe at prompt-build time — and a fact the runtime does not own (an `external` server's `--parallel`) is left unsaid rather than guessed, because a guessed number is one the model will plan against. They are config values, so they move only when the operator writes the config file, which is the same event that already flips the descriptor gate; nothing per-turn goes in that line or the KV cache would drop on every step. +`fusion.delegate` ([src/tools/fusion/](src/tools/fusion/)) is the tool that makes the mode more than a label. The orchestrator plans, then hands independent parts down in one call: `{ tasks: [{ id, title, instructions, deliverable?, files? }] (1..8), maxWorkers?, contract? }`. The optional `contract` ([contract.ts](src/tools/fusion/contract.ts): `owners` path → task, `provides` — a `symbol` / `file` / `id` / `endpoint` / `env` / `flag` / `other` a task must produce, `requires`, and `checks` as `verify.run` specs) is the interface between the parts: it is validated against the task ids (a `requires` name must match a `provides` name; ≤ 64 provides, ≤ 16 checks, ≤ 8,000 rendered chars), prepended to every brief with a per-task "You own / You provide / You may rely on", and after the fan-out each provide is checked for presence by language-agnostic means and the checks run through the injected `runChecks` seam ([contract-checks.ts](src/tools/fusion/contract-checks.ts)) — a missing provide is a note on its owner's row and a `contract:` line in the status table; a failing declared check makes its task `failed`; checks with no runner wired are reported as not run, never as passed. `maxWorkers` has no parser ceiling — an over-ambitious number runs as wide as the machine allows rather than coming back as a validation error the model has to notice and retry; only a value below one is invalid. Each task becomes one ephemeral worker session running a turn pinned to the local leg — `origin: "fusion"`, `maxSteps: workerMaxSteps`, `taskMaxDurationMs: workerTimeoutMs`, `toolFilter: isWorkerVisibleTool`, an approval policy of `refuse` — and the call returns every reply plus a per-task status (`ok` / `no_changes` / `failed` / `cancelled` / `max_steps` / `needs_orchestrator`). The brief ([worker-prompt.ts](src/tools/fusion/worker-prompt.ts)) tells the worker it has no memory of the parent conversation, that it must never ask a question, and that a refused approval is handed back up rather than retried. It also quotes the operator's **original request** above the task, labelled "context only; your task is below" and clipped at `ORIGINAL_REQUEST_CHAR_BUDGET` (16,000 chars) with an explicit note: orchestrator briefs are summaries, and thin ones made workers build the wrong thing or scavenge the disk for the spec. `executeTurn` records the request per session for the running turn (`pickOriginalRequest` — the turn's message, plus the message before it when the turn started with a short follow-up like "continue") and the tool reads it through `resolveOriginalRequest`. With the spec carried that way, `instructions` is capped at 32,000 chars, and the rejection names the limit and the exact overage. Statuses are ground-truthed rather than taken from the reply: a `reply` on the loop's forced finalization step comes back with `RunTurnResult.stopCause` and is reported `max_steps`, as is a worker stopped by its own `workerTimeoutMs` (which is not a cancellation); an `ok` task whose `files` entry does not exist afterwards is downgraded to `failed` ([declared-files.ts](src/tools/fusion/declared-files.ts)), an existing but untouched entry is only a note, since `files` may be inputs, and an `ok` task that declared `files` but made no successful write / edit / patch call and changed none of them on disk is `no_changes` (a task without `files` never is). A failed worker's loop/provider error lands on the task's head line with a remediation hint for the recognised classes (context exceeded, first-token/idle timeout, 402/429). A worker's filesystem **reads** are confined to its working directory plus the fan-out's write scope ([worker-read-scope.ts](src/tools/fusion/worker-read-scope.ts), installed on the registry by `confineWorkerReads` at boot); other sessions are unaffected. `### fusion` in the stable prefix ([src/prompt/fusion-guidance.ts](src/prompt/fusion-guidance.ts)) is what makes a cloud model reach for the tool at all — it is present only while the descriptor is mounted, so a non-fusion install's prefix is byte-identical to a build without the feature. It carries two things the orchestrator cannot get anywhere else: a push to delegate whenever the work splits into independent, self-contained parts (with the honest limit — a part that only makes sense with this conversation in front of it stays the orchestrator's), and one line of **machine facts** ([fusion-machine-facts.ts](src/prompt/fusion-machine-facts.ts)) so the width it picks is informed rather than guessed: the llama-server request slots, the local model behind them, and therefore how many workers run at once before the rest queue. Those facts are read from the already-loaded config by `buildPrompt` — no probe at prompt-build time — and a fact the runtime does not own (an `external` server's `--parallel`) is left unsaid rather than guessed, because a guessed number is one the model will plan against. They are config values, so they move only when the operator writes the config file, which is the same event that already flips the descriptor gate; nothing per-turn goes in that line or the KV cache would drop on every step. **Nothing about the mode is decided at boot.** The tool is registered unconditionally, because its own live `resolveRunMode()` refusal is the correct and only gate it needs; and `bootstrap.ts` resolves the descriptor gate (`fusion: { enabled: … }`) on *every* read of `effectiveToolDescriptors()`, memoised on the gate's own value so the array identity — and therefore the prefix bytes — only changes when the mode does. Both were boot-time `if`s once, and the pair made a mid-session switch inert: the operator got the chip, the tint and the config, no `fusion.delegate`, no `### fusion`, and a fusion mode that silently did nothing until a restart. When the gate does flip, that session's KV cache drops once — the same cost, for the same reason, as installing a skill or live-adding an MCP server (`refreshMcp`): the tool catalog changed, so the prefix must. The GBNF grammar's `tool-name` rule lists `fusion.delegate` unconditionally (a local orchestrator has to be able to emit it — see the comment in `grammars/tool-call.gbnf`); a worker's per-request grammar drops it again through `toolFilter`, and the tool refuses from a worker session in any case. **The orchestrator sizes the fan-out.** Concurrency is `min(args.maxWorkers ?? runMode.workers, tasks.length, slotManager.poolSize())`, the last term only when the worker provider has slot affinity — and the first term is the model's own number, taken as asked. `llm.runMode.fusion.workers` is the **default for a call that named no `maxWorkers`**, never a ceiling on one that did: the model is the party that knows how divisible a particular job is, and `### fusion` tells it what this machine can serve, so the remaining bounds are the two physical ones (tasks, slots). The pool is read **after** `warmWorkerBackend` (the fallback seam's `prepareLink`): a fusion boot is cloud-active, so the local `/props` probe is deferred and the pool is still sized 1 until that warm runs — reading first would cap every fan-out at one worker. When the pool is what held the width down, the result adds a line naming how many workers were wanted, how many actually ran at once, and `localModels.managed.parallel` — it goes into the *tool result*, because the orchestrator is the party that can adapt to it. -The call is `approval_gated` in [tool-resource-class.ts](src/agent/tool-resource-class.ts) — not because it prompts, but because that is the class meaning "must be solo": one call runs several turns internally, for minutes. It refuses from inside a worker session (one level of fan-out), refuses when the resolver no longer says fusion, and otherwise returns `status: "ok"` even when every worker failed — an orchestrator handed a bare error learns nothing about which parts survived, and partial results are the whole value of a fan-out. Progress reaches the chat as the `fusion_worker` `AgentLoopEvent` (`started` / `tool` / `finished` / `failed` / `cancelled`), emitted through `emitAgentLoopEventFor(parentSessionId, …)`: `started` and `tool` fire from inside the worker turn's own event hook, which runs under the **worker's** async context, and an event routed by the ambient frame would be tagged with a throwaway session that has no recorder, no hook and no UI. +The call is `approval_gated` in [tool-resource-class.ts](src/agent/tool-resource-class.ts) — not because it prompts, but because that is the class meaning "must be solo": one call runs several turns internally, for minutes. It refuses from inside a worker session (one level of fan-out), refuses when the resolver no longer says fusion, and otherwise its status summarises its tasks (`details.outcome`: `all_ok` / `partial` / `all_failed`): `ok` while any task delivered anything — an orchestrator handed a bare error learns nothing about which parts survived, and partial results are the whole value of a fan-out — and `error` only when every task failed or was cancelled; the head line of the result counts every status (`7 tasks: 5 ok, 1 no_changes, 1 failed`). Progress reaches the chat as the `fusion_worker` `AgentLoopEvent` (`started` / `tool` / `finished` / `failed` / `cancelled`), emitted through `emitAgentLoopEventFor(parentSessionId, …)`: `started` and `tool` fire from inside the worker turn's own event hook, which runs under the **worker's** async context, and an event routed by the ambient frame would be tagged with a throwaway session that has no recorder, no hook and no UI. **Attribution.** Fusion is the one mode where two models on two bills share a turn, so every one of those lines carries `role` (`worker` / `orchestrator`) and `model` — `runMode.workerModel` / `.orchestratorModel`, falling back to the provider id and *never* to an invented string. [format-fusion-worker-line.ts](src/tui/format-fusion-worker-line.ts) renders them as `» worker 2 · qwen-3.5-4b — os.fs.write` and `» worker 2 · qwen-3.5-4b: done — 4 steps`, colours unchanged. `phase: "tool"` is what makes a worker's work visible at all: the TUI reducer drops events whose session id is not the visible one, so a worker's own `tool_call_parsed` never reaches the parent's feed on its own. It is bounded twice — a consecutive repeat of the same tool is swallowed, and each worker announces at most `WORKER_TOOL_LINES_PER_TASK` (5) tools, so a fan-out of 8 costs at most 40 lines instead of an unbounded log; the complete per-tool tally still comes back on the task's result row. The orchestrator's own contribution is exactly two lines bracketing the fan-out (`role: "orchestrator"`, its `fusion.delegate` call and the merge), and deliberately no more: the parent turn's other steps run through the fallback chain and may not have been served by `orchestratorModel` at all, so labelling them would be attribution the runtime cannot stand behind. diff --git a/src/tools/fusion/contract-checks.test.ts b/src/tools/fusion/contract-checks.test.ts index 5b69c01a..fd2a4a62 100644 --- a/src/tools/fusion/contract-checks.test.ts +++ b/src/tools/fusion/contract-checks.test.ts @@ -26,7 +26,7 @@ function row(over: Partial = {}): WorkerTaskResult { reply: "done", stepCount: 1, durationMs: 1, - tools: { calls: 0, errors: 0, byTool: {} }, + tools: { calls: 0, errors: 0, writes: 0, byTool: {} }, ...over, }; } diff --git a/src/tools/fusion/declared-files.test.ts b/src/tools/fusion/declared-files.test.ts index 405e5a4e..50e18513 100644 --- a/src/tools/fusion/declared-files.test.ts +++ b/src/tools/fusion/declared-files.test.ts @@ -11,6 +11,7 @@ import { join } from "node:path"; import { applyDeclaredFileReport, + applyNoChangesRule, inspectDeclaredFiles, MTIME_SLACK_MS, } from "./declared-files.js"; @@ -53,6 +54,7 @@ describe("inspectDeclaredFiles", () => { expect(report).toEqual({ missing: ["js/scene.js", absoluteMissing, "spec.md/child.txt"], unchanged: ["spec.md"], + modified: ["js/main.js"], }); }); @@ -64,6 +66,7 @@ describe("inspectDeclaredFiles", () => { expect(await inspectDeclaredFiles(["out.js"], dir, startedAt)).toEqual({ missing: [], unchanged: [], + modified: ["out.js"], }); }); }); @@ -76,17 +79,54 @@ function row(over: Partial = {}): WorkerTaskResult { reply: "Implemented `js/scene.js`", stepCount: 5, durationMs: 1000, - tools: { calls: 0, errors: 0, byTool: {} }, + tools: { calls: 0, errors: 0, writes: 0, byTool: {} }, ...over, }; } +describe("applyNoChangesRule", () => { + const untouched = { missing: [], unchanged: ["js/main.js"], modified: [] }; + + it("turns an ok task with no write call and no changed file into no_changes, and says why", () => { + const result = applyNoChangesRule( + row({ reply: "I'm done!", notes: ["js/main.js unchanged by this task"] }), + untouched, + ); + expect(result.status).toBe("no_changes"); + expect(result).not.toHaveProperty("error"); + expect(result.notes).toEqual([ + "js/main.js unchanged by this task", + "no write, edit or patch call succeeded and no declared file changed", + ]); + }); + + it("leaves the task ok when either a write call succeeded or the disk shows a change", () => { + // A successful write whose target is not among the declared files + // (or the declared files are globs): the call is the evidence. + const wrote = row({ tools: { calls: 1, errors: 0, writes: 1, byTool: { "os.fs.write": 1 } } }); + expect(applyNoChangesRule(wrote, untouched)).toBe(wrote); + // A shell command wrote the file: the disk is the evidence. + const shelled = row(); + expect( + applyNoChangesRule(shelled, { missing: [], unchanged: [], modified: ["js/main.js"] }), + ).toBe(shelled); + }); + + it("never touches a status that already says why the work is incomplete", () => { + for (const status of ["failed", "cancelled", "max_steps", "needs_orchestrator"] as const) { + const original = row({ status }); + expect(applyNoChangesRule(original, untouched)).toBe(original); + } + }); +}); + describe("applyDeclaredFileReport", () => { it("fails an ok row whose declared file is absent — the reply is contradicted", () => { expect( applyDeclaredFileReport(row(), { missing: ["js/scene.js"], unchanged: [], + modified: [], }), ).toMatchObject({ status: "failed", @@ -96,6 +136,7 @@ describe("applyDeclaredFileReport", () => { applyDeclaredFileReport(row(), { missing: ["a.js", "b.js"], unchanged: [], + modified: [], }).error, ).toBe( "declared file a.js does not exist after the task; declared file b.js does not exist after the task", @@ -106,7 +147,7 @@ describe("applyDeclaredFileReport", () => { for (const status of ["max_steps", "needs_orchestrator"] as const) { const result = applyDeclaredFileReport( row({ status, notes: ["earlier note"] }), - { missing: ["js/scene.js"], unchanged: [] }, + { missing: ["js/scene.js"], unchanged: [], modified: [] }, ); expect(result.status).toBe(status); expect(result).not.toHaveProperty("error"); @@ -121,6 +162,7 @@ describe("applyDeclaredFileReport", () => { const result = applyDeclaredFileReport(row(), { missing: [], unchanged: ["spec.md", "README.md"], + modified: ["js/scene.js"], }); expect(result.status).toBe("ok"); expect(result.notes).toEqual(["spec.md, README.md unchanged by this task"]); @@ -129,7 +171,7 @@ describe("applyDeclaredFileReport", () => { it("returns the row untouched when there is nothing to report", () => { const original = row(); expect( - applyDeclaredFileReport(original, { missing: [], unchanged: [] }), + applyDeclaredFileReport(original, { missing: [], unchanged: [], modified: [] }), ).toBe(original); }); }); diff --git a/src/tools/fusion/declared-files.ts b/src/tools/fusion/declared-files.ts index f71b8072..b8cee988 100644 --- a/src/tools/fusion/declared-files.ts +++ b/src/tools/fusion/declared-files.ts @@ -23,6 +23,8 @@ export interface DeclaredFileReport { missing: string[]; /** Declared paths that exist but were not modified during the task. */ unchanged: string[]; + /** Declared paths that exist and were written during the task. */ + modified: string[]; } /** Globs are patterns, not paths: whether they "exist" is meaningless. */ @@ -51,7 +53,7 @@ export async function inspectDeclaredFiles( workingDir: string, startedAt: number, ): Promise { - const report: DeclaredFileReport = { missing: [], unchanged: [] }; + const report: DeclaredFileReport = { missing: [], unchanged: [], modified: [] }; for (const file of files) { if (GLOB_CHARS.test(file)) continue; let absolute: string; @@ -64,6 +66,8 @@ export async function inspectDeclaredFiles( const info = await stat(absolute); if (info.mtimeMs < startedAt - MTIME_SLACK_MS) { report.unchanged.push(file); + } else { + report.modified.push(file); } } catch (error) { if (isNotFound(error)) report.missing.push(file); @@ -113,3 +117,33 @@ export function applyDeclaredFileReport( ...(notes.length === 0 ? {} : { notes }), }; } + +/** + * An `ok` task that declared `files`, made no successful write, edit + * or patch call, and changed none of those files on disk is + * `no_changes` — the truthful name for "I'm done!" over an untouched + * tree, which used to pass for `ok`. + * + * Both halves are needed. The call count alone would misread a worker + * that wrote through the shell (the disk shows the change); the disk + * alone would misread a task whose declared paths are all globs (there + * is nothing to stat). A task with no declared `files` — research, a + * review — is never `no_changes`; the caller only asks for tasks that + * declared some. Statuses other than `ok` already say why the work is + * incomplete and are left alone. + */ +export function applyNoChangesRule( + result: WorkerTaskResult, + report: DeclaredFileReport, +): WorkerTaskResult { + if (result.status !== "ok") return result; + if (result.tools.writes > 0 || report.modified.length > 0) return result; + return { + ...result, + status: "no_changes", + notes: [ + ...(result.notes ?? []), + "no write, edit or patch call succeeded and no declared file changed", + ], + }; +} diff --git a/src/tools/fusion/fusion-delegate.test.ts b/src/tools/fusion/fusion-delegate.test.ts index 77860a96..89d3e886 100644 --- a/src/tools/fusion/fusion-delegate.test.ts +++ b/src/tools/fusion/fusion-delegate.test.ts @@ -412,13 +412,37 @@ describe("fusion.delegate", () => { ); const result = await tool.run({ tasks: TASKS }, ctx()); expect(result.status).toBe("ok"); + expect(result.details.outcome).toBe("partial"); const rows = result.details.tasks as WorkerTaskResult[]; expect(rows.map((r) => r.status)).toEqual(["failed", "ok"]); + expect(result.summary.split("\n")[0]).toBe("2 tasks: 1 ok, 1 failed"); expect(result.summary).toContain("[t1] failed"); expect(result.summary).toContain("[t2] ok"); }); - it("survives an aborted orchestrator turn without throwing", async () => { + it("reports all_ok when every task delivered", async () => { + const result = await buildFusionDelegateTool(deps()).run({ tasks: TASKS }, ctx()); + expect(result.status).toBe("ok"); + expect(result.details.outcome).toBe("all_ok"); + }); + + it("is status:error only when every task failed — the per-task rows still come back", async () => { + // A fan-out where every worker died used to return `ok`; an + // orchestrator reading the status merged nothing as something. + const tool = buildFusionDelegateTool( + deps({ runTurn: async () => Promise.reject(new Error("worker died")) }), + ); + const result = await tool.run({ tasks: TASKS }, ctx()); + expect(result.status).toBe("error"); + expect(result.details.outcome).toBe("all_failed"); + const rows = result.details.tasks as WorkerTaskResult[]; + expect(rows.map((r) => r.status)).toEqual(["failed", "failed"]); + expect(result.summary).toContain("2 tasks: 2 failed"); + expect(result.summary).toContain("[t1] failed"); + expect(result.summary).toContain("[t2] failed"); + }); + + it("survives an aborted orchestrator turn without throwing, and reports it as every task cancelled", async () => { const controller = new AbortController(); controller.abort(); const tool = buildFusionDelegateTool( @@ -434,7 +458,10 @@ describe("fusion.delegate", () => { { tasks: TASKS }, ctx({ signal: controller.signal }), ); - expect(result.status).toBe("ok"); + // Nothing was delivered, so the call itself is an error — but a + // readable one, with the rows. + expect(result.status).toBe("error"); + expect(result.details.outcome).toBe("all_failed"); expect( (result.details.tasks as WorkerTaskResult[]).every( (r) => r.status === "cancelled", diff --git a/src/tools/fusion/fusion-delegate.ts b/src/tools/fusion/fusion-delegate.ts index 4ca16eea..da72cf76 100644 --- a/src/tools/fusion/fusion-delegate.ts +++ b/src/tools/fusion/fusion-delegate.ts @@ -20,6 +20,7 @@ import { } from "./contract-checks.js"; import { runWorkerTasks, type WorkerRunnerDeps } from "./worker-runner.js"; import { + delegateOutcome, formatDelegateOutput, type WorkerTaskResult, } from "./worker-result.js"; @@ -93,11 +94,12 @@ function error( * just walked away from. * 3. **Only on valid args.** See `parseDelegateArgs`. * - * Once the call runs it returns `status: "ok"` even when every worker - * failed. Per-task status lives in the output and in - * `details.tasks` — an orchestrator that gets a bare error learns - * nothing about which parts survived, and partial results are the whole - * value of a fan-out. + * Once the call runs, its status summarises its tasks + * (`details.outcome`): `ok` while any task delivered anything — partial + * results are the whole value of a fan-out, and an orchestrator handed + * a bare error learns nothing about which parts survived — and `error` + * only when every task failed or was cancelled. Per-task status lives + * in the output and in `details.tasks` either way. * * **Width is the model's call.** `args.maxWorkers` is honoured as asked; * `llm.runMode.fusion.workers` only fills in for a call that named @@ -353,15 +355,20 @@ export function buildFusionDelegateTool( const hint = poolIsBinding ? `\n\nNote: ${Math.max(wanted, parsed.tasks.length)} workers' worth of work was sent but the local server has ${poolSize} request slot${poolSize === 1 ? "" : "s"}, so only ${maxWorkers} ran at a time and the rest queued. That number comes from the machine — every slot draws on one shared llama-server context pool (\`localModels.managed.parallel\`, \`"auto"\` by default). Split into fewer, larger tasks if the queueing is costing more than the parallelism buys.` : ""; + // The call's own status is the tasks' summary: a fan-out where + // every worker failed used to come back `ok`, and an orchestrator + // reading only the status merged nothing as if it were something. + const outcome = delegateOutcome(results); return compressToolResult( { tool: FUSION_DELEGATE_TOOL, - status: "ok", + status: outcome === "all_failed" ? "error" : "ok", output: `${formatDelegateOutput(results, deps.outputCharCap, { ...(contractLine === undefined ? {} : { contractLine }), })}${hint}`, details: { tasks: results, + outcome, maxWorkers, requestedWorkers: requested, ...(Number.isFinite(poolSize) ? { slotPoolSize: poolSize } : {}), diff --git a/src/tools/fusion/index.ts b/src/tools/fusion/index.ts index a396fffc..0fb843aa 100644 --- a/src/tools/fusion/index.ts +++ b/src/tools/fusion/index.ts @@ -54,14 +54,18 @@ export { export { WorkerRunCollector, classifyWorkerStatus, + delegateOutcome, formatDelegateOutput, resultCarriesApprovalRefusal, workerFailureHint, + FILE_WRITING_TOOLS, WORKER_HINT_CONTEXT, WORKER_HINT_SATURATED, WORKER_HINT_QUOTA, + WORKER_STATUS_ORDER, } from "./worker-result.js"; export type { + DelegateOutcome, TaskCheckSummary, WorkerStopCause, WorkerTaskResult, @@ -70,6 +74,7 @@ export type { } from "./worker-result.js"; export { applyDeclaredFileReport, + applyNoChangesRule, inspectDeclaredFiles, } from "./declared-files.js"; export type { DeclaredFileReport } from "./declared-files.js"; diff --git a/src/tools/fusion/worker-result.test.ts b/src/tools/fusion/worker-result.test.ts index 2a3921ad..8661d1e3 100644 --- a/src/tools/fusion/worker-result.test.ts +++ b/src/tools/fusion/worker-result.test.ts @@ -7,6 +7,7 @@ import { WORKER_HINT_SATURATED, WorkerRunCollector, classifyWorkerStatus, + delegateOutcome, formatDelegateOutput, resultCarriesApprovalRefusal, workerFailureHint, @@ -92,12 +93,26 @@ describe("WorkerRunCollector", () => { tools: { calls: 3, errors: 1, + writes: 0, byTool: { "os.fs.read": 2, "os.fs.grep": 1 }, }, usage: { promptTokens: 15, completionTokens: 5, totalTokens: 20 }, }); }); + it("counts only the write, edit and patch calls that succeeded", () => { + const c = new WorkerRunCollector(); + c.observe(toolExecuted("os.fs.write", "ok")); + c.observe(toolExecuted("os.fs.edit", "ok")); + c.observe(toolExecuted("os.fs.patch", "error", "no such file")); + // A refused write is not a write. + c.observe(toolExecuted("os.fs.write", "error", `denied: ${FUSION_WORKER_APPROVAL_REFUSED}`)); + // A shell command may write, but the collector cannot know; the disk check does. + c.observe(toolExecuted("os.shell.run", "ok")); + const result = c.finish({ id: "t", title: "T", reason: "reply", stepCount: 5, durationMs: 1 }); + expect(result.tools).toMatchObject({ calls: 5, errors: 2, writes: 2 }); + }); + it("keeps the last reply when an auto-continued turn emits several", () => { const c = new WorkerRunCollector(); c.observe(reply("first leg")); @@ -138,7 +153,7 @@ describe("WorkerRunCollector", () => { stepCount: 0, durationMs: 0, }); - expect(result.tools).toEqual({ calls: 0, errors: 0, byTool: {} }); + expect(result.tools).toEqual({ calls: 0, errors: 0, writes: 0, byTool: {} }); }); it("reports needs_orchestrator when a tool result carried the refusal", () => { @@ -206,11 +221,25 @@ function row(over: Partial = {}): WorkerTaskResult { reply: "the map", stepCount: 2, durationMs: 3000, - tools: { calls: 1, errors: 0, byTool: { "os.fs.read": 1 } }, + tools: { calls: 1, errors: 0, writes: 0, byTool: { "os.fs.read": 1 } }, ...over, }; } +describe("delegateOutcome", () => { + it("is all_ok only when every task is ok, all_failed only when every task failed or was cancelled", () => { + expect(delegateOutcome([row(), row({ id: "t2" })])).toBe("all_ok"); + expect(delegateOutcome([row({ status: "failed" }), row({ id: "t2", status: "cancelled" })])).toBe("all_failed"); + expect(delegateOutcome([row({ status: "failed" }), row({ id: "t2" })])).toBe("partial"); + // A task that produced nothing usable is still not "failed": the + // orchestrator gets its reply and re-delegates from it. + for (const status of ["no_changes", "max_steps", "needs_orchestrator"] as const) { + expect(delegateOutcome([row({ status })])).toBe("partial"); + expect(delegateOutcome([row({ status }), row({ id: "t2", status: "failed" })])).toBe("partial"); + } + }); +}); + describe("formatDelegateOutput", () => { it("renders one headed block per task", () => { const out = formatDelegateOutput( @@ -369,6 +398,26 @@ describe("formatDelegateOutput — the contract", () => { }); }); +describe("formatDelegateOutput — the head line", () => { + it("counts every status, in a fixed order, whatever order the tasks finished in", () => { + const rows = [ + row({ id: "t1", status: "failed", error: "boom" }), + row({ id: "t2", status: "no_changes", notes: ["js/main.js unchanged by this task"] }), + row({ id: "t3" }), + row({ id: "t4", status: "cancelled" }), + row({ id: "t5" }), + row({ id: "t6", status: "max_steps" }), + row({ id: "t7", status: "needs_orchestrator" }), + ]; + const out = formatDelegateOutput(rows, 16000); + expect(out.split("\n")[0]).toBe( + "7 tasks: 2 ok, 1 no_changes, 1 needs_orchestrator, 1 max_steps, 1 failed, 1 cancelled", + ); + expect(out).toContain("- [t2] no_changes — Map — js/main.js unchanged by this task"); + expect(out).toContain("[t2] no_changes — Map (2 steps, 3s, 1 tool calls, 0 errors)"); + }); +}); + describe("classifyWorkerStatus — a ceiling that ended the task", () => { it("reports max_steps for a reply written on the forced final step", () => { // A worker at 40/40 replied "the step limit was reached before the diff --git a/src/tools/fusion/worker-result.ts b/src/tools/fusion/worker-result.ts index 22dc4d72..630211b0 100644 --- a/src/tools/fusion/worker-result.ts +++ b/src/tools/fusion/worker-result.ts @@ -17,7 +17,26 @@ import { FUSION_WORKER_APPROVAL_MARKER } from "./worker-tool-policy.js"; * itself without re-reading a transcript it no longer has. */ export type WorkerTaskStatus = - "ok" | "failed" | "cancelled" | "needs_orchestrator" | "max_steps"; + | "ok" + | "no_changes" + | "failed" + | "cancelled" + | "needs_orchestrator" + | "max_steps"; + +/** + * The order the head line counts statuses in: what was delivered first, + * then what was not and why. Fixed so two fan-outs with the same + * outcome read the same, whichever task finished first. + */ +export const WORKER_STATUS_ORDER: readonly WorkerTaskStatus[] = [ + "ok", + "no_changes", + "needs_orchestrator", + "max_steps", + "failed", + "cancelled", +]; /** Which ceiling ended a worker's loop — see `RunTurnResult.stopCause`. */ export type WorkerStopCause = NonNullable; @@ -25,9 +44,40 @@ export type WorkerStopCause = NonNullable; export interface WorkerToolStats { calls: number; errors: number; + /** + * Successful write / edit / patch calls. A task that declared `files` + * and made none is `no_changes` (`declared-files.ts`), whatever its + * reply says. + */ + writes: number; byTool: Record; } +/** The calls that change a file. Shell commands can too, but the disk check catches those. */ +export const FILE_WRITING_TOOLS: ReadonlySet = new Set([ + "os.fs.write", + "os.fs.edit", + "os.fs.patch", +]); + +/** + * How the call as a whole went, for `details.outcome` and the tool + * result's own status: `all_ok` when every task is `ok`, `all_failed` + * when every task is `failed` or `cancelled` — the one case the result + * is `status: "error"` — and `partial` for everything in between. + */ +export type DelegateOutcome = "all_ok" | "partial" | "all_failed"; + +export function delegateOutcome( + results: readonly WorkerTaskResult[], +): DelegateOutcome { + if (results.every((r) => r.status === "ok")) return "all_ok"; + if (results.every((r) => r.status === "failed" || r.status === "cancelled")) { + return "all_failed"; + } + return "partial"; +} + export interface WorkerTaskResult { id: string; title: string; @@ -99,6 +149,7 @@ export class WorkerRunCollector { private approvalRefused = false; private calls = 0; private errors = 0; + private writes = 0; private readonly byTool: Record = {}; private usage: CompletionUsage | undefined; private lastLoopError: string | undefined; @@ -138,6 +189,7 @@ export class WorkerRunCollector { this.calls += 1; this.byTool[result.tool] = (this.byTool[result.tool] ?? 0) + 1; if (result.status === "error") this.errors += 1; + else if (FILE_WRITING_TOOLS.has(result.tool)) this.writes += 1; if (resultCarriesApprovalRefusal(result.summary, result.details)) { this.approvalRefused = true; } @@ -211,6 +263,7 @@ export class WorkerRunCollector { tools: { calls: this.calls, errors: this.errors, + writes: this.writes, byTool: { ...this.byTool }, }, ...(this.usage ? { usage: this.usage } : {}), @@ -345,12 +398,12 @@ function renderStatusTable( results: readonly WorkerTaskResult[], contractLine: string | undefined, ): string { - const counts = new Map(); + const counts = new Map(); for (const r of results) { counts.set(r.status, (counts.get(r.status) ?? 0) + 1); } - const tally = [...counts] - .map(([status, n]) => `${n} ${status}`) + const tally = WORKER_STATUS_ORDER.filter((status) => counts.has(status)) + .map((status) => `${counts.get(status)} ${status}`) .join(", "); const lines = results.map((r) => [ diff --git a/src/tools/fusion/worker-runner.test.ts b/src/tools/fusion/worker-runner.test.ts index c7716309..69f49e63 100644 --- a/src/tools/fusion/worker-runner.test.ts +++ b/src/tools/fusion/worker-runner.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from "vitest"; -import { mkdtempSync, rmSync, utimesSync, writeFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, rmSync, utimesSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -530,6 +530,108 @@ describe("runWorkerTasks", () => { } }); + it("reports no_changes for an ok task that declared files, wrote nothing and changed nothing", async () => { + // "js/main.js unchanged by this task / I'm done!" came back `ok`. + const dir = mkdtempSync(join(tmpdir(), "fusion-runner-files-")); + try { + mkdirSync(join(dir, "js")); + writeFileSync(join(dir, "js", "main.js"), "old"); + const past = new Date(Date.now() - 60_000); + utimesSync(join(dir, "js", "main.js"), past, past); + const { deps, events } = harness(async ({ options }) => { + options.eventHook?.({ + type: "llm_event", + event: { + type: "tool_call_executed", + result: { tool: "os.fs.read", status: "ok", summary: "old", details: {}, truncated: false }, + batchIndex: 0, + batchSize: 1, + }, + }); + options.eventHook?.({ + type: "llm_event", + event: { type: "assistant_reply", text: "I'm done!" }, + }); + return turnResult({ stepCount: 2 }); + }); + deps.workingDir = dir; + const results = await runWorkerTasks(deps, { + ...BASE, + tasks: [ + { id: "t0", title: "Main", instructions: "Fix main", files: ["js/main.js"] }, + ], + maxWorkers: 1, + signal: new AbortController().signal, + }); + expect(results[0]).toMatchObject({ + status: "no_changes", + reply: "I'm done!", + tools: { writes: 0 }, + notes: [ + "js/main.js unchanged by this task", + "no write, edit or patch call succeeded and no declared file changed", + ], + }); + expect(results[0]).not.toHaveProperty("error"); + expect(events.at(-1)!.event).toMatchObject({ + type: "fusion_worker", + phase: "finished", + summary: "no changes — I'm done!", + }); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("never reports no_changes for a task without declared files, or one whose write succeeded", async () => { + const dir = mkdtempSync(join(tmpdir(), "fusion-runner-files-")); + try { + writeFileSync(join(dir, "notes.md"), "old"); + const past = new Date(Date.now() - 60_000); + utimesSync(join(dir, "notes.md"), past, past); + const research = harness(async ({ options }) => { + options.eventHook?.({ + type: "llm_event", + event: { type: "assistant_reply", text: "the answer" }, + }); + return turnResult(); + }); + research.deps.workingDir = dir; + const [plain] = await runWorkerTasks(research.deps, { + ...BASE, + tasks: [{ id: "t0", title: "Research", instructions: "Read and report" }], + maxWorkers: 1, + signal: new AbortController().signal, + }); + expect(plain!.status).toBe("ok"); + + // A successful write call is the evidence, even when the declared + // path is a glob the disk check cannot stat. + const wrote = harness(async ({ options }) => { + options.eventHook?.({ + type: "llm_event", + event: { + type: "tool_call_executed", + result: { tool: "os.fs.write", status: "ok", summary: "wrote", details: {}, truncated: false }, + batchIndex: 0, + batchSize: 1, + }, + }); + return turnResult(); + }); + wrote.deps.workingDir = dir; + const [written] = await runWorkerTasks(wrote.deps, { + ...BASE, + tasks: [{ id: "t0", title: "Write", instructions: "x", files: ["js/**/*.js"] }], + maxWorkers: 1, + signal: new AbortController().signal, + }); + expect(written).toMatchObject({ status: "ok", tools: { writes: 1 } }); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + it("leaves an ok task ok when every declared file was written", async () => { const dir = mkdtempSync(join(tmpdir(), "fusion-runner-files-")); try { diff --git a/src/tools/fusion/worker-runner.ts b/src/tools/fusion/worker-runner.ts index 78bc5010..5d1da7fb 100644 --- a/src/tools/fusion/worker-runner.ts +++ b/src/tools/fusion/worker-runner.ts @@ -7,6 +7,7 @@ import type { DelegateTask } from "./delegate-args.js"; import type { DelegateContract } from "./contract.js"; import { applyDeclaredFileReport, + applyNoChangesRule, inspectDeclaredFiles, } from "./declared-files.js"; import { @@ -308,9 +309,10 @@ async function runOneTask( } // Ground truth before the orchestrator reads the reply: a worker that - // says it wrote a file it never wrote must not come back `ok`. Only - // for statuses that still claim some work; a failed or cancelled task - // is expected to have left its files missing. + // says it wrote a file it never wrote must not come back `ok`, and one + // that wrote nothing at all is `no_changes`. Only for statuses that + // still claim some work; a failed or cancelled task is expected to + // have left its files missing. if ( task.files !== undefined && task.files.length > 0 && @@ -318,10 +320,12 @@ async function runOneTask( result.status === "max_steps" || result.status === "needs_orchestrator") ) { - result = applyDeclaredFileReport( - result, - await inspectDeclaredFiles(task.files, deps.workingDir, startedAt), + const report = await inspectDeclaredFiles( + task.files, + deps.workingDir, + startedAt, ); + result = applyNoChangesRule(applyDeclaredFileReport(result, report), report); } // Keep the feed paired: a turn that died before it ever stepped never @@ -356,7 +360,9 @@ function summarise(result: WorkerTaskResult): string { const text = (result.error ?? result.reply).replace(/\s+/g, " ").trim(); if (text.length === 0) return result.status; const cap = Math.min(120, WORKER_REPLY_CHAR_BUDGET); - return text.length > cap ? `${text.slice(0, cap)}…` : text; + const clipped = text.length > cap ? `${text.slice(0, cap)}…` : text; + // "I'm done!" over an untouched tree must not read as done in the feed. + return result.status === "no_changes" ? `no changes — ${clipped}` : clipped; } function isAbortError(error: unknown): boolean {