diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f5d92b..eb0af44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,63 @@ # Changelog +## v2.7.0 + +Routing, concurrency and cost. The factory now picks a model per issue type and stage, shares +one machine's slots and daily budget across every repo it watches, and can prove non-test work +(a blog post, a docs page) without ever writing a failing test. + +- **`routes` in `.factory/config.json`.** Each type label (`bug`, `feature`, `docs`, `security`, + `dependency`, or one a repo adds, e.g. `content`) can name its own agent per stage and its own + routed skills. `resolveRoute(agents, stages, routes, stage, type)` resolves + `routes[type].stages[stage]` → `stages[stage]` → `stages.default` → `"claude"`; triage always + uses `stages.triage`, since the type isn't known yet. `template/.factory/config.example.json` + ships the recommended default: triage on Haiku, plan on Opus, build and verify on Sonnet, PR + on Haiku, with `docs` builds on Haiku. With no config, the Claude CLI's own default model still + applies. +- **`src/context.ts`: a context pack, built once per stage.** `buildContextPack(stage, issue, cwd, + skills)` assembles AGENTS.md, the route's skills, `ARCHITECTURE.md` if the repo has one, and the + files named in `plan.json`, capped at 64 KiB; whatever doesn't fit is named, never truncated + mid-file. Claude gets it appended after `STAGE_GUIDANCE` on the same `--append-system-prompt`; + every other agent gets it inlined ahead of the artifact contract in its rendered prompt. +- **Pricing for the current models.** `claude-sonnet-5` and `claude-opus-5-5` price correctly + instead of costing NULL; `research/pricing/anthropic.md` captures the source table these prices + (and the already-priced `claude-haiku-4-5`) come from. +- **Concurrency belongs to the machine, not the repo.** `src/machine.ts` reads slots from + `FACTORY_SLOTS`, or `FACTORY_HOME/machine.json` (`{ "slots": N, "dailyUsd": N }`), validated at + boot. A repo's own `concurrency` becomes a cap on top of that: the limit is whichever is + smaller. Slots are leases in `machine.db` (`BEGIN IMMEDIATE`), reclaimed when a holder's pid has + died, so two watchers on one machine (splitbill-demo and lwp-website) share one pool instead of + each assuming the whole machine. `factory doctor` prints a suggested slot count from cores and + free memory, as advice only. +- **Continuous dispatch.** The watcher no longer waits for the whole pool to drain before polling + again; it keeps polling while jobs run and picks up new work the moment a slot frees, tracking + in-flight issues so nothing double-starts. +- **Spend caps.** `spend.perIssueUsd` and `dailyUsd` (repo and machine) are checked before every + stage, against the running sum of `stage_runs.cost_usd`. Over a cap, the issue parks with a + "budget" inbox item and intake pauses while the daily cap is hit; a stage with unknown cost + counts against a separate token cap so an unpriced model can't spend past every limit for free. +- **`proof: "test" | "check"` on the plan.** `test` (the default) keeps today's rule: build writes + a failing test first, verify checks it catches the bug. `check` names the commands that prove + each acceptance criterion instead: build runs them, verify re-runs them and judges the diff, and + never asks for a test. Both skills read `proof` off the plan artifact, never off the type name, so + a repo can declare work `proof: check` under any type, including one it added itself. The type + list is config-driven everywhere now; `TYPE_LABELS` is the default, not the only list. + +Structural tests: every `(type, stage)` pair in `routes` and `TYPE_LABELS` resolves to a defined +agent; `loadConfig` refuses at boot when a routed skill has no `SKILL.md` on disk, instead of +no-opping mid-run in the context pack; a `proof: check` plan (Markdown files, a prose-lint gate, +no test file) ships exactly like a `proof: test` plan; the app-agnostic grep now covers every +file under `template/.claude/skills` and `template/.claude/agents` for a named stack tool +(`npm`, `bun`, `pytest`, `cargo`, `go test`), since those are the files an agent actually reads at +runtime, unlike `gates.sh` or the CI workflow, which are allowed to be stack-specific; a property +test holds running jobs to the machine's slots across two watcher processes and confirms a freed +slot is picked up within one poll. + +Not in v2.7.0: +- The lwp-website pilot itself, `merge.policy`, and anything that merges (v2.8). +- A live agent run against splitbill-demo exercising the new routes and concurrency: pending your + reset OK. + ## v2.6.2 Any repo, safely. No new features: this closes the gaps a second repo (splitbill-demo plus diff --git a/bin/factory b/bin/factory index d24c960..8b01491 100755 --- a/bin/factory +++ b/bin/factory @@ -21,6 +21,7 @@ import { plain } from "../src/display"; import { act, buildInbox, inboxPositionals, type InboxAction } from "../src/inbox"; import { reset, rebaseline } from "../src/reset"; import { runDoctor, fixDoctor } from "../src/doctor"; +import { loadMachineConfig, MachineLeases, MachineSpend } from "../src/machine"; import { createDashboard } from "../dashboard/server"; import { versionOf, which } from "../src/probes"; import { workspacesDir as defaultWorkspacesDir, defaultStatePath } from "../src/paths"; @@ -77,12 +78,16 @@ function buildWatchDeps(cloneDir: string, config: FactoryConfig): WatchDeps { github: new GitHub(), git: new Git(new GitCommandRunner()), state: new FactoryState(flag("db") ?? process.env.FACTORY_DB_PATH ?? defaultStatePath(process.env, config.repo)), - executor: new CommandExecutor(config.agents, config.stages), + executor: new CommandExecutor(config.agents, config.stages, config.routes), gateRunner: new ShellGateRunner(), rechecker: verifierIsClaude(config) ? new ClaudeRechecker(new BinaryRunner("claude"), cloneDir) : undefined, cloneDir, workspacesDir: flag("workspaces") ?? defaultWorkspacesDir(process.env, config.repo), setupRunner: new ShellSetupRunner(), + // Real cross-process leases: every real CLI invocation (watch, run, tick) + // shares this machine's FACTORY_HOME/machine.db, so a second repo's + // watcher on the same machine is respected (plan v2.7.0 item 5). + machine: { leases: new MachineLeases(), config: loadMachineConfig(), spend: new MachineSpend() }, }; } @@ -306,9 +311,11 @@ async function cmdDoctor(): Promise { factoryMode: process.env.FACTORY_MODE, agents: config.agents, stages: config.stages, + routes: config.routes, templateSkills: templateSkills(), legacyStatePath: defaultStatePath(process.env), legacyWorkspacesDir: defaultWorkspacesDir(process.env), + configuredSlots: loadMachineConfig().slots, }, ); const allOk = checks.every((c) => c.ok || c.warn); @@ -316,7 +323,7 @@ async function cmdDoctor(): Promise { else for (const c of checks) console.log(` [${c.ok ? "ok" : c.warn ? "warn" : "FAIL"}] ${c.name} — ${c.detail}`); if (!allOk && has("fix")) { console.error("factory doctor --fix: creating missing labels"); - await fixDoctor(github, config.repo); + await fixDoctor(github, config.repo, config.routes); } if (!allOk && !has("fix")) process.exit(EXIT.checksFailed); } @@ -382,7 +389,7 @@ async function cmdVerifyAgent(): Promise { const loaded = await loadConfig(cloneDir); const config: FactoryConfig = { ...loaded, ...configFor(name) }; const out = resolve(flag("out") ?? `tests/fixtures/agents/${name}`); - const deps = { ...buildWatchDeps(cloneDir, config), executor: new CommandExecutor(config.agents, config.stages, new FixtureRecorder(out, name)) }; + const deps = { ...buildWatchDeps(cloneDir, config), executor: new CommandExecutor(config.agents, config.stages, config.routes, new FixtureRecorder(out, name)) }; const outcome = await advanceIssue(deps, config, await deps.github.getIssue(config.repo, issueNumber)); if (outcome === "awaiting-approval") { console.log(`Plan gate: read the plan on #${issueNumber}, approve it (/factory approve), then run this command again to finish and record the rest.`); diff --git a/dashboard/analytics.ts b/dashboard/analytics.ts index aaaabea..aeedf3f 100644 --- a/dashboard/analytics.ts +++ b/dashboard/analytics.ts @@ -22,6 +22,7 @@ export interface Analytics { readonly spend30dUsd: number; readonly byStage: readonly Bucket[]; readonly byAgent: readonly Bucket[]; + readonly byModel: readonly Bucket[]; } const FINISHED = new Set(["shipped", "failed", "rejected", "cancelled", "needs-human"]); @@ -56,5 +57,9 @@ export function analytics(runs: readonly Run[], stageRuns: readonly StageRun[], spend30dUsd: spendSince(30), byStage: bucketBy(stageRuns, (r) => r.stage), byAgent: bucketBy(stageRuns, (r) => r.agent), + // A run with no price on record (no agent-reported total, no pricing.ts + // entry) is still counted, under "unknown" — never dropped silently + // (plan v2.7.0 item 7: spend per model, visible even when incomplete). + byModel: bucketBy(stageRuns, (r) => r.model ?? "unknown"), }; } diff --git a/dashboard/public/app.js b/dashboard/public/app.js index ff997e0..c05fcf9 100644 --- a/dashboard/public/app.js +++ b/dashboard/public/app.js @@ -243,7 +243,7 @@ function analyticsView() { h("div", { class: "kpis" }, [[a.successRate == null ? "None yet" : `${Math.round(a.successRate * 1000) / 10}%`, "Runs that shipped, of those finished"], [String(a.runs), "Runs started"], [money(a.spend7dUsd), "Spent in 7 days"], [money(a.spend30dUsd), "Spent in 30 days"]] .map(([v, l]) => h("div", { class: "kpi" }, h("span", { class: "value" }, v), h("span", { class: "label" }, l)))), - bars("Cost by stage", a.byStage), bars("Cost by agent", a.byAgent)))); + bars("Cost by stage", a.byStage), bars("Cost by agent", a.byAgent), bars("Cost by model", a.byModel)))); } /* ---- Agents ---- */ diff --git a/package.json b/package.json index f6d8d7d..3723d81 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "software-factory", - "version": "2.6.2", + "version": "2.7.0", "private": true, "type": "module", "description": "GitHub-native SDLC loop for coding agents: triage, plan, build, verify, PR.", diff --git a/research/pricing/anthropic.md b/research/pricing/anthropic.md new file mode 100644 index 0000000..0f93529 --- /dev/null +++ b/research/pricing/anthropic.md @@ -0,0 +1,20 @@ +# Anthropic pricing capture + +Source: https://www.anthropic.com/pricing (redirects to https://claude.com/pricing). +Captured: 2026-09-27, via WebFetch. + +USD per million tokens. + +| Model | Input | Output | Cache read | Cache write | +|---|---|---|---|---| +| Opus 5.5 | 4 | 20 | 0.20 | 5 | +| Sonnet 5 | 2 | 10 | 0.20 | 2.50 | +| Haiku 4.5 | 1 | 5 | 0.10 | 1.25 | +| Fable 5.1 | 10 | 50 | 0.25 | 12.50 | +| Opus 5 / Opus 4.8 / 4.7 / 4.6 / 4.5 (legacy) | 5 | 25 | 0.50 | 6.25 | +| Sonnet 4.6 / 4.5 (legacy) | 3 | 15 | 0.30 | 3.75 | +| Fable 5 (legacy) | 10 | 50 | 1 | 12.50 | + +`src/pricing.ts` already carries `claude-haiku-4-5`, `claude-sonnet-4-5`, `claude-opus-4-5` under +these exact numbers (they match this capture; no change needed there). This capture adds +`claude-opus-5-5` and `claude-sonnet-5`, which were previously unpriced (NULL cost). diff --git a/src/agents/executor.ts b/src/agents/executor.ts index 944f625..66f1c09 100644 --- a/src/agents/executor.ts +++ b/src/agents/executor.ts @@ -7,12 +7,15 @@ import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { runDir } from "../artifacts"; +import type { RouteConfig } from "../config"; +import { buildContextPack } from "../context"; import { aggregateStageEvents, type Executor, type StageEvent, type StageRunOptions, type StageRunResult } from "../executor"; import { sanitizeEnv } from "./env"; import { renderPrompt } from "./prompt"; import { PRESETS } from "./presets"; import { structuredCommand } from "./structured"; import { replySchema } from "../schemas"; +import { typesFor } from "../labels"; import { writeReply } from "./reply"; import type { FixtureRecorder } from "./record"; import { type AgentConfig, type AgentPreset, type StageAgents, stagePolicy } from "./types"; @@ -30,8 +33,26 @@ export interface ResolvedAgent { readonly preset?: AgentPreset; } +// `routes[type].stages[stage]`, then `stages[stage]`, then `stages.default`, +// then "claude" (plan v2.7.0 item 1). Triage runs before the type is known, +// so it always uses `stages.triage` regardless of what `type` is passed. +export function resolveRoute( + agents: Record, + stages: StageAgents, + routes: Readonly> | undefined, + stage: keyof StageAgents & string, + type: string | undefined, +): ResolvedAgent { + const routed = stage !== "triage" && type ? routes?.[type]?.stages?.[stage] : undefined; + const name = routed ?? stages[stage] ?? stages.default ?? "claude"; + return resolveAgentByName(agents, name, stage); +} + export function resolveAgent(agents: Record, stages: StageAgents, stage: keyof StageAgents & string): ResolvedAgent { - const name = stages[stage] ?? stages.default ?? "claude"; + return resolveRoute(agents, stages, undefined, stage, undefined); +} + +function resolveAgentByName(agents: Record, name: string, stage: string): ResolvedAgent { const config = agents[name]; if (!config) throw new Error(`stage ${stage} uses agent "${name}", which is not in config.agents`); const preset = config.preset ? PRESETS[config.preset] : undefined; @@ -58,11 +79,12 @@ export class CommandExecutor implements Executor { constructor( private readonly agents: Record, private readonly stages: StageAgents, + private readonly routes: Readonly> = {}, private readonly recorder?: FixtureRecorder, ) {} async runStage(opts: StageRunOptions): Promise { - const agent = resolveAgent(this.agents, this.stages, opts.stage); + const agent = resolveRoute(this.agents, this.stages, this.routes, opts.stage, opts.type); const scratch = mkdtempSync(join(tmpdir(), `factory-scratch-${opts.issue}-`)); try { return await this.spawnStage(opts, agent, scratch); @@ -79,10 +101,13 @@ export class CommandExecutor implements Executor { const readOnly = !stagePolicy(opts.stage).write && agent.preset?.returnsArtifact === true && !agent.config.command; let argv: readonly string[]; let stdin: string | undefined; + const types = typesFor(this.routes); + const skills = opts.type ? this.routes[opts.type]?.skills ?? [] : []; + const contextPack = await buildContextPack(opts.stage, opts.issue, opts.cwd, skills); if (agent.preset?.ownsPrompt) { - ({ argv, stdin } = agent.preset.command(opts, agent.config, "")); + ({ argv, stdin } = agent.preset.command(opts, agent.config, contextPack)); } else { - const prompt = await renderPrompt(opts, readOnly); + const prompt = await renderPrompt(opts, readOnly, types, contextPack); if (agent.config.command) { const promptFile = join(scratch, "prompt.md"); writeFileSync(promptFile, prompt); @@ -93,7 +118,7 @@ export class CommandExecutor implements Executor { let schemaFile: string | undefined; if (readOnly && agent.config.outputSchema) { schemaFile = join(scratch, "reply.schema.json"); - writeFileSync(schemaFile, JSON.stringify(replySchema(opts.stage))); + writeFileSync(schemaFile, JSON.stringify(replySchema(opts.stage, types))); } ({ argv, stdin } = agent.preset.command(opts, agent.config, prompt, { schemaFile })); } else { diff --git a/src/agents/presets/claude.ts b/src/agents/presets/claude.ts index eda46c4..70c36c1 100644 --- a/src/agents/presets/claude.ts +++ b/src/agents/presets/claude.ts @@ -71,7 +71,11 @@ export function parseStreamJsonLine(line: string): StageEvent[] { return events; } -export function claudeArgs(opts: StageRunOptions): string[] { +// `contextPack` (buildContextPack, plan v2.7.0 item 3) rides the same +// --append-system-prompt as STAGE_GUIDANCE, appended after it. --help gives +// no guarantee that repeating the flag concatenates rather than overriding, +// so this sends one flag with one value rather than risk losing STAGE_GUIDANCE. +export function claudeArgs(opts: StageRunOptions, contextPack = ""): string[] { return [ "-p", `/factory-${opts.stage} ${opts.issue}`, @@ -87,7 +91,7 @@ export function claudeArgs(opts: StageRunOptions): string[] { "--settings", stageSettings(opts.stage, opts.issue, opts.agentCommands), "--append-system-prompt", - STAGE_GUIDANCE, + contextPack ? `${STAGE_GUIDANCE}\n\n${contextPack}` : STAGE_GUIDANCE, "--no-session-persistence", "--max-budget-usd", String(opts.maxBudgetUsd), @@ -104,7 +108,9 @@ export const claudePreset: AgentPreset = { skillsDir: ".claude/skills", contextFile: "CLAUDE.md", ownsPrompt: true, - command: (opts, agent) => ({ argv: ["claude", ...claudeArgs(opts), ...(agent.model ? ["--model", agent.model] : [])] }), + // `prompt` is the context pack for an ownsPrompt agent: Claude runs its own + // skill and ignores everything else spawnStage would otherwise put there. + command: (opts, agent, prompt) => ({ argv: ["claude", ...claudeArgs(opts, prompt), ...(agent.model ? ["--model", agent.model] : [])] }), parseLine: parseStreamJsonLine, isUsageCandidate: (line) => isUsageResultCandidate(line, "result"), }; diff --git a/src/agents/prompt.ts b/src/agents/prompt.ts index 754830e..9c81f9f 100644 --- a/src/agents/prompt.ts +++ b/src/agents/prompt.ts @@ -12,7 +12,9 @@ export function stripFrontmatter(text: string): string { return text.startsWith("---\n") ? text.slice(text.indexOf("\n---", 3) + 4).replace(/^\n+/, "") : text; } -export function artifactContract(opts: Pick, readOnly = false): string { +// `types` bounds the triage schema's `type` enum to the repo's actual type +// list (typesFor(config.routes)); omitted, it falls back to TYPE_LABELS. +export function artifactContract(opts: Pick, readOnly = false, types?: readonly string[]): string { if (readOnly) { return [ "## Artifact contract", @@ -20,7 +22,7 @@ export function artifactContract(opts: Pick, "This stage is read-only: you cannot write files. Do not try. Your final message must be ONE JSON object and nothing else, with no code fence:", `{"artifact": , "comment": "", "question": ""}`, "", - `The artifact must match this JSON Schema: ${JSON.stringify(stageSchema(opts.stage))}`, + `The artifact must match this JSON Schema: ${JSON.stringify(stageSchema(opts.stage, types))}`, "", "You have no GitHub access and cannot push or merge; the runner does that.", ].join("\n"); @@ -29,7 +31,7 @@ export function artifactContract(opts: Pick, "## Artifact contract", "", `You run in the issue's worktree. Write your results as files in $FACTORY_ARTIFACT_DIR (${runDir(opts.issue)}/):`, - `- ${JSON_FILENAMES[opts.stage]}: the structured result, exactly as the instructions above describe. Schema: ${JSON.stringify(stageSchema(opts.stage))}`, + `- ${JSON_FILENAMES[opts.stage]}: the structured result, exactly as the instructions above describe. Schema: ${JSON.stringify(stageSchema(opts.stage, types))}`, `- ${COMMENT_FILENAMES[opts.stage]}: the comment the runner posts on the issue.`, "- question-comment.md: only when you cannot proceed without an answer from a human.", "", @@ -37,7 +39,10 @@ export function artifactContract(opts: Pick, ].join("\n"); } -export async function renderPrompt(opts: StageRunOptions, readOnly = false): Promise { +// `contextPack` (buildContextPack, plan v2.7.0 item 3) is inlined here for +// every non-Claude agent; Claude gets the same text through +// --append-system-prompt instead (see agents/presets/claude.ts). +export async function renderPrompt(opts: StageRunOptions, readOnly = false, types?: readonly string[], contextPack = ""): Promise { const skill = await readFile(`${opts.cwd}/.claude/skills/factory-${opts.stage}/SKILL.md`, "utf8").catch(() => { throw new Error(`stage skill .claude/skills/factory-${opts.stage}/SKILL.md not found in ${opts.cwd}; run \`factory install --update\``); }); @@ -46,7 +51,8 @@ export async function renderPrompt(opts: StageRunOptions, readOnly = false): Pro "", stripFrontmatter(skill).trim(), "", - artifactContract(opts, readOnly), + ...(contextPack ? [contextPack, ""] : []), + artifactContract(opts, readOnly, types), "", ].join("\n"); } diff --git a/src/artifacts.ts b/src/artifacts.ts index e13e807..4db7802 100644 --- a/src/artifacts.ts +++ b/src/artifacts.ts @@ -22,7 +22,9 @@ export interface TriageArtifact { readonly outcome?: StepOutcome; readonly summary?: string; readonly disposition: Disposition; - readonly type: "bug" | "feature" | "docs" | "security" | "dependency"; + // One of TYPE_LABELS, or a type this repo added via config.routes (e.g. + // "content"); validated against the repo's actual type list in schemas.ts. + readonly type: string; readonly risk: Risk; readonly done_when: string; readonly files_expected: string[]; @@ -39,6 +41,9 @@ export interface PlanArtifact { readonly files: string[]; readonly autoApproveEligible: boolean; readonly commentId?: number; + // "test" (build writes a failing test first) or "check" (build runs named + // commands instead). Absent means "test" (plan v2.7.0 item 8). + readonly proof?: "test" | "check"; } export interface BuildArtifact { @@ -98,18 +103,20 @@ function stepEnvelopeProblem(o: Record): string | undefined { // pass as a silent no-op. The verdict has its own, deeper validator above. export const STEP_KEYS: Record, readonly string[]> = { triage: ["disposition", "type", "risk", "done_when", "files_expected", "gate_level", "confidence", "outcome", "summary"], - plan: ["status", "risk", "revision", "files", "autoApproveEligible", "commentId", "outcome", "summary"], + plan: ["status", "risk", "revision", "files", "autoApproveEligible", "commentId", "proof", "outcome", "summary"], build: ["status", "gate_line", "rounds", "outcome", "summary"], pr: ["outcome", "summary"], }; -export function validateStepJson(stage: Exclude, raw: unknown): { ok: true } | { ok: false; reason: string } { +// `types` bounds triage's `type` enum to the repo's actual type list +// (typesFor(config.routes)); omit it to fall back to the fixed TYPE_LABELS. +export function validateStepJson(stage: Exclude, raw: unknown, types?: readonly string[]): { ok: true } | { ok: false; reason: string } { const name = JSON_FILENAMES[stage]; if (typeof raw !== "object" || raw === null || Array.isArray(raw)) return { ok: false, reason: `${name} is not a JSON object` }; const o = raw as Record; const extra = unknownKey(o, new Set(STEP_KEYS[stage])); if (extra) return { ok: false, reason: `${name} has unknown field "${extra}"` }; - const problem = stepEnvelopeProblem(o) ?? schemaProblem(stage, o); + const problem = stepEnvelopeProblem(o) ?? schemaProblem(stage, o, types); return problem ? { ok: false, reason: `${name}: ${problem}` } : { ok: true }; } diff --git a/src/config.ts b/src/config.ts index 8d89caf..4499049 100644 --- a/src/config.ts +++ b/src/config.ts @@ -39,6 +39,20 @@ export interface GateSpec { readonly required: boolean; } +// A route for one issue type (a label: the five in TYPE_LABELS, or one a repo +// adds, e.g. lwp's "content"). `resolveRoute` (src/agents/executor.ts) checks +// `stages[stage]` here before falling back to the repo-wide `stages`. Triage +// runs before the type is known, so it never consults a route. +export interface RouteConfig { + readonly stages?: Partial; + readonly skills?: readonly string[]; + // "test" (default): build writes a failing test first, verify checks it + // catches the bug. "check": build runs the named commands instead of a + // test, verify re-runs them and a reviewer judges the diff (plan v2.7.0 + // item 8: a blog post can only ever end uncertain/reject under "test"). + readonly proof?: "test" | "check"; +} + export interface FactoryConfig { readonly repo: string; // "owner/name" readonly protectedPaths: readonly string[]; // globs the guard hook and triage refuse @@ -63,6 +77,21 @@ export interface FactoryConfig { // Named agents, each a preset or a command; `stages` says which one runs a stage. readonly agents: Readonly>; readonly stages: StageAgents; + // Per-type overrides, keyed by the type label. See RouteConfig. + readonly routes: Readonly>; + // Checked before every stage (plan v2.7.0 item 7). perIssueUsd is that + // issue's own lifetime spend; dailyUsd is this repo's spend since 00:00 UTC + // today. Either breached parks the issue with a "budget" inbox item. + // maxUnreportedRuns caps the count of stage runs with unknown cost (no + // price, no self-reported total) on one issue — a separate cap from the + // dollar ones, since an unpriced run can't be summed into them. + readonly spend: SpendConfig; +} + +export interface SpendConfig { + readonly perIssueUsd?: number; + readonly dailyUsd?: number; + readonly maxUnreportedRuns?: number; } export const DEFAULT_CONFIG: FactoryConfig = { @@ -83,6 +112,8 @@ export const DEFAULT_CONFIG: FactoryConfig = { agentCommands: { read: [], build: [], verify: [] }, agents: { claude: { preset: "claude" } }, stages: { default: "claude" }, + routes: {}, + spend: {}, }; export function mergeConfig(partial: Partial): FactoryConfig { @@ -94,6 +125,8 @@ export function mergeConfig(partial: Partial): FactoryConfig { agentCommands: { ...DEFAULT_CONFIG.agentCommands, ...partial.agentCommands }, agents: { ...DEFAULT_CONFIG.agents, ...partial.agents }, stages: { ...DEFAULT_CONFIG.stages, ...partial.stages }, + routes: { ...DEFAULT_CONFIG.routes, ...partial.routes }, + spend: { ...DEFAULT_CONFIG.spend, ...partial.spend }, }; } @@ -120,6 +153,8 @@ const TOP_LEVEL: Record = { agentCommands: "object", agents: "object", stages: "object", + routes: "object", + spend: "object", riskCriteria: "object", }; @@ -155,6 +190,7 @@ export function configProblems(raw: unknown): string[] { }; nested("riskPolicy", { autoApproveLowRisk: "boolean" }); nested("maxBudgetUsd", { triage: "positive", plan: "positive", build: "positive", verify: "positive", pr: "positive" }); + nested("spend", { perIssueUsd: "positive", dailyUsd: "positive", maxUnreportedRuns: "posInt" }); nested("agentCommands", { read: "strings", build: "strings", verify: "strings" }); if (cfg.gates !== undefined) { if (!Array.isArray(cfg.gates)) problems.push("gates: expected a list"); @@ -163,15 +199,17 @@ export function configProblems(raw: unknown): string[] { else checkKeys(g as Record, { name: "string", cmd: "string", required: "boolean" }, `gates[${i}].`, problems); }); } - problems.push(...agentProblems(cfg.agents, cfg.stages)); + problems.push(...agentProblems(cfg.agents, cfg.stages, cfg.routes)); return problems; } const STAGE_KEYS = ["default", "triage", "plan", "build", "verify", "pr"]; +// Triage runs before an issue's type is known, so a route can never override it. +const ROUTABLE_STAGE_KEYS = STAGE_KEYS.filter((s) => s !== "triage" && s !== "default"); // An agent is a preset or a command. `{{prompt}}` in the executable slot would // run the prompt as a program (assembler validateConfig), so it is refused. -function agentProblems(agents: unknown, stages: unknown): string[] { +function agentProblems(agents: unknown, stages: unknown, routes?: unknown): string[] { const problems: string[] = []; const named = new Set(["claude"]); if (agents !== undefined && typeof agents === "object" && agents !== null && !Array.isArray(agents)) { @@ -205,6 +243,29 @@ function agentProblems(agents: unknown, stages: unknown): string[] { else if (typeof agent !== "string" || !named.has(agent)) problems.push(`stages.${stage}: "${String(agent)}" is not an agent in config.agents`); } } else if (stages !== undefined) problems.push("stages: expected an object"); + if (routes !== undefined && typeof routes === "object" && routes !== null && !Array.isArray(routes)) { + for (const [type, raw] of Object.entries(routes as Record)) { + if (type.startsWith("_")) continue; + const where = `routes.${type}.`; + if (typeof raw !== "object" || raw === null || Array.isArray(raw)) { + problems.push(`routes.${type}: expected an object`); + continue; + } + const r = raw as Record; + checkKeys(r, { stages: "object", skills: "strings", proof: "string" }, where, problems); + if (r.proof !== undefined && r.proof !== "test" && r.proof !== "check") problems.push(`${where}proof: expected "test" or "check", got ${JSON.stringify(r.proof)}`); + if (r.stages !== undefined) { + if (typeof r.stages !== "object" || r.stages === null || Array.isArray(r.stages)) problems.push(`${where}stages: expected an object`); + else { + for (const [stage, agent] of Object.entries(r.stages as Record)) { + if (stage.startsWith("_")) continue; + if (!ROUTABLE_STAGE_KEYS.includes(stage)) problems.push(`${where}stages.${stage}: unknown stage (allowed: ${ROUTABLE_STAGE_KEYS.join(", ")})`); + else if (typeof agent !== "string" || !named.has(agent)) problems.push(`${where}stages.${stage}: "${String(agent)}" is not an agent in config.agents`); + } + } + } + } + } else if (routes !== undefined) problems.push("routes: expected an object"); return problems; } @@ -237,6 +298,16 @@ export async function loadConfig(targetRepoDir: string): Promise if (origin && origin.toLowerCase() !== config.repo.toLowerCase()) { throw new ConfigError(`${path}: "repo" is ${config.repo} but this clone's origin is ${origin}; fix "repo" or remove it to use the origin`); } + // configProblems is pure and cannot see the worktree, so a route naming a + // skill that was never installed would otherwise only surface as a silent + // no-op in buildContextPack, mid-run (plan v2.7.0 item 3). + const missing: string[] = []; + for (const [type, route] of Object.entries(config.routes)) { + for (const name of route.skills ?? []) { + if (!existsSync(`${targetRepoDir}/.claude/skills/${name}/SKILL.md`)) missing.push(`routes.${type}.skills: "${name}" (.claude/skills/${name}/SKILL.md not found)`); + } + } + if (missing.length > 0) throw new ConfigError(`${path} names a skill that does not exist:\n - ${missing.join("\n - ")}`); return config; } diff --git a/src/context.ts b/src/context.ts new file mode 100644 index 0000000..dee3986 --- /dev/null +++ b/src/context.ts @@ -0,0 +1,85 @@ +// Plan v2.7.0 item 3: what an agent would otherwise spend tool calls +// discovering, assembled once, up front. Held pack: AGENTS.md, the skills its +// route adds for the issue's type, ARCHITECTURE.md if the repo has one, and +// the files the plan named. Capped at 64 KiB; whatever does not fit is named +// in the pack, never silently truncated mid-file. + +import type { PlanArtifact } from "./artifacts"; +import { readStageArtifacts } from "./artifacts"; +import type { StageName } from "./executor"; +import { stripFrontmatter } from "./agents/prompt"; + +export const MAX_CONTEXT_PACK_BYTES = 64 * 1024; + +interface Section { + readonly label: string; + readonly body: string; +} + +async function readOptional(path: string): Promise { + const file = Bun.file(path); + if (!(await file.exists())) return undefined; + const text = await file.text(); + return text.trim() ? text : undefined; +} + +async function planFiles(cwd: string, issue: number): Promise { + const { json } = await readStageArtifacts(cwd, issue, "plan"); + const files = (json as Partial | undefined)?.files; + return Array.isArray(files) ? files.filter((f): f is string => typeof f === "string") : []; +} + +async function sections(issue: number, cwd: string, skills: readonly string[]): Promise { + const out: Section[] = []; + const agentsMd = await readOptional(`${cwd}/AGENTS.md`); + if (agentsMd) out.push({ label: "AGENTS.md", body: agentsMd.trim() }); + + for (const name of skills) { + const body = await readOptional(`${cwd}/.claude/skills/${name}/SKILL.md`); + if (body) out.push({ label: `skill: ${name}`, body: stripFrontmatter(body).trim() }); + } + + const architectureMd = await readOptional(`${cwd}/ARCHITECTURE.md`); + if (architectureMd) out.push({ label: "ARCHITECTURE.md", body: architectureMd.trim() }); + + // Triage runs before a plan exists, so this is empty until plan and later. + for (const path of await planFiles(cwd, issue)) { + const body = await readOptional(`${cwd}/${path}`); + if (body) out.push({ label: `file: ${path}`, body: body.trim() }); + } + + return out; +} + +// Sections are added highest-priority first (repo conventions and skills +// ahead of the plan's own files), and the budget is enforced in that same +// order: the first sections over budget get dropped, not the file content +// inside a kept section truncated. +function assemble(built: readonly Section[]): string { + const header = "## Context pack\n\n"; + let used = Buffer.byteLength(header); + const kept: Section[] = []; + const dropped: string[] = []; + for (const s of built) { + const block = `### ${s.label}\n\n${s.body}\n\n`; + const bytes = Buffer.byteLength(block); + if (used + bytes > MAX_CONTEXT_PACK_BYTES) { + dropped.push(s.label); + continue; + } + used += bytes; + kept.push(s); + } + if (!kept.length && !dropped.length) return ""; + const droppedNote = dropped.length ? `Dropped for the 64 KiB context-pack budget: ${dropped.join(", ")}.\n\n` : ""; + return `${header}${droppedNote}${kept.map((s) => `### ${s.label}\n\n${s.body}\n\n`).join("")}`.trim(); +} + +// "" means there is nothing to add: callers skip injecting it rather than +// appending an empty "## Context pack" header. `stage` is part of the +// signature (plan v2.7.0 item 3) for a future stage-specific pack; every +// stage gets the same sections today. +export async function buildContextPack(stage: StageName, issue: number, cwd: string, skills: readonly string[] = []): Promise { + void stage; + return assemble(await sections(issue, cwd, skills)); +} diff --git a/src/doctor.ts b/src/doctor.ts index 96c4bc8..ef5bb48 100644 --- a/src/doctor.ts +++ b/src/doctor.ts @@ -2,9 +2,11 @@ // starts, checked once instead of failing three stages in. import type { CommandRunner, GitHub } from "./github"; -import { LABELS } from "./labels"; +import { labelsFor } from "./labels"; +import type { RouteConfig } from "./config"; import type { AgentConfig, StageAgents } from "./agents/types"; import { PRESETS } from "./agents/presets"; +import { suggestSlots } from "./machine"; export interface DoctorCheck { readonly name: string; @@ -33,6 +35,7 @@ export interface DoctorContext { readonly factoryMode?: string; // FACTORY_MODE, "actions" when CI drives the loop readonly agents?: Record; readonly stages?: StageAgents; + readonly routes?: Readonly>; // Shipped skill files (path relative to the repo root -> content), to spot an install that predates this runner. readonly templateSkills?: Record; // The pre-v2.6.2 shared paths (`defaultStatePath`/`workspacesDir` called @@ -42,6 +45,10 @@ export interface DoctorContext { // check only names the path and leaves the move to the operator. readonly legacyStatePath?: string; readonly legacyWorkspacesDir?: string; + // The machine's own slot count (loadMachineConfig().slots), so doctor can + // compare it against suggestSlots()'s cores/memory heuristic. Advice only + // (plan v2.7.0 item 5): nothing here enforces the suggestion. + readonly configuredSlots?: number; } // Flags that let a CLI run headless without waiting on an approval prompt. @@ -204,7 +211,7 @@ export async function runDoctor(deps: DoctorDeps, ctx: DoctorContext): Promise !existingLabels.has(l.name)); + const missing = labelsFor(ctx.routes).filter((l) => !existingLabels.has(l.name)); checks.push({ name: "all factory labels exist", ok: missing.length === 0, @@ -237,11 +244,22 @@ export async function runDoctor(deps: DoctorDeps, ctx: DoctorContext): Promise { - for (const label of LABELS) { +export async function fixDoctor(github: GitHub, repo: string, routes?: Readonly>): Promise { + for (const label of labelsFor(routes)) { await github.ensureLabel(repo, label.name, label.color, label.description); } } diff --git a/src/executor.ts b/src/executor.ts index 9035571..c45e20d 100644 --- a/src/executor.ts +++ b/src/executor.ts @@ -38,6 +38,10 @@ export interface StageRunOptions { readonly agentCommands?: AgentCommands; // The agent name from config; recorded in stage_runs. Unset means "claude". readonly agent?: string; + // The issue's type label (e.g. "docs", or a repo-added type like "content"), + // read from its GitHub labels. Unset on triage, which runs before the type + // is known and so never routes on it (plan v2.7.0 item 1). + readonly type?: string; } export interface StageRunResult { diff --git a/src/inbox.ts b/src/inbox.ts index 713ac27..695b3ac 100644 --- a/src/inbox.ts +++ b/src/inbox.ts @@ -4,11 +4,12 @@ // through chatops.ts and its trust rule instead of a second command path. import { parseChatOps } from "./chatops"; +import { parseDataMarkers } from "./derive"; import { plain } from "./display"; import type { GhComment, GhIssue, GitHub } from "./github"; import { LABEL, PARKED_LABELS } from "./labels"; -export type InboxKind = "approve-plan" | "answer-question" | "review-pr" | "parked" | "failed"; +export type InboxKind = "approve-plan" | "answer-question" | "review-pr" | "parked" | "failed" | "budget"; export type InboxAction = "approve" | "revise" | "answer" | "retry" | "cancel"; // Actions that carry the human's own words. @@ -48,12 +49,23 @@ function latestRunnerComment(comments: readonly GhComment[]): GhComment | undefi return [...comments].reverse().find((c) => c.body.includes("\ntriage"); + writeFileSync( + join(dir, "triage.json"), + JSON.stringify({ disposition: "needs-info", type: "bug", risk: "low", done_when: "x", files_expected: ["src/a.ts"], gate_level: "make check", confidence: 0.9 }), + ); + writeFileSync(join(dir, "question-comment.md"), "\nWhich one?"); + this.active.delete(n); + return { events: [], toolCalls: 0, tokensIn: 0, tokensOut: 0, costUsd: 0.01, costReported: true, exitCode: 0, permissionDenials: [] }; + } +} + +function setup(delayMsFor: (issue: number) => number) { + const workspacesDir = mkdtempSync(join(tmpdir(), "factory-ws-")); + const cloneDir = mkdtempSync(join(tmpdir(), "factory-clone-")); + const github = new FakeGitHub([baseIssue(1, ["factory:ready"])]); + const executor = new SlowTriageExecutor(delayMsFor); + const config = mergeConfig({ repo: "acme/repo", concurrency: 1 }); + const deps = { github, git: new FakeGit(), state: new FactoryState(":memory:"), executor, gateRunner: new FakeGateRunner(), cloneDir, workspacesDir }; + return { deps, config, github, executor, workspacesDir, cloneDir }; +} + +describe("pollOnce's inFlight set makes overlapping polls (what startWatch now runs) safe", () => { + test("a second, overlapping poll never redispatches an issue the first poll is still running", async () => { + const { deps, config, executor, workspacesDir, cloneDir } = setup(() => 30); + const inFlight = new Set(); + + // Fired back to back, with no await between them: `dispatch()` relabels + // the issue away from "ready" before its slow stage run, so a real gap + // here would let that relabel alone close the race and the test would + // pass whether or not `inFlight` does anything. Only a same-tick start + // puts both polls' own `listIssuesByLabel` calls before either one's + // relabel resolves, so `inFlight` (set synchronously, no await) is the + // only thing left that can close it. + const first = pollOnce(deps, config, inFlight); + const second = pollOnce(deps, config, inFlight); + await Promise.all([first, second]); + + expect(executor.concurrentDup).toBe(false); + expect(executor.starts).toEqual([1]); // the second poll saw #1 already in flight and skipped it + expect(inFlight.size).toBe(0); // bookkeeping is cleared once each poll's batch finishes + + rmSync(workspacesDir, { recursive: true, force: true }); + rmSync(cloneDir, { recursive: true, force: true }); + }); + + test("a newly-surfaced issue is picked up by an overlapping poll without waiting for an earlier poll's batch to finish", async () => { + // #1 is deliberately much slower than #2, so #2's dispatch (started by + // the later, overlapping poll) can finish while #1's is still running. + const { deps, config, github, executor, workspacesDir, cloneDir } = setup((n) => (n === 1 ? 200 : 10)); + const inFlight = new Set(); + + const first = pollOnce(deps, config, inFlight); // dispatches #1, takes 200ms + await Bun.sleep(5); + github.issues.set(2, baseIssue(2, ["factory:ready"])); // surfaces mid-batch + const second = pollOnce(deps, config, inFlight); // must not wait on `first` + + await second; + // #2 was picked up and #1 was still running: proves the second poll did + // not block on the first poll's still-in-flight batch. + expect(executor.active.has(1)).toBe(true); + expect(executor.starts).toContain(2); + + await first; + expect(executor.concurrentDup).toBe(false); + + rmSync(workspacesDir, { recursive: true, force: true }); + rmSync(cloneDir, { recursive: true, force: true }); + }); +}); diff --git a/tests/machine-dispatch.test.ts b/tests/machine-dispatch.test.ts new file mode 100644 index 0000000..7754e37 --- /dev/null +++ b/tests/machine-dispatch.test.ts @@ -0,0 +1,95 @@ +// The unit MachineLeases test proves the SQLite lease table itself never +// oversubscribes; this proves watch.ts's own dispatch path actually calls +// into it. Two independent WatchDeps (standing in for two watcher processes +// — splitbill-demo's and lwp-website's, say) share one MachineLeases pointed +// at the same db file. Whatever the mix of stage-run lengths, the number of +// simultaneously-running stages across both never exceeds the machine's slots. + +import { describe, expect, test } from "bun:test"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { runDir } from "../src/artifacts"; +import { mergeConfig } from "../src/config"; +import type { StageRunOptions, StageRunResult } from "../src/executor"; +import { FactoryState } from "../src/state"; +import { pollOnce } from "../src/watch"; +import { MachineLeases, MachineSpend } from "../src/machine"; +import { baseIssue, FakeGateRunner, FakeGit, FakeGitHub } from "./harness"; + +// Every stage run this executor serves takes `delayMs` and always ends the +// issue at "needs-info" after triage alone — one runStage call is enough to +// create overlap between issues, and stopping there keeps the fixture tiny. +// `shared` is the same object across both "processes" in the test below, so +// it measures real concurrent execution directly, independent of whether the +// lease mechanism under test is the thing actually bounding it. +class SlowTriageExecutor { + constructor(private readonly delayMs: number, private readonly shared: { active: number; max: number }) {} + + async runStage(opts: StageRunOptions): Promise { + this.shared.active += 1; + this.shared.max = Math.max(this.shared.max, this.shared.active); + await Bun.sleep(this.delayMs); + const dir = join(opts.cwd, runDir(opts.issue)); + mkdirSync(dir, { recursive: true }); + writeFileSync(join(dir, "triage-comment.md"), "\ntriage"); + writeFileSync( + join(dir, "triage.json"), + JSON.stringify({ disposition: "needs-info", type: "bug", risk: "low", done_when: "x", files_expected: ["src/a.ts"], gate_level: "make check", confidence: 0.9 }), + ); + writeFileSync(join(dir, "question-comment.md"), "\nWhich one?"); + this.shared.active -= 1; + return { events: [], toolCalls: 0, tokensIn: 0, tokensOut: 0, costUsd: 0.01, costReported: true, exitCode: 0, permissionDenials: [] }; + } +} + +function makeDeps(repo: string, issues: number[], executor: SlowTriageExecutor, leases: MachineLeases, slots: number, spend: MachineSpend) { + const workspacesDir = mkdtempSync(join(tmpdir(), "factory-ws-")); + const cloneDir = mkdtempSync(join(tmpdir(), "factory-clone-")); + const github = new FakeGitHub(issues.map((n) => baseIssue(n, ["factory:ready"]))); + const config = mergeConfig({ repo, concurrency: issues.length }); + const deps = { + github, + git: new FakeGit(), + state: new FactoryState(":memory:"), + executor, + gateRunner: new FakeGateRunner(), + cloneDir, + workspacesDir, + machine: { leases, config: { slots }, spend }, + }; + return { deps, config, workspacesDir, cloneDir }; +} + +describe("pollOnce respects a machine-wide lease across two independent WatchDeps", () => { + test("running stages never exceed the machine's slot count, whatever the mix of job lengths", async () => { + const dbDir = mkdtempSync(join(tmpdir(), "factory-machine-")); + const dbPath = join(dbDir, "machine.db"); + const leasesA = new MachineLeases(dbPath); + const leasesB = new MachineLeases(dbPath); + const spendA = new MachineSpend(dbPath); + const spendB = new MachineSpend(dbPath); + const slots = 2; + + // Shared across both "processes": measures real concurrent execution + // directly, so a broken lease shows up here even if held() itself can't + // be trusted (e.g. a break that skips acquiring a lease altogether). + const shared = { active: 0, max: 0 }; + const execA = new SlowTriageExecutor(15, shared); + const execB = new SlowTriageExecutor(5, shared); + const a = makeDeps("acme/repo-a", [1, 2, 3], execA, leasesA, slots, spendA); + const b = makeDeps("acme/repo-b", [1, 2, 3], execB, leasesB, slots, spendB); + + await Promise.all([pollOnce(a.deps, a.config), pollOnce(b.deps, b.config)]); + + expect(shared.max).toBeLessThanOrEqual(slots); + expect(shared.max).toBeGreaterThan(0); // the run actually overlapped, not just ran serially + expect(leasesA.held()).toBe(0); // every lease is released once its stage finishes + + leasesA.close(); + leasesB.close(); + spendA.close(); + spendB.close(); + for (const dir of [a.workspacesDir, a.cloneDir, b.workspacesDir, b.cloneDir, dbDir]) rmSync(dir, { recursive: true, force: true }); + }); +}); diff --git a/tests/machine.test.ts b/tests/machine.test.ts new file mode 100644 index 0000000..476aeac --- /dev/null +++ b/tests/machine.test.ts @@ -0,0 +1,124 @@ +// Machine-wide slot leases (plan v2.7.0 item 5): two watcher processes on one +// machine must never together hold more leases than the machine's own slot +// count, whatever the mix of job lengths. Simulated here as two independent +// MachineLeases instances pointed at the same on-disk db, which is exactly +// what two real `factory watch` processes sharing FACTORY_HOME would be. + +import { describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { effectiveSlots, loadMachineConfig, MachineLeases, suggestSlots } from "../src/machine"; + +function tmpDb(): string { + const dir = mkdtempSync(join(tmpdir(), "factory-machine-")); + return join(dir, "machine.db"); +} + +describe("MachineLeases", () => { + test("a slot can be acquired and released", () => { + const leases = new MachineLeases(tmpDb()); + const id = leases.acquire("a", 2); + expect(id).not.toBeUndefined(); + expect(leases.held()).toBe(1); + leases.release(id!); + expect(leases.held()).toBe(0); + leases.close(); + }); + + test("acquiring past the slot count returns undefined, never oversubscribes", () => { + const leases = new MachineLeases(tmpDb()); + const a = leases.acquire("a", 1); + const b = leases.acquire("b", 1); + expect(a).not.toBeUndefined(); + expect(b).toBeUndefined(); + expect(leases.held()).toBe(1); + leases.close(); + }); + + test("a lease held by a dead pid is reclaimed on the next acquire", () => { + const leases = new MachineLeases(tmpDb()); + // A pid this high is never a real running process. + const dead = leases.acquire("a", 1, 999_999_999); + expect(dead).not.toBeUndefined(); + const next = leases.acquire("b", 1); + expect(next).not.toBeUndefined(); + expect(leases.held()).toBe(1); + leases.close(); + }); + + // Property: whatever the mix of job lengths (random delays here), the + // number of simultaneously-held leases across two independent leaseholders + // sharing one db file never exceeds the machine's own slot count. + test("across two processes sharing one db, held leases never exceed the slot count", async () => { + const path = tmpDb(); + const slots = 3; + const procA = new MachineLeases(path); + const procB = new MachineLeases(path); + let maxHeld = 0; + let observedSlots: string[] = []; + + async function job(leases: MachineLeases, holder: string): Promise { + for (let i = 0; i < 8; i++) { + let id: number | undefined; + for (;;) { + id = leases.acquire(holder, slots); + if (id !== undefined) break; + await Bun.sleep(1); + } + observedSlots.push(holder); + maxHeld = Math.max(maxHeld, Math.max(procA.held(), procB.held())); + // Re-check the true global count (both processes' leases are in the + // same table), not just this instance's local view. + const global = procA.held(); + maxHeld = Math.max(maxHeld, global); + expect(global).toBeLessThanOrEqual(slots); + await Bun.sleep(Math.random() * 5); + leases.release(id); + } + } + + await Promise.all([job(procA, "A"), job(procB, "B")]); + expect(observedSlots.length).toBe(16); + expect(procA.held()).toBe(0); + procA.close(); + procB.close(); + }); +}); + +describe("effectiveSlots", () => { + test("is the smaller of the repo's own concurrency and the machine's slots", () => { + expect(effectiveSlots(5, { slots: 2 })).toBe(2); + expect(effectiveSlots(1, { slots: 5 })).toBe(1); + }); +}); + +describe("loadMachineConfig", () => { + test("FACTORY_SLOTS wins over machine.json and must be a positive integer", () => { + expect(loadMachineConfig({ FACTORY_SLOTS: "4", HOME: "/nonexistent" })).toEqual({ slots: 4 }); + expect(() => loadMachineConfig({ FACTORY_SLOTS: "0", HOME: "/nonexistent" })).toThrow(/positive integer/); + expect(() => loadMachineConfig({ FACTORY_SLOTS: "abc", HOME: "/nonexistent" })).toThrow(/positive integer/); + }); + + test("naming neither falls back to the default", () => { + const dir = mkdtempSync(join(tmpdir(), "factory-home-")); + expect(loadMachineConfig({ FACTORY_HOME: dir })).toEqual({ slots: 2 }); + rmSync(dir, { recursive: true, force: true }); + }); + + test("machine.json is read and validated when present", () => { + const dir = mkdtempSync(join(tmpdir(), "factory-home-")); + Bun.write(`${dir}/machine.json`, JSON.stringify({ slots: 3, dailyUsd: 20 })); + expect(loadMachineConfig({ FACTORY_HOME: dir })).toEqual({ slots: 3, dailyUsd: 20 }); + Bun.write(`${dir}/machine.json`, JSON.stringify({ slots: 0 })); + expect(() => loadMachineConfig({ FACTORY_HOME: dir })).toThrow(/positive integer/); + rmSync(dir, { recursive: true, force: true }); + }); +}); + +describe("suggestSlots", () => { + test("is a positive integer", () => { + expect(Number.isInteger(suggestSlots())).toBe(true); + expect(suggestSlots()).toBeGreaterThanOrEqual(1); + }); +}); diff --git a/tests/routes.test.ts b/tests/routes.test.ts new file mode 100644 index 0000000..3038c47 --- /dev/null +++ b/tests/routes.test.ts @@ -0,0 +1,156 @@ +// v2.7.0 item 1/8: a route overrides which agent runs a stage for one issue +// type, and the type list is config-driven, not fixed at the five defaults. +// The structural test: whatever routes and TYPE_LABELS name, every (type, +// stage) pair resolves to a real agent, so a typo in config.json fails at +// boot, not mid-run on some future issue that happens to hit the gap. + +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterAll, describe, expect, test } from "bun:test"; +import { resolveAgent, resolveRoute } from "../src/agents/executor"; +import { configProblems, DEFAULT_CONFIG, loadConfig, mergeConfig, type FactoryConfig } from "../src/config"; +import type { StageName } from "../src/executor"; +import { issueType, labelsFor, TYPE_LABELS, typesFor } from "../src/labels"; + +const STAGES: readonly (keyof FactoryConfig["stages"] & string)[] = ["triage", "plan", "build", "verify", "pr"]; + +describe("resolveRoute", () => { + const agents = { opus: { preset: "claude", model: "opus" }, sonnet: { preset: "claude", model: "sonnet" }, haiku: { preset: "claude", model: "haiku" } }; + const stages = { default: "sonnet", triage: "haiku" }; + const routes = { docs: { stages: { build: "haiku" } } }; + + test("a route's stage agent wins over the repo-wide stage agent", () => { + expect(resolveRoute(agents, stages, routes, "build", "docs").name).toBe("haiku"); + }); + + test("falls back to the repo-wide stage agent when the route has none for this stage", () => { + expect(resolveRoute(agents, stages, routes, "verify", "docs").name).toBe("sonnet"); + }); + + test("falls back to stages.default when neither the route nor the stage names an agent", () => { + expect(resolveRoute(agents, stages, routes, "pr", "docs").name).toBe("sonnet"); + }); + + test("falls back to \"claude\" when nothing names an agent and there is no default", () => { + expect(resolveRoute({ claude: { preset: "claude" } }, {}, undefined, "pr", undefined).name).toBe("claude"); + }); + + test("triage never routes on type, even when a route names a triage agent", () => { + const withTriageRoute = { docs: { stages: { triage: "opus", build: "haiku" } } }; + expect(resolveRoute(agents, stages, withTriageRoute, "triage", "docs").name).toBe("haiku"); // stages.triage, not the route + }); + + test("an unrouted type behaves exactly like resolveAgent", () => { + expect(resolveRoute(agents, stages, routes, "verify", "bug").name).toBe(resolveAgent(agents, stages, "verify").name); + }); + + test("every type in routes and TYPE_LABELS, crossed with every stage, resolves to a defined agent", () => { + const config = mergeConfig({ repo: "a/b", agents, stages: { default: "sonnet", triage: "haiku", plan: "opus", build: "sonnet", verify: "sonnet", pr: "haiku" }, routes }); + for (const type of typesFor(config.routes)) { + for (const stage of STAGES) { + expect(() => resolveRoute(config.agents, config.stages, config.routes, stage, type)).not.toThrow(); + } + } + }); + + test("the shipped example config resolves every (type, stage) pair", () => { + const example = JSON.parse(readFileSync(join(import.meta.dir, "../template/.factory/config.example.json"), "utf8")); + const config = mergeConfig({ ...example, repo: "a/b" }); + for (const type of typesFor(config.routes)) { + for (const stage of STAGES) { + expect(() => resolveRoute(config.agents, config.stages, config.routes, stage, type)).not.toThrow(); + } + } + }); +}); + +describe("config validation of routes", () => { + test("a route naming an agent outside config.agents is refused", () => { + const problems = configProblems({ ...DEFAULT_CONFIG, repo: "a/b", agents: { claude: { preset: "claude" } }, routes: { docs: { stages: { build: "ghost" } } } }); + expect(problems).toContain('routes.docs.stages.build: "ghost" is not an agent in config.agents'); + }); + + test("a route naming triage is refused: triage never routes on type", () => { + const problems = configProblems({ ...DEFAULT_CONFIG, repo: "a/b", routes: { docs: { stages: { triage: "claude" } } } }); + expect(problems).toContain("routes.docs.stages.triage: unknown stage (allowed: plan, build, verify, pr)"); + }); + + test("proof must be \"test\" or \"check\"", () => { + const problems = configProblems({ ...DEFAULT_CONFIG, repo: "a/b", routes: { content: { proof: "vibes" } } }); + expect(problems).toContain('routes.content.proof: expected "test" or "check", got "vibes"'); + }); + + test("a well-formed routes block validates cleanly", () => { + const problems = configProblems({ + ...DEFAULT_CONFIG, + repo: "a/b", + agents: { claude: { preset: "claude" }, haiku: { preset: "claude", model: "haiku" } }, + routes: { docs: { stages: { build: "haiku" }, skills: ["writing"], proof: "check" } }, + }); + expect(problems).toEqual([]); + }); +}); + +describe("loadConfig checks every routed skill exists on disk", () => { + const dir = mkdtempSync(join(tmpdir(), "factory-routes-")); + afterAll(() => rmSync(dir, { recursive: true, force: true })); + + function repoWith(config: object): string { + const d = mkdtempSync(join(dir, "r-")); + mkdirSync(join(d, ".factory")); + writeFileSync(join(d, ".factory/config.json"), JSON.stringify(config)); + return d; + } + + test("a route naming a skill with no SKILL.md on disk is refused at boot, not left to no-op mid-run", async () => { + const target = repoWith({ repo: "a/b", routes: { docs: { skills: ["writing"] } } }); + await expect(loadConfig(target)).rejects.toThrow(/routes\.docs\.skills: "writing" \(\.claude\/skills\/writing\/SKILL\.md not found\)/); + }); + + test("a route naming a skill that is on disk loads cleanly", async () => { + const target = repoWith({ repo: "a/b", routes: { docs: { skills: ["writing"] } } }); + mkdirSync(join(target, ".claude/skills/writing"), { recursive: true }); + writeFileSync(join(target, ".claude/skills/writing/SKILL.md"), "---\nname: writing\n---\nWrite in plain language."); + const config = await loadConfig(target); + expect(config.routes.docs?.skills).toEqual(["writing"]); + }); +}); + +describe("typesFor / issueType", () => { + test("with no routes, the type list is exactly the five defaults", () => { + expect(typesFor(undefined)).toEqual([...TYPE_LABELS]); + }); + + test("a route key not already in TYPE_LABELS extends the type list", () => { + expect(typesFor({ content: {} })).toEqual([...TYPE_LABELS, "content"]); + }); + + test("a route key already in TYPE_LABELS does not duplicate it", () => { + expect(typesFor({ docs: {} })).toEqual([...TYPE_LABELS]); + }); + + test("issueType picks the one label that is in the type list", () => { + expect(issueType({ content: {} }, ["factory:ready", "content", "factory:triaging"])).toBe("content"); + }); + + test("issueType is undefined when no label is a known type", () => { + expect(issueType(undefined, ["factory:ready"])).toBeUndefined(); + }); +}); + +describe("labelsFor", () => { + test("with no routes, labelsFor is exactly LABELS", () => { + expect(labelsFor(undefined).map((l) => l.name).sort()).toEqual(labelsFor({}).map((l) => l.name).sort()); + }); + + test("a repo-added type gets one extra label doctor can create", () => { + const names = labelsFor({ content: {} }).map((l) => l.name); + expect(names).toContain("content"); + }); + + test("a route reusing an existing type name never duplicates its label", () => { + const names = labelsFor({ docs: {} }).map((l) => l.name); + expect(names.filter((n) => n === "docs")).toHaveLength(1); + }); +}); diff --git a/tests/scenarios.test.ts b/tests/scenarios.test.ts index eb159be..4ee0977 100644 --- a/tests/scenarios.test.ts +++ b/tests/scenarios.test.ts @@ -623,6 +623,26 @@ describe("cancel from every waiting state", () => { }); }); +// Plan v2.7.0 item 8: proof:check exists so Markdown-only work (a blog post, +// a docs page) is never forced through "build writes a failing test first". +// The runner reads proof off plan.json and never branches on it itself (that +// branching is the skills' job), so this pins the property that a +// proof:check plan naming no test file still reaches shipped like any other. +describe("proof: check ships markdown-only work with no test file", () => { + test("23. a blog-post plan proved by a prose lint ships with no test file anywhere in its artifact", async () => { + const c = setup([LABEL.ready]); + c.state.setToggle("auto_approve_low_risk", true); + c.push("triage", triage({ type: "docs", gate_level: "npm run check-prose" })); + c.push("plan", plan("low", { proof: "check", files: ["content/blog/2026-09-27-launch.md"] })); + build_to_pr(c); + expect(await c.step()).toBe("shipped"); + const planJson = JSON.parse(readFileSync(join(c.workspacesDir, "issue-1", runDir(1), "plan.json"), "utf8")); + expect(planJson.proof).toBe("check"); + expect(planJson.files.some((f: string) => /test|spec/i.test(f))).toBe(false); + done(c); + }); +}); + test("every state and parked label is reached by a scenario", () => { const unreached = Object.values(LABEL).filter((l) => l !== LABEL.monitor && !seen.has(l)); expect(unreached).toEqual([]); diff --git a/tests/schemas.test.ts b/tests/schemas.test.ts index d0b882d..61f15cf 100644 --- a/tests/schemas.test.ts +++ b/tests/schemas.test.ts @@ -50,3 +50,33 @@ describe("validators enforce the schema, not just the key list", () => { expect(validateVerdict({ outcome: "complete" }).ok).toBe(false); }); }); + +// v2.7.0 item 1: the triage schema's `type` enum is bounded by the repo's +// actual type list, not a fixed five, so a route-added type (e.g. "content") +// validates when the caller passes it, and still fails closed by default. +describe("triage's type enum follows the repo's routes, not a fixed list", () => { + const contentTriage = { disposition: "proceed", type: "content", risk: "low", done_when: "x", files_expected: [], gate_level: "g", confidence: 0.9 }; + + test("a route-added type is rejected against the default TYPE_LABELS", () => { + expect(validateStepJson("triage", contentTriage).ok).toBe(false); + }); + + test("the same type validates once the repo's type list is passed", () => { + expect(validateStepJson("triage", contentTriage, ["bug", "feature", "docs", "security", "dependency", "content"])).toEqual({ ok: true }); + }); +}); + +// v2.7.0 item 8: build/verify read `proof` from the plan artifact, never the +// type name, so the plan schema has to carry and validate it. +describe("plan's proof field", () => { + test("\"test\" and \"check\" both validate; a typo does not", () => { + const base = { risk: "low", revision: 1, files: [] as string[], autoApproveEligible: true }; + expect(validateStepJson("plan", { ...base, proof: "test" })).toEqual({ ok: true }); + expect(validateStepJson("plan", { ...base, proof: "check" })).toEqual({ ok: true }); + expect(validateStepJson("plan", { ...base, proof: "vibes" }).ok).toBe(false); + }); + + test("proof is optional: an older plan with none still validates", () => { + expect(validateStepJson("plan", { risk: "low", revision: 1, files: [], autoApproveEligible: true })).toEqual({ ok: true }); + }); +}); diff --git a/tests/spend-caps.test.ts b/tests/spend-caps.test.ts new file mode 100644 index 0000000..f4c3776 --- /dev/null +++ b/tests/spend-caps.test.ts @@ -0,0 +1,132 @@ +// Plan v2.7.0 item 7: a cap breached before a stage starts parks the issue +// with a "budget" inbox item, and the daily cap pauses new factory:ready +// pickups. Every case below proves the cap is checked directly: the fake +// executor throws if a stage ever actually runs, so a test can only pass by +// the cap catching the issue *before* any stage work, never by coincidence. + +import { describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { mergeConfig } from "../src/config"; +import { buildInbox } from "../src/inbox"; +import { MachineLeases, MachineSpend } from "../src/machine"; +import { FactoryState } from "../src/state"; +import { pollOnce, processReadyIssue } from "../src/watch"; +import { baseIssue, FakeGateRunner, FakeGit, FakeGitHub } from "./harness"; + +class ThrowingExecutor { + async runStage(): Promise { + throw new Error("a stage ran despite a breached spend cap"); + } +} + +function setup(config: Parameters[0]) { + const workspacesDir = mkdtempSync(join(tmpdir(), "factory-ws-")); + const cloneDir = mkdtempSync(join(tmpdir(), "factory-clone-")); + const github = new FakeGitHub([baseIssue(1, ["factory:ready"])]); + const state = new FactoryState(":memory:"); + const deps = { github, git: new FakeGit(), state, executor: new ThrowingExecutor(), gateRunner: new FakeGateRunner(), cloneDir, workspacesDir }; + const merged = mergeConfig({ repo: "acme/repo", concurrency: 1, ...config }); + return { deps, config: merged, github, state, workspacesDir, cloneDir }; +} + +function cleanup(ctx: { workspacesDir: string; cloneDir: string }): void { + rmSync(ctx.workspacesDir, { recursive: true, force: true }); + rmSync(ctx.cloneDir, { recursive: true, force: true }); +} + +describe("checkSpendCap parks an issue before any stage runs", () => { + test("an issue at or over its own perIssueUsd cap parks with a budget inbox item", async () => { + const ctx = setup({ spend: { perIssueUsd: 5 } }); + ctx.state.recordStageRun({ + repo: "acme/repo", issue: 1, stage: "triage", agent: "claude", model: "claude-sonnet-5", + started_at: "2020-01-01T00:00:00Z", finished_at: "2020-01-01T00:01:00Z", duration_ms: 60_000, + tool_calls: 1, tokens_in: 1, tokens_out: 1, tokens_cached: 0, cost_usd: 5, usage_complete: 1, exit_code: 0, killed_reason: null, + }); + + const outcome = await processReadyIssue(ctx.github.issues.get(1)!, ctx.deps, ctx.config); + + expect(outcome).toBe("needs-human"); + const issue = ctx.github.issues.get(1)!; + expect(issue.labels.map((l) => l.name)).toContain("factory:needs-human"); + expect(issue.comments.at(-1)?.body).toContain("perIssueUsd"); + expect(buildInbox([issue])[0]?.kind).toBe("budget"); + + cleanup(ctx); + }); + + test("a repo at or over its dailyUsd cap parks a fresh issue the same way", async () => { + const ctx = setup({ spend: { dailyUsd: 2 } }); + ctx.state.recordStageRun({ + repo: "acme/repo", issue: 99, stage: "build", agent: "claude", model: "claude-sonnet-5", + started_at: new Date().toISOString(), finished_at: new Date().toISOString(), duration_ms: 1000, + tool_calls: 1, tokens_in: 1, tokens_out: 1, tokens_cached: 0, cost_usd: 2, usage_complete: 1, exit_code: 0, killed_reason: null, + }); + + const outcome = await processReadyIssue(ctx.github.issues.get(1)!, ctx.deps, ctx.config); + + expect(outcome).toBe("needs-human"); + expect(ctx.github.issues.get(1)!.comments.at(-1)?.body).toContain("dailyUsd"); + + cleanup(ctx); + }); + + test("an issue at or over its maxUnreportedRuns cap parks even though every run's own cost is unknown", async () => { + const ctx = setup({ spend: { maxUnreportedRuns: 2 } }); + for (let i = 0; i < 2; i++) { + ctx.state.recordStageRun({ + repo: "acme/repo", issue: 1, stage: "triage", agent: "claude", model: "claude-sonnet-5", + started_at: "2020-01-01T00:00:00Z", finished_at: "2020-01-01T00:01:00Z", duration_ms: 60_000, + tool_calls: 1, tokens_in: 1, tokens_out: 1, tokens_cached: 0, cost_usd: null, usage_complete: 0, exit_code: 0, killed_reason: null, + }); + } + + const outcome = await processReadyIssue(ctx.github.issues.get(1)!, ctx.deps, ctx.config); + + expect(outcome).toBe("needs-human"); + expect(ctx.github.issues.get(1)!.comments.at(-1)?.body).toContain("maxUnreportedRuns"); + + cleanup(ctx); + }); + + test("the machine's own dailyUsd cap parks a repo that never spent a cent itself, because another repo sharing the machine did", async () => { + const dbDir = mkdtempSync(join(tmpdir(), "factory-machine-")); + const dbPath = join(dbDir, "machine.db"); + const machineSpend = new MachineSpend(dbPath); + machineSpend.record("acme/other-repo", 10); // a different repo, same machine + + const ctx = setup({}); + const deps = { ...ctx.deps, machine: { leases: new MachineLeases(":memory:"), config: { slots: 2, dailyUsd: 5 }, spend: machineSpend } }; + + const outcome = await processReadyIssue(ctx.github.issues.get(1)!, deps, ctx.config); + + expect(outcome).toBe("needs-human"); + expect(ctx.github.issues.get(1)!.comments.at(-1)?.body).toContain("machine has spent"); + + machineSpend.close(); + rmSync(dbDir, { recursive: true, force: true }); + cleanup(ctx); + }); +}); + +describe("pollOnce pauses new pickups while a daily spend cap is hit", () => { + test("factory:ready issues are skipped, but an issue already parked keeps surfacing in the inbox buckets", async () => { + const ctx = setup({ spend: { dailyUsd: 1 } }); + ctx.github.issues.set(2, baseIssue(2, ["factory:needs-human"])); + ctx.state.recordStageRun({ + repo: "acme/repo", issue: 50, stage: "build", agent: "claude", model: "claude-sonnet-5", + started_at: new Date().toISOString(), finished_at: new Date().toISOString(), duration_ms: 1000, + tool_calls: 1, tokens_in: 1, tokens_out: 1, tokens_cached: 0, cost_usd: 1, usage_complete: 1, exit_code: 0, killed_reason: null, + }); + + const result = await pollOnce(ctx.deps, ctx.config); + + expect(result.paused).toBe(true); + expect(result.reason).toContain("budget"); + expect(result.processed).not.toContain(1); // the ready pickup never started + expect(ctx.github.issues.get(1)!.labels.map((l) => l.name)).toContain("factory:ready"); // untouched, still waiting + + cleanup(ctx); + }); +}); diff --git a/tests/verify-agent.test.ts b/tests/verify-agent.test.ts index fbd6621..c6df4c0 100644 --- a/tests/verify-agent.test.ts +++ b/tests/verify-agent.test.ts @@ -49,7 +49,7 @@ describe("verify-agent", () => { const script = join(cwd, "agent.sh"); await Bun.write(script, `#!/bin/sh\necho '{"type":"x","text":"sk-abcdefghijklmnopqrstuv"}'\n`); Bun.spawnSync(["chmod", "+x", script]); - const ex = new CommandExecutor({ x: { command: [script] } }, { default: "x" }, new FixtureRecorder(dir, "x")); + const ex = new CommandExecutor({ x: { command: [script] } }, { default: "x" }, {}, new FixtureRecorder(dir, "x")); await ex.runStage({ stage: "triage", issue: 1, cwd, maxBudgetUsd: 1 }); const lines = readFileSync(join(dir, "triage.jsonl"), "utf8"); expect(lines).toContain("");