diff --git a/.claude/tasks/0008-arm-isolation-hardening.md b/.claude/tasks/0008-arm-isolation-hardening.md new file mode 100644 index 0000000..dc1f4c4 --- /dev/null +++ b/.claude/tasks/0008-arm-isolation-hardening.md @@ -0,0 +1,66 @@ +--- +spec: skill-benchmarking +blocked-by: 0004-efficacy-benchmark-tracer-bullet +--- + +## What to build + +The isolation fix the tracer bullet flagged as a follow-up: give every arm a hermetic **fixture-only world** so the no-skill baseline stops discovering the skill on disk, turning that run's contaminated red into an honest counterfactual. + +In 0004 the arms ran with the repo as their working directory. +The no-skill baseline, solving its task inside the real repo, grepped up `skills/axi-review/AXI-PRINCIPLES.md` and graded against it — its own output admitted as much — so the measured "no-skill" arm was not the counterfactual of the skill not existing. +The Efficacy verdict came out red as a direct artifact of that contamination rather than a real result. + +Each arm-and-trial now runs in a fixture-only world. +The arm subagent's working directory is its fresh fixture copy rather than the repo root, and nothing under `skills/` or the grading `tests/` tree sits on any path it explores from there. +This is what makes the no-skill baseline honest, because it can no longer find and read the skill's assets, and it also stops any arm from reading its own case's soft criteria or hard assertions and tuning its answer to the bar it will be judged against. + +A with-skill arm is handed its skill as an **isolated temp materialization** placed outside the fixture, and the subagent is pointed there to force-invoke it. +For this slice's two-arm shape only the new-skill arm has one: a copy of the working-tree skill directory, so it reflects uncommitted edits and carries none of its repo surroundings. +The no-skill arm is handed nothing. +Each temp materialization is cleaned up after the run, like the fresh fixture copies. + +Isolation here is deliberately soft. +An in-session subagent shares the machine and could in principle reach the repo by absolute path, so the baseline is additionally instructed to stay within its working directory. +Relocating each arm's world and giving that instruction moves contamination from near-certain to requiring an arm to deliberately wander outside its world. +A hard filesystem guarantee would require an OS sandbox and is out of scope. + +`$WORLD` binds to the arm's fixture copy — now also its working directory — and `$OUTPUT` to its captured final message, with the hard-assertion gate unchanged from 0004. +The deterministic core and its fixture unit test are untouched, because isolation is a runner orchestration concern rather than a scoring one. + +Correctness is established by re-running the harness on the real skill and reading the result. +A live `/benchmark-skill axi-review` run confirms the baseline arm's transcript no longer references `AXI-PRINCIPLES.md` or any `skills/` or `tests/` path, and reports the now-uncontaminated Efficacy verdict that replaces 0004's documented-artifact red. + +This slice establishes the "materialize a with-skill arm's skill into an isolated temp path" pattern that the previous-version arm reuses for the old skill version. + +## Acceptance criteria + +- [x] Each arm-and-trial subagent runs with its working directory set to its fresh fixture copy, never the repo root. +- [x] Nothing under `skills/` or the `tests/` tree is present on any relative path an arm explores from its world. +- [x] The new-skill arm's skill is materialized into an isolated temp path outside the fixture — a copy of the working-tree directory that reflects uncommitted edits — and the subagent is pointed there to force-invoke it, with the temp materialization cleaned up after the run. +- [x] The no-skill baseline is handed no skill materialization and is instructed to stay within its working directory. +- [x] `$WORLD` binds to the arm's fixture copy and `$OUTPUT` to its captured final message, with the hard-assertion gate semantics unchanged from 0004. +- [x] The deterministic core and its fixture unit test are unchanged, since isolation is an orchestration concern and not a scoring one. +- [x] A live `/benchmark-skill axi-review` run confirms the baseline arm's transcript references no `AXI-PRINCIPLES.md`, `skills/`, or `tests/` path, and its Efficacy verdict reflects the honest counterfactual rather than the earlier contamination artifact. + +## Implementation Notes + +Changed `skills/benchmark-skill/SKILL.md` only: the world/materialization steps in §3, the `$WORLD` binding in §4, and the scratch cleanup in §6. +`core/benchmark_core.py` and its check were left untouched, and the unchanged core was confirmed to still run clean on its committed fixture. + +- **Worlds are materialized outside the repo, not merely relocated within it.** + The spec calls for a "fixture-only world", and the sharp form that actually kills contamination is a world under a system temp path (`mktemp -d`), because a world still under `tests/.reports/` leaves `skills/` and the grading `tests/` tree reachable by upward navigation. + The prose now says so explicitly. +- **Live verification passed and reversed 0004's contaminated result.** + A real `/benchmark-skill axi-review`-shaped run of 5 paired trials, orchestrated through the workflow mechanism with each arm's world under a temp path, came back with the new arm winning 5/5 and the Efficacy verdict **green** — where 0004 had scored red at 2/5 purely because the baseline had read the skill's `AXI-PRINCIPLES.md`. + All five no-skill-baseline transcripts were clean of any `AXI-PRINCIPLES.md`, `skills/`, or `tests/` reference, and the baselines instead invented their own generic CLI rubric — the honest counterfactual of the skill not existing. +- **Soft isolation is realized by instruction, since a subagent's working directory is not hard-settable through the workflow surface.** + Each arm is told to `cd` into its world first and work only there, and the baseline is additionally told to stay within it. + This is the achievable in-session bar, and hard OS-sandbox isolation stays deferred per the spec's "Out of Scope". +- **Review follow-through.** + A spec-axis review caught that "copy the case fixture into the world" could be read as nesting the fixture one level down, which would break `$WORLD/` predicates. + The wording now states that the world *is* the fresh fixture copy, so a `$WORLD/` predicate resolves against the fixture root. + A semicolon in one acceptance-criterion line was recast. +- **The feature spec is not part of this commit.** + `.claude/spec/skill-benchmarking.md` carries the isolation decisions but remains untracked, matching how 0004 was committed (code and task file, without the spec), and committing it here would pull in prose-standard breaches from sections written earlier. +- The materialization pattern this establishes for the new arm is reused by 0005's previous-version arm, which points at a temp checkout of the skill at main's `HEAD` instead of a copy of the working-tree directory. diff --git a/skills/benchmark-skill/SKILL.md b/skills/benchmark-skill/SKILL.md index b4195bb..ed5239c 100644 --- a/skills/benchmark-skill/SKILL.md +++ b/skills/benchmark-skill/SKILL.md @@ -57,15 +57,21 @@ Use the workflow mechanism (the `Workflow` tool) to fan out the arms and judges. For each case, for each of the 5 trials, run both arms, then judge the pair. **Both arms receive the identical `## Prompt`, authored once and arm-agnostically.** -Each arm-and-trial combination runs against a **fresh copy** of the case fixture, made under `tests/.reports/.work/`, so writes never leak between runs. For a conversation-driven case, inject the `## Seed` transcript as the subagent's prior context before the prompt. +**Give each arm-and-trial a hermetic fixture-only world.** +Make a fresh copy of the case fixture *outside the repo* — under a system temp path such as one from `mktemp -d`, never under `tests/.reports/`. +That copy is the **world**: it is the arm subagent's **working directory** and the `$WORLD` the hard-assertion gate reads, so a `$WORLD/` predicate resolves against the fixture root. +Because the world lives outside the repo, nothing under `skills/` or the grading `tests/` tree sits on any path the arm reaches from there, so the baseline cannot discover the skill's assets and no arm can read its own case's soft criteria or hard assertions. +Each arm-and-trial gets its own fresh world, so writes never leak between them. + - **New-skill arm** — force-invoke the skill. - Point the subagent at the skill's own directory, tell it to use that skill, and have it read the skill's `SKILL.md` and any assets it references, so the real skill machinery is exercised. - Give it the fresh fixture copy as its working target and the prompt. + Materialize the skill into an **isolated temp path outside the world** — a copy of its working-tree directory, so it reflects uncommitted edits and carries none of its repo surroundings. + Point the subagent at that copy, tell it to use that skill, and have it read the skill's `SKILL.md` and any assets it references, so the real skill machinery is exercised. + Its working directory is the fresh world, and it also receives the prompt. - **No-skill baseline arm** — the honest counterfactual of the skill not existing. - Give it the bare prompt with the skill absent from its context. - Do not mention the skill or hint that one exists. + Materialize no skill for it: give it the bare prompt with the skill absent from its context, and do not mention the skill or hint that one exists. + Instruct it to stay within its working directory, since the isolation is soft and the subagent shares the machine. Capture each arm's **final message** to a file — this is the `$OUTPUT` the hard-assertion gate reads and the text the judge compares. @@ -77,7 +83,7 @@ Have it return a single winner: A, B, or tie. ## 4. Run the hard-assertion gate Run the case's `## Hard assertions` against the **new arm only**, once per trial. -Bind `$OUTPUT` to the path of that trial's new-arm final message and `$WORLD` to the path of that trial's fresh new-arm fixture copy. +Bind `$OUTPUT` to the path of that trial's new-arm final message and `$WORLD` to that trial's new-arm world (its fixture copy, which is the arm's working directory). Run each predicate. A non-zero exit fails the assertion. Record pass/fail per trial — a failed hard assertion fails the case outright regardless of the head-to-head. @@ -144,7 +150,7 @@ The core collapses each efficacy trial to WIN, TIE, or LOSS for the new arm (a t It renders a self-contained HTML report: the Efficacy badge and run metadata, a two-row per-arm cost table (footnoted that absolute cost is inflated by shared-context cache overhead, so the trustworthy signal is the new-vs-no-skill ratio), and the cases in stable authored order. Cost is reported alongside quality but never gates the verdict. -Clean up the `tests/.reports/.work/` scratch directory when done. +Clean up the scratch when done: remove the `tests/.reports/.work/` directory and every per-arm world and skill materialization you created under the system temp path. Done when `tests/.reports/.html` exists. Report the Efficacy verdict, the path to the report, and any flagged losses to the user.