diff --git a/.claude/tasks/0026-bench-arm-scaffolding.md b/.claude/tasks/0026-bench-arm-scaffolding.md index 0b569ca..a247113 100644 --- a/.claude/tasks/0026-bench-arm-scaffolding.md +++ b/.claude/tasks/0026-bench-arm-scaffolding.md @@ -11,8 +11,25 @@ The deliberate asymmetries follow the shipped products: the gitea-axi arm loads ## Acceptance criteria -- [ ] All arms share the identical base prompt and are handed the same repository coordinates and token. -- [ ] The gitea-axi arm's assembled context carries the bundled Agent Skill. -- [ ] The tea and raw-API arms each receive only a one-line native-discovery pointer. -- [ ] The gitea-mcp arm loads its dispatcher schemas eagerly, has the shell tool disabled, and is attached only the MCP tools. -- [ ] Each non-MCP arm's tool/PATH configuration comes from the guard and exposes only that arm's allowed binary. +- [x] All arms share the identical base prompt and are handed the same repository coordinates and token. +- [x] The gitea-axi arm's assembled context carries the bundled Agent Skill. +- [x] The tea and raw-API arms each receive only a one-line native-discovery pointer. +- [x] The gitea-mcp arm loads its dispatcher schemas eagerly, has the shell tool disabled, and is attached only the MCP tools. +- [x] Each non-MCP arm's tool/PATH configuration comes from the guard and exposes only that arm's allowed binary. + +## Implementation Notes + +The scaffolding lives in `bench/arm.ts`, following the existing `bench/` seam pattern. Two exports: + +- `basePrompt(context)` — the identical, task-agnostic base prompt, carrying only the facts shared by every arm (repository coordinates, host URL, token) and naming no tool or task, so it is byte-for-byte identical across arms and the per-arm bootstrap is the only difference. +- `buildArm(arm, context, options)` — assembles the single `ArmDefinition` the runner consumes: the fully assembled system prompt plus the tool configuration (`shell` xor `mcp`). The shell arms' `binDir`/PATH/guard come from `guard.ts` (`provisionArmBin` + `guardCommand`); the gitea-mcp arm gets `shell: null` and the MCP server attachment. + +Decisions and deviations worth flagging: + +- **AC4's "loads dispatcher schemas eagerly" and "attached only the MCP tools" are conveyed structurally, not enforced here.** Eager schema loading is inherent to attaching an MCP server — the Agent SDK lists the server's tools on connect — so the arm definition materializes it by carrying the `mcp` server config (with `shell: null`). Actually attaching *only* the MCP tools (granting no shell/other builtin tools) is the runner's job in task 0027; the arm definition expresses the intent via `shell: null` + a populated `mcp`. This split matches the spec, which places the SDK wiring in the runner slice. +- **`loadSkillBody` strips the skill's YAML frontmatter, embedding only the instructional body.** AC2 says the gitea-axi arm "carries the bundled Agent Skill"; the frontmatter's `description` is metadata Claude Code loads ambiently for *every* skill, so folding it into this one arm would double-count it and overcharge gitea-axi's ambient cost (User Story 4: "each tool's real ambient-context cost is charged honestly"). The body is what an active skill contributes. +- **The MCP env uses the official gitea-mcp server's own contract** (`GITEA_HOST`, `GITEA_ACCESS_TOKEN`, launched `-t stdio`), pointed at the shared host and token. The tests assert the env *values* (host + token), not the key names, to avoid coupling to launch details. +- **Path resolution matches the product's house style** (`new URL("../skills/gitea-axi/SKILL.md", import.meta.url)`, as in `src/commands/setup.ts`), rather than `import.meta.dirname`, per a review note; `bench/` runs from source so `import.meta.url` resolves to the shipped skill. +- **`skillPath` and `locate` options** are injectable seams (skill location; binary resolver) that keep the module host-independently testable; `locate` mirrors `provisionArmBin`'s existing parameter in `guard.ts`. + +Review: Risk **Low**. No unaddressed Standards or Spec findings — the one actionable Standards note (house-style path resolution) was applied; the remaining review points are principled deviations documented above. No criteria dropped. diff --git a/bench/README.md b/bench/README.md index 8f5038a..7c9dbd8 100644 --- a/bench/README.md +++ b/bench/README.md @@ -38,8 +38,9 @@ The raw component breakdown is retained on every sample so the data can be re-we - `checker.ts` — the deterministic scorer: the full-state diff for mutation tasks and the answer-match for read tasks, plus the `score` entry point that dispatches on task kind. - `seed-plan.ts` — the deterministic ground truth every throwaway repository is seeded to: the fixed labels, the open/closed issue spread across the discriminating dimensions, and the pull requests, as pure data plus `groundTruth(user)`, which realizes it into the `RepoState` the checker scores against. - `seed.ts` — the idempotent seeding scripted over the live Gitea API: `resolveBenchAccess` (which reuses gitea-axi's own tea-login credential discovery), `provisionRepo`, and `seedRepo`, reconciling each label, issue, pull request, comment, and review by its natural key so a re-run never duplicates the ground truth. +- `arm.ts` — the per-arm scaffolding: `basePrompt` (the identical task-agnostic base every arm shares) and `buildArm`, which produces the single `ArmDefinition` the runner consumes — the assembled prompt plus the tool configuration. The gitea-axi arm embeds the bundled Agent Skill, the tea and raw-api arms get a one-line native-discovery pointer, and the gitea-mcp arm runs with the shell disabled and only the MCP server attached (its dispatcher schemas load eagerly). The shell arms' PATH and guard come from `guard.ts`. -Later slices add the arm scaffolding, the single-cell runner, the task suite, the run-loop CLI, and the aggregator. +Later slices add the single-cell runner, the task suite, the run-loop CLI, and the aggregator. ## Tests diff --git a/bench/arm.test.ts b/bench/arm.test.ts new file mode 100644 index 0000000..c397ec8 --- /dev/null +++ b/bench/arm.test.ts @@ -0,0 +1,152 @@ +import { mkdtempSync, readdirSync, readlinkSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import type { Arm } from "./result.js"; +import { basePrompt, buildArm, type SharedContext } from "./arm.js"; + +/** + * The shared context every arm is handed. Its values are distinctive literals so + * that finding them echoed back in a prompt is unambiguous evidence, not a + * coincidence — the coordinates ("acme/bench-xyz"), the host URL, and the token + * are all chosen here, independent of the module under test. + */ +const context: SharedContext = { + coords: { owner: "acme", repo: "bench-xyz" }, + access: { apiUrl: "https://git.example.test", token: "s3cr3t-token" }, +}; + +/** Every arm the benchmark compares (ADR / result.ts); an independent literal list. */ +const allArms: ReadonlyArray = ["gitea-axi", "tea", "gitea-mcp", "raw-api"]; + +describe("basePrompt", () => { + // Behavior: the task-agnostic base prompt carries the same repository + // coordinates, host URL, and token the harness was given — these are the facts + // every arm must share (benchmark-harness spec, "Scaffolding"). + it("echoes the repository coordinates, host URL, and token from the context", () => { + const prompt = basePrompt(context); + + expect(prompt).toContain("acme/bench-xyz"); + expect(prompt).toContain("https://git.example.test"); + expect(prompt).toContain("s3cr3t-token"); + }); +}); + +describe("buildArm", () => { + let binRoot: string; + + beforeEach(() => { + binRoot = mkdtempSync(join(tmpdir(), "bench-arm-")); + }); + + afterEach(() => { + rmSync(binRoot, { recursive: true, force: true }); + }); + + // Fake resolver so provisioning a curated bin dir never depends on binaries + // present on the host; dangling symlinks are fine. + const locate = (binary: string) => `/fake/bin/${binary}`; + + // Behavior: all arms share the identical base prompt — the base is a common + // prefix of every arm's system prompt, and arms differ only in the bootstrap + // appended after it. The expected prefix is basePrompt(context), computed + // independently of buildArm. + it.each(allArms)( + "prefixes the %s arm's system prompt with the identical shared base prompt", + (arm) => { + const base = basePrompt(context); + const definition = buildArm(arm, context, { binRoot, locate }); + + expect(definition.systemPrompt.startsWith(base)).toBe(true); + }, + ); + + // Behavior: the gitea-axi arm's assembled context carries the bundled Agent + // Skill, because the Skill ships with the product and its token cost is charged + // to gitea-axi (benchmark-harness spec, "Scaffolding"). "agent-ergonomic CLI" + // is a distinctive phrase from the shipped skills/gitea-axi/SKILL.md body — an + // independent literal anchor, not a value recomputed from the module. + it("carries the bundled Agent Skill in the gitea-axi arm's system prompt", () => { + const definition = buildArm("gitea-axi", context, { binRoot, locate }); + + expect(definition.systemPrompt).toContain("agent-ergonomic CLI"); + }); + + // Behavior: the tea and raw-API arms each receive only a one-line + // native-discovery pointer beyond the shared base — unlike gitea-axi (whole + // skill) or gitea-mcp (eager schemas) (benchmark-harness spec, "Scaffolding"). + // The bootstrap is the systemPrompt with the shared base prefix removed. Each + // pointer must be a single line and name that arm's own tool. The tool names + // ("tea", "curl" — raw-api drives the API via curl, ADR 0016) are independent + // literals fixed by the spec, not recomputed from the module. + it.each([ + { arm: "tea" as Arm, tool: "tea" }, + { arm: "raw-api" as Arm, tool: "curl" }, + ])( + "gives the $arm arm only a one-line native-discovery pointer naming $tool", + ({ arm, tool }) => { + const definition = buildArm(arm, context, { binRoot, locate }); + const bootstrap = definition.systemPrompt.slice(basePrompt(context).length).trim(); + + expect(bootstrap.split("\n")).toHaveLength(1); + expect(bootstrap).toContain(tool); + }, + ); + + // Behavior: the gitea-mcp arm is MCP-only — its shell tool is disabled and only + // the MCP tools are attached, reaching the same host and token as the shared + // context (benchmark-harness spec, "Tool isolation" / "Scaffolding"). Eager + // schema loading is inherent to attaching the MCP server, so the observable + // facts are: no shell config, an attached MCP server, and that server's env + // carrying the fixture's host URL and token (independent literals, not read + // from the module). Env-var KEY names are deliberately not asserted, so the + // test does not couple to launch-detail naming. + it("makes the gitea-mcp arm MCP-only: shell disabled, MCP attached with the shared host and token", () => { + const definition = buildArm("gitea-mcp", context, { binRoot, locate }); + + expect(definition.shell).toBeNull(); + expect(definition.mcp).not.toBeNull(); + + const envValues = Object.values(definition.mcp!.server.env); + expect(envValues).toContain("https://git.example.test"); + expect(envValues).toContain("s3cr3t-token"); + }); + + // Behavior: each non-MCP arm's tool/PATH configuration comes from the guard and + // exposes only that arm's allowed binary (benchmark-harness spec, "Tool + // isolation" / ADR 0016). The (arm, binary) pairs are independent literals — + // ADR 0016 fixes exactly one allowed binary per shell arm — not values read + // back from the module. For each shell arm: it is not an MCP arm; its curated + // bin dir exposes only its own binary (symlinked to the injected target); its + // PATH leads with that curated dir; and its guard permits its own binary while + // denying a foreign one. + const shellArms = [ + { arm: "gitea-axi", binary: "gitea-axi", foreign: "tea issues list" }, + { arm: "tea", binary: "tea", foreign: "curl https://x" }, + { arm: "raw-api", binary: "curl", foreign: "tea issues list" }, + ] as const; + + it.each(shellArms)( + "configures the $arm arm's PATH and guard from the guard, exposing only $binary", + ({ arm, binary, foreign }) => { + const definition = buildArm(arm, context, { binRoot, locate }); + + expect(definition.mcp).toBeNull(); + const shell = definition.shell; + expect(shell).not.toBeNull(); + if (shell === null) return; + + // Curated bin dir exposes ONLY this arm's allowed binary, symlinked to the + // injected resolver's target. + expect(readdirSync(shell.binDir)).toEqual([binary]); + expect(readlinkSync(join(shell.binDir, binary))).toBe(`/fake/bin/${binary}`); + + // PATH leads with the curated dir, so the arm's binary is found there first. + expect(shell.path.split(":")[0]).toBe(shell.binDir); + + // The guard is bound to this arm: its own binary passes, a foreign one is denied. + expect(shell.guard(`${binary} --help`).allowed).toBe(true); + expect(shell.guard(foreign).allowed).toBe(false); + }, + ); +}); diff --git a/bench/arm.ts b/bench/arm.ts new file mode 100644 index 0000000..a952803 --- /dev/null +++ b/bench/arm.ts @@ -0,0 +1,198 @@ +// Per-arm scaffolding: the single arm definition the runner consumes for one +// cell. Every arm shares one task-agnostic base prompt and the same repository +// coordinates and token; each arm then receives a minimal, symmetric bootstrap +// naming its tool and pointing at that tool's own native discovery affordance. +// +// The deliberate asymmetries follow the shipped products (see the benchmark +// spec's Scaffolding section): the gitea-axi arm loads the bundled Agent Skill, +// because the Skill ships with the product and its token cost belongs to +// gitea-axi; the tea and raw-api arms get a one-line native-discovery pointer; +// the gitea-mcp arm's dispatcher schemas load eagerly as its ambient cost and it +// runs with the shell disabled, attaching only the MCP tools. +// +// This module assembles the prompt and composes the guard (guard.ts) for the +// tool/PATH configuration; it does not run the agent — the runner (a later +// slice) consumes an ArmDefinition and drives the Claude Agent SDK. + +import { readFileSync } from "node:fs"; +import { delimiter, join } from "node:path"; +import { guardCommand, provisionArmBin, type GuardDecision } from "./guard.js"; +import type { Arm } from "./result.js"; +import type { BenchAccess, RepoCoords } from "./seed.js"; + +/** + * The task-agnostic inputs handed identically to every arm of a cell: the + * throwaway repository's coordinates and the host access (base URL and token). + */ +export interface SharedContext { + coords: RepoCoords; + access: BenchAccess; +} + +/** + * The tool/PATH configuration for a shell-driving arm, derived from the guard. + * `null` on an ArmDefinition marks an arm that runs with the shell disabled. + */ +export interface ArmShell { + /** Curated bin directory exposing only the arm's one allowed binary. */ + binDir: string; + /** PATH value: the curated bin dir prepended to the ambient PATH. */ + path: string; + /** The authoritative tool-isolation guard, bound to this arm. */ + guard: (command: string) => GuardDecision; +} + +/** + * The MCP attachment for the gitea-mcp arm. The runner launches the server over + * stdio and attaches its dispatcher tools, whose schemas load eagerly on connect + * as the arm's ambient cost. `null` on an ArmDefinition marks an arm that reaches + * Gitea through the shell instead. + */ +export interface ArmMcp { + server: { + command: string; + args: string[]; + env: Record; + }; +} + +/** + * Everything the runner needs to run one arm: the fully assembled system prompt + * and the tool configuration. Exactly one of `shell` / `mcp` is non-null. + */ +export interface ArmDefinition { + arm: Arm; + systemPrompt: string; + shell: ArmShell | null; + mcp: ArmMcp | null; +} + +/** Options controlling how an arm is built; the runner supplies a trial scratch dir. */ +export interface BuildArmOptions { + /** Directory under which the arm's curated bin dir is created (shell arms). */ + binRoot: string; + /** Resolver for a binary's absolute path; injectable for host-independent tests. */ + locate?: (binary: string) => string | null; + /** Override the bundled skill file's path (defaults to the shipped SKILL.md). */ + skillPath?: string; +} + +/** + * The identical, task-agnostic base prompt every arm's assembled prompt begins + * with. It carries only the facts shared by all arms — the repository + * coordinates, the host URL, and the token — and never names a specific tool or + * task, so the base is byte-for-byte the same across arms and the per-arm + * bootstrap is the only difference in the assembled prompt. + */ +export function basePrompt(context: SharedContext): string { + const { owner, repo } = context.coords; + const { apiUrl, token } = context.access; + return [ + "You are a coding agent operating on a single Gitea repository.", + "", + `Repository: ${owner}/${repo}`, + `Gitea host: ${apiUrl}`, + `Access token: ${token}`, + "", + "Authenticate every request with that token, and confine your work to that", + "repository. When the task asks a question, state your final answer plainly.", + ].join("\n"); +} + +/** + * The bundled Agent Skill's shipped location, resolved relative to this module + * the same way the product resolves it (see src/commands/setup.ts's + * `SKILL_SOURCE`). bench/ runs from source, so `import.meta.url` points at this + * file and `../skills/...` lands at the repository's shipped skill. + */ +const DEFAULT_SKILL_PATH = new URL("../skills/gitea-axi/SKILL.md", import.meta.url); + +/** + * Read the bundled Agent Skill's body, stripping its YAML frontmatter. Only the + * instructional body is charged to the gitea-axi arm: the frontmatter's + * `description` is metadata Claude Code loads ambiently for every skill, so + * folding it in here would double-count it against this one arm. + */ +function loadSkillBody(skillPath: string | URL): string { + const raw = readFileSync(skillPath, "utf8"); + const match = raw.match(/^---\n[\s\S]*?\n---\n/); + return (match ? raw.slice(match[0].length) : raw).trim(); +} + +/** + * The per-arm bootstrap appended after the shared base: the minimal, symmetric + * text naming the arm's tool and pointing at its native discovery affordance. + * The gitea-axi arm is the deliberate asymmetry — it embeds the bundled Agent + * Skill, whose token cost belongs to the shipped product. + */ +function armBootstrap(arm: Arm, context: SharedContext, options: BuildArmOptions): string { + switch (arm) { + case "gitea-axi": { + const skill = loadSkillBody(options.skillPath ?? DEFAULT_SKILL_PATH); + return [ + "You have the `gitea-axi` CLI available in your shell. Its bundled Agent", + "Skill follows; treat it as your guide to the tool.", + "", + skill, + ].join("\n"); + } + case "tea": + return "You have the `tea` CLI available in your shell; run `tea --help` to discover its commands."; + case "raw-api": + return `You have \`curl\` available in your shell; the Gitea REST API is documented at ${context.access.apiUrl}/api/swagger.`; + case "gitea-mcp": + return "The Gitea MCP server's tools are attached; use them to operate on the repository."; + } +} + +/** + * The MCP attachment for the gitea-mcp arm: the official server launched over + * stdio, pointed at the shared host and token through the environment variables + * it reads (`GITEA_HOST`, `GITEA_ACCESS_TOKEN`). Attaching it is what loads the + * dispatcher schemas eagerly — the SDK lists the server's tools on connect — so + * that ambient cost is charged to this arm. + */ +function mcpAttachment(context: SharedContext): ArmMcp { + return { + server: { + command: "gitea-mcp", + args: ["-t", "stdio"], + env: { + GITEA_HOST: context.access.apiUrl, + GITEA_ACCESS_TOKEN: context.access.token, + }, + }, + }; +} + +/** + * Build the tool/PATH configuration for a shell-driving arm from the guard: + * provision a curated bin directory exposing only the arm's one allowed binary, + * lead the PATH with it, and bind the authoritative guard to the arm. The + * gitea-mcp arm has no shell binary (`provisionArmBin` exposes nothing for it), + * so this returns null there and the arm reaches Gitea through its MCP tools. + */ +function buildShell(arm: Arm, options: BuildArmOptions): ArmShell | null { + if (arm === "gitea-mcp") { + return null; + } + const binDir = join(options.binRoot, arm); + provisionArmBin(arm, binDir, options.locate); + const ambient = process.env.PATH ?? ""; + return { + binDir, + path: ambient === "" ? binDir : `${binDir}${delimiter}${ambient}`, + guard: (command) => guardCommand(arm, command), + }; +} + +/** Assemble the single arm definition the runner consumes for the given arm. */ +export function buildArm(arm: Arm, context: SharedContext, options: BuildArmOptions): ArmDefinition { + const systemPrompt = `${basePrompt(context)}\n\n${armBootstrap(arm, context, options)}`; + return { + arm, + systemPrompt, + shell: buildShell(arm, options), + mcp: arm === "gitea-mcp" ? mcpAttachment(context) : null, + }; +}