From ff896f0316c586d10f89f5a12ce10e1dca60ded7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Jul 2026 16:36:28 +0000 Subject: [PATCH 1/5] Add swarm dispatch scope and dispatch-mode benchmark harness Adds a new 'swarm' triage scope alongside single/team: a lead + workers share one checkout and a live relay channel (agent-relay MCP messaging, already fully plumbed via AgentSpec.channel -> SpawnInput.channel) instead of team's isolated parallel fan-out. Opt-in only via the agent:swarm label, never inferred heuristically. renderAgentTask now briefs the lead/workers with their coordination channel and each other's names. Adds benchmark/ machinery to actually measure whether team/swarm beat a single agent, and whether swarm's live collaboration beats team's isolated fan-out: a task corpus schema + loader, a resumable (task x mode x repeat) matrix builder, a real-dispatch runner (gh + factory dispatch + verify.sh scoring) behind a testable DispatchRunner interface, and a markdown report generator grouped by coordination-benefit difficulty tier. Pure logic is unit-tested; the real-IO adapter is proven by running it, not by mocking child_process. Includes a SWE-bench Verified adapter script (tested live against 2 real instances) for an externally-comparable subset, with loud caveats about repointing to a controlled fork before dispatching. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01H1jnbLuMdTR2hTWv9EUnkq Session-Id: 8c8fcd37-6ade-4756-9a98-e5ced3cf5871 --- benchmark/README.md | 91 ++++++++++++ benchmark/cli-dispatch-runner.ts | 101 ++++++++++++++ benchmark/dispatch-runner.ts | 22 +++ benchmark/matrix.test.ts | 66 +++++++++ benchmark/matrix.ts | Bin 0 -> 1596 bytes benchmark/orchestrate.test.ts | 95 +++++++++++++ benchmark/orchestrate.ts | 55 ++++++++ benchmark/report.test.ts | 77 ++++++++++ benchmark/report.ts | Bin 0 -> 3752 bytes benchmark/results-store.test.ts | 43 ++++++ benchmark/results-store.ts | 30 ++++ benchmark/run.ts | 87 ++++++++++++ benchmark/schema.ts | 83 +++++++++++ benchmark/swe-bench-adapter.mjs | 132 ++++++++++++++++++ benchmark/tasks/.gitkeep | 0 .../templates/multi-service-feature/task.json | 13 ++ .../templates/multi-service-feature/verify.sh | 5 + benchmark/templates/single-file-fix/task.json | 13 ++ benchmark/templates/single-file-fix/verify.sh | 8 ++ benchmark/tsconfig.json | 9 ++ package.json | 1 + src/dispatch/templates.test.ts | 50 +++++++ src/dispatch/templates.ts | 28 ++++ src/orchestrator/factory.test.ts | 45 ++++++ src/orchestrator/factory.ts | 53 +++++-- src/ports/fleet.ts | 2 + src/triage/heuristic.ts | 48 ++++++- src/triage/index.ts | 2 +- src/triage/schema.ts | 3 +- src/triage/triage.test.ts | 48 +++++++ src/types.ts | 2 +- vitest.config.ts | 1 + 32 files changed, 1199 insertions(+), 14 deletions(-) create mode 100644 benchmark/README.md create mode 100644 benchmark/cli-dispatch-runner.ts create mode 100644 benchmark/dispatch-runner.ts create mode 100644 benchmark/matrix.test.ts create mode 100644 benchmark/matrix.ts create mode 100644 benchmark/orchestrate.test.ts create mode 100644 benchmark/orchestrate.ts create mode 100644 benchmark/report.test.ts create mode 100644 benchmark/report.ts create mode 100644 benchmark/results-store.test.ts create mode 100644 benchmark/results-store.ts create mode 100644 benchmark/run.ts create mode 100644 benchmark/schema.ts create mode 100644 benchmark/swe-bench-adapter.mjs create mode 100644 benchmark/tasks/.gitkeep create mode 100644 benchmark/templates/multi-service-feature/task.json create mode 100755 benchmark/templates/multi-service-feature/verify.sh create mode 100644 benchmark/templates/single-file-fix/task.json create mode 100755 benchmark/templates/single-file-fix/verify.sh create mode 100644 benchmark/tsconfig.json diff --git a/benchmark/README.md b/benchmark/README.md new file mode 100644 index 00000000..13144ea0 --- /dev/null +++ b/benchmark/README.md @@ -0,0 +1,91 @@ +# Factory dispatch-mode benchmark + +Machinery to answer, with real numbers instead of vibes: does Factory's +multi-agent dispatch (`team`) beat a single agent (`single`) on real coding +tasks, and does live collaboration (the new `swarm` scope — a lead + workers +sharing one checkout and a relay channel, see `../src/triage/heuristic.ts`) +beat parallel task-splitting, or is it just coordination overhead? + +## How it works + +``` +benchmark/tasks//task.json + verify.sh — the task corpus (ground truth: real tests, not a self-report) + │ + ▼ +benchmark/matrix.ts — builds the (task x mode x repeat) matrix, skipping cells already recorded (resumable) + │ + ▼ +benchmark/run.ts — for each cell: open a labeled GitHub issue, `factory dispatch` at the forced mode, + │ wait for a PR, check it out, run verify.sh — via benchmark/cli-dispatch-runner.ts + ▼ +benchmark/results.jsonl (append-only, one line per cell) + │ + ▼ +benchmark/report.ts — aggregates into success rate / wall-clock / cost per task x mode + │ + ▼ +benchmark/report.md +``` + +`matrix.ts`, `report.ts`, `orchestrate.ts`, `results-store.ts`, and +`schema.ts` are pure logic with full unit coverage +(`npx vitest run benchmark/`). `cli-dispatch-runner.ts` is the real-IO +adapter (shells out to `gh` and `factory dispatch`) — same category as +`../scripts/verify-tailscale-preview-e2e.mjs`: proven by running it for +real against live infrastructure, not by mocking `child_process`. + +## Before running this for real + +1. **Point it at a disposable sandbox repo, never a product repo.** Every + task's `targetRepo` must be a repo Factory can freely open/close PRs and + issues against with no consequence — e.g. a dedicated + `AgentWorkforce/factory-benchmark-fixtures` repo (not created by this + change; create and seed it before authoring real tasks). +2. **Author real tasks.** `benchmark/tasks/` ships empty (only `.gitkeep`). + `benchmark/templates/` has two fully-shaped examples — a `single-file` + control-group task and a `multi-service` task where coordination should + matter — to copy the shape from. Aim for ~12-15 tasks spanning + `single-file` → `multi-file` → `multi-service`; single-file is the + control (little coordination benefit expected), multi-service is where + team/swarm should differentiate from single, if they differentiate at + all. +3. **Optionally add a public-benchmark subset** with + `node benchmark/swe-bench-adapter.mjs --count 20 --out benchmark/tasks`. + **Read the script's header comment first** — every generated task points + `targetRepo` at the real upstream OSS repo (e.g. `django/django`); you + must repoint it to a fork you control before dispatching, and the + generated `verify.sh` is a best-effort pytest reconstruction, not a + guarantee of official-SWE-bench-harness parity. Say so plainly in any + report built from these tasks — don't imply leaderboard-comparable + numbers if any instance needed reshaping. +4. **`factory.config.json`** for the workspace must map every task's + `targetRepo` to a real `clonePath`, same as any other Factory config. + +## Running it + +```bash +npx tsx benchmark/run.ts --config ./factory.config.json +# narrow while iterating: +npx tsx benchmark/run.ts --config ./factory.config.json --only-task rename-error-type --only-mode swarm --repeats 1 +``` + +This dispatches real agents and spends real money/time — there is +deliberately no offline/fixture mode for the scored runs, because fixture +output can't be scored for code quality. Smoke-test one task across all +three modes with `--repeats 1` before committing to the full matrix. + +It's safe to kill and re-run: `results.jsonl` is append-only, and `run.ts` +skips any `(task, mode, repeat)` cell that already has a recorded result. + +## Reading the result + +- A `single-file` task where `team`/`swarm` don't at least match `single` + means dispatch overhead is hurting on tasks too small to benefit from it — + expected and fine, it's the control group. +- A `multi-file`/`multi-service` task is where `team` (parallel fan-out) and + `swarm` (live collaboration) get a real chance to beat `single`. If they + don't, that's a legitimate, reportable finding — don't only report the + runs that flatter multi-agent dispatch. +- Compare `team` vs `swarm` directly on the same tasks to answer the + original question: does *live* collaboration beat *isolated parallel* + collaboration, or is the extra coordination channel just overhead? diff --git a/benchmark/cli-dispatch-runner.ts b/benchmark/cli-dispatch-runner.ts new file mode 100644 index 00000000..176e2cd3 --- /dev/null +++ b/benchmark/cli-dispatch-runner.ts @@ -0,0 +1,101 @@ +import { execFile } from 'node:child_process' +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { promisify } from 'node:util' + +import type { DispatchRunner } from './dispatch-runner' + +const exec = promisify(execFile) + +export interface CliDispatchRunnerOptions { + /** Path to the factory.config.json for the workspace that owns task.targetRepo's clonePath. */ + factoryConfigPath: string + /** Bounds how long we poll for a PR to appear after dispatch, in addition to the task's own verify timeout. */ + dispatchTimeoutMs?: number + pollIntervalMs?: number + log?: (message: string) => void +} + +/** + * The real, real-infrastructure implementation of DispatchRunner: creates a + * labeled GitHub issue via `gh`, dispatches Factory for real via the CLI + * (mirroring scripts/factory-canary.sh's "wrap the command" pattern), polls + * for the resulting PR, then checks it out into a scratch clone and runs the + * task's verify command there. + * + * This talks to real GitHub and spends real agent time/cost — never run it + * against a product repo, only a disposable benchmark-fixtures sandbox. It is + * intentionally not unit-tested (same category as + * scripts/verify-tailscale-preview-e2e.mjs): correctness here is proven by + * running it for real, not by mocking child_process. + */ +export function createCliDispatchRunner(options: CliDispatchRunnerOptions): DispatchRunner { + const log = options.log ?? (() => {}) + const dispatchTimeoutMs = options.dispatchTimeoutMs ?? 30 * 60_000 + const pollIntervalMs = options.pollIntervalMs ?? 15_000 + + return { + async dispatch(task, mode) { + const label = `agent:${mode}` + const { stdout: createOut } = await exec('gh', [ + 'issue', 'create', + '--repo', task.targetRepo, + '--title', task.title, + '--body', task.issueBody, + '--label', 'factory', + '--label', label, + ]) + const issueUrl = createOut.trim() + const issueNumber = issueUrl.split('/').pop() + if (!issueNumber) { + throw new Error(`gh issue create returned an unexpected URL: ${issueUrl}`) + } + log(`created ${task.targetRepo}#${issueNumber} (${label})`) + + await exec('node', ['bin/factory.mjs', 'dispatch', issueNumber, '--config', options.factoryConfigPath]) + log(`dispatched ${task.targetRepo}#${issueNumber}`) + + const prBranch = await pollForPrBranch(task.targetRepo, issueNumber, dispatchTimeoutMs, pollIntervalMs, log) + return { runId: `${task.targetRepo}#${issueNumber}`, prBranch } + }, + + async verify(task, outcome) { + const clonePath = await mkdtemp(join(tmpdir(), 'factory-benchmark-verify-')) + try { + await exec('git', ['clone', '--depth', '1', '--branch', outcome.prBranch, `https://github.com/${task.targetRepo}.git`, clonePath]) + try { + await exec('bash', ['-c', task.verify.command], { cwd: clonePath, timeout: task.verify.timeoutMs }) + return { passed: true } + } catch (error) { + return { passed: false, notes: `verify failed: ${error instanceof Error ? error.message : String(error)}` } + } + } finally { + await rm(clonePath, { recursive: true, force: true }) + } + }, + } +} + +async function pollForPrBranch( + repo: string, + issueNumber: string, + timeoutMs: number, + intervalMs: number, + log: (message: string) => void, +): Promise { + const deadline = Date.now() + timeoutMs + while (Date.now() < deadline) { + const { stdout } = await exec('gh', [ + 'issue', 'view', issueNumber, + '--repo', repo, + '--json', 'closedByPullRequestsReferences', + ]) + const parsed = JSON.parse(stdout) as { closedByPullRequestsReferences?: Array<{ number: number; headRefName: string }> } + const pr = parsed.closedByPullRequestsReferences?.[0] + if (pr) return pr.headRefName + log(`waiting for a PR on ${repo}#${issueNumber}...`) + await new Promise((resolve) => setTimeout(resolve, intervalMs)) + } + throw new Error(`timed out after ${timeoutMs}ms waiting for a PR on ${repo}#${issueNumber}`) +} diff --git a/benchmark/dispatch-runner.ts b/benchmark/dispatch-runner.ts new file mode 100644 index 00000000..4d1f921a --- /dev/null +++ b/benchmark/dispatch-runner.ts @@ -0,0 +1,22 @@ +import type { BenchmarkMode, BenchmarkTask } from './schema' + +export interface DispatchOutcome { + runId: string + /** The PR head branch the implementer(s) pushed, once dispatch completes. */ + prBranch: string +} + +/** + * The real-world side of running one matrix cell: ensure a GitHub issue + * exists for the task, dispatch Factory at the forced mode, wait for a PR, + * check it out, and run the task's verify command against it. Kept as an + * interface (not a concrete class) so orchestrate.ts's matrix-walking logic + * is unit-testable against a fake, the same separation this repo already + * uses for FleetClient/GithubRead/etc. in src/ports. + */ +export interface DispatchRunner { + /** Creates (or reuses) the GitHub issue and dispatches Factory at the given mode. Returns once a PR exists. */ + dispatch(task: BenchmarkTask, mode: BenchmarkMode): Promise + /** Checks out `prBranch` in `task.targetRepo` and runs `task.verify.command` there. */ + verify(task: BenchmarkTask, outcome: DispatchOutcome): Promise<{ passed: boolean; notes?: string }> +} diff --git a/benchmark/matrix.test.ts b/benchmark/matrix.test.ts new file mode 100644 index 00000000..346adb2c --- /dev/null +++ b/benchmark/matrix.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, it } from 'vitest' + +import { buildMatrix } from './matrix' +import type { BenchmarkTask } from './schema' + +function task(id: string): BenchmarkTask { + return { + id, + title: `Task ${id}`, + issueBody: 'Do the thing.', + targetRepo: 'AgentWorkforce/factory-benchmark-fixtures', + baseRef: 'main', + difficulty: 'single-file', + verify: { command: 'npm test', timeoutMs: 300_000 }, + source: 'authored', + } +} + +describe('buildMatrix', () => { + it('produces every task x mode x repeat combination by default', () => { + const cells = buildMatrix([task('a'), task('b')], ['single', 'team'], [], { repeats: 2 }) + + expect(cells).toHaveLength(8) + expect(cells.filter((cell) => cell.task.id === 'a' && cell.mode === 'single')).toHaveLength(2) + }) + + it('skips cells that already have a recorded result (resumable)', () => { + const cells = buildMatrix( + [task('a')], + ['single', 'team', 'swarm'], + [ + { taskId: 'a', mode: 'single', repeat: 0 }, + { taskId: 'a', mode: 'team', repeat: 0 }, + ], + { repeats: 1 }, + ) + + expect(cells).toEqual([{ task: task('a'), mode: 'swarm', repeat: 0 }]) + }) + + it('only re-runs the repeats not yet recorded, per mode', () => { + const cells = buildMatrix( + [task('a')], + ['single'], + [{ taskId: 'a', mode: 'single', repeat: 0 }], + { repeats: 3 }, + ) + + expect(cells.map((cell) => cell.repeat)).toEqual([1, 2]) + }) + + it('narrows to --only-task and --only-mode without touching other tasks', () => { + const cells = buildMatrix( + [task('a'), task('b')], + ['single', 'team', 'swarm'], + [], + { onlyTaskIds: ['b'], onlyModes: ['swarm'], repeats: 1 }, + ) + + expect(cells).toEqual([{ task: task('b'), mode: 'swarm', repeat: 0 }]) + }) + + it('rejects a non-positive repeats value instead of silently producing an empty matrix', () => { + expect(() => buildMatrix([task('a')], ['single'], [], { repeats: 0 })).toThrow(/repeats must be >= 1/) + }) +}) diff --git a/benchmark/matrix.ts b/benchmark/matrix.ts new file mode 100644 index 0000000000000000000000000000000000000000..bcf3a264e1973a7bdf84252e0dd1a361222806a9 GIT binary patch literal 1596 zcmZ`(?`zvY4BhAa6>m_8(m`+_*QmuxA z0c``EHlA#uGQxxMR&PI3Rbf8>JjcyWxB|o0n%T~*Le5r88&ZA-)7H;q2Z#Tnf7KP! z1^mWTI{{thZZU&nZu9c^#9(qDxMIGjGMq*#IXjU}udfHV#(k@++`*TGqU9>20&GLd z{D^_5LzSqtX&sc=A(Mg?HdZ$lwl#vJDoZNgaZJ|OoN_+5L01%w9m6$Pv1jVt3fjS` zg5Vo=nP~I7Srb$tRVsnXta2q7S|O)OvpmWZR+EX8Igu1sFv{BImFvf=_GQz0fpQx% zx!BYT881*?*EG!<=UJjY#fjq#IYFtyrR+8B8(Xv{^O`-MMV8pyr0c6YQ&xKL_%!Z% z%O_I0B==T)aiOhto|B4G#?Ie$w!S@fsUN)1M(yTk()IkCKi;RXDh?5z#@#eOQRS`A|!#lQ|HY8eX31X zwY*iqE7f_R17;E9AL_7ym({m zFR=E0kOf3iQ9lcu;m1$iT;R4JkfY@TH}(`82Sgf~mdYiPx3owyV?oBDF6oL~X50>G z1`m5HH(dP0E!(uNO!h&y%NQJ_h4MLsr~Jy-l5eevJ1UpsaW&5~SmymL7(q{OF`@p# f { appended.push(result) }, + log: (message: string) => { logged.push(message) }, + now: () => 'fixed-timestamp', + }, + appended, + logged, + } +} + +describe('runMatrix', () => { + it('dispatches, verifies, and appends one result per cell in order', async () => { + const runner: DispatchRunner = { + dispatch: vi.fn(async (t, mode) => ({ runId: `${t.id}-${mode}`, prBranch: `factory/${t.id}` })), + verify: vi.fn(async () => ({ passed: true })), + } + const { opts, appended } = deps(runner) + + const results = await runMatrix([ + { task: task('a'), mode: 'single', repeat: 0 }, + { task: task('a'), mode: 'team', repeat: 0 }, + ], opts) + + expect(results).toEqual([ + { taskId: 'a', mode: 'single', repeat: 0, runId: 'a-single', passed: true, notes: undefined, timestamp: 'fixed-timestamp' }, + { taskId: 'a', mode: 'team', repeat: 0, runId: 'a-team', passed: true, notes: undefined, timestamp: 'fixed-timestamp' }, + ]) + expect(appended).toEqual(results) + expect(runner.dispatch).toHaveBeenCalledTimes(2) + }) + + it('records a failed result and keeps going when one cell throws, instead of aborting the run', async () => { + const runner: DispatchRunner = { + dispatch: vi.fn() + .mockRejectedValueOnce(new Error('fleet spawn failed')) + .mockResolvedValueOnce({ runId: 'a-team', prBranch: 'factory/a' }), + verify: vi.fn(async () => ({ passed: true })), + } + const { opts } = deps(runner) + + const results = await runMatrix([ + { task: task('a'), mode: 'single', repeat: 0 }, + { task: task('a'), mode: 'team', repeat: 0 }, + ], opts) + + expect(results[0]).toMatchObject({ mode: 'single', passed: false, notes: expect.stringContaining('fleet spawn failed') }) + expect(results[1]).toMatchObject({ mode: 'team', passed: true }) + expect(runner.dispatch).toHaveBeenCalledTimes(2) // second cell still ran after the first threw + }) + + it('processes cells strictly sequentially, never overlapping two dispatches', async () => { + const order: string[] = [] + const runner: DispatchRunner = { + dispatch: vi.fn(async (t) => { + order.push(`start:${t.id}`) + await new Promise((resolve) => setTimeout(resolve, 5)) + order.push(`end:${t.id}`) + return { runId: t.id, prBranch: `factory/${t.id}` } + }), + verify: vi.fn(async () => ({ passed: true })), + } + const { opts } = deps(runner) + + await runMatrix([ + { task: task('a'), mode: 'single', repeat: 0 }, + { task: task('b'), mode: 'single', repeat: 0 }, + ], opts) + + expect(order).toEqual(['start:a', 'end:a', 'start:b', 'end:b']) + }) +}) diff --git a/benchmark/orchestrate.ts b/benchmark/orchestrate.ts new file mode 100644 index 00000000..2c23a6e8 --- /dev/null +++ b/benchmark/orchestrate.ts @@ -0,0 +1,55 @@ +import type { DispatchRunner } from './dispatch-runner' +import type { MatrixCell } from './matrix' +import type { BenchmarkResult } from './schema' + +export interface OrchestrateDeps { + runner: DispatchRunner + appendResult: (result: BenchmarkResult) => Promise + log: (message: string) => void + now: () => string +} + +/** + * Walks the matrix sequentially (one cell at a time — real dispatches spend + * real money and share a fleet, so this deliberately does not parallelize) + * recording one result per cell as soon as it finishes, so a crash partway + * through only loses the in-flight cell, not the whole run. + */ +export async function runMatrix(cells: MatrixCell[], deps: OrchestrateDeps): Promise { + const results: BenchmarkResult[] = [] + for (const [index, cell] of cells.entries()) { + deps.log(`[${index + 1}/${cells.length}] ${cell.task.id} x ${cell.mode} (repeat ${cell.repeat})`) + const result = await runCell(cell, deps) + await deps.appendResult(result) + results.push(result) + } + return results +} + +async function runCell(cell: MatrixCell, deps: OrchestrateDeps): Promise { + try { + const outcome = await deps.runner.dispatch(cell.task, cell.mode) + const verdict = await deps.runner.verify(cell.task, outcome) + return { + taskId: cell.task.id, + mode: cell.mode, + repeat: cell.repeat, + runId: outcome.runId, + passed: verdict.passed, + notes: verdict.notes, + timestamp: deps.now(), + } + } catch (error) { + // A dispatch/verify failure is itself a real, reportable data point (the + // mode failed this task) — never let one cell's exception abort the run. + return { + taskId: cell.task.id, + mode: cell.mode, + repeat: cell.repeat, + runId: `error:${cell.task.id}:${cell.mode}:${cell.repeat}`, + passed: false, + notes: `runner error: ${error instanceof Error ? error.message : String(error)}`, + timestamp: deps.now(), + } + } +} diff --git a/benchmark/report.test.ts b/benchmark/report.test.ts new file mode 100644 index 00000000..da0c8475 --- /dev/null +++ b/benchmark/report.test.ts @@ -0,0 +1,77 @@ +import { describe, expect, it } from 'vitest' + +import { buildReportRows, renderMarkdownReport } from './report' +import type { BenchmarkTask } from './schema' + +const task: BenchmarkTask = { + id: 'rename-error-type', + title: 'Rename NetworkError to TransportError', + issueBody: 'Do the thing.', + targetRepo: 'AgentWorkforce/factory-benchmark-fixtures', + baseRef: 'main', + difficulty: 'single-file', + verify: { command: 'npm test', timeoutMs: 300_000 }, + source: 'authored', +} + +describe('buildReportRows', () => { + it('computes success rate and sample size per task x mode', () => { + const rows = buildReportRows([task], [ + { taskId: 'rename-error-type', mode: 'single', repeat: 0, runId: 'r1', passed: true, timestamp: 't' }, + { taskId: 'rename-error-type', mode: 'single', repeat: 1, runId: 'r2', passed: false, timestamp: 't' }, + { taskId: 'rename-error-type', mode: 'team', repeat: 0, runId: 'r3', passed: true, timestamp: 't' }, + ]) + + expect(rows).toEqual([ + expect.objectContaining({ taskId: 'rename-error-type', mode: 'single', sampleSize: 2, successRate: 0.5 }), + expect.objectContaining({ taskId: 'rename-error-type', mode: 'team', sampleSize: 1, successRate: 1 }), + ]) + }) + + it('joins cost/duration samples by runId and averages them', () => { + const rows = buildReportRows([task], [ + { taskId: 'rename-error-type', mode: 'single', repeat: 0, runId: 'r1', passed: true, timestamp: 't' }, + { taskId: 'rename-error-type', mode: 'single', repeat: 1, runId: 'r2', passed: true, timestamp: 't' }, + ], [ + { runId: 'r1', usd: 1.0, durationMs: 60_000 }, + { runId: 'r2', usd: 3.0, durationMs: 180_000 }, + ]) + + expect(rows[0]?.meanCostUsd).toBe(2.0) + expect(rows[0]?.meanWallClockMs).toBe(120_000) + }) + + it('leaves cost fields undefined when no cost sample matches a run, rather than defaulting to 0', () => { + const rows = buildReportRows([task], [ + { taskId: 'rename-error-type', mode: 'single', repeat: 0, runId: 'r1', passed: true, timestamp: 't' }, + ]) + + expect(rows[0]?.meanCostUsd).toBeUndefined() + expect(rows[0]?.meanWallClockMs).toBeUndefined() + }) + + it('drops results for tasks no longer in the corpus instead of throwing', () => { + const rows = buildReportRows([task], [ + { taskId: 'deleted-task', mode: 'single', repeat: 0, runId: 'r1', passed: true, timestamp: 't' }, + ]) + + expect(rows).toEqual([]) + }) +}) + +describe('renderMarkdownReport', () => { + it('groups rows under a heading per difficulty tier', () => { + const markdown = renderMarkdownReport(buildReportRows([task], [ + { taskId: 'rename-error-type', mode: 'single', repeat: 0, runId: 'r1', passed: true, timestamp: 't' }, + ])) + + expect(markdown).toContain('## single-file') + expect(markdown).toContain('rename-error-type') + expect(markdown).toContain('100%') + expect(markdown).not.toContain('## multi-file') + }) + + it('reports no results without crashing on an empty matrix', () => { + expect(renderMarkdownReport([])).toContain('No results recorded yet.') + }) +}) diff --git a/benchmark/report.ts b/benchmark/report.ts new file mode 100644 index 0000000000000000000000000000000000000000..97e0b633673e9ebc2a2bbf953e1e22b29123e5ca GIT binary patch literal 3752 zcma)9+iu%N5Y4l{Vv4vRnMjn(TZG2ta%Z*%gAahpdZmM z?3eV+UPvl-kQy*R4rkBY&yKQUTh*HMeke*0wl=ju zxH`*8Xv(Ef+8UEv3Bg7v5{Yh*r9C?o18L;@WYv@j$3x2|%hR^5Dn!=UN%TS6lBR<5 z0lv6&^uF2toSCnj7~63;-L>CrP7`8{=!{xJ5OP;0RSBor;Q9VCg_z)L( zGLI2WUeSz&ah49ufq4|Q0Breuf5k1{!|3^Ak!*iiEc+arLJeI08M4J(*HyDc30j|C z+S%#Nq7VL6$gZrannD|p%@b8CpRve9>*KwZd%!MEKU=++GZKF1!1!k|PJ)-Q90ZPtb{1N@{EuceDwSIlLX!zU< zYX*z{_?)A2-W*3~%5;e;aTHv8oQl;p&vY0(2_iyU&K{(a8Ni0owFHj(&M@)XG})lC z6@}liAmSd2b5N2ped<^ z+86~pUYsAFp{-ErqQbnvFPH08mV<6XpKfkaS5j7%X%y$OTBJG;-;;TehbEH=js1wb!~{?29qyK@W;fJU=wvhG z=xFKpm=EQL@>jm4cz?mADlu^>=yWf2{Gklca6R%_?%%en17^cr0xvr@I@%uweVTg6 zy|`+bcu_j%Bj`Pa?jCuW>whgo%amcHj_24q`7c;_Gf{@ zC7#P6##sgf3JT;X`{GT>`csz3V9tbTAdoW)m1EPeINvM-qnnMZN8{a4bE3Ro|2J9P z=^hii+@kf5c)<4#+%^<#XvTPNS!nU}DV=`q5DyD>*W&inf_u6F)nj#=a*y3piT`WM zEm+qyi>BZT+F{ojCcF}Q39|>?KL$A*4(}iPce=fNp}d?Z8<|fNQP07d<4=P3o`wwD zI#M{6@t|~)YqOD?9_skCB{SkR!G7$x6dU1-@i>Zg^)9=UX*iA!-!D2uZb#<5-hONf z(Qd}{nu2m90y^W{y%74XTmOxrFOK$H@xXl5V5R;l=;9QcjB~>2w!Z!lp~#PCVHi-;F)f59VGIL+xm7d%80rTj{J}bW(fCgJ%zoQy@woHKe||Ng7brN|$Vu8#yoe6P0zH6FQ=vC87Tz)q O4H()BO{EsNtNsI|YtkP8 literal 0 HcmV?d00001 diff --git a/benchmark/results-store.test.ts b/benchmark/results-store.test.ts new file mode 100644 index 00000000..c975a9a9 --- /dev/null +++ b/benchmark/results-store.test.ts @@ -0,0 +1,43 @@ +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +import { afterEach, beforeEach, describe, expect, it } from 'vitest' + +import { appendResult, loadResults } from './results-store' + +let dir: string + +beforeEach(async () => { + dir = await mkdtemp(join(tmpdir(), 'factory-benchmark-results-')) +}) + +afterEach(async () => { + await rm(dir, { recursive: true, force: true }) +}) + +describe('results-store', () => { + it('returns an empty list when the results file does not exist yet', async () => { + await expect(loadResults(join(dir, 'results.jsonl'))).resolves.toEqual([]) + }) + + it('round-trips appended results as JSONL, one per line', async () => { + const path = join(dir, 'results.jsonl') + await appendResult(path, { taskId: 'a', mode: 'single', repeat: 0, runId: 'r1', passed: true, timestamp: 't1' }) + await appendResult(path, { taskId: 'a', mode: 'team', repeat: 0, runId: 'r2', passed: false, timestamp: 't2', notes: 'flaked' }) + + const results = await loadResults(path) + + expect(results).toEqual([ + { taskId: 'a', mode: 'single', repeat: 0, runId: 'r1', passed: true, timestamp: 't1' }, + { taskId: 'a', mode: 'team', repeat: 0, runId: 'r2', passed: false, timestamp: 't2', notes: 'flaked' }, + ]) + }) + + it('rejects a malformed result instead of silently writing bad data', async () => { + const path = join(dir, 'results.jsonl') + const malformed = { taskId: 'a', mode: 'not-a-mode', repeat: 0, runId: 'r1', passed: true, timestamp: 't1' } as unknown as Parameters[1] + await expect(appendResult(path, malformed)).rejects.toThrow() + await expect(loadResults(path)).resolves.toEqual([]) + }) +}) diff --git a/benchmark/results-store.ts b/benchmark/results-store.ts new file mode 100644 index 00000000..7e84900a --- /dev/null +++ b/benchmark/results-store.ts @@ -0,0 +1,30 @@ +import { appendFile, readFile } from 'node:fs/promises' + +import { BenchmarkResultSchema, type BenchmarkResult } from './schema' + +/** Reads every recorded result, tolerating a not-yet-created results file (a fresh benchmark run). */ +export async function loadResults(path: string): Promise { + let raw: string + try { + raw = await readFile(path, 'utf8') + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return [] + throw error + } + + return raw + .split('\n') + .map((line) => line.trim()) + .filter((line) => line.length > 0) + .map((line) => BenchmarkResultSchema.parse(JSON.parse(line))) +} + +/** + * Appends one result as a JSONL line. Append-only by design: the runner is + * meant to be safe to kill and resume, and a JSONL file survives a crash + * mid-write far better than a single JSON array file would. + */ +export async function appendResult(path: string, result: BenchmarkResult): Promise { + BenchmarkResultSchema.parse(result) // fail fast on a malformed result instead of writing bad data + await appendFile(path, `${JSON.stringify(result)}\n`) +} diff --git a/benchmark/run.ts b/benchmark/run.ts new file mode 100644 index 00000000..1124b375 --- /dev/null +++ b/benchmark/run.ts @@ -0,0 +1,87 @@ +#!/usr/bin/env -S npx tsx +// Runs the benchmark matrix for real against a live Factory workspace. +// +// This dispatches real coding agents, opens real PRs, and spends real +// money/time — see benchmark/README.md before running it. Never point +// factoryConfigPath at a config whose repos map to a product repo. +// +// Usage: +// npx tsx benchmark/run.ts --config ./factory.config.json [--only-task id] [--only-mode single] [--repeats 3] + +import { join } from 'node:path' + +import { createCliDispatchRunner } from './cli-dispatch-runner' +import { BENCHMARK_MODES, loadTasks, type BenchmarkMode } from './schema' +import { buildMatrix } from './matrix' +import { runMatrix } from './orchestrate' +import { renderMarkdownReport, buildReportRows } from './report' +import { appendResult, loadResults } from './results-store' + +const TASKS_DIR = join(import.meta.dirname, 'tasks') +const RESULTS_PATH = join(import.meta.dirname, 'results.jsonl') +const REPORT_PATH = join(import.meta.dirname, 'report.md') + +function parseArgs(argv: string[]) { + const args: { config?: string; onlyTaskIds?: string[]; onlyModes?: BenchmarkMode[]; repeats?: number } = {} + for (let i = 0; i < argv.length; i += 1) { + const arg = argv[i] + if (arg === '--config') args.config = argv[++i] + else if (arg === '--only-task') args.onlyTaskIds = [...(args.onlyTaskIds ?? []), argv[++i]!] + else if (arg === '--only-mode') { + const mode = argv[++i] + if (!BENCHMARK_MODES.includes(mode as BenchmarkMode)) { + throw new Error(`--only-mode must be one of ${BENCHMARK_MODES.join(', ')}, got "${mode}"`) + } + args.onlyModes = [...(args.onlyModes ?? []), mode as BenchmarkMode] + } else if (arg === '--repeats') args.repeats = Number(argv[++i]) + else throw new Error(`Unknown argument: ${arg}`) + } + if (!args.config) { + throw new Error('--config is required') + } + return args +} + +async function main() { + const args = parseArgs(process.argv.slice(2)) + + const tasks = await loadTasks(TASKS_DIR) + if (tasks.length === 0) { + throw new Error(`No tasks found in ${TASKS_DIR}. Author real task.json files there (see benchmark/templates/ and benchmark/swe-bench-adapter.mjs) before running the benchmark.`) + } + + const existingResults = await loadResults(RESULTS_PATH) + const cells = buildMatrix(tasks, BENCHMARK_MODES, existingResults, { + onlyTaskIds: args.onlyTaskIds, + onlyModes: args.onlyModes, + repeats: args.repeats, + }) + if (cells.length === 0) { + console.log('Every requested (task x mode x repeat) cell already has a recorded result. Nothing to run.') + } else { + console.log(`Running ${cells.length} cell(s); ${existingResults.length} already recorded and skipped.`) + + const runner = createCliDispatchRunner({ + factoryConfigPath: args.config!, + log: (message) => console.log(`[dispatch] ${message}`), + }) + await runMatrix(cells, { + runner, + appendResult: (result) => appendResult(RESULTS_PATH, result), + log: (message) => console.log(message), + now: () => new Date().toISOString(), + }) + } + + const allResults = await loadResults(RESULTS_PATH) + const rows = buildReportRows(tasks, allResults) + const markdown = renderMarkdownReport(rows) + const { writeFile } = await import('node:fs/promises') + await writeFile(REPORT_PATH, markdown) + console.log(`Report written to ${REPORT_PATH}`) +} + +main().catch((error) => { + console.error(error) + process.exitCode = 1 +}) diff --git a/benchmark/schema.ts b/benchmark/schema.ts new file mode 100644 index 00000000..69ed7252 --- /dev/null +++ b/benchmark/schema.ts @@ -0,0 +1,83 @@ +import { readFile, readdir } from 'node:fs/promises' +import { join } from 'node:path' + +import { z } from 'zod' + +/** + * A dispatch mode under test. Mirrors TriageDecision['scope'] minus 'workflow' + * (workflow dispatch isn't a coding-agent comparison — it runs a fixed script). + */ +export const BENCHMARK_MODES = ['single', 'team', 'swarm'] as const +export type BenchmarkMode = (typeof BENCHMARK_MODES)[number] + +export const BenchmarkTaskSchema = z.object({ + id: z.string().min(1), + title: z.string().min(1), + /** Full issue body dispatched to Factory as the task description. */ + issueBody: z.string().min(1), + /** owner/repo the task runs against. Must be a disposable sandbox repo, never a product repo. */ + targetRepo: z.string().regex(/^[\w.-]+\/[\w.-]+$/u, 'targetRepo must be "owner/repo"'), + /** Branch PRs are opened against. Defaults to the repo's default branch. */ + baseRef: z.string().min(1).default('main'), + /** + * Coordination-benefit tier: single-file tasks are the control (little to no + * expected benefit from team/swarm); multi-service tasks are where dispatch + * mode should differentiate most, if it differentiates at all. + */ + difficulty: z.enum(['single-file', 'multi-file', 'multi-service']), + verify: z.object({ + /** Shell command run from the repo root against the checked-out PR branch. Exit 0 = pass. */ + command: z.string().min(1), + timeoutMs: z.number().int().positive().default(300_000), + }), + source: z.enum(['authored', 'swe-bench']), + /** Only for source: 'swe-bench' — the upstream instance_id, for traceability. */ + sweBenchInstanceId: z.string().optional(), + /** Only for source: 'swe-bench' — a caveat surfaced next to targetRepo (e.g. "repoint to a fork"). */ + targetRepoNote: z.string().optional(), +}) + +export type BenchmarkTask = z.infer + +export interface BenchmarkResult { + taskId: string + mode: BenchmarkMode + repeat: number + runId: string + passed: boolean + notes?: string + /** ISO timestamp the result was recorded. */ + timestamp: string +} + +export const BenchmarkResultSchema = z.object({ + taskId: z.string().min(1), + mode: z.enum(BENCHMARK_MODES), + repeat: z.number().int().min(0), + runId: z.string().min(1), + passed: z.boolean(), + notes: z.string().optional(), + timestamp: z.string().min(1), +}) + +/** + * Loads every `benchmark/tasks//task.json`, validating each against + * BenchmarkTaskSchema and asserting the directory name matches `id` (catches + * copy-paste task authoring mistakes before they reach a real dispatch run). + */ +export async function loadTasks(tasksDir: string): Promise { + const entries = await readdir(tasksDir, { withFileTypes: true }) + const dirs = entries.filter((entry) => entry.isDirectory()).map((entry) => entry.name).sort() + + const tasks: BenchmarkTask[] = [] + for (const dir of dirs) { + const path = join(tasksDir, dir, 'task.json') + const raw = JSON.parse(await readFile(path, 'utf8')) + const task = BenchmarkTaskSchema.parse(raw) + if (task.id !== dir) { + throw new Error(`benchmark task at ${path} has id "${task.id}" but lives in directory "${dir}" — they must match`) + } + tasks.push(task) + } + return tasks +} diff --git a/benchmark/swe-bench-adapter.mjs b/benchmark/swe-bench-adapter.mjs new file mode 100644 index 00000000..0aeaff33 --- /dev/null +++ b/benchmark/swe-bench-adapter.mjs @@ -0,0 +1,132 @@ +#!/usr/bin/env node +// Fetches instances from SWE-bench Verified (a public, human-filtered subset of +// SWE-bench: https://huggingface.co/datasets/princeton-nlp/SWE-bench_Verified) +// and writes each as a benchmark/tasks//task.json + verify.sh, matching +// BenchmarkTaskSchema (see ./schema.ts). +// +// IMPORTANT — read before running: +// 1. Every instance's `repo` field is an upstream open-source repo (e.g. +// django/django) that Factory does NOT own push/PR access to and MUST NOT +// open real PRs against. Before dispatching these tasks for real, fork +// each distinct `repo` this script pulls into an org we control (or mirror +// it), then repoint `targetRepo` in the generated task.json files at that +// fork. This script deliberately does NOT do that forking for you — it +// only writes the upstream repo name plus a loud `targetRepoNote` so a +// human/agent has to make that call explicitly instead of silently +// dispatching against someone else's repository. +// 2. `verify.command` is a best-effort reconstruction: it re-runs the +// instance's own FAIL_TO_PASS pytest node IDs with `python -m pytest`. +// SWE-bench's official harness uses per-repo, sometimes non-pytest, test +// runners and pinned conda environments — this script does not replicate +// that. Spot-check the generated verify.sh against the instance's actual +// repo before trusting a "fail" result, especially for non-pytest repos. +// 3. This is best-effort, not a claim of strict SWE-bench-Verified parity — +// say so plainly in any report generated from these tasks. +// +// Usage: +// node benchmark/swe-bench-adapter.mjs --count 20 --out benchmark/tasks + +import { mkdir, writeFile } from 'node:fs/promises' +import { join } from 'node:path' + +const DATASET = 'princeton-nlp/SWE-bench_Verified' +const ROWS_PAGE_SIZE = 100 + +function parseArgs(argv) { + const args = { count: 20, out: 'benchmark/tasks', offset: 0 } + for (let i = 0; i < argv.length; i += 1) { + const arg = argv[i] + if (arg === '--count') args.count = Number(argv[++i]) + else if (arg === '--out') args.out = argv[++i] + else if (arg === '--offset') args.offset = Number(argv[++i]) + else throw new Error(`Unknown argument: ${arg}`) + } + if (!Number.isInteger(args.count) || args.count < 1) { + throw new Error(`--count must be a positive integer, got ${args.count}`) + } + return args +} + +async function fetchInstances(count, offset) { + const instances = [] + let cursor = offset + while (instances.length < count) { + const length = Math.min(ROWS_PAGE_SIZE, count - instances.length) + const url = `https://datasets-server.huggingface.co/rows?dataset=${encodeURIComponent(DATASET)}&config=default&split=test&offset=${cursor}&length=${length}` + const response = await fetch(url) + if (!response.ok) { + throw new Error(`SWE-bench Verified fetch failed: HTTP ${response.status} for ${url}`) + } + const body = await response.json() + if (body.rows.length === 0) break + instances.push(...body.rows.map((entry) => entry.row)) + cursor += body.rows.length + } + return instances +} + +function difficultyFor(instance) { + // SWE-bench Verified's own `difficulty` field is a human-estimated time + // bucket, not a coordination-benefit signal — approximate ours from how + // many distinct files the gold patch touches instead. + const filesTouched = new Set( + [...instance.patch.matchAll(/^diff --git a\/(\S+) /gmu)].map((match) => match[1]), + ).size + if (filesTouched <= 1) return 'single-file' + if (filesTouched <= 3) return 'multi-file' + return 'multi-service' +} + +function taskIdFor(instance) { + return `swe-bench-${instance.instance_id}`.toLowerCase().replace(/[^a-z0-9-]+/gu, '-') +} + +function toTask(instance) { + const failToPass = Array.isArray(instance.FAIL_TO_PASS) + ? instance.FAIL_TO_PASS + : JSON.parse(instance.FAIL_TO_PASS) + + return { + id: taskIdFor(instance), + title: `[SWE-bench Verified] ${instance.instance_id}`, + issueBody: instance.problem_statement, + targetRepo: instance.repo, + targetRepoNote: 'UPSTREAM OSS REPO — repoint to a controlled fork before dispatching for real. See adapter script header.', + baseRef: instance.base_commit, + difficulty: difficultyFor(instance), + verify: { + // Best-effort reconstruction — see script header caveat #2. + command: `python -m pytest ${failToPass.map((testId) => JSON.stringify(testId)).join(' ')}`, + timeoutMs: 600_000, + }, + source: 'swe-bench', + sweBenchInstanceId: instance.instance_id, + } +} + +async function main() { + const args = parseArgs(process.argv.slice(2)) + const instances = await fetchInstances(args.count, args.offset) + if (instances.length < args.count) { + console.warn(`Requested ${args.count} instances but only ${instances.length} were available from offset ${args.offset}.`) + } + + for (const instance of instances) { + const task = toTask(instance) + const dir = join(args.out, task.id) + await mkdir(dir, { recursive: true }) + await writeFile(join(dir, 'task.json'), `${JSON.stringify(task, null, 2)}\n`) + await writeFile( + join(dir, 'verify.sh'), + `#!/usr/bin/env bash\n# Adapted from SWE-bench Verified instance ${task.sweBenchInstanceId}. See swe-bench-adapter.mjs header before trusting this.\nset -euo pipefail\n\n${task.verify.command}\n`, + { mode: 0o755 }, + ) + } + + console.log(`Wrote ${instances.length} SWE-bench-derived task(s) to ${args.out}. Read the script header before dispatching any of them.`) +} + +main().catch((error) => { + console.error(error) + process.exitCode = 1 +}) diff --git a/benchmark/tasks/.gitkeep b/benchmark/tasks/.gitkeep new file mode 100644 index 00000000..e69de29b diff --git a/benchmark/templates/multi-service-feature/task.json b/benchmark/templates/multi-service-feature/task.json new file mode 100644 index 00000000..5bcdf241 --- /dev/null +++ b/benchmark/templates/multi-service-feature/task.json @@ -0,0 +1,13 @@ +{ + "id": "example-multi-service-feature", + "title": "TEMPLATE (multi-service — where team/swarm should differentiate) — add a feature spanning API, worker, and frontend", + "issueBody": "TEMPLATE — replace this whole file with a real task authored against benchmark-fixtures repo `` before running the corpus for real.\n\nThe intent of a multi-service task: acceptance criteria that genuinely span independently-touchable surfaces (an API route, a background worker, a frontend component), so isolated parallel work (team) or live-coordinated work (swarm) has real room to beat a single agent serializing through all three.\n\nExample shape: add a `/export` API endpoint that enqueues a background job, a worker that processes the queued job and writes a result file, and a frontend button that triggers the export and polls for the result. Acceptance: an end-to-end test exercises all three surfaces together.", + "targetRepo": "AgentWorkforce/factory-benchmark-fixtures", + "baseRef": "main", + "difficulty": "multi-service", + "verify": { + "command": "npm test -- export-flow.e2e.test.ts", + "timeoutMs": 600000 + }, + "source": "authored" +} diff --git a/benchmark/templates/multi-service-feature/verify.sh b/benchmark/templates/multi-service-feature/verify.sh new file mode 100755 index 00000000..cc8f724f --- /dev/null +++ b/benchmark/templates/multi-service-feature/verify.sh @@ -0,0 +1,5 @@ +#!/usr/bin/env bash +# TEMPLATE verify script. Run from the repo root, on the checked-out PR branch. +set -euo pipefail + +npm test -- export-flow.e2e.test.ts diff --git a/benchmark/templates/single-file-fix/task.json b/benchmark/templates/single-file-fix/task.json new file mode 100644 index 00000000..44add73e --- /dev/null +++ b/benchmark/templates/single-file-fix/task.json @@ -0,0 +1,13 @@ +{ + "id": "example-single-file-fix", + "title": "TEMPLATE (single-file control group) — fix a null-check bug in one file", + "issueBody": "TEMPLATE — replace this whole file with a real task authored against benchmark-fixtures repo `` before running the corpus for real.\n\nThe intent of a single-file task: a change small and self-contained enough that splitting it across multiple agents (team/swarm) should offer little or no coordination benefit. These are the control group — if team/swarm don't at least match single here, dispatch overhead is hurting, not helping.\n\nExample shape: `formatUserName(user)` in `src/format.ts` throws when `user.middleName` is undefined. Fix it to treat a missing middle name as absent (no double space), and add a regression test.", + "targetRepo": "AgentWorkforce/factory-benchmark-fixtures", + "baseRef": "main", + "difficulty": "single-file", + "verify": { + "command": "npm test -- format.test.ts", + "timeoutMs": 300000 + }, + "source": "authored" +} diff --git a/benchmark/templates/single-file-fix/verify.sh b/benchmark/templates/single-file-fix/verify.sh new file mode 100755 index 00000000..79756b69 --- /dev/null +++ b/benchmark/templates/single-file-fix/verify.sh @@ -0,0 +1,8 @@ +#!/usr/bin/env bash +# TEMPLATE verify script. Run from the repo root, on the checked-out PR branch. +# Exit 0 = task passed; any other exit code = failed. Keep this narrow and +# deterministic — it is the ground truth the runner scores against, not a +# smoke test the implementer agent can talk its way around. +set -euo pipefail + +npm test -- format.test.ts diff --git a/benchmark/tsconfig.json b/benchmark/tsconfig.json new file mode 100644 index 00000000..544a956e --- /dev/null +++ b/benchmark/tsconfig.json @@ -0,0 +1,9 @@ +{ + "extends": "../tsconfig.json", + "compilerOptions": { + "composite": false, + "noEmit": true, + "rootDir": "." + }, + "include": ["**/*"] +} diff --git a/package.json b/package.json index 1419a14f..87241805 100644 --- a/package.json +++ b/package.json @@ -85,6 +85,7 @@ "featuremap:check": "node bin/factory.mjs featuremap check", "test": "vitest run", "test:e2e:kubernetes": "bash scripts/test-kubernetes-provider-e2e.sh", + "benchmark": "tsx benchmark/run.ts", "test:e2e:load": "tsx test/e2e/load-harness.e2e.ts", "test:e2e:verification": "tsx test/e2e/verification-gate.e2e.ts", "test:stack-e2e": "tsx test/e2e/stack-deployer.e2e.ts", diff --git a/src/dispatch/templates.test.ts b/src/dispatch/templates.test.ts index 7aa9fdac..c5c26772 100644 --- a/src/dispatch/templates.test.ts +++ b/src/dispatch/templates.test.ts @@ -69,6 +69,56 @@ describe('renderAgentTask', () => { expect(task).toContain('Merge policy: never') }) + it('renders swarm lead coordination clauses naming the workers and shared channel', () => { + const task = renderAgentTask({ + issue, + route: { repo: 'pear', clonePath: '/work/pear' }, + role: 'implementer', + config: baseConfig, + reviewerName: 'ar-123-review', + agentName: 'ar-123-impl-lead', + swarm: { role: 'lead', channel: 'swarm-ar-123', otherMemberNames: ['ar-123-impl-worker-1'] }, + }) + + expect(task).toContain('SWARM LEAD') + expect(task).toContain('ar-123-impl-worker-1 (workers) are collaborating with you live in this SAME checkout.') + expect(task).toContain('#swarm-ar-123') + expect(task).toContain('Integrate everyone\'s changes into one coherent result') + expect(task).not.toContain('SWARM WORKER') + }) + + it('renders swarm worker coordination clauses naming the lead and forbidding its own PR', () => { + const task = renderAgentTask({ + issue, + route: { repo: 'pear', clonePath: '/work/pear' }, + role: 'implementer', + config: baseConfig, + reviewerName: 'ar-123-review', + agentName: 'ar-123-impl-worker-1', + swarm: { role: 'worker', channel: 'swarm-ar-123', otherMemberNames: ['ar-123-impl-lead'] }, + }) + + expect(task).toContain('SWARM WORKER') + expect(task).toContain('a lead (ar-123-impl-lead) live in this SAME checkout') + expect(task).toContain('#swarm-ar-123') + expect(task).toContain('Do not open your own pull request') + expect(task).not.toContain('SWARM LEAD') + }) + + it('omits swarm clauses entirely for non-swarm dispatch', () => { + const task = renderAgentTask({ + issue, + route: { repo: 'pear', clonePath: '/work/pear' }, + role: 'implementer', + config: baseConfig, + reviewerName: 'ar-123-review', + agentName: 'ar-123-impl', + }) + + expect(task).not.toContain('SWARM') + expect(task).not.toContain('#swarm-') + }) + it('renders reviewer coordination clauses for team dispatch', () => { const task = renderAgentTask({ issue, diff --git a/src/dispatch/templates.ts b/src/dispatch/templates.ts index fee09e7f..15dbc103 100644 --- a/src/dispatch/templates.ts +++ b/src/dispatch/templates.ts @@ -75,6 +75,13 @@ export interface RenderAgentTaskInput { branchPrepared?: boolean /** Registered relay identity used in durable human-input request comments. */ agentName?: string + /** Set only for scope 'swarm': this implementer collaborates live with named others over a shared relay channel. */ + swarm?: { + role: 'lead' | 'worker' + channel: string + /** The other swarm members sharing this checkout and channel (excludes this agent). */ + otherMemberNames: string[] + } /** Durable Relay action owned by the active Factory process. */ lifecycleActionName?: string /** @@ -119,8 +126,11 @@ export function renderAgentTask(input: RenderAgentTaskInput): string { ] : []), ] + const swarmInstructions = input.swarm ? renderSwarmInstructions(input.swarm) : [] + const common = [ ...header, + ...swarmInstructions, '', input.branchName && input.branchPrepared ? `Factory already prepared this isolated checkout on branch \`${input.branchName}\`. Do not reset it, switch branches, or recreate it; commit and push only this branch.` @@ -319,6 +329,24 @@ export function renderAgentTask(input: RenderAgentTaskInput): string { ].join('\n') } +function renderSwarmInstructions(swarm: NonNullable): string[] { + const others = swarm.otherMemberNames.length > 0 ? swarm.otherMemberNames.join(', ') : 'the rest of the swarm' + if (swarm.role === 'lead') { + return [ + '', + `You are the SWARM LEAD. ${others} (workers) are collaborating with you live in this SAME checkout.`, + `Coordinate over the shared relay channel #${swarm.channel}: break the work into subtasks, assign them to workers by name, and check their progress before you finish.`, + `Integrate everyone's changes into one coherent result before handing off to review. Do not finish until you've confirmed on #${swarm.channel} that every worker is done or blocked.`, + ] + } + return [ + '', + `You are a SWARM WORKER collaborating with a lead (${others}) live in this SAME checkout.`, + `Watch the shared relay channel #${swarm.channel} for direction from the lead. Announce what you're starting, ask there if scope is ambiguous, and post when a subtask is done or blocked.`, + 'Do not open your own pull request or push to origin yourself — the lead integrates everyone\'s work and finishes it.', + ] +} + function lifecycleInstructions( input: Pick, kind: 'completed' | 'ready', diff --git a/src/orchestrator/factory.test.ts b/src/orchestrator/factory.test.ts index 7231a15d..61fda72f 100644 --- a/src/orchestrator/factory.test.ts +++ b/src/orchestrator/factory.test.ts @@ -11360,6 +11360,51 @@ describe('FactoryLoop', () => { expect(comments[0]).not.toContain('Too many repo labels') }) + // Companion to the single/workflow guard tests: an explicit swarm-scope issue + // shares one checkout across a lead + worker (unlike team, which fans out one + // implementer per repo label) and puts every member on the same relay channel + // so they can coordinate live, even when extra repo labels are present. + it('dispatches explicit swarm labels as a lead and worker sharing one checkout and channel', async () => { + const routedIssue = realIssueFile(729, ready, { + labels: [{ name: 'pear' }, { name: 'cloud' }, { name: 'agent:swarm' }], + }) + const mount = new FakeMountClient({ [issuePath(729)]: routedIssue }) + const fleet = new FakeFleetClient() + const factory = createFactory(config({ + triage: { maxImplementers: 2 }, + repos: { + byLabel: { + pear: 'AgentWorkforce/pear', + cloud: 'AgentWorkforce/cloud', + }, + byProject: {}, + keywordRules: [], + clonePaths: { + 'AgentWorkforce/pear': '/work/pear', + 'AgentWorkforce/cloud': '/work/cloud', + }, + default: 'AgentWorkforce/pear', + }, + }), { mount, fleet, triage: new StaticTriage() }) + + const result = await factory.dispatch(await factory.triageIssue(parseLinearIssue(issuePath(729), routedIssue))) + + expect(result.agents.map((agent) => agent.name)).toEqual([ + 'ar-729-impl-lead', + 'ar-729-impl-worker-1', + 'ar-729-review', + ]) + expect(fleet.spawns.map((spawn) => [spawn.name, spawn.cwd, spawn.channel])).toEqual([ + ['ar-729-impl-lead', '/work/pear', 'swarm-ar-729'], + ['ar-729-impl-worker-1', '/work/pear', 'swarm-ar-729'], + ['ar-729-review', '/work/pear', undefined], + ]) + const [lead, worker] = fleet.spawns + expect(lead?.task).toMatch(/SWARM LEAD/) + expect(lead?.task).toContain('#swarm-ar-729') + expect(worker?.task).toMatch(/SWARM WORKER/) + }) + it('fails dispatch loudly when no labels are present and no default repo is configured', async () => { const unlabeledIssue = realIssueFile(722, ready, { labels: [] }) const mount = new FakeMountClient({ [issuePath(722)]: unlabeledIssue }) diff --git a/src/orchestrator/factory.ts b/src/orchestrator/factory.ts index 4bfbcb8e..268699aa 100644 --- a/src/orchestrator/factory.ts +++ b/src/orchestrator/factory.ts @@ -66,7 +66,7 @@ import { type GithubHumanInputRequest, } from '../dispatch/templates' import { resolveTestGuidance } from '../dispatch/test-guidance' -import { HeuristicTriage, TieredTriage, babysitterSpec, isShapeLabel, scopeFromLabels } from '../triage' +import { HeuristicTriage, TieredTriage, babysitterSpec, isShapeLabel, scopeFromLabels, swarmChannel, swarmMemberSlugs, swarmTaskFor } from '../triage' import { agentNameForRole, sanitizeAgentSlug } from '../triage/agent-names' import { isResourceSubscriptionsUnavailable, type ResourceSubscription } from '../subscriptions' import type { @@ -10397,6 +10397,15 @@ export class FactoryLoop implements Factory { previewStartCommand: spec.preview?.startCommand, } : {}), ...(this.#fleet.lifecycleActionName ? { lifecycleActionName: this.#fleet.lifecycleActionName } : {}), + ...(spec.swarmRole && spec.channel ? { + swarm: { + role: spec.swarmRole, + channel: spec.channel, + otherMemberNames: decision.implementers + .filter((implementer) => implementer.channel === spec.channel && implementer.name !== spec.name) + .map((implementer) => implementer.name), + }, + } : {}), }), } } @@ -15837,15 +15846,19 @@ function labelDerivedDispatchDecision( } } - const implementers = routesByLabel.routes.map(({ slug, route }) => - routeImplementerSpec(liveIssue, config, slug, route), - ) - const selectedRoutes = scope === 'single' ? routesByLabel.routes.slice(0, 1) : routesByLabel.routes + // Swarm always shares one checkout (lead + workers collaborate live over a + // shared relay channel), so — like single/workflow — only the first matched + // label route is used; it is never fanned out across repo labels like team. + const selectedRoutes = scope === 'single' || scope === 'swarm' + ? routesByLabel.routes.slice(0, 1) + : routesByLabel.routes const selectedImplementers = scope === 'team' - ? implementers + ? routesByLabel.routes.map(({ slug, route }) => routeImplementerSpec(liveIssue, config, slug, route)) : scope === 'single' - ? implementers.slice(0, 1) - : [] + ? routesByLabel.routes.slice(0, 1).map(({ slug, route }) => routeImplementerSpec(liveIssue, config, slug, route)) + : scope === 'swarm' + ? routeSwarmImplementerSpecs(liveIssue, config, selectedRoutes[0]?.route, maxImplementers) + : [] const routes = selectedRoutes.map(({ route }) => route) const workflow = scope === 'workflow' ? routeWorkflowSpec(liveIssue, config, selectedRoutes, decision.workflow) @@ -16003,6 +16016,30 @@ function routeImplementerSpec( } } +function routeSwarmImplementerSpecs( + issue: LinearIssue, + config: FactoryConfig, + route: TriageDecision['routes'][number] | undefined, + maxImplementers: number, +): AgentSpec[] { + if (!route) { + return [] + } + const channel = swarmChannel(issue) + return swarmMemberSlugs(maxImplementers).map((slug) => ({ + name: agentNameForRole(issue, 'impl', { repo: route.repo, discriminator: slug }), + role: 'implementer' as const, + capability: config.agentCapabilities.implementer, + model: config.models.implementer, + task: swarmTaskFor(issue, route, slug, channel), + repo: route.repo, + clonePath: route.clonePath, + channel, + swarmRole: slug === 'lead' ? 'lead' as const : 'worker' as const, + node: 'self', + })) +} + function decisionWithLifecycleBranches( decision: TriageDecision, runId: string, diff --git a/src/ports/fleet.ts b/src/ports/fleet.ts index 0ba3354f..a032024b 100644 --- a/src/ports/fleet.ts +++ b/src/ports/fleet.ts @@ -225,4 +225,6 @@ export type AgentSpec = { existingPullRequestBranch?: boolean /** Shared live preview owned by the issue lifecycle, not this agent process. */ preview?: PreviewReference + /** Set only for scope 'swarm': this implementer's position in the live-collaborating team. */ + swarmRole?: 'lead' | 'worker' } diff --git a/src/triage/heuristic.ts b/src/triage/heuristic.ts index d5ad6a3a..9ca38b1e 100644 --- a/src/triage/heuristic.ts +++ b/src/triage/heuristic.ts @@ -1,7 +1,7 @@ import type { FactoryConfig } from '../config/schema' import type { AgentSpec } from '../ports' import type { IssueRef, LinearIssue, RepoMapEntry, TriageContext, TriageDecision, TriageEngine } from '../types' -import { agentNameForRole, repoSlugFromName } from './agent-names' +import { agentBaseName, agentNameForRole, repoSlugFromName } from './agent-names' type RouteSource = RepoMapEntry['source'] type Route = TriageDecision['routes'][number] @@ -14,6 +14,14 @@ const SHAPE_LABELS: Record = { 'agent:single': 'single', 'agent:workflow': 'workflow', 'agent:team': 'team', + 'agent:swarm': 'swarm', +} + +/** First slug is always the swarm lead; the rest are workers, in spawn order. */ +export function swarmMemberSlugs(maxImplementers: number): string[] { + const workerCount = Math.max(maxImplementers - 1, 0) + const slugs = ['lead', ...Array.from({ length: workerCount }, (_, index) => `worker-${index + 1}`)] + return slugs.slice(0, Math.max(maxImplementers, 0)) } const SURFACE_BUCKETS: Array<{ name: string; patterns: RegExp[] }> = [ @@ -252,6 +260,14 @@ function implementationAssignments( return route ? [{ route, slug: repoSlugFromName(route.repo) ?? 'scope' }] : [] } + if (scope === 'swarm') { + // Swarm always shares one checkout (lead + workers collaborate live over a + // shared relay channel) — unlike team, extra matched routes are not fanned + // out, only the first is used. + const route = routes[0] + return route ? swarmMemberSlugs(maxImplementers).map((slug) => ({ route, slug })) : [] + } + if (routes.length >= 2) { return routes.slice(0, maxImplementers).map((route, index) => ({ route, @@ -286,20 +302,45 @@ function implementerSpec(input: { }): AgentSpec { const name = agentNameForRole(input.issue, 'impl', { repo: input.route.repo, - discriminator: input.scope === 'team' ? input.slug : undefined, + discriminator: input.scope === 'team' || input.scope === 'swarm' ? input.slug : undefined, }) + const channel = input.scope === 'swarm' ? swarmChannel(input.issue) : undefined return { name, role: 'implementer', capability: input.config.agentCapabilities.implementer, model: input.config.models.implementer, - task: taskFor(input.issue, input.route, 'implementer'), + task: channel + ? swarmTaskFor(input.issue, input.route, input.slug, channel) + : taskFor(input.issue, input.route, 'implementer'), repo: input.route.repo, clonePath: input.route.clonePath, + ...(channel ? { channel, swarmRole: input.slug === 'lead' ? 'lead' as const : 'worker' as const } : {}), node: 'self', } } +/** Shared relay channel every lead/worker for one issue's swarm joins live. */ +export function swarmChannel(issue: LinearIssue): string { + return `swarm-${agentBaseName(issue)}` +} + +export function swarmTaskFor(issue: LinearIssue, route: Route, slug: string, channel: string): string { + const base = taskFor(issue, route, 'implementer') + const roleBriefing = slug === 'lead' + ? [ + `You are the SWARM LEAD for this task. Worker agents are collaborating with you in the same checkout, in real time.`, + `Coordinate over the shared relay channel #${channel}: break the work into subtasks, assign them to workers by name, and check their progress before you finish.`, + `Integrate everyone's changes into one coherent result before handing off to review. Do not finish until you've confirmed on #${channel} that every worker is done or blocked.`, + ] + : [ + `You are a SWARM WORKER (${slug}) for this task, collaborating with a lead and other workers in the same checkout, in real time.`, + `Watch the shared relay channel #${channel} for direction from the lead. Announce what you're starting, ask there if scope is ambiguous, and post when a subtask is done or blocked.`, + `Do not open your own pull request — the lead integrates and finishes the work.`, + ] + return [base, ...roleBriefing].join('\n\n') +} + function workflowSpec(issue: LinearIssue, _config: FactoryConfig, routes: Route[]): AgentSpec { const primaryRoute = routes[0] // NOTE: this derives repoLabels from raw label strings, whereas the dispatch @@ -379,6 +420,7 @@ function taskFor(issue: LinearIssue, route: Route, role: AgentSpec['role']): str export function scopeFromLabels(labels: string[]): Scope | undefined { const normalized = new Set(labels.map((label) => label.trim().toLowerCase())) + if (normalized.has('agent:swarm')) return 'swarm' if (normalized.has('agent:team')) return 'team' if (normalized.has('agent:workflow')) return 'workflow' if (normalized.has('agent:single')) return 'single' diff --git a/src/triage/index.ts b/src/triage/index.ts index 2760a661..a44d9eb0 100644 --- a/src/triage/index.ts +++ b/src/triage/index.ts @@ -1,4 +1,4 @@ -export { HeuristicTriage, babysitterSpec, isShapeLabel, scopeFromLabels } from './heuristic' +export { HeuristicTriage, babysitterSpec, isShapeLabel, scopeFromLabels, swarmChannel, swarmMemberSlugs, swarmTaskFor } from './heuristic' export type { HeuristicTriageOptions } from './heuristic' export { LlmTriage, buildPrompt } from './llm' export type { LlmTriageOptions } from './llm' diff --git a/src/triage/schema.ts b/src/triage/schema.ts index 6428a257..1c05c439 100644 --- a/src/triage/schema.ts +++ b/src/triage/schema.ts @@ -13,6 +13,7 @@ export const AgentSpecSchema = z.object({ repo: z.string(), clonePath: z.string().optional(), channel: z.string().optional(), + swarmRole: z.enum(['lead', 'worker']).optional(), node: z.string().optional(), sessionRef: z.string().optional(), invocationId: z.string().optional(), @@ -49,7 +50,7 @@ export const TriageDecisionSchema = z.object({ clonePath: z.string().optional(), rationale: z.string(), })), - scope: z.enum(['single', 'workflow', 'team']), + scope: z.enum(['single', 'workflow', 'team', 'swarm']), implementers: z.array(AgentSpecSchema), workflow: AgentSpecSchema.optional(), reviewer: AgentSpecSchema, diff --git a/src/triage/triage.test.ts b/src/triage/triage.test.ts index a643cf53..7f06e9ea 100644 --- a/src/triage/triage.test.ts +++ b/src/triage/triage.test.ts @@ -191,6 +191,13 @@ describe('HeuristicTriage thin and scope detection', () => { expectedImplementers: ['ar-123-impl-pear', 'ar-123-impl-scope'], expectedWorkflow: undefined, }, + { + name: 'agent:swarm produces a lead and one worker sharing a checkout', + labels: ['pear', 'agent:swarm'], + expectedScope: 'swarm', + expectedImplementers: ['ar-123-impl-lead', 'ar-123-impl-worker-1'], + expectedWorkflow: undefined, + }, ])('$name', async ({ labels, expectedScope, expectedImplementers, expectedWorkflow }) => { const decision = await new HeuristicTriage().triage(issue({ labels, @@ -222,6 +229,47 @@ describe('HeuristicTriage thin and scope detection', () => { expect(decision.implementers.every((implementer) => implementer.repo === 'AgentWorkforce/pear')).toBe(true) }) + it('is never inferred heuristically, only from the agent:swarm label', async () => { + const decision = await new HeuristicTriage().triage(issue({ + labels: ['pear', 'agents'], + description: richDescription('Update renderer and broker surfaces with tests in src/main/broker.ts.'), + }), ctx) + + // Same fixture as the multi-route team test above — proves swarm is opt-in only. + expect(decision.scope).toBe('team') + }) + + it('gives every swarm member the same checkout and a shared relay channel', async () => { + const decision = await new HeuristicTriage().triage(issue({ + labels: ['pear', 'agent:swarm'], + }), ctx) + + expect(decision.scope).toBe('swarm') + expect(decision.implementers).toHaveLength(2) + expect(decision.implementers.every((implementer) => implementer.clonePath === '/work/pear')).toBe(true) + expect(decision.implementers.every((implementer) => implementer.channel === 'swarm-ar-123')).toBe(true) + expect(decision.implementers[0]?.task).toMatch(/SWARM LEAD/) + expect(decision.implementers[0]?.task).toMatch(/#swarm-ar-123/) + expect(decision.implementers[1]?.task).toMatch(/SWARM WORKER/) + }) + + it('caps swarm members at triage.maxImplementers, same as team', async () => { + const config = FactoryConfigSchema.parse({ + workspaceId: 'ws_123', + triage: { maxImplementers: 3 }, + repos: baseConfig.repos, + models: { implementer: 'codex-test', reviewer: 'claude-test' }, + }) + + const decision = await new HeuristicTriage().triage(issue({ labels: ['pear', 'agent:swarm'] }), { ...ctx, config }) + + expect(decision.implementers.map((implementer) => implementer.name)).toEqual([ + 'ar-123-impl-lead', + 'ar-123-impl-worker-1', + 'ar-123-impl-worker-2', + ]) + }) + it('builds AgentSpec entries from config models and clone paths', async () => { const decision = await new HeuristicTriage().triage(issue(), ctx) diff --git a/src/types.ts b/src/types.ts index c300cd5c..d402774b 100644 --- a/src/types.ts +++ b/src/types.ts @@ -325,7 +325,7 @@ export interface TriageDecision { issue: IssueRef issueResolution?: IssueResolution routes: Array<{ repo: string; clonePath?: string; rationale: string }> - scope: 'single' | 'workflow' | 'team' + scope: 'single' | 'workflow' | 'team' | 'swarm' implementers: AgentSpec[] workflow?: AgentSpec reviewer: AgentSpec diff --git a/vitest.config.ts b/vitest.config.ts index 5ffb289e..2e4fbe6f 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -4,6 +4,7 @@ export default defineConfig({ test: { include: [ 'src/**/*.test.ts', + 'benchmark/**/*.test.ts', '.agentworkforce/agents/**/*.test.ts', 'test/e2e/dispatch-identity-real-broker.test.ts', 'test/e2e/run-cost-accounting.test.ts', From 8ccb1a91316b9f15166fa34baf80624c5a2ea753 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Mon, 17 Aug 2026 11:30:21 +0200 Subject: [PATCH 2/5] fix(factory,benchmark): address PR #199 review threads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six unresolved review threads on PR #199, fixed: P1 — Gate swarm publication on lead completion (src/orchestrator/factory.ts) Swarm workers register as ordinary implementers on the same repo, checkout and lifecycle branch as the lead. Without a worker-specific gate, whichever worker exited first would publish partial state on the shared branch and release every swarm member via the shared-branch PR probe, killing the still-working lead. #handleAgentExit now short-circuits swarm-role workers with a swarmWorkerExitsSuppressed counter increment before the completion path runs. Tests: swarm-worker-exit does not publish or complete (red before the guard, green after); swarm-lead-exit stays on the normal path. P1 — Team cell dispatches only one agent (benchmark/cli-dispatch-runner.ts) Factory's team scope fans out one implementer per configured repo route. A single-repository BenchmarkTask produces one route, so `team` and `single` become the same one-implementer dispatch and the harness would produce team-vs-single numbers that measure the same thing. The runner now refuses `team` for single-repo tasks with an explicit reason — surfacing the gap as a per-cell notes string rather than manufacturing data. Tests: guard rejects team-mode dispatch on a single-repo task. P1 — Runner ignores task.baseRef (benchmark/cli-dispatch-runner.ts) Factory always cuts the lifecycle branch and PR base from the repository default branch (#publishImplementerPullRequest / #githubDefaultBranch). A task carrying a specific baseRef — most obviously a SWE-bench instance base_commit — would silently score against the wrong revision. The runner now refuses a non-`main` baseRef until Factory honors per-issue base overrides. Tests: guard rejects non-default baseRef. P1 — Pass a repo-qualified issue to Factory (src/cli/fleet.ts, benchmark/cli-dispatch-runner.ts) Dispatching a bare number in a multi-repo config either fails with an ambiguity error or silently resolves through repos.default to the wrong repository — the same defect class as factory#276 that PR #278 fixed by repo-qualifying with githubIssuePathParts. `findIssuePath` now accepts an `owner/repo#N` argument (the same shape githubIssueIdentity / githubLifecycleIdentity already use) and the runner passes `${task.targetRepo}#${issueNumber}`. Tests: repo-qualified argument wins over repos.default; repo-qualified argument outside configured routes is rejected. P2 — Record wall-clock samples per run (benchmark/orchestrate.ts, benchmark/report.ts, benchmark/schema.ts) `buildReportRows` was called without `costSamples`, so every report printed `n/a` for the headline wall-clock metric. `runMatrix` now measures each cell with an injected monotonic clock and stamps `durationMs` on the BenchmarkResult (also captured on the failure path). Report reads `durationMs`/`costUsd` off result rows; the legacy `costSamples` sidecar remains supported. Cost stays `undefined` until Factory exposes per-dispatch spend — the report reports `n/a` for cost rather than a lying 0. Tests: durationMs captured on success and failure paths; report averages only sampled rows. P2 — Remove contradictory worker push instruction (src/dispatch/templates.ts) Worker prompt said "do not push", but `renderAgentTask`'s common block then appended "Commit the implementation and tests" and "Push the branch to origin". Common publication lines (commit/push/PR-open, reviewer handoff, lifecycle action) are now emitted only for roles that own publication — every implementer except a swarm worker. Workers still commit locally on the shared branch and post done/blocked on the shared swarm channel. Tests: worker prompt no longer contains push/PR-open/reviewer-DM instructions; non-swarm implementer prompt still contains them (regression fence). Co-Authored-By: Claude Opus 4.7 Session-Id: 8c8fcd37-6ade-4756-9a98-e5ced3cf5871 Session-Id: 8c8fcd37-6ade-4756-9a98-e5ced3cf5871 --- benchmark/cli-dispatch-runner.test.ts | 43 +++++++++++++++++ benchmark/cli-dispatch-runner.ts | 47 ++++++++++++++++++- benchmark/orchestrate.test.ts | 28 ++++++++++- benchmark/orchestrate.ts | 9 ++++ benchmark/report.test.ts | 24 ++++++++++ benchmark/report.ts | Bin 3752 -> 4144 bytes benchmark/schema.ts | 14 ++++++ src/dispatch/templates.test.ts | 26 +++++++++++ src/dispatch/templates.ts | 46 +++++++++++++----- src/orchestrator/factory.test.ts | 64 ++++++++++++++++++++++++++ src/orchestrator/factory.ts | 11 +++++ 11 files changed, 297 insertions(+), 15 deletions(-) create mode 100644 benchmark/cli-dispatch-runner.test.ts diff --git a/benchmark/cli-dispatch-runner.test.ts b/benchmark/cli-dispatch-runner.test.ts new file mode 100644 index 00000000..5e4cd7c2 --- /dev/null +++ b/benchmark/cli-dispatch-runner.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from 'vitest' + +import { createCliDispatchRunner } from './cli-dispatch-runner' +import type { BenchmarkTask } from './schema' + +// The runner is deliberately real-IO; the guards below are pure input +// validation that runs BEFORE any child_process call, so they are safe (and +// worth) unit-testing on their own. They exist to prevent the harness from +// producing plausibly-shaped-but-wrong benchmark numbers when Factory cannot +// honor the requested cell — see the guards' comments in cli-dispatch-runner.ts. + +function task(overrides: Partial = {}): BenchmarkTask { + return { + id: 'ex', + title: 'Example', + issueBody: 'Do the thing.', + targetRepo: 'AgentWorkforce/factory-benchmark-fixtures', + baseRef: 'main', + difficulty: 'single-file', + verify: { command: 'npm test', timeoutMs: 300_000 }, + source: 'authored', + ...overrides, + } +} + +describe('createCliDispatchRunner guards', () => { + it('refuses team mode for a single-repository task (Factory team scope needs multiple repo routes)', async () => { + const runner = createCliDispatchRunner({ factoryConfigPath: '/tmp/nowhere.json' }) + await expect(runner.dispatch(task(), 'team')).rejects.toThrow( + /Cannot dispatch team mode for single-repository task ex/, + ) + // If the guard failed to fire, `gh issue create` would run for real — + // proving the guard is the boundary, not the network. + }) + + it('refuses a non-default baseRef because Factory always cuts from the repo default branch', async () => { + const runner = createCliDispatchRunner({ factoryConfigPath: '/tmp/nowhere.json' }) + await expect(runner.dispatch(task({ baseRef: 'abc123def456' }), 'single')).rejects.toThrow( + /Cannot dispatch task ex with baseRef=abc123def456/, + ) + }) + +}) diff --git a/benchmark/cli-dispatch-runner.ts b/benchmark/cli-dispatch-runner.ts index 176e2cd3..87d7523f 100644 --- a/benchmark/cli-dispatch-runner.ts +++ b/benchmark/cli-dispatch-runner.ts @@ -29,6 +29,14 @@ export interface CliDispatchRunnerOptions { * intentionally not unit-tested (same category as * scripts/verify-tailscale-preview-e2e.mjs): correctness here is proven by * running it for real, not by mocking child_process. + * + * Two categories of dispatch call are refused up front instead of quietly + * producing wrong numbers — see the guards in `dispatch()` below. Both are + * limitations of Factory's dispatch, not this runner: fixing them belongs in + * Factory (multi-implementer-per-repo team scope, per-issue base override), + * and the benchmark should report `n/a` for the affected cells until those + * land rather than mislabel single-agent runs as `team` or measure the wrong + * SWE-bench base commit. */ export function createCliDispatchRunner(options: CliDispatchRunnerOptions): DispatchRunner { const log = options.log ?? (() => {}) @@ -37,6 +45,36 @@ export function createCliDispatchRunner(options: CliDispatchRunnerOptions): Disp return { async dispatch(task, mode) { + // Factory's `team` scope fans out ONE implementer per configured repo + // route. A single-repository benchmark task produces exactly one route, + // so `team` and `single` become the same one-implementer dispatch and + // the benchmark's team-vs-single comparison would be a null test + // wearing the wrong label. Refuse the cell so the report shows the gap + // instead of manufacturing data. + if (mode === 'team' && !task.targetRepo.includes(',')) { + throw new Error( + `Cannot dispatch team mode for single-repository task ${task.id} (targetRepo=${task.targetRepo}): ` + + `Factory's team scope requires multiple repository routes and would spawn one implementer here — ` + + `the same cardinality as single. This cell is unsupported until BenchmarkTask carries multiple ` + + `repositories (or Factory's team scope splits one repository across implementers).`, + ) + } + // Factory always cuts the lifecycle branch from the repository's + // default branch and opens the PR against it (see + // Factory#publishImplementerPullRequest / #githubDefaultBranch). A + // task carrying a specific baseRef — most obviously a SWE-bench + // instance's `base_commit` — would silently be measured against the + // wrong revision. Until Factory honors a per-issue base override, + // refuse instead of scoring against `main`. + if (task.baseRef && task.baseRef !== 'main') { + throw new Error( + `Cannot dispatch task ${task.id} with baseRef=${task.baseRef}: ` + + `Factory cuts the lifecycle branch and PR base from the repository's default branch and does not ` + + `honor a per-issue baseRef. Scoring this task from a different base would silently measure the ` + + `wrong revision.`, + ) + } + const label = `agent:${mode}` const { stdout: createOut } = await exec('gh', [ 'issue', 'create', @@ -53,7 +91,14 @@ export function createCliDispatchRunner(options: CliDispatchRunnerOptions): Disp } log(`created ${task.targetRepo}#${issueNumber} (${label})`) - await exec('node', ['bin/factory.mjs', 'dispatch', issueNumber, '--config', options.factoryConfigPath]) + // Repo-qualify the dispatch argument (owner/repo#N — the same shape + // githubIssueIdentity / githubLifecycleIdentity already use). A bare + // number is either rejected as ambiguous when the config maps + // multiple repositories, or is silently resolved through + // repos.default to a different repository — the same defect class + // as factory#276 (unqualified merge-advance closed cross-repo + // issues; fixed by repo-qualifying with githubIssuePathParts). + await exec('node', ['bin/factory.mjs', 'dispatch', `${task.targetRepo}#${issueNumber}`, '--config', options.factoryConfigPath]) log(`dispatched ${task.targetRepo}#${issueNumber}`) const prBranch = await pollForPrBranch(task.targetRepo, issueNumber, dispatchTimeoutMs, pollIntervalMs, log) diff --git a/benchmark/orchestrate.test.ts b/benchmark/orchestrate.test.ts index c487981a..eb333495 100644 --- a/benchmark/orchestrate.test.ts +++ b/benchmark/orchestrate.test.ts @@ -46,9 +46,13 @@ describe('runMatrix', () => { ], opts) expect(results).toEqual([ - { taskId: 'a', mode: 'single', repeat: 0, runId: 'a-single', passed: true, notes: undefined, timestamp: 'fixed-timestamp' }, - { taskId: 'a', mode: 'team', repeat: 0, runId: 'a-team', passed: true, notes: undefined, timestamp: 'fixed-timestamp' }, + expect.objectContaining({ taskId: 'a', mode: 'single', repeat: 0, runId: 'a-single', passed: true, notes: undefined, timestamp: 'fixed-timestamp' }), + expect.objectContaining({ taskId: 'a', mode: 'team', repeat: 0, runId: 'a-team', passed: true, notes: undefined, timestamp: 'fixed-timestamp' }), ]) + // Every result now carries a non-negative wall-clock — the report can no + // longer print `n/a` for it. (Cost stays undefined until Factory exposes + // per-dispatch spend to the runner.) + for (const result of results) expect(result.durationMs).toBeGreaterThanOrEqual(0) expect(appended).toEqual(results) expect(runner.dispatch).toHaveBeenCalledTimes(2) }) @@ -72,6 +76,26 @@ describe('runMatrix', () => { expect(runner.dispatch).toHaveBeenCalledTimes(2) // second cell still ran after the first threw }) + it('records durationMs on every cell — including failures — using the injected monotonic clock', async () => { + let clock = 1_000 + const runner: DispatchRunner = { + dispatch: vi.fn() + .mockImplementationOnce(async () => { clock += 42_000; return { runId: 'ok', prBranch: 'factory/x' } }) + .mockImplementationOnce(async () => { clock += 17_000; throw new Error('boom') }), + verify: vi.fn(async () => ({ passed: true })), + } + const { opts, appended } = deps(runner) + + await runMatrix([ + { task: task('a'), mode: 'single', repeat: 0 }, + { task: task('a'), mode: 'team', repeat: 0 }, + ], { ...opts, monotonicMs: () => clock }) + + expect(appended[0]?.durationMs).toBe(42_000) + expect(appended[1]?.durationMs).toBe(17_000) // the failure path timed the failed cell just like a success + expect(appended[1]?.notes).toContain('runner error') + }) + it('processes cells strictly sequentially, never overlapping two dispatches', async () => { const order: string[] = [] const runner: DispatchRunner = { diff --git a/benchmark/orchestrate.ts b/benchmark/orchestrate.ts index 2c23a6e8..2ff8a2e7 100644 --- a/benchmark/orchestrate.ts +++ b/benchmark/orchestrate.ts @@ -7,6 +7,11 @@ export interface OrchestrateDeps { appendResult: (result: BenchmarkResult) => Promise log: (message: string) => void now: () => string + /** + * Monotonic clock in milliseconds. Injected so tests can drive it + * deterministically; defaults to `performance.now()` at call sites. + */ + monotonicMs?: () => number } /** @@ -27,6 +32,8 @@ export async function runMatrix(cells: MatrixCell[], deps: OrchestrateDeps): Pro } async function runCell(cell: MatrixCell, deps: OrchestrateDeps): Promise { + const monotonic = deps.monotonicMs ?? (() => performance.now()) + const startedAtMs = monotonic() try { const outcome = await deps.runner.dispatch(cell.task, cell.mode) const verdict = await deps.runner.verify(cell.task, outcome) @@ -38,6 +45,7 @@ async function runCell(cell: MatrixCell, deps: OrchestrateDeps): Promise { expect(rows[0]?.meanWallClockMs).toBeUndefined() }) + // New shape: durationMs and costUsd are recorded directly on the result by + // orchestrate.runMatrix, so the report never needs an out-of-band samples + // file to fill in the headline wall-clock column. + it('reads durationMs and costUsd off the result row directly', () => { + const rows = buildReportRows([task], [ + { taskId: 'rename-error-type', mode: 'single', repeat: 0, runId: 'r1', passed: true, timestamp: 't', durationMs: 60_000, costUsd: 1 }, + { taskId: 'rename-error-type', mode: 'single', repeat: 1, runId: 'r2', passed: true, timestamp: 't', durationMs: 180_000, costUsd: 3 }, + ]) + + expect(rows[0]?.meanWallClockMs).toBe(120_000) + expect(rows[0]?.meanCostUsd).toBe(2) + }) + + it('averages only the rows that carry duration/cost, treating missing values as absent rather than 0', () => { + const rows = buildReportRows([task], [ + { taskId: 'rename-error-type', mode: 'single', repeat: 0, runId: 'r1', passed: true, timestamp: 't', durationMs: 120_000 }, + { taskId: 'rename-error-type', mode: 'single', repeat: 1, runId: 'r2', passed: false, timestamp: 't' }, // no durationMs + ]) + + // Only the sampled row counts toward the mean — no silent 0 bringing it down. + expect(rows[0]?.meanWallClockMs).toBe(120_000) + expect(rows[0]?.meanCostUsd).toBeUndefined() + }) + it('drops results for tasks no longer in the corpus instead of throwing', () => { const rows = buildReportRows([task], [ { taskId: 'deleted-task', mode: 'single', repeat: 0, runId: 'r1', passed: true, timestamp: 't' }, diff --git a/benchmark/report.ts b/benchmark/report.ts index 97e0b633673e9ebc2a2bbf953e1e22b29123e5ca..8f92537d326dc2047df1d2288fe51cf125e32989 100644 GIT binary patch delta 590 zcmb7=u};G<5Qe8x!N>#>NVhc^n(m-biLFDW3dCL~z7VU4ZTaldOnm{iz64_>MqY`9 zH^6Zk)CCDVoqhNJ_xXRj-oyTDu$@d`fo5!+gf*)i5=2^QS22qmvIdmNOC)RjJ@G?x zW{udyVA%>m8_bcT%<&BT@m6Y$25gB?;WNt{SmuiCBdc|Vv;-0yb7tVBLny6|wrq$}gMo+hEq~4Ax9Y0&-d_(*m|aLgw-n+R)qrBsJ4fWRP5CV@U6yE0bO^Czwuy^L_KsLM6{=5A$s#GagTmhU4Jt zZ3|IZG*J{{jc>YmK@dn+XFevs$u<`nRdHygX$qsRIf{3mgUfAi^$TJ9+b80Gz;nd^ c#}%;713DaY?dp?{(>@v6Er#&^I=mWw1JlUJJOBUy delta 190 zcmdm>utIjjM^=^O{Ji24h2;F=l41p0g{0Et?9>vy+{6M6jiS`z(wq`a1zWqxC)nkz zH1*Olb4pT+G&G76a|?1(HLVmN428^M1!tiCV6ZSyxq?%Eeoks)9!%k61CHGqU>hI? nL-m7HLYR6frA3J)nfZCX#hR19a45*&Qde4>qPbawQ-&P?B9cMC diff --git a/benchmark/schema.ts b/benchmark/schema.ts index 69ed7252..924357cc 100644 --- a/benchmark/schema.ts +++ b/benchmark/schema.ts @@ -48,6 +48,18 @@ export interface BenchmarkResult { notes?: string /** ISO timestamp the result was recorded. */ timestamp: string + /** + * Wall-clock the cell took, dispatch → verify inclusive. Optional so + * historical results.jsonl lines that pre-date this field still load; new + * rows always include it. + */ + durationMs?: number + /** + * USD spend attributed to this cell. Optional and left unset until Factory + * surfaces per-dispatch cost — the report treats undefined as "no cost + * sample" (renders `n/a`) rather than as 0. + */ + costUsd?: number } export const BenchmarkResultSchema = z.object({ @@ -58,6 +70,8 @@ export const BenchmarkResultSchema = z.object({ passed: z.boolean(), notes: z.string().optional(), timestamp: z.string().min(1), + durationMs: z.number().nonnegative().optional(), + costUsd: z.number().nonnegative().optional(), }) /** diff --git a/src/dispatch/templates.test.ts b/src/dispatch/templates.test.ts index c5c26772..015e99a5 100644 --- a/src/dispatch/templates.test.ts +++ b/src/dispatch/templates.test.ts @@ -103,6 +103,32 @@ describe('renderAgentTask', () => { expect(task).toContain('#swarm-ar-123') expect(task).toContain('Do not open your own pull request') expect(task).not.toContain('SWARM LEAD') + // The common-instructions block used to append "Push the branch to + // origin" for every implementer, contradicting the swarm-worker "do not + // push" clause and letting workers race the lead's commits on the shared + // branch. Workers must never see a push instruction, must not be told to + // open the PR, and must not be asked to DM the reviewer (the lead owns + // publication and reviewer handoff). + expect(task).not.toContain('Push the branch to origin') + expect(task).not.toContain('Factory will open the PR') + expect(task).not.toMatch(/Send reviewer .* a concise branch and commit summary/) + }) + + it('does not strip the push/PR instructions from a non-swarm implementer', () => { + const task = renderAgentTask({ + issue, + route: { repo: 'pear', clonePath: '/work/pear' }, + role: 'implementer', + config: baseConfig, + reviewerName: 'ar-123-review', + agentName: 'ar-123-impl', + }) + + // Regression fence for the swarm-worker gate: single/team implementers + // still receive the full commit/push/PR pipeline. Only role=worker in a + // swarm is stripped. + expect(task).toContain('Push the branch to origin') + expect(task).toContain('Factory will open the PR') }) it('omits swarm clauses entirely for non-swarm dispatch', () => { diff --git a/src/dispatch/templates.ts b/src/dispatch/templates.ts index 15dbc103..6c12a037 100644 --- a/src/dispatch/templates.ts +++ b/src/dispatch/templates.ts @@ -128,22 +128,44 @@ export function renderAgentTask(input: RenderAgentTaskInput): string { const swarmInstructions = input.swarm ? renderSwarmInstructions(input.swarm) : [] + // Swarm workers share the lead's checkout and branch. The lead alone + // commits/pushes the integrated result; workers must not race the lead's + // history nor expose an incomplete branch (see renderSwarmInstructions). + // Render commit/push/reviewer/lifecycle lines only for roles that own + // publication — everyone except a swarm worker. + const isSwarmWorker = input.swarm?.role === 'worker' + const branchLine = input.branchName && input.branchPrepared + ? isSwarmWorker + ? `Factory already prepared this isolated checkout on branch \`${input.branchName}\`. Do not reset it, switch branches, or recreate it; the lead commits and pushes the integrated result.` + : `Factory already prepared this isolated checkout on branch \`${input.branchName}\`. Do not reset it, switch branches, or recreate it; commit and push only this branch.` + : input.branchName + ? isSwarmWorker + ? `Continue on the exact branch \`${input.branchName}\` in this shared checkout. Do not reset it, switch branches, or push it — the lead publishes.` + : `Create a branch for this issue before editing. Create or reset the exact branch \`${input.branchName}\` from the repository default branch, then commit and push only this branch.` + : 'Create a branch for this issue before editing.' + const publicationInstructions = isSwarmWorker + ? [ + // A worker still commits its subtask locally so the lead can integrate + // it. It must not push, open a PR, or coordinate with the reviewer — + // the lead does that once the integrated branch is ready. + 'Commit your subtask locally on the shared branch so the lead can integrate it. Do NOT push, do NOT run `gh pr create`, and do NOT DM the reviewer — the lead owns publication and reviewer handoff.', + 'When your subtask is done or blocked, post on the shared swarm channel and output `/exit` on its own line. Do not call any Factory lifecycle action — the lead reports issue completion, not workers.', + ] + : [ + 'Commit the implementation and tests.', + 'Push the branch to origin.', + 'When implementation is complete, Factory will open the PR targeting the repository default branch through the connected GitHub workspace.', + 'Do not run `gh pr create` or require local GitHub CLI authentication.', + `Factory will hand the opened PR to reviewer \`${input.reviewerName}\`.`, + `Send reviewer \`${input.reviewerName}\` a concise branch and commit summary. If that direct delivery fails, do not fall back to a shared channel; Factory completion does not depend on this coordination message.`, + ] const common = [ ...header, ...swarmInstructions, '', - input.branchName && input.branchPrepared - ? `Factory already prepared this isolated checkout on branch \`${input.branchName}\`. Do not reset it, switch branches, or recreate it; commit and push only this branch.` - : input.branchName - ? `Create a branch for this issue before editing. Create or reset the exact branch \`${input.branchName}\` from the repository default branch, then commit and push only this branch.` - : 'Create a branch for this issue before editing.', - 'Commit the implementation and tests.', - 'Push the branch to origin.', - 'When implementation is complete, Factory will open the PR targeting the repository default branch through the connected GitHub workspace.', - 'Do not run `gh pr create` or require local GitHub CLI authentication.', - `Factory will hand the opened PR to reviewer \`${input.reviewerName}\`.`, - `Send reviewer \`${input.reviewerName}\` a concise branch and commit summary. If that direct delivery fails, do not fall back to a shared channel; Factory completion does not depend on this coordination message.`, - ...lifecycleInstructions(input, 'completed'), + branchLine, + ...publicationInstructions, + ...(isSwarmWorker ? [] : lifecycleInstructions(input, 'completed')), 'Do NOT auto-merge.', mergePolicyLine(input.config.mergePolicy), ] diff --git a/src/orchestrator/factory.test.ts b/src/orchestrator/factory.test.ts index 61fda72f..3da49a69 100644 --- a/src/orchestrator/factory.test.ts +++ b/src/orchestrator/factory.test.ts @@ -11405,6 +11405,70 @@ describe('FactoryLoop', () => { expect(worker?.task).toMatch(/SWARM WORKER/) }) + // Swarm workers register as implementers on the SAME shared checkout and + // lifecycle branch as the lead. Without a worker-specific gate, whichever + // worker finished first would publish (or wave through) the partial shared + // branch and mark every swarm member "done" via the shared-branch PR probe, + // releasing the still-working lead. A worker exit must therefore be a + // non-event for publication and completion. + it('suppresses a swarm worker exit so it never publishes or completes the shared checkout', async () => { + const routedIssue = realIssueFile(732, ready, { + labels: [{ name: 'pear' }, { name: 'agent:swarm' }], + }) + const mount = new FakeMountClient({ [issuePath(732)]: routedIssue }) + const fleet = new FakeFleetClient() + const factory = createFactory(config({ + triage: { maxImplementers: 2 }, + repos: { + byLabel: { pear: 'AgentWorkforce/pear' }, + byProject: {}, + keywordRules: [], + clonePaths: { 'AgentWorkforce/pear': '/work/pear' }, + default: 'AgentWorkforce/pear', + }, + }), { mount, fleet, triage: new StaticTriage() }) + + await factory.dispatch(await factory.triageIssue(parseLinearIssue(issuePath(732), routedIssue))) + + fleet.emitAgentExit('ar-732-impl-worker-1', 'issue-done') + await flush() + await flush() + + expect(factory.status().counters.swarmWorkerExitsSuppressed).toBe(1) + expect(factory.status().counters.done ?? 0).toBe(0) + expect(fleet.releases).toEqual([]) + // Lead is still in flight — its exit is the authoritative publication / + // completion signal, and we must not have raced it via the worker. + expect(factory.status().inFlight.map((issue) => issue.key)).toContain('AR-732') + }) + + it('does not suppress a swarm lead exit — the lead drives the normal completion path', async () => { + const routedIssue = realIssueFile(733, ready, { + labels: [{ name: 'pear' }, { name: 'agent:swarm' }], + }) + const mount = new FakeMountClient({ [issuePath(733)]: routedIssue }) + const fleet = new FakeFleetClient() + const factory = createFactory(config({ + triage: { maxImplementers: 2 }, + repos: { + byLabel: { pear: 'AgentWorkforce/pear' }, + byProject: {}, + keywordRules: [], + clonePaths: { 'AgentWorkforce/pear': '/work/pear' }, + default: 'AgentWorkforce/pear', + }, + }), { mount, fleet, triage: new StaticTriage() }) + + await factory.dispatch(await factory.triageIssue(parseLinearIssue(issuePath(733), routedIssue))) + + fleet.emitAgentExit('ar-733-impl-lead', 'issue-done') + await flush() + await flush() + + // The lead exit is NEVER treated as a worker suppression event. + expect(factory.status().counters.swarmWorkerExitsSuppressed ?? 0).toBe(0) + }) + it('fails dispatch loudly when no labels are present and no default repo is configured', async () => { const unlabeledIssue = realIssueFile(722, ready, { labels: [] }) const mount = new FakeMountClient({ [issuePath(722)]: unlabeledIssue }) diff --git a/src/orchestrator/factory.ts b/src/orchestrator/factory.ts index 268699aa..758a79ba 100644 --- a/src/orchestrator/factory.ts +++ b/src/orchestrator/factory.ts @@ -7407,6 +7407,17 @@ export class FactoryLoop implements Factory { } } + // Swarm workers share the lead's checkout and lifecycle branch. If a worker + // exit reached the publication/completion paths below, whichever worker + // finished first would publish whatever partial state was on the shared + // branch and mark every swarm member "done" via the shared-branch PR probe, + // releasing the still-working lead. The lead alone is authoritative for + // publication and completion in a swarm. + if (exiting?.spec.swarmRole === 'worker') { + this.#increment('swarmWorkerExitsSuppressed') + return + } + if (isCompletionReason(reason)) { if (exiting?.spec.role === 'implementer' && await this.#issueHasCompletionPr(record, { openOnly: this.#config.babysitter.enabled, From 8bf2dbc7f8ce991d79c8e0f73d685207e0c8c5f3 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Mon, 17 Aug 2026 11:39:29 +0200 Subject: [PATCH 3/5] chore: re-trigger CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GitHub Actions did not create a workflow run for 3e4496e — no check-suite exists for that commit — so the review threads landed but the CI table was empty. Nudge a new push so the CI workflow enqueues against the same tree. Session-Id: 8c8fcd37-6ade-4756-9a98-e5ced3cf5871 Session-Id: 8c8fcd37-6ade-4756-9a98-e5ced3cf5871 From 10ebede00db307ca60521d324f3c9357f22a2936 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Mon, 17 Aug 2026 12:10:24 +0200 Subject: [PATCH 4/5] fix(templates): split done/blocked instructions for swarm workers Cubic flagged that "Do not call any Factory lifecycle action" contradicts the durable human-input flow rendered below, which asks a blocked worker on a Linear-only issue to `invoke_action { kind: "blocked" }`. Rescope the ban to the completion action only, and point workers explicitly at the durable question flow for the blocked case. See PR #199 cubic thread on templates.ts:152. Co-Authored-By: Claude Opus 4.7 Session-Id: 8c8fcd37-6ade-4756-9a98-e5ced3cf5871 --- src/dispatch/templates.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/dispatch/templates.ts b/src/dispatch/templates.ts index 6c12a037..205e9c1e 100644 --- a/src/dispatch/templates.ts +++ b/src/dispatch/templates.ts @@ -149,7 +149,12 @@ export function renderAgentTask(input: RenderAgentTaskInput): string { // it. It must not push, open a PR, or coordinate with the reviewer — // the lead does that once the integrated branch is ready. 'Commit your subtask locally on the shared branch so the lead can integrate it. Do NOT push, do NOT run `gh pr create`, and do NOT DM the reviewer — the lead owns publication and reviewer handoff.', - 'When your subtask is done or blocked, post on the shared swarm channel and output `/exit` on its own line. Do not call any Factory lifecycle action — the lead reports issue completion, not workers.', + // Split "done" from "blocked": the lead is authoritative for the + // completion lifecycle action, but a worker blocked on a human answer + // still needs the durable question flow rendered below (github-issue + // writeback, or lifecycle `invoke_action { kind: "blocked" }`). + 'When your subtask is done, post the result on the shared swarm channel and output `/exit` on its own line. Do not call the Factory completion lifecycle action — the lead reports issue completion, not workers.', + 'If you are blocked and need a human answer instead, follow the durable human-input instructions below (do not exit before recording the request).', ] : [ 'Commit the implementation and tests.', From 6eab0f79642d029f7426d95e7872834debdcfb35 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Mon, 17 Aug 2026 12:19:10 +0200 Subject: [PATCH 5/5] fix(templates): make swarm worker blocked hint match the available route MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cubic's follow-up: my blocked-line assumed the durable question flow would always render below, but questionInstructions has a no-durable-route fallback (neither github metadata nor lifecycleActionName) that only says "report in your final outcome" — which contradicts "do not exit before recording the request." Split the blocked instruction on whether a durable recording route actually exists. Applied cubic's suggestion. Co-Authored-By: Claude Opus 4.7 Session-Id: 8c8fcd37-6ade-4756-9a98-e5ced3cf5871 --- src/dispatch/templates.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/dispatch/templates.ts b/src/dispatch/templates.ts index 205e9c1e..521dbeb9 100644 --- a/src/dispatch/templates.ts +++ b/src/dispatch/templates.ts @@ -154,7 +154,13 @@ export function renderAgentTask(input: RenderAgentTaskInput): string { // still needs the durable question flow rendered below (github-issue // writeback, or lifecycle `invoke_action { kind: "blocked" }`). 'When your subtask is done, post the result on the shared swarm channel and output `/exit` on its own line. Do not call the Factory completion lifecycle action — the lead reports issue completion, not workers.', - 'If you are blocked and need a human answer instead, follow the durable human-input instructions below (do not exit before recording the request).', + // The "durable instructions below" only exist when the issue has + // github metadata OR a lifecycle action; otherwise questionInstructions + // falls back to "report in your final outcome", which is a plain + // report, not a durable recording route. + ...(sourceGithubIssue || input.lifecycleActionName + ? ['If you are blocked and need a human answer instead, follow the durable human-input instructions below (do not exit before recording the request).'] + : ['If you are blocked, report one concrete question in your final outcome so Factory can route it for an answer.']), ] : [ 'Commit the implementation and tests.',