From db303be509bdd19aa0ce5ca5a8b48127f4d0cda1 Mon Sep 17 00:00:00 2001 From: Prem Mallappa Date: Fri, 11 Sep 2026 14:45:15 +0530 Subject: [PATCH 1/4] #323: the transcription audit, and the gap published as three assertions Cases 16, 17 and 18 of docs/test-plans/293-plan.md. Build target 7's CI job is deliberately NOT here -- it is a job-list edit whose promotion sequencing is unresolved, and it lands separately. docs/audits/293-transcription-audit.md is case 18's model-judged verdict: sixteen rows, one per `## Re-entry` row across the four skills, each naming the transcription step that implements it, the mutation that step gates, and the reviewer's reasoning. It is not an evidence artifact because every file under docs/evidence/ must match `{issue}-{ts}.md` (evidence-upsert.test.ts) and because an artifact is immutable once pushed, which is the wrong lifetime for a record that must be amended whenever a table or a transcription changes. The audit's finding: the ordering obligation the skill bodies state is per-row -- every mutating step preceded by the read that decides it -- never table-wide. Three of the four transcriptions therefore execute their rows out of table order, each for a reason recorded row by row, and all three are faithful. That is the same fact that makes case 16 pass when the harness fails to notice a reordered table, and the reason is written into the assertion's own comment so it is not "repaired". Two authorised mutations are dead in the roster -- goal-create's milestone PATCH and story-create's `--milestone` reassign. Both are the fake forge's unmodelled shapes, not transcription defects. The audit records them; only the live rung can close them. test/transcription-gap.test.ts carries the tool-checked halves: the audited (skill, row) pair set must equal exactly the set reEntryRows() reads from disk, by cardinality and not by length; CLAIM-41.8's class in docs/test-plans/41-plan.md must still read `model-judged`, gate ruling G2 made mechanical; and a row reorder in each of 4 of 4 bodies must leave the harness green, with the mutation asserting its own pattern matched first. Seven mutations were injected against these assertions. Six were killed on first contact. M5 survived: folding the summary table back under `## The audit` left the parser reading the right rows by accident, because it took the first contiguous run rather than asserting the structure it depended on. The parser now asserts `## The audit` holds exactly one table, and M5 is killed. A control test proves `--skills-dir` is load-bearing, without which all four of case 16's green runs would have been vacuous. --- docs/audits/293-transcription-audit.md | 152 ++++++++++ test/transcription-gap.test.ts | 392 +++++++++++++++++++++++++ 2 files changed, 544 insertions(+) create mode 100644 docs/audits/293-transcription-audit.md create mode 100644 test/transcription-gap.test.ts diff --git a/docs/audits/293-transcription-audit.md b/docs/audits/293-transcription-audit.md new file mode 100644 index 0000000..5814191 --- /dev/null +++ b/docs/audits/293-transcription-audit.md @@ -0,0 +1,152 @@ +# Transcription audit — the four skills of S2.6 + +Case 18 of `docs/test-plans/293-plan.md`, anchoring `CLAIM-293.11`. Task #323. + +`CLAIM-293.11` is **`model-judged` deliberately**: the harness holds a +procedure, the `SKILL.md` holds prose, and whether the two agree is meaning. No +regex closes it. **The denominator is tool-checked** — the row count below is +asserted at run time against `countReEntryRows()` +(`packages/harness/src/re-entry.ts:98`) by +`test/transcription-gap.test.ts` — **only the verdict is not.** + +## Why this document is not an evidence artifact + +Every model-judged verdict in this repository so far has been recorded in +`docs/evidence/`. This one is not, for two mechanical reasons: + +1. `packages/core/test/evidence-upsert.test.ts:33-50` asserts that **every** + `.md` under `docs/evidence/` matches `^(\d+)-(\d{8}T\d{6}Z)\.md$`. A durable, + re-checkable audit has no timestamp to carry. +2. An evidence artifact is **immutable once pushed**, and is measured on the + tree that contains it. This audit is a standing record that must be re-read + and amended whenever a `## Re-entry` table or a transcription changes — the + opposite lifetime. + +The #323 evidence artifact records the verdict of the audit; this file is the +audit itself. + +## Method, and what would have made a row fail + +Each row below was resolved by hand, in both directions: + +- the `## Re-entry` cell was read from `skills//SKILL.md` and quoted + verbatim, not paraphrased; +- the harness step was located by its `ctx.read(, …)` call and its line + number read off the file, not carried from a prior document; +- the mutation that the row's *"Then, and only then"* half authorises was + located separately, and checked to be **after** its own read. + +A row fails this audit if any of: the transcription implements a different +read than the cell names; a mutation the cell authorises runs **before** the +read that decides it; the cell's *"never"* half is violated; or the row has no +step at all. + +**The ordering constraint is per-row, not table-wide.** Each skill body states +it in terms — `skills/story-create/SKILL.md:95` says *"Every mutating step is +preceded by the read that decides it"*, and `skills/story-design/SKILL.md:90-91` +says the same. It does **not** say the rows execute top to bottom. Three of the +four transcriptions therefore execute their rows out of table order, each for a +recorded reason, and all three are faithful. **This is the same fact that makes +case 16 pass when the harness fails to notice a reordered table** — see the +closing section. + +## The audit + +`Executes` is this step's position in the transcription's own execution order. +`Mutation` is the write the row's *"Then, and only then"* half authorises. + +| Skill | Row | "Read first" cell, verbatim | Harness step | Executes | Mutation it gates | Verdict | Reviewer's reasoning | +|---|---|---|---|---|---|---|---| +| goal-create | 1 | Does a milestone with this title exist? | `packages/harness/src/transcription-goal-create.ts:68` | 2nd of 3 | `POST …/milestones` at `:72` | faithful | Identity is matched on the milestone **title**, which is what `skills/goal-create/SKILL.md:79-81` names as the identity a second run matches on. The read is a `--paginate` list, so a truncated page cannot be read as absence; the create at `:72` is inside the `if not found` branch the read decides. | +| goal-create | 2 | Does its description already carry the feature table? | `packages/harness/src/transcription-goal-create.ts:90` | 3rd of 3 | `PATCH …/milestones/{number}` at `:111` | faithful | The list is **re-read** rather than reusing row 1's read. That is not redundancy: `skills/goal-create/SKILL.md:107-110` names a rate-limited read of this same list as the failure that must never be read as *"no such milestone"*, and trusting the create above to have landed would be exactly that mistake. The amend branch is dead in the roster — see *Dead branches* below — for a reason that is about the fake, not about this step. | +| goal-create | 3 | Did the goal resolve at all? | `packages/harness/src/transcription-goal-create.ts:53` | **1st of 3** | none — the hard-failure `throw` at `:55` | faithful | Executed first although it is the table's third row. The cell's second half is *"proceed, or emit the hard-failure block and stop"*, and a goal that does not resolve must mutate **nothing** — so every create-or-leave row above it is only meaningful once this read has passed. Running it third would make the hard-failure block unreachable after a milestone had already been created. The out-of-order execution is what makes the row faithful, not a departure from it. | +| story-create | 1 | Does a Story already exist for this row? | `packages/harness/src/transcription-story-create.ts:74` | 2nd of 4 | `gh issue create` at `:92` | faithful | Identity is the row's description text, per `skills/story-create/SKILL.md:93`. The read is fetched ahead of row 4's orphan comparison and ahead of every create; row 4's own constraint is only that it precede row 1's **creates**, never row 1's **read**. | +| story-create | 2 | Does the declared domain resolve to a binding? | `packages/harness/src/transcription-story-create.ts:58` | **1st of 4** | none — the hard-failure `throw` at `:60` | faithful | Executed first although it is the table's second row, for the same reason as `goal-create`'s row 3: `skills/story-create/SKILL.md:34-37` says a Story labelled with a domain no later verb can resolve is a Story that stalls the pipeline **after it has already been created**. Validating after the creates would be a different skill. | +| story-create | 3 | Is the existing Story assigned to this milestone? | `packages/harness/src/transcription-story-create.ts:117` | 4th of 4 | `gh issue edit --milestone` at `:121` | faithful | The issue list is read **again** at `:117` rather than reusing `:74`'s result — the creates just issued changed which issues exist, and `skills/story-create/SKILL.md:119-123` names a rate-limited read here as one that must never be mistaken for *"no Stories exist"*. Dead in the roster; see below. | +| story-create | 4 | Are there Stories with no row? | `packages/harness/src/transcription-story-create.ts:83` | 3rd of 4 | **none, and that is the point** | faithful | The cell says *"report them; **never** create, edit or close"*, and `skills/story-create/SKILL.md:102` carries the same `never`. The step reads the milestone document, computes the orphan set and reports it. There is no mutation to audit, and a transcription that quietly fixed an orphan would be a transcription of a different skill. Placed before row 1's creates, per `skills/story-create/SKILL.md:73-85`. | +| story-design | 1 | Does `docs/design/stories/{n}.md` exist? | `packages/harness/src/transcription-story-design.ts:44` | 1st of 4 | `writeFile` + `git add` + `git commit` at `:46-48` | faithful | `git cat-file -e` is an existence probe that does not read the blob, which is the correct read for a create-or-leave decision. When the file exists the transcription writes **nothing at all** — not even identical bytes. That matters: case 10 of `docs/test-plans/293-plan.md` makes an identical-bytes rewrite a recorded mutation, so a "harmless" rewrite here would score as a run-2 mutation and be a real defect. | +| story-design | 2 | Does the Story already carry a design sentinel? | `packages/harness/src/transcription-story-design.ts:77` | **4th of 4** | `gh issue comment` at `:88`, or `PATCH …/issues/comments/{id}` at `:99` | faithful | Executed last although it is the table's second row, because the sentinel body pins the permalink that row 3's read resolves — the sentinel cannot be posted before the sha it quotes exists. Both halves of the cell are implemented: post when absent, **edit that comment in place** when present, never append a second. | +| story-design | 3 | Does the pinned commit resolve on the remote? | `packages/harness/src/transcription-story-design.ts:63` | 2nd of 4 | none — its output is consumed by row 2's sentinel body | faithful | Read **after** row 1's write, never before: the sha that resolves is the one the write, if any, just produced. Reading it first would pin a permalink to the parent commit — the precise error the handoff records as *"an evidence artifact cannot carry the hash of the commit that contains it"*. The fixture has no remote, so this is `rev-parse HEAD`; the audit records that as the honest limit of the transcription, not as a divergence. | +| story-design | 4 | Do the registered claims already exist? | `packages/harness/src/transcription-story-design.ts:74` | 3rd of 4 | **none — observational by construction** | faithful | Claims live as checklist text **inside** the Design file, not as a separate forge object, so there is no second mutation that could "register" one beyond row 1's write. The cell's second half, *"register them, or leave the existing identifiers alone"*, is satisfied by construction: this transcription never rewrites a Design that already exists. A step that invented a registration call would be transcribing a skill that does not exist. | +| story-test-plan | 1 | Does the plan file exist? | `packages/harness/src/transcription-story-test-plan.ts:61` | 1st of 5 | `writeFile` + `git add` + `git commit` at `:95-97` | faithful | Same existence-probe shape as `story-design`'s row 1, and the same no-rewrite-when-present rule. Note the write at `:95` executes **after** rows 2, 3 and 4's reads, all of which need the pre-write state. | +| story-test-plan | 2 | Which claims have no case? | `packages/harness/src/transcription-story-test-plan.ts:67` | 2nd of 5 | none — feeds the body written by row 1 | faithful | The Design is read as the **only** source of claims, per `skills/story-test-plan/SKILL.md:29-31`. The transcription never invents a claim to plan against, which is the failure this row exists to prevent. | +| story-test-plan | 3 | Which cases anchor to a claim that is gone? | `packages/harness/src/transcription-story-test-plan.ts:78` | 3rd of 5 | **none — reports only** | faithful | Read **before** the write at `:95`, deliberately. On a first run the plan file does not exist and `git show HEAD:` exits non-zero — that is expected, and it is still a **recorded read**, never a reason to skip the call. Skipping it would be the *"decided to do nothing"* versus *"did nothing"* collapse that `CLAIM-293.3` exists to separate. The cell says *"do not silently delete a case"*; the step reports into `danglingCases` and deletes nothing. | +| story-test-plan | 4 | Does every case already declare a `Corpus`? | `packages/harness/src/transcription-story-test-plan.ts:90` | 4th of 5 | **none — observational** | faithful | Reads the same pre-write state as row 3 and does so **under its own row number**, rather than reusing row 3's result. That is what makes the read count honest: the runner's threshold is `countReads(report.calls)` (`packages/harness/src/argv-kind.ts`), computed from calls actually issued, never from a declared row. The cell's *"add the column"* half has no separate mutation because the plan's shape is always written whole, once, by row 1. | +| story-test-plan | 5 | Is a test-plan sentinel already posted? | `packages/harness/src/transcription-story-test-plan.ts:109` | 5th of 5 | `gh issue comment` at `:114`, or `PATCH …/issues/comments/{id}` at `:126` | faithful | The cell asks for both halves in its own text — *"edit that comment, never append a second"* — and both are implemented. Contrast `story-design`'s row 2, whose cell asks the same and is implemented the same way; the two are consistent with each other and each with its own body. | + +## Denominator + +**16 rows audited; 16 `## Re-entry` rows on disk; 0 divergent.** + +| Skill | `## Re-entry` rows | Rows audited | +|---|---|---| +| goal-create | 3 | 3 | +| story-create | 4 | 4 | +| story-design | 4 | 4 | +| story-test-plan | 5 | 5 | +| **Total** | **16** | **16** | + +Neither direction has an orphan: no `## Re-entry` row lacks a step, and no +transcription step is attributable to no row. + +> **THIS TABLE IS DELIBERATELY NOT UNDER `## The audit`, AND MUST NOT BE MOVED +> BACK.** `test/transcription-gap.test.ts` parses the audit table by section, +> and every row of this summary also begins with a skill name followed by an +> integer. While the two tables shared a section the parser counted 23 pairs +> instead of 16, silently double-counting four skills — an error in the +> direction that *inflates* a denominator, which is the direction that hides a +> missing row. `## The audit` holds exactly one table; keep it that way. + +## Dead branches — two, and both are the fake's limit, not the transcription's + +Two authorised mutations exist in the transcriptions and are never reached by +any scenario in `packages/harness/src/scenario-roster.ts`: + +| Branch | Why it never fires | Why no scenario forces it | +|---|---|---| +| `goal-create` row 2's `PATCH …/milestones/{number}` | Scenario 1 creates the milestone with the feature table already correct, so run 2 finds no *"description differs"* state | `packages/harness/src/fake-forge.ts` models **no** `PATCH` on a milestone at all. A scenario reaching it lands on `unmodelled`, never a real update | +| `story-create` row 3's `gh issue edit --milestone` | Every create in scenario 2 carries `--milestone` from the start, so `milestone === null` is never true | The fake's `create` never fails to honour `--milestone`, so the null-assignment state is unreachable without making the fake lie in a way it does not model | + +**Both are correct re-entrant behaviour**, and neither is a transcription +defect: the code implements what its cell authorises, and the state that would +trigger it cannot honestly be produced by this fake. A scenario built to force +either would be asserting something about the fake rather than about the skill. + +**This is an unmodelled-shape gap in the fake forge, and it is now recorded +rather than built around.** It is the item the #322 handoff listed as *needing +an issue if #323's audit or #324's live rung does not absorb it*. This audit +absorbs the **record**; it does not close the coverage. The live rung (#324) is +the first thing that can, because a real forge honours both shapes. + +## What this audit does NOT establish + +- **It does not test the model reading a `SKILL.md`.** The population audited + here is the transcription. Gate ruling G2 (`docs/design/stories/293.md`) + forbids any later Story treating this audit, or a green harness run, as + licence to upgrade `CLAIM-41.8` or `CLAIM-47.4` from `model-judged`. +- **It does not make the transcriptions correct by construction.** It records + that a reviewer checked sixteen correspondences on a named tree. A change to + either side invalidates the row that covers it, and the denominator assertion + will catch a row **added or removed** — it cannot catch a cell **edited**. +- **It is not the roster's coverage report.** Two authorised mutations above + are audited as faithful and are never executed. + +## Why case 16 passes, and why it must not be "repaired" + +The transcriptions attribute each read to a row number **written by hand**. +Nothing parses the `## Re-entry` table at run time except the row **count**. +Reorder two rows of a shipped table and the count does not change, so +`bun run skill-harness` stays green and not one assertion moves. + +That is `NEVER-293.5` working, and this audit is why it is the right behaviour +rather than a hole: the table is a set of read-before-write obligations, not a +sequence, and three of the four transcriptions already execute their rows out +of table order **for reasons recorded row by row above**. A harness that went +red on a reordered table would be asserting an ordering the skill bodies never +claimed — and would go red on three of the four skills as they are shipped +today. + +The gap that remains real is the one this document exists to bound: the harness +would not notice if a cell's **text** changed to mean something else. That is +measured, published, and owned by `CLAIM-293.11` — which is why it is +`model-judged`. diff --git a/test/transcription-gap.test.ts b/test/transcription-gap.test.ts new file mode 100644 index 0000000..36adf45 --- /dev/null +++ b/test/transcription-gap.test.ts @@ -0,0 +1,392 @@ +// Tests for the transcription gap NEVER-293.5 (Task #323). Three cases: +// case 16 (P0, NEVER-293.5), case 17 (P0, NEVER-293.5), and case 18's +// tool-checked denominator (P0, CLAIM-293.11). +// +// Copied from test/skill-harness.test.ts's own conventions exactly: the +// `spawnHarness` helper shape, `repoRoot` from `import.meta.dir`, +// `createTempDirs()` with an `afterAll` teardown, and the `HarnessArtifact` +// interface. THE EXIT CODE IS ASSERTED FIRST in any spawn test. + +import { afterAll, describe, expect, test } from "bun:test"; +import { cpSync, existsSync, mkdirSync, readFileSync, readdirSync, statSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { createTempDirs } from "../packages/harness/src/tempdir"; +import { countReEntryRows, readSkillBodies, reEntryRows } from "../packages/harness/src/re-entry"; + +const repoRoot = join(import.meta.dir, ".."); +const realSkillsDir = join(repoRoot, "skills"); + +const temps = createTempDirs(); +afterAll(() => temps.cleanup()); + +interface Spawned { + readonly stdout: string; + readonly stderr: string; + readonly exitCode: number; +} + +async function spawnHarness(args: readonly string[]): Promise { + const proc = Bun.spawn(["bun", join(repoRoot, "scripts", "skill-harness.ts"), ...args], { + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + return { stdout, stderr, exitCode }; +} + +interface HarnessArtifact { + readonly schemaVersion: number; + readonly verdict: "pass" | "fail"; + readonly exitCode: 0 | 1; + readonly failures: readonly unknown[]; + readonly denominators: { + readonly skillsOnDisk: readonly string[]; + readonly rosterLength: number; + readonly scenariosExecuted: number; + readonly scenariosPerSkill: Readonly>; + readonly reEntryRows: Readonly>; + readonly seedTreeHashes: readonly string[]; + }; + readonly notVerified: readonly string[]; +} + +// =========================================================================== +// Helpers shared by the mutation in block 1. +// =========================================================================== + +/** Find the `## Re-entry` section's line range in a SKILL.md's text: the + * heading line whose trimmed text is exactly `## Re-entry`, up to the next + * line starting with `## ` (or the end of the file). Mirrors + * packages/harness/src/re-entry.ts's own section-boundary rule exactly, so + * the mutation below operates on the same section the reader parses. */ +function findReEntrySection(lines: readonly string[]): { start: number; end: number } { + const start = lines.findIndex((l) => l.trim() === "## Re-entry"); + if (start === -1) throw new Error("## Re-entry heading not found"); + let end = lines.length; + for (let i = start + 1; i < lines.length; i += 1) { + if ((lines[i] as string).startsWith("## ")) { + end = i; + break; + } + } + return { start, end }; +} + +/** Swap the first two `## Re-entry` DATA rows (header + separator dropped, + * same `.slice(2)` re-entry.ts itself takes) of one SKILL.md's text, and + * return both the mutated text and the two swapped-row strings so the + * caller can assert the swap actually happened. */ +function swapFirstTwoReEntryRows(text: string): { + mutatedText: string; + originalDataRows: readonly string[]; + mutatedDataRows: readonly string[]; +} { + const lines = text.split("\n"); + const { start, end } = findReEntrySection(lines); + const sectionLines = lines.slice(start, end); + const tableLineIndices: number[] = []; + for (let i = 0; i < sectionLines.length; i += 1) { + if ((sectionLines[i] as string).startsWith("|")) tableLineIndices.push(i); + } + // Drop the header row and the `|---|---|` separator: the first two + // `|`-prefixed lines in the section. + const dataRowIndices = tableLineIndices.slice(2); + if (dataRowIndices.length < 2) { + throw new Error("fewer than 2 data rows in ## Re-entry table"); + } + const originalDataRows = dataRowIndices.map((i) => sectionLines[i] as string); + + const [firstIdx, secondIdx] = dataRowIndices as [number, number]; + const firstLine = sectionLines[firstIdx] as string; + const secondLine = sectionLines[secondIdx] as string; + const mutatedSectionLines = sectionLines.slice(); + mutatedSectionLines[firstIdx] = secondLine; + mutatedSectionLines[secondIdx] = firstLine; + + const mutatedDataRows = dataRowIndices.map((i) => mutatedSectionLines[i] as string); + + const mutatedLines = lines.slice(); + mutatedLines.splice(start, end - start, ...mutatedSectionLines); + return { mutatedText: mutatedLines.join("\n"), originalDataRows, mutatedDataRows }; +} + +// =========================================================================== +// BLOCK 1 -- case 16 (P0, NEVER-293.5): mutating a SKILL.md leaves the seam +// rung green. +// =========================================================================== +// +// THIS CASE PASSES WHEN THE HARNESS DOES NOT NOTICE. That is not a bug in +// this test, and it is not a bug in the harness -- it is NEVER-293.5, and +// gate ruling G2 (docs/design/stories/293.md) made mechanical: no green +// harness run, however thorough, may be read as licence to upgrade +// CLAIM-41.8 or CLAIM-47.4 off `model-judged`, and this case is the proof +// that the harness's own population cannot carry that upgrade even by +// accident. +// +// The harness's population is a TRANSCRIPTION of a `## Re-entry` table, not +// the model reading the markdown. Concretely: `packages/harness/src/re-entry.ts` +// attributes each `ctx.read(, ...)` call in +// `packages/harness/src/transcription-.ts` to a row NUMBER written by +// hand, by a reviewer, once -- see `docs/audits/293-transcription-audit.md`. +// At run time the harness (via `countReEntryRows`) reads only the row COUNT +// off disk. Nothing at run time re-derives which table row a transcribed +// step corresponds to, or checks that the table's ORDER matches the +// transcription's execution order. +// +// That is exactly why swapping two data rows must leave the harness green: +// the count is unchanged (it's a permutation, not a deletion -- asserted +// below), so `countReEntryRows` agrees between the mutated tree and the real +// one, and the harness has no other lever it pulls on this table. +// +// It would be tempting to call that a hole and demand the harness go red on +// a reordered table. IT MUST NOT. `docs/audits/293-transcription-audit.md` +// records, row by row, that three of the four transcriptions already +// execute their `## Re-entry` rows OUT OF the table's written order, each +// for a reason recorded in that document (a hard-failure read that must run +// before any create, a re-read that must not reuse an earlier result, a +// sentinel body that cannot be written before the read that pins it). Each +// skill body states the read-before-write CONSTRAINT itself, never a +// top-to-bottom sequence. A harness that went red on THIS reorder would be +// asserting an ordering the skill bodies never claimed, and would fail on +// three of the four skills as they are honestly shipped today. +// +// DO NOT "REPAIR" THIS TEST. Deleting it, or "fixing" it so the harness must +// go red here, deletes the only published, tool-checked measurement of the +// transcription gap. The gap that remains real -- a cell's TEXT changing to +// mean something else -- is bounded instead by CLAIM-293.11 (block 3) and by +// case 17 (block 2), which is exactly why this repository keeps that gap +// `model-judged` rather than pretending a green run here closes it. +describe("case 16 (P0, NEVER-293.5): mutating a SKILL.md leaves the seam rung green", () => { + const bodies = readSkillBodies(realSkillsDir); + + // DENOMINATOR NON-ZERO FIRST, and exactly 4 -- so "4 of 4 bodies" is + // visible in the output rather than a silently short loop. + test("0. the real skills directory carries exactly 4 skill bodies", () => { + expect(bodies.length).toBeGreaterThan(0); + expect(bodies.length).toBe(4); + }); + + // THE CONTROL. Four green runs against mutated copies prove nothing unless + // `--skills-dir` is load-bearing: if the flag were ignored -- or silently + // fell back to the repository's real `skills/` -- every mutation below + // would run against an UNMUTATED corpus and pass vacuously, and the whole + // block would be green for the wrong reason. A pure row reorder is + // invisible in the artifact (that is the point of case 16), so nothing in + // a green run can distinguish the two directories. This test supplies the + // distinguishing evidence separately: a copy carrying a FIFTH skill, which + // no scenario covers, must turn the harness RED. If it does not, the flag + // is not being honoured and every assertion below is worthless. + test("0b. control: --skills-dir is load-bearing, so the four runs below are not vacuous", async () => { + const controlDir = temps.create("iai-transcription-gap-control-"); + cpSync(realSkillsDir, controlDir, { recursive: true }); + const extra = join(controlDir, "not-a-real-skill"); + mkdirSync(extra, { recursive: true }); + writeFileSync(join(extra, "SKILL.md"), "# not a real skill\n\n## Re-entry\n\n| Read first | Then, and only then |\n|---|---|\n| Does this exist? | it does not |\n", "utf8"); + + // Assert the control corpus really differs from the real one before + // drawing any conclusion from the run -- a control that is not a control + // reports the answer you were hoping for. + expect(readSkillBodies(controlDir).length).toBe(bodies.length + 1); + + const outPath = join(temps.create("iai-transcription-gap-control-out-"), "report.json"); + const spawned = await spawnHarness(["--skills-dir", controlDir, "--out", outPath]); + expect(spawned.exitCode).not.toBe(0); + const artifact = JSON.parse(readFileSync(outPath, "utf8")) as HarnessArtifact; + expect(artifact.verdict).toBe("fail"); + expect(artifact.failures.length).toBeGreaterThan(0); + }); + + // Driven one at a time -- 4 separate `test()` calls -- rather than a + // single loop-wrapped assertion, so each skill's pass/fail is its own + // line in the test output. + for (const body of bodies) { + test(`mutating only ${body.name}'s SKILL.md (swap first two Re-entry rows) leaves the harness green`, async () => { + const skillNames = readdirSync(realSkillsDir).filter((name) => + statSync(join(realSkillsDir, name)).isDirectory(), + ); + expect(skillNames.length).toBe(4); + + const mutatedSkillsDir = temps.create(`iai-transcription-gap-${body.name}-`); + for (const name of skillNames) { + cpSync(join(realSkillsDir, name), join(mutatedSkillsDir, name), { recursive: true }); + } + // All four skills must be present in the copy, or the harness's + // per-skill coverage denominator fails for reasons unrelated to this + // mutation. + for (const name of skillNames) { + expect(existsSync(join(mutatedSkillsDir, name, "SKILL.md"))).toBe(true); + } + + const skillMdPath = join(mutatedSkillsDir, body.name, "SKILL.md"); + const originalText = readFileSync(skillMdPath, "utf8"); + const { mutatedText, originalDataRows, mutatedDataRows } = swapFirstTwoReEntryRows(originalText); + + // THE MUTATION MUST ASSERT ITS PATTERN MATCHED. + // (a) the section was found and there are at least 2 data rows -- + // implicit: swapFirstTwoReEntryRows throws otherwise, and a throw + // inside this test would fail it, not silently no-op it. + expect(originalDataRows.length).toBeGreaterThanOrEqual(2); + // (b) the mutated text differs from the original. + expect(mutatedText).not.toBe(originalText); + // (c) the two swapped line strings are exchanged at their indices. + expect(mutatedDataRows[0]).toBe(originalDataRows[1]); + expect(mutatedDataRows[1]).toBe(originalDataRows[0]); + // (d) the multiset of data-row lines is unchanged -- a permutation, + // not a deletion. + expect([...mutatedDataRows].sort()).toEqual([...originalDataRows].sort()); + + writeFileSync(skillMdPath, mutatedText, "utf8"); + + // The reorder must not change the denominator. + expect(countReEntryRows(mutatedSkillsDir)).toEqual(countReEntryRows(realSkillsDir)); + + const outPath = join(temps.create("iai-transcription-gap-out-"), "report.json"); + const spawned = await spawnHarness(["--skills-dir", mutatedSkillsDir, "--out", outPath]); + + // THE EXIT CODE FIRST. + expect(spawned.exitCode).toBe(0); + + const artifact = JSON.parse(readFileSync(outPath, "utf8")) as HarnessArtifact; + expect(artifact.verdict).toBe("pass"); + expect(artifact.failures).toEqual([]); + }); + } +}); + +// =========================================================================== +// BLOCK 2 -- case 17 (P0, NEVER-293.5): the transcription gap is published. +// =========================================================================== +// +// Gate ruling G2 says the seam rung's green may never upgrade CLAIM-41.8 or +// CLAIM-47.4 off `model-judged`. CLAIM-41.8's population is the MODEL +// reading the markdown; the harness's population (block 1, above) is a +// TRANSCRIPTION of it -- a different population, however green the run is. +// This assertion is what makes that prohibition MECHANICAL rather than +// cultural: it reads `docs/test-plans/41-plan.md` itself, at run time, and +// fails if a future edit ever quietly promoted CLAIM-41.8's row out of +// `model-judged`, or split it into more than one row. +describe("case 17 (P0, NEVER-293.5): the transcription gap is published", () => { + const planPath = join(repoRoot, "docs", "test-plans", "41-plan.md"); + const planText = readFileSync(planPath, "utf8"); + const rows = planText + .split("\n") + .filter((l) => l.startsWith("|")) + .map((l) => l.split("|")); + + // | # | Case | anchors_to | Target | Priority | Verifier | Corpus | Command | Passes when | + // 0 1 2 3 4 5 6 7 8 (index 0 is empty, from the leading `|`) + const CLAIM_41_8_ROWS = rows.filter((cells) => (cells[3] ?? "").trim() === "CLAIM-41.8"); + + test("exactly one row in the case table anchors to CLAIM-41.8", () => { + expect(CLAIM_41_8_ROWS.length).toBe(1); + }); + + test("that row's Verifier column is model-judged, never tool-checked", () => { + const row = CLAIM_41_8_ROWS[0] as string[]; + expect((row[6] ?? "").trim()).toBe("model-judged"); + }); +}); + +// =========================================================================== +// BLOCK 3 -- case 18's tool-checked denominator (P0, CLAIM-293.11). +// =========================================================================== +// +// CLAIM-293.11 is `model-judged`, and its VERDICT lives in +// docs/audits/293-transcription-audit.md, written by hand -- not here. This +// block is only its DENOMINATOR, which IS tool-checked: the row count is +// read off each skill body at run time via `countReEntryRows`, never +// hardcoded, and this test asserts the audit's own row set against it. That +// catches a row ADDED to a `## Re-entry` table (or the audit) with no +// counterpart on the other side, and a row REMOVED the same way. IT CANNOT +// CATCH A CELL EDITED -- a row whose "Read first" text changed to mean +// something else, with the audit's corresponding cell edited to match, +// passes this test exactly as cleanly as a row nobody touched. Do not read +// a green run of this block as CLAIM-293.11 itself; it is the denominator +// CLAIM-293.11 depends on, and no more. +describe("case 18's tool-checked denominator (P0, CLAIM-293.11)", () => { + const auditPath = join(repoRoot, "docs", "audits", "293-transcription-audit.md"); + const auditText = readFileSync(auditPath, "utf8"); + const auditLines = auditText.split("\n"); + + // CRITICAL PARSING RULE: parse ONLY the table inside `## The audit`, up to + // the next line starting with `## `. The document also carries a LATER + // summary table whose rows also begin with a skill name followed by an + // integer -- parsing the whole file would double-count and hide a defect + // rather than reveal one. + const sectionStart = auditLines.findIndex((l) => l.trim() === "## The audit"); + if (sectionStart === -1) throw new Error("## The audit section not found"); + let sectionEnd = auditLines.length; + for (let i = sectionStart + 1; i < auditLines.length; i += 1) { + if ((auditLines[i] as string).startsWith("## ")) { + sectionEnd = i; + break; + } + } + const sectionLines = auditLines.slice(sectionStart, sectionEnd); + // `## The audit` MUST CONTAIN EXACTLY ONE TABLE, AND THAT IS ASSERTED, NOT + // ASSUMED. The document also carries a per-skill summary table whose rows + // likewise begin with a skill name followed by an integer; while the two + // shared a section, a section-wide parse collected 23 pairs instead of 16, + // silently double-counting four skills. That error INFLATES the + // denominator, which is the direction that hides a missing row. + // + // Taking "the first contiguous run of `|` lines" would tolerate the second + // table instead of reporting it — and would then silently read the WRONG + // table if one were ever inserted above this one. So the structural + // constraint the summary table's own admonition states is enforced here: + // every `|`-prefixed line in the section must form ONE contiguous block. + // A second table in this section fails the test that depends on there + // being one, which is the only way a document convention stays true. + const tableIdx = sectionLines + .map((l, i) => (l.startsWith("|") ? i : -1)) + .filter((i) => i !== -1); + if (tableIdx.length === 0) throw new Error("no table found in ## The audit section"); + const first = tableIdx[0] as number; + const last = tableIdx[tableIdx.length - 1] as number; + const contiguous = last - first + 1 === tableIdx.length; + const tableLines = sectionLines.slice(first, last + 1); + // Drop the header row and the `|---|---|...|` separator. + const dataLines = tableLines.slice(2); + const audited = dataLines.map((line) => { + const cells = line.split("|"); + return { skill: (cells[1] ?? "").trim(), row: Number((cells[2] ?? "").trim()) }; + }); + + const onDiskRows = reEntryRows(realSkillsDir); + const onDiskBodies = readSkillBodies(realSkillsDir); + + test("0. `## The audit` holds exactly one table, so the parse reads the rows it means to", () => { + // If this fails, a second table has been added to the section (most + // likely the summary table moved back under it). Do not relax the + // parser: move the table out, under its own heading. + expect(contiguous).toBe(true); + }); + + test("1. denominator non-zero first: on-disk Re-entry rows and skill bodies are both non-empty", () => { + expect(onDiskRows.length).toBeGreaterThan(0); + expect(onDiskBodies.length).toBeGreaterThan(0); + }); + + test("2. cardinality, not length: the audited (skill, row) pairs contain no duplicate", () => { + const pairs = audited.map((a) => `${a.skill}#${a.row}`); + expect(new Set(pairs).size).toBe(audited.length); + }); + + test("3. the audited (skill, row) pair set equals exactly the on-disk set", () => { + const auditedPairs = audited.map((a) => `${a.skill}#${a.row}`).sort(); + const onDiskPairs = onDiskRows.map((r) => `${r.skill}#${r.row}`).sort(); + expect(auditedPairs).toEqual(onDiskPairs); + expect(onDiskPairs).toEqual(auditedPairs); + }); + + test("4. per-skill audited row counts deep-equal countReEntryRows on disk", () => { + const auditedCounts: Record = {}; + for (const name of readSkillBodies(realSkillsDir).map((b) => b.name)) auditedCounts[name] = 0; + for (const a of audited) auditedCounts[a.skill] = (auditedCounts[a.skill] ?? 0) + 1; + expect(auditedCounts).toEqual(countReEntryRows(realSkillsDir)); + }); +}); From 2b4f87df196fbd6cae5ed7e8175e3f203a2bceea Mon Sep 17 00:00:00 2001 From: Prem Mallappa Date: Sat, 12 Sep 2026 15:57:25 +0530 Subject: [PATCH 2/4] #323: the seam job, and --inject to prove the harness can fail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Case 9 of docs/test-plans/293-plan.md, closing CLAIM-293.6 half (ii), plus build target 7's job. The job is NOT promoted to a required context here; the promotion is sequenced separately and its procedure is written into ci.yml. --inject selects INJECTION_ROSTER, five scenarios, one per FailureMode. The truncated-list scenario makes goal-create's run 2 issue a duplicate create: run 1 POSTs the milestone "M9 — universal lifecycle", the lie drops the last element of the one-element list, run 2 reads absence and POSTs the identical title again. The fake has no duplicate check on any identity key, so it lands and the harness reports run-2-mutated. The assertion is on the recorded argv, never the exit code -- #320's M9/M9b pair proved a double that refuses and a double that accepts return the same verdict and differ only in state. The lie is armed BETWEEN the runs, never before run 1. Every one of the five is a lie about what run 2 sees; arming it for run 1 leaves nothing genuine for run 2's create to duplicate, and the harness would prove nothing. `bun run skill-harness --inject` EXITS 1, and that non-zero exit is the success condition of the mode. The wrapper still assigns exitCode from verdict.exitCode unconditionally: inverting it under --inject would move the branch-on-the-verdict one file over from decideVerdict, which case 24 statically forbids, and a branch does not stop being a branch by relocating. Two findings, reported and not forced. Three of the five modes collapse to one signature -- the read-lie trio all crash run 2 on JSON.parse of empty stdout, because no transcription checks a read's exit code first. That is #320's finding reproduced one layer up, and it belongs to #331. silent-no-op-mutation is armed but never engaged: all four transcriptions are correctly idempotent under honest reads, so run 2 attempts no mutation for it to intercept. It is verified at createFakeForge's own seam instead. The seam job runs `bun run skill-harness`, carries no `if:`, no matrix and no paths filter, and is deliberately not yet required. A never-reported context sits pending forever (#204), and story/47 and its six task issues carry no seam job on their heads -- promoting now would permanently block PR #313 and every S2.3 task PR. ci.yml names all four in-tree promotion sites, flagging the `required = [...]` list inside the PYEOF heredoc as the one that gets missed, plus the two out-of-tree settings a human must change afterwards. test/ci-workflow.test.ts makes that gap mechanical: the four hardcoded lists must agree as ordered arrays, and ci.yml's job set must PARTITION into required-and-listed union NOT_REQUIRED, each entry carrying its reason. A carve-out cannot detect an unclassified new member; a partition must. Five mutations were injected against it and all five were killed. The claim-lint comment at ci.yml:114-123 was stale in both halves: it said claim-lint was not in branch protection or ruleset 21656025, and it is in both. Corrected, and the promotion commit 15e5a36 is now cited so the next reader can check it rather than trust it. #304's class, found in the very comment the new one is modelled on. Every subprocess-spawning test now declares an explicit 120s timeout. They inherited bun's 5s default and one had already flaked under parallel load, which breaks the required `test` job. --- .github/workflows/ci.yml | 89 +++- packages/harness/src/index.ts | 3 + packages/harness/src/injection-roster.ts | 384 ++++++++++++++++++ packages/harness/src/runner.ts | 30 +- packages/harness/test/fake-forge.test.ts | 24 +- .../harness/test/injection-roster.test.ts | 302 ++++++++++++++ scripts/skill-harness.ts | 47 ++- test/ci-workflow.test.ts | 297 ++++++++++++++ test/skill-harness.test.ts | 67 ++- test/transcription-gap.test.ts | 8 +- 10 files changed, 1220 insertions(+), 31 deletions(-) create mode 100644 packages/harness/src/injection-roster.ts create mode 100644 packages/harness/test/injection-roster.test.ts create mode 100644 test/ci-workflow.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 10926b1..31b8ae3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -112,15 +112,27 @@ jobs: - run: bun run skill-lint skills/ # claim-lint enforces CLAIM-194.1 through CLAIM-194.4 of - # docs/design/stories/194.md. It is intentionally NOT in branch protection - # or ruleset 21656025 yet. Per the decision recorded on issue #204, a - # required context that has never reported sits at "pending" forever and - # blocks every PR permanently, so the context must first merge to main and - # report on a real PR before it can be made required. Making it required is - # a follow-up commit that must also update scripts/verify-required-checks.sh - # and scripts/verify-workflow-hygiene.sh (each hardcodes the job list twice) - # in that same commit. Unlike smoke-install, this job can meaningfully fail - # today — it is a real check, merely not yet enforced. + # docs/design/stories/194.md. It HAS SINCE BEEN PROMOTED: it is now one of + # the six required contexts in both main's branch protection and ruleset + # 21656025 — verified live via `gh api repos/.../branches/main/protection` + # and the ruleset, each listing claim-lint among exactly six contexts. + # scripts/verify-required-checks.sh and scripts/verify-workflow-hygiene.sh + # were updated in that same promotion commit, 15e5a36 ("#204: make + # claim-lint the sixth required check") — each hardcodes the job list + # twice, and one commit carries all four sites. + # + # THIS COMMENT WAS FALSE FOR AS LONG AS THE PROMOTION HAD BEEN DONE. It + # asserted the opposite of the live configuration, in the same file that + # tells the next person how to promote a job. Re-read a comment's claim + # against the thing it describes before you copy its advice. + # + # The durable lesson from getting here stands, because the new `seam` job + # below depends on it: per the decision recorded on issue #204, a required + # context that has never reported sits at "pending" forever and blocks + # every PR permanently, so a context must first merge to main and report on + # a real PR before it can be made required. That sequencing worked for + # claim-lint; `seam` is currently at the "merged but not yet required" + # stage claim-lint itself just passed through. claim-lint: runs-on: ubuntu-latest steps: @@ -134,6 +146,65 @@ jobs: - run: bun install - run: bun run claim-lint + # seam is build target 7 of docs/design/stories/293.md: the deterministic, + # offline, secret-free seam rung. It runs the skill harness's default + # scenario roster (`bun run skill-harness`, no --inject) against the + # in-memory fake forge, never a real one, so it needs no token and cannot + # flake on network state. Unlike smoke-install below, this job can + # MEANINGFULLY FAIL today: `bun run skill-harness --inject` is the proof — + # each of the five injectable failure modes (a zeroed rate-limit budget, + # the header-absent third state, a truncated unpaginated list, a silent + # no-op mutation, a transient connection error) makes the harness report a + # real, non-vacuous failure, per docs/test-plans/293-plan.md case 9. This + # job runs the plain (non-`--inject`) roster, so it is the everyday check; + # the `--inject` roster is exercised by test, not by this job. + # + # It is deliberately NOT YET REQUIRED, same reasoning that governed + # claim-lint above, for two independently sufficient reasons: + # 1. A required context that has never reported sits at "pending" + # forever and blocks every PR permanently (issue #204, restated + # above). This job must first merge to main and report on a real PR. + # 2. story/47-s2-3-execution-skills and its six open task issues have no + # seam job on their branch heads yet. Promoting this job to required + # now would permanently block PR #313 and every S2.3 task PR, since + # their heads predate this commit and can never report this context. + # + # PROMOTION PROCEDURE (a follow-up commit, once the two reasons above no + # longer hold). All FOUR of the following in-tree sites must be edited in + # ONE commit, or the four-way agreement the tests in + # test/ci-workflow.test.ts assert on will go red: + # (i) scripts/verify-required-checks.sh's REQUIRED_CONTEXTS array + # (ii) the same file's inline `required_status_checks.contexts[]=` + # list in the branch-protection help text (the `gh api` example + # under the 404 branch) + # (iii) scripts/verify-workflow-hygiene.sh's REQUIRED_JOBS array + # (iv) the `required = [...]` Python list inside that same file's + # `<<'PYEOF'` heredoc (run_yaml_parse) — THIS FOURTH ONE IS THE + # ONE THAT GETS MISSED, because it is nested inside a heredoc and + # does not show up in a plain grep for REQUIRED_JOBS or + # REQUIRED_CONTEXTS. + # Beyond the four in-tree sites, TWO OUT-OF-TREE settings must also be + # updated by a human, as a SEPARATE action taken only after this job has + # actually reported on main at least once: + # - main's branch protection required status checks + # - ruleset 21656025 (story-branch-checks, covering refs/heads/story/**) + # + # NEVER-9.8 still applies to this job even though it is not required: no + # `if:`, no `strategy:`/matrix, no `paths:` filter — a skipped job reports + # success, and a job that cannot fail is worse than useless as a signal. + seam: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: oven-sh/setup-bun@v2 + with: + bun-version: ${{ env.BUN_VERSION }} + - uses: actions/setup-node@v4 + with: + node-version: ${{ env.NODE_VERSION }} + - run: bun install + - run: bun run skill-harness + # This job is intentionally NOT added to branch protection and must never # be made required. docs/milestones/M1.md and PLAN.md's risk table both # describe a both-host install smoke test, but packages/installer has no diff --git a/packages/harness/src/index.ts b/packages/harness/src/index.ts index c938852..f21a51a 100644 --- a/packages/harness/src/index.ts +++ b/packages/harness/src/index.ts @@ -88,3 +88,6 @@ export type { StoryTestPlanOutcome, StoryTestPlanParams } from "./transcription- export { SCENARIO_ROSTER } from "./scenario-roster"; export type { ScenarioId } from "./scenario-roster"; + +export { INJECTION_ROSTER } from "./injection-roster"; +export type { InjectionScenarioId } from "./injection-roster"; diff --git a/packages/harness/src/injection-roster.ts b/packages/harness/src/injection-roster.ts new file mode 100644 index 0000000..5ad43f3 --- /dev/null +++ b/packages/harness/src/injection-roster.ts @@ -0,0 +1,384 @@ +// The injection roster: five scenarios, one per `FailureMode` +// (`fake-forge.ts`'s closed set), each arming its lie via +// `Scenario.injectBeforeRun2` after run 1 and before run 2. Case 9 of +// `docs/test-plans/293-plan.md`. Build target of #323 (task #323, closing +// CLAIM-293.6 half (ii)). +// +// NOT A SEVENTH ROSTER FREE TO GROW. `INJECTION_ROSTER` is a frozen module +// constant, exactly like `SCENARIO_ROSTER` (`scenario-roster.ts`) is. It +// reuses the same four `runXxx` transcriptions that module already imports +// -- goal-create, story-create, story-design, story-test-plan -- rather than +// authoring a sixth transcription. No new skill body is transcribed here; +// this module only supplies a different SEED and a different LIE to +// existing, already-reviewed re-entry logic. +// +// ============================================================================= +// THE CRITICAL DESIGN CONSTRAINT: `--inject` DOES NOT REOPEN THE VACUITY HOLE +// `scenario-roster.ts`'s HEADER CLOSES. +// ============================================================================= +// +// `scenario-roster.ts:9-17` says, in terms: "NO FLAG, NO PARAMETERISATION, NO +// INJECTION POINT -- AND THAT IS DELIBERATE", because a flag that can SHRINK +// a roster on demand is a harness that can be silenced from outside itself -- +// that is the exact shape the `empty-roster` failure code exists to catch. +// +// `scripts/skill-harness.ts`'s `--inject` flag does NOT have that shape, and +// here is the argument for why, stated once, in the one place a reader +// auditing both rosters together would look for it: +// +// 1. `--inject` selects between exactly TWO FROZEN CONSTANTS -- +// `SCENARIO_ROSTER` (six scenarios, unconditionally) or +// `INJECTION_ROSTER` (five scenarios, unconditionally, defined below). +// It is a SWITCH between two fixed values, never a parameter that +// shapes either one. There is no `--inject=` that trims +// `INJECTION_ROSTER` to fewer than five entries, no `--inject=` +// that swaps which `FailureMode` a given scenario arms, and no +// argument threading from the CLI into either roster's construction at +// all -- the flag is boolean, and everything downstream of "which +// constant" is exactly as frozen as `SCENARIO_ROSTER` already is. +// +// 2. `SCENARIO_ROSTER` remains UNREACHABLE FROM ANY ARGUMENT in the sense +// that matters: no combination of flags can make it smaller, reorder +// it, or remove a scenario from it. `--inject` does not touch it at +// all -- it *replaces* it wholesale with a second, equally-frozen +// roster; it does not parameterise the six-scenario roster into +// producing fewer than six results. +// +// 3. `INJECTION_ROSTER` is EQUALLY FROZEN. It is a `readonly Scenario[]` +// built exactly the way `SCENARIO_ROSTER` is (`Object.freeze([...])`, +// module-level, no factory function, no argument taken anywhere in +// this file). The only way to change its length or contents is a +// source edit to THIS file -- the same discipline +// `scenario-roster.ts`'s header states for its own roster, and the +// same thing a mutation test on this module would be built to defend. +// +// So the flag widens WHICH FROZEN CONSTANT runs, never HOW MANY SCENARIOS a +// constant contains, and never WHAT A SCENARIO DOES once selected. That is +// the property `scenario-roster.ts`'s header actually forbids losing, and +// `--inject` does not lose it. +// +// ============================================================================= +// WHY THE TRUNCATED-LIST SCENARIO IS THE ONE THAT PROVES THE HARNESS CAN FAIL +// ============================================================================= +// +// `goal-create`'s row-1 read is `gh api --paginate repos/OWNER/REPO/ +// milestones?...` (`transcription-goal-create.ts`'s `milestonesListArgv`), +// and it creates a milestone exactly when that list does not already carry +// one with the wanted title (skills/goal-create/SKILL.md:79's identity key). +// `fake-forge.ts`'s `truncate()` drops the LAST element of any list while +// `truncated-list` is armed -- "the newest object is the one a re-entrant +// skill just created and is about to look for" (`fake-forge.ts:358-360`). +// +// `INJECTION_TRUNCATED_LIST` below seeds NO milestone (run 1 creates the +// first, and only, one -- exactly `SCENARIO_1`'s shape in +// `scenario-roster.ts`). Between run 1 and run 2, `truncated-list` is armed. +// Run 2's re-read of a ONE-ELEMENT list, truncated by one, is an EMPTY list +// -- the milestone run 1 just created is now indistinguishable from a +// milestone that was never created. `goal-create`'s transcription reads +// exactly that empty list and does exactly what its own re-entry contract +// says to do when a milestone is absent: it creates one. `fake-forge.ts` +// has NO DUPLICATE CHECK ON ANY IDENTITY KEY (NEVER-293.7, +// `fake-forge.ts:26-32`), so the second `POST .../milestones` call, carrying +// the SAME title as run 1's, is accepted and lands as a second, real +// milestone. This is verified empirically in +// `packages/harness/test/injection-roster.test.ts`, not assumed: the test +// reads run 2's recorded forge mutations and asserts a second +// `-f title=` argv is present. + +import type { FailureMode, FakeForge } from "./fake-forge"; +import { GOALS_FIXTURE_CORPUS_DECLARATION, renderGoalsFixture } from "./goals-fixture"; +import type { Scenario } from "./runner"; +import { runGoalCreate } from "./transcription-goal-create"; +import { runStoryCreate } from "./transcription-story-create"; +import { runStoryDesign } from "./transcription-story-design"; +import { runStoryTestPlan } from "./transcription-story-test-plan"; + +/** A CLOSED SET: the five scenario ids below, and nothing else -- the same + * discipline `scenario-roster.ts`'s `ScenarioId` applies to its own six. */ +export type InjectionScenarioId = + | "goal-create/injected-truncated-list" + | "story-create/injected-rate-limit" + | "story-design/injected-forbidden-header" + | "story-test-plan/injected-transient-error" + | "goal-create/injected-silent-no-op"; + +type IdentifiedInjectionScenario = Scenario & { readonly id: InjectionScenarioId; readonly injectBeforeRun2: FailureMode }; + +const REPO_FLAG = ["--repo", "OWNER/REPO"] as const; + +function designBodyFor(issue: number): string { + return [ + `# Story ${String(issue)} Design`, + "", + "## Problem", + "", + `CLAIM-${String(issue)}.1: the subject behaves correctly the first time it is entered.`, + "", + ].join("\n"); +} + +function designSentinelBodyFor(issue: number): string { + return `## iai-design pinned\n\nDesign for #${String(issue)} committed by injection-roster. Corpus: synthetic.`; +} + +function planBodyFor(issue: number): string { + return [ + `# Test Plan for Story ${String(issue)}`, + "", + "| Case | anchors_to | Corpus |", + "|---|---|---|", + `| 1 | CLAIM-${String(issue)}.1 | synthetic |`, + "", + ].join("\n"); +} + +function testPlanSentinelBodyFor(issue: number): string { + return `## iai-test-plan pinned\n\nTest plan for #${String(issue)} committed by injection-roster. Corpus: synthetic.`; +} + +// ============================================================================= +// 1. goal-create/injected-truncated-list -- THE REQUIRED CASE. +// ============================================================================= +// +// Byte-for-byte `SCENARIO_1` of `scenario-roster.ts` (same goals fixture, +// same milestone title, same feature table), except for its marker line +// (own seed tree hash) and `injectBeforeRun2`. Run 1: milestone absent, +// created. Run 2, with `truncated-list` armed: the one-element list is +// truncated to zero elements, the milestone reads as absent again, and +// `goal-create` creates a SECOND milestone with the SAME title -- the +// duplicate create this case exists to prove. + +const INJECTION_TRUNCATED_LIST: IdentifiedInjectionScenario = { + id: "goal-create/injected-truncated-list", + skill: "goal-create", + corpus: GOALS_FIXTURE_CORPUS_DECLARATION, + files: [ + { path: "goals/GOALS.md", contents: renderGoalsFixture({ goals: ["universal lifecycle"] }) }, + { + path: "docs/milestones/M9.md", + contents: "# M9\n\n\n", + }, + ], + milestones: [], + injectBeforeRun2: "truncated-list", + async run(ctx): Promise { + await runGoalCreate(ctx, { + goalsPath: "goals/GOALS.md", + goalId: "G0", + milestoneTitle: "M9 — universal lifecycle", + featureTable: "| Feature | Description |\n|---|---|\n| Onboarding | user completes onboarding |\n", + }); + }, +}; + +// ============================================================================= +// 2. story-create/injected-rate-limit +// ============================================================================= +// +// `story-create`'s row 1 read is `gh issue list --json number,title, +// milestone` (`transcription-story-create.ts`'s `issueListArgv`), a plain +// `gh issue list` classified by `handleIssue`'s `action === "list"` branch +// -- exactly the shape `lieOnRead` intercepts. Run 1 seeds no issues, so it +// creates both feature rows (non-vacuous). Run 2, with +// `rate-limit-zeroed-budget` armed: the re-read fails (exit 1, empty +// stdout), and `parseIssues` (`JSON.parse` over that empty string) throws -- +// this transcription never checks the read's exit code before parsing it, +// so the failure surfaces as `scenario-threw`, not a graceful re-entry +// decision. Reported, not hidden: see this module's header and +// `injection-roster.test.ts`'s report on signature collapse. + +const FEATURE_ROWS_INJECTED = ["Feature: create the injected widget", "Feature: retire the injected widget"] as const; + +const INJECTION_RATE_LIMIT: IdentifiedInjectionScenario = { + id: "story-create/injected-rate-limit", + skill: "story-create", + corpus: "synthetic — a two-row feature table authored for #323's injection roster; no real binding behind it", + files: [ + { + path: "docs/milestones/M9.md", + contents: [ + "# M9 — full lifecycle", + "", + "", + "", + "| Feature | Description |", + "|---|---|", + `| A | ${FEATURE_ROWS_INJECTED[0]} |`, + `| B | ${FEATURE_ROWS_INJECTED[1]} |`, + "", + ].join("\n"), + }, + { path: "bindings/dev.md", contents: "domain: dev\nowner: platform\n" }, + ], + injectBeforeRun2: "rate-limit-zeroed-budget", + async run(ctx): Promise { + await runStoryCreate(ctx, { + milestonePath: "docs/milestones/M9.md", + milestoneTitle: "M9 — full lifecycle", + bindingPath: "bindings/dev.md", + featureRows: FEATURE_ROWS_INJECTED, + }); + }, +}; + +// ============================================================================= +// 3. story-design/injected-forbidden-header +// ============================================================================= +// +// `story-design`'s row 2 read is `gh issue view --json comments` +// (`transcription-story-design.ts`'s `issueViewArgv`), classified by +// `handleIssue`'s `action === "view"` branch -- also intercepted by +// `lieOnRead`. `seed()` issues one `gh issue create` on the raw fake +// (becoming issue 1), same reasoning as `SCENARIO_3`. Run 1: no Design file +// yet, so it writes the Design and posts the sentinel (non-vacuous). Run 2, +// with `forbidden-no-budget-header` armed (the THIRD STATE -- +// `fake-forge.ts:130-144` -- a 403 with no budget header captured): the +// sentinel re-read fails (exit 1, empty stdout) and `JSON.parse` over that +// empty stdout throws, again surfacing as `scenario-threw`. + +const INJECTION_FORBIDDEN_HEADER: IdentifiedInjectionScenario = { + id: "story-design/injected-forbidden-header", + skill: "story-design", + corpus: "synthetic — a Story with no Design yet, authored for #323's injection roster", + files: [{ path: "MARKER.md", contents: "injection scenario 3: forbidden-header marker\n" }], + injectBeforeRun2: "forbidden-no-budget-header", + async seed(forge: FakeForge): Promise { + await forge.run([ + "gh", + "issue", + "create", + ...REPO_FLAG, + "--title", + "Story: injected-forbidden-header", + "--body", + "seed body for injection scenario 3", + "--label", + "type:story", + ]); + }, + async run(ctx): Promise { + await runStoryDesign(ctx, { + issue: 1, + designPath: "docs/design/stories/1.md", + designBody: designBodyFor(1), + sentinelBody: designSentinelBodyFor(1), + }); + }, +}; + +// ============================================================================= +// 4. story-test-plan/injected-transient-error +// ============================================================================= +// +// `story-test-plan`'s row 5 read is `gh issue view --json comments` +// (`transcription-story-test-plan.ts`), also intercepted by `lieOnRead`. +// `seed()` issues one `gh issue create` (issue 1), same reasoning as +// `SCENARIO_4`. Run 1: no plan yet, so it writes the plan and posts the +// sentinel (non-vacuous). Run 2, with `transient-connection-error` armed: +// the mode is TRANSIENT (`fake-forge.ts:288-292` -- it "spends itself on the +// first call ... and the identical argv then succeeds"), but this +// transcription issues its row-5 read exactly ONCE per run and never +// retries, so the one call that fails is the only call made -- the failure +// still surfaces as `scenario-threw`, for the same reason as scenarios 2 and +// 3: none of the four transcriptions implement retry-on-failure logic. + +const INJECTION_TRANSIENT_ERROR: IdentifiedInjectionScenario = { + id: "story-test-plan/injected-transient-error", + skill: "story-test-plan", + corpus: "synthetic — a Design with one claim and no plan yet, authored for #323's injection roster", + files: [{ path: "docs/design/stories/1.md", contents: designBodyFor(1) }], + injectBeforeRun2: "transient-connection-error", + async seed(forge: FakeForge): Promise { + await forge.run([ + "gh", + "issue", + "create", + ...REPO_FLAG, + "--title", + "Story: injected-transient-error", + "--body", + "seed body for injection scenario 4", + "--label", + "type:story", + ]); + }, + async run(ctx): Promise { + await runStoryTestPlan(ctx, { + issue: 1, + designPath: "docs/design/stories/1.md", + planPath: "docs/test-plans/1-plan.md", + planBody: planBodyFor(1), + sentinelBody: testPlanSentinelBodyFor(1), + }); + }, +}; + +// ============================================================================= +// 5. goal-create/injected-silent-no-op +// ============================================================================= +// +// Reuses `goal-create` a second time, deliberately -- the roster has five +// modes and only four transcriptions, and `goal-create`'s is the only +// re-entry contract this module found a genuine duplicate-create hazard +// in, so it earns the repeat rather than a fifth transcription being +// invented to avoid one. Same shape as scenario 1: run 1 creates the one +// milestone from an empty list (non-vacuous). Run 2, with +// `silent-no-op-mutation` armed, but WITHOUT `truncated-list`: the re-read +// is answered HONESTLY (`lieOnRead` has no clause for this mode -- +// `fake-forge.ts:325-337` -- it only touches `lieOnMutation`), so +// `goal-create` correctly sees the milestone it already created and +// attempts NO create at all. `silent-no-op-mutation`'s own clause in +// `lieOnMutation` (`fake-forge.ts:345-351`, "exit 0, nothing written") is +// therefore armed but never reached by this scenario's run 2 -- there is no +// mutation attempt for it to intercept. +// +// THIS IS REPORTED, NOT PAPERED OVER (see this module's header and +// `injection-roster.test.ts`). None of the four transcriptions attempts a +// mutation on a truthfully-re-read run 2 by design -- that is the exact +// re-entrance property `scenario-roster.ts`'s six scenarios exist to prove. +// `silent-no-op-mutation` only has an effect on an ATTEMPTED mutation, and +// producing one honestly (without also lying about a read, which is a +// different mode's job) is not available from any of the four existing +// transcriptions' correct behaviour. `injection-roster.test.ts` instead +// exercises `silent-no-op-mutation`'s own behaviour directly against +// `createFakeForge`, at the fake's own seam, so the mode's mechanics are +// still verified even though this particular scenario's run 2 cannot +// observe them. + +const INJECTION_SILENT_NO_OP: IdentifiedInjectionScenario = { + id: "goal-create/injected-silent-no-op", + skill: "goal-create", + corpus: GOALS_FIXTURE_CORPUS_DECLARATION, + files: [ + { path: "goals/GOALS.md", contents: renderGoalsFixture({ goals: ["universal lifecycle"] }) }, + { + path: "docs/milestones/M9.md", + contents: "# M9\n\n\n", + }, + ], + milestones: [], + injectBeforeRun2: "silent-no-op-mutation", + async run(ctx): Promise { + await runGoalCreate(ctx, { + goalsPath: "goals/GOALS.md", + goalId: "G0", + milestoneTitle: "M9 — silent no-op lifecycle", + featureTable: "| Feature | Description |\n|---|---|\n| Onboarding | user completes onboarding |\n", + }); + }, +}; + +// ============================================================================= +// The roster +// ============================================================================= + +/** Five scenarios, one per `FailureMode`, FROZEN -- see this module's own + * header for why that freeze is exactly as load-bearing as + * `SCENARIO_ROSTER`'s. */ +export const INJECTION_ROSTER: readonly Scenario[] = Object.freeze([ + INJECTION_TRUNCATED_LIST, + INJECTION_RATE_LIMIT, + INJECTION_FORBIDDEN_HEADER, + INJECTION_TRANSIENT_ERROR, + INJECTION_SILENT_NO_OP, +]); diff --git a/packages/harness/src/runner.ts b/packages/harness/src/runner.ts index cd39a32..7b40cb2 100644 --- a/packages/harness/src/runner.ts +++ b/packages/harness/src/runner.ts @@ -41,7 +41,7 @@ import type { ExecResult, Port, RecordedCall, RecordingPort } from "iai-exec"; import { classifyArgv, countReads } from "./argv-kind"; -import { createFakeForge, type FakeForge } from "./fake-forge"; +import { createFakeForge, type FailureMode, type FakeForge } from "./fake-forge"; import { createFixtureRepo, type FixtureFile } from "./fixture-repo"; import { createMutationRecorder, mutationEvidence, type MutationReport } from "./mutation-recorder"; import { countReEntryRows, readSkillNames } from "./re-entry"; @@ -96,6 +96,25 @@ export interface Scenario { * mutation the subject made. */ seed?(forge: FakeForge): Promise; run(ctx: ScenarioContext): Promise; + /** Case 9 of docs/test-plans/293-plan.md. Arm one of the fake forge's five + * lies (`fake-forge.ts`'s `FailureMode`) AFTER run 1 completes and BEFORE + * run 2 begins -- never before run 1, and this is not an oversight to + * "simplify" later. + * + * RUN 1 MUST BE ALLOWED TO ESTABLISH REAL STATE. Every one of the five + * lies is a lie about what run 2 SEES; if it were also armed for run 1, + * run 1 could never create the milestone, issue or comment that run 2's + * lied read is then asked to misreport. A "duplicate create" is only a + * duplicate of something -- lying to run 1 leaves nothing for run 2's + * create to duplicate, and the harness would have proven nothing. + * + * Absent (the default for all six of `scenario-roster.ts`'s frozen + * scenarios) means exactly what it always meant before this field + * existed: `runHarness` still calls `forge.inject(null)` between the + * runs, which is a no-op against a forge that starts with `injected = + * null` and was never armed. The six frozen scenarios are bit-for-bit + * unaffected by this field's existence. */ + readonly injectBeforeRun2?: FailureMode; } // =========================================================================== @@ -728,6 +747,15 @@ export async function runHarness(options: RunHarnessOptions): Promise 0` (and, per +// this task's brief, `=== 5` rather than `>= 5`, so the denominator is exact, +// not merely non-vacuous) before anything else, lists asserted rather than +// counts, `new Set(...)` wherever cardinality (not length) is the property. + +import { afterAll, describe, expect, test } from "bun:test"; +import { join } from "node:path"; +import { createRealPort } from "iai-exec"; +import { + createFakeForge, + createTempDirs, + FAILURE_MODES, + INJECTION_ROSTER, + runHarness, + SCENARIO_ROSTER, + type FailureMode, + type HarnessVerdict, +} from "../src/index"; + +const repoRoot = join(import.meta.dir, "../../.."); +const skillsDir = join(repoRoot, "skills"); +const temps = createTempDirs(); +afterAll(() => temps.cleanup()); + +// =========================================================================== +// Roster hygiene -- no full run required for any of this +// =========================================================================== + +describe("injection roster hygiene", () => { + test("the roster is non-empty and has exactly five entries", () => { + expect(INJECTION_ROSTER.length).toBeGreaterThan(0); // denominator first + expect(INJECTION_ROSTER.length).toBe(5); + }); + + test("scenario ids are pairwise distinct", () => { + const ids = INJECTION_ROSTER.map((s) => s.id); + expect(new Set(ids).size).toBe(ids.length); + }); + + test("every one of the five FailureMode values appears exactly once, asserted against FAILURE_MODES itself", () => { + // ASSERTED AGAINST THE EXPORTED MODE LIST, NOT A LOCAL LITERAL OF FIVE + // STRINGS -- a sixth `FailureMode` added to `fake-forge.ts` without a + // matching scenario here must fail this test, not silently pass because + // this file's own copy of the set went stale alongside it. + expect(FAILURE_MODES.length).toBeGreaterThan(0); // denominator first + expect(FAILURE_MODES.length).toBe(5); + + const armed = INJECTION_ROSTER.map((s) => s.injectBeforeRun2).filter( + (m): m is FailureMode => m !== undefined, + ); + // EVERY SCENARIO ARMS A MODE. A scenario present in the roster but not + // arming anything would inflate `INJECTION_ROSTER.length` without + // covering a `FailureMode` -- this line makes that a distinct, + // separately-visible failure from the coverage check below. + expect(armed.length).toBe(INJECTION_ROSTER.length); + + // THE DENOMINATOR IS EXACT (`toBe`, never `toBeGreaterThanOrEqual`): a + // mode covered twice while another goes uncovered would satisfy `>= 5` + // on `armed.length` alone while leaving a real `FailureMode` untested. + const coveredModes = new Set(armed); + expect(coveredModes.size).toBe(FAILURE_MODES.length); + const uncovered = FAILURE_MODES.filter((mode) => !coveredModes.has(mode)); + // ASSERT THE LIST: an uncovered mode should name itself. + expect(uncovered).toEqual([]); + }); + + test("every scenario's skill is a known skill on disk", () => { + const knownSkills = new Set(["goal-create", "story-create", "story-design", "story-test-plan"]); + const unknown = INJECTION_ROSTER.filter((s) => !knownSkills.has(s.skill)).map((s) => `${s.id} -> ${s.skill}`); + expect(unknown).toEqual([]); + }); +}); + +// =========================================================================== +// The one full run, memoised +// =========================================================================== + +let verdictPromise: Promise | undefined; + +function runInjectionRoster(): Promise { + verdictPromise ??= runHarness({ + roster: INJECTION_ROSTER, + skillsDir, + temps, + makePort: () => createRealPort(), + }); + return verdictPromise; +} + +// =========================================================================== +// THE REQUIRED CASE: the truncated-list scenario's run 2 issues a duplicate +// milestone create, and the harness reports a failure for it. +// =========================================================================== +// +// Explicit 120 s timeout on every test below that calls runInjectionRoster(): +// each call runs a real git fixture + fake forge over the whole five-scenario +// roster; inheriting the 5 s default made the required `test` CI job flaky +// under parallel load. + +describe('"goal-create/injected-truncated-list": run 2 issues a duplicate create', () => { + test("run 1 and run 2 both mutate, and the harness's overall verdict is fail with a non-empty failures list", async () => { + const verdict = await runInjectionRoster(); + expect(verdict.verdict).toBe("fail"); + expect(verdict.exitCode).toBe(1); + expect(verdict.failures.length).toBeGreaterThan(0); // denominator first + + const scenario = verdict.scenarios.find((s) => s.id === "goal-create/injected-truncated-list"); + expect(scenario).toBeDefined(); + if (scenario === undefined) return; + expect(scenario.run1).not.toBeNull(); + expect(scenario.run2).not.toBeNull(); + if (scenario.run1 === null || scenario.run2 === null) return; + + expect(scenario.run1.mutations).toBeGreaterThan(0); + expect(scenario.run2.mutations).toBeGreaterThan(0); + }, 120_000); + + test("run 2's recorded forge mutations contain a milestone-create argv carrying the SAME title as run 1's -- asserted on the recorded argv, never on the exit code or on stdout", async () => { + // #320's M9/M9b pair proved a double that refuses and a double that + // accepts return the SAME verdict and differ only in STATE -- so this + // assertion reads `run1.forgeMutations` / `run2.forgeMutations` + // directly, never `verdict.exitCode`, `verdict.verdict`, or anything + // rendered to a string a human reads. + const verdict = await runInjectionRoster(); + const scenario = verdict.scenarios.find((s) => s.id === "goal-create/injected-truncated-list"); + expect(scenario).toBeDefined(); + if (scenario === undefined) return; + const run1 = scenario.run1; + const run2 = scenario.run2; + expect(run1).not.toBeNull(); + expect(run2).not.toBeNull(); + if (run1 === null || run2 === null) return; + + function milestoneCreateTitles(mutations: readonly (readonly string[])[]): readonly string[] { + return mutations + .filter((argv) => argv[0] === "gh" && argv[1] === "api" && argv.includes("--method") && argv.includes("POST")) + .map((argv) => { + const titleFlag = argv.find((part) => part.startsWith("title=")); + return titleFlag === undefined ? "" : titleFlag.slice("title=".length); + }) + .filter((title) => title.length > 0); + } + + const run1Titles = milestoneCreateTitles(run1.forgeMutations); + const run2Titles = milestoneCreateTitles(run2.forgeMutations); + + expect(run1Titles.length).toBe(1); // denominator first + expect(run2Titles.length).toBe(1); + + // THE DUPLICATE, QUOTED: run 2's create carries the identical title run + // 1's create carried -- a second, real milestone with the same identity + // key skills/goal-create/SKILL.md:79 keys existence on. + expect(run2Titles[0]).toBe(run1Titles[0]); + expect(run2Titles[0]).toBe("M9 — universal lifecycle"); + + // The full argv, quoted for the record: run 1's and run 2's + // milestone-create calls are byte-identical, because nothing about the + // TRANSCRIPTION changed between the two runs -- only the forge's answer + // to the list read did. + const run1CreateArgv = run1.forgeMutations.find( + (argv) => argv[0] === "gh" && argv.includes("--method") && argv.includes("POST"), + ); + const run2CreateArgv = run2.forgeMutations.find( + (argv) => argv[0] === "gh" && argv.includes("--method") && argv.includes("POST"), + ); + expect(run1CreateArgv).toBeDefined(); + expect(run2CreateArgv).toBeDefined(); + expect(run2CreateArgv).toEqual(run1CreateArgv); + }, 120_000); + + test("the harness reports it: a run-2-mutated failure names this scenario", async () => { + const verdict = await runInjectionRoster(); + const named = verdict.failures.filter((f) => f.code === "run-2-mutated" && f.scenario === "goal-create/injected-truncated-list"); + expect(named.length).toBe(1); + expect(named[0]?.message).toContain("goal-create/injected-truncated-list"); + }, 120_000); +}); + +// =========================================================================== +// The other four modes: each is armed (case 9 half (i) redux, at the +// scenario level rather than the raw-forge level), and each scenario's +// outcome is asserted precisely -- never inferred from the overall verdict. +// =========================================================================== + +describe("the remaining four injected scenarios", () => { + test("the three read-lie modes (rate-limit, forbidden-header, transient) each crash run 2 with scenario-threw -- REPORTED, NOT FORCED: their messages are IDENTICAL, which is the collapse #320 recorded (a zeroed budget, a header-absent 403 and a transient error all read as \"the read exited 1\") reproduced one layer up, at the transcription level", async () => { + const verdict = await runInjectionRoster(); + const ids = [ + "story-create/injected-rate-limit", + "story-design/injected-forbidden-header", + "story-test-plan/injected-transient-error", + ] as const; + + const messages: string[] = []; + for (const id of ids) { + const scenario = verdict.scenarios.find((s) => s.id === id); + expect(scenario).toBeDefined(); + if (scenario === undefined) continue; + expect(scenario.status).toBe("threw"); + expect(scenario.threw?.runIndex).toBe(2); + expect(scenario.run2).toBeNull(); + const message = scenario.threw?.message ?? ""; + expect(message.length).toBeGreaterThan(0); // denominator first + messages.push(message); + } + + expect(messages.length).toBe(ids.length); + // THE COLLAPSE, PINNED: none of the four transcriptions + // (`transcription-*.ts`) checks a forge read's exit code before parsing + // its stdout as JSON (only the git-backed hard-failure reads do that), + // so a lied read of ANY of these three modes crashes on + // `JSON.parse("")` with the SAME engine error message. Asserting + // equality here, rather than merely observing it, is what keeps this + // fact from silently drifting the next time one of these transcriptions + // changes -- a future transcription that starts checking exit codes + // would need to update this test, which is the point. + expect(new Set(messages).size).toBe(1); + + const failureCodes = verdict.failures + .filter((f) => ids.includes(f.scenario as (typeof ids)[number])) + .map((f) => f.code); + expect(failureCodes.length).toBe(ids.length); + expect(failureCodes.every((c) => c === "scenario-threw")).toBe(true); + }, 120_000); + + test("silent-no-op-mutation's scenario is armed but never engaged: run 2 attempts no mutation, so the mode has nothing to intercept -- REPORTED, NOT FORCED", async () => { + const verdict = await runInjectionRoster(); + const scenario = verdict.scenarios.find((s) => s.id === "goal-create/injected-silent-no-op"); + expect(scenario).toBeDefined(); + if (scenario === undefined) return; + expect(scenario.status).toBe("ok"); + expect(scenario.run1).not.toBeNull(); + expect(scenario.run2).not.toBeNull(); + if (scenario.run1 === null || scenario.run2 === null) return; + expect(scenario.run1.mutations).toBeGreaterThan(0); + expect(scenario.run2.mutations).toBe(0); + expect(scenario.run2.forgeMutations).toEqual([]); + + // No `run-2-mutated` failure for THIS scenario specifically (other + // scenarios in this same roster fail for their own, distinct reasons). + const named = verdict.failures.filter( + (f) => f.scenario === "goal-create/injected-silent-no-op", + ); + expect(named).toEqual([]); + }, 120_000); +}); + +// =========================================================================== +// silent-no-op-mutation, verified directly at the fake's own seam -- since +// none of the four transcriptions attempts a mutation on an honestly-read +// run 2 (the property the whole roster exists to prove), this is the mode's +// own mechanics, checked where they CAN be observed. +// =========================================================================== + +describe("silent-no-op-mutation's own mechanics, at createFakeForge's seam", () => { + test("exit 0, and the milestone is NOT added to the fake's state", async () => { + const forge = createFakeForge({ milestones: [] }); + forge.inject("silent-no-op-mutation"); + const result = await forge.run([ + "gh", + "api", + "--method", + "POST", + "repos/OWNER/REPO/milestones", + "-f", + "title=silent-no-op-probe", + "-f", + "description=synthetic", + ]); + expect(result.exitCode).toBe(0); + + forge.inject(null); + const after = await forge.run(["gh", "api", "--paginate", "repos/OWNER/REPO/milestones?state=all&per_page=100"]); + expect(after.exitCode).toBe(0); + const milestones = JSON.parse(after.stdout) as readonly { readonly title: string }[]; + // ASSERT THE LIST, NOT A COUNT: the probe title must be ABSENT, not + // merely "the count did not grow by exactly one" (which a mutation that + // both added and removed something could also satisfy). + expect(milestones.map((m) => m.title)).toEqual([]); + }); +}); + +// =========================================================================== +// The default path is unchanged: SCENARIO_ROSTER's own length and behaviour +// do not move because INJECTION_ROSTER and `injectBeforeRun2` now exist. +// =========================================================================== + +describe("the six frozen scenarios are bit-for-bit unaffected by injectBeforeRun2's existence", () => { + test("SCENARIO_ROSTER.length is still 6, and none of its entries declares injectBeforeRun2", () => { + expect(SCENARIO_ROSTER.length).toBe(6); + const armed = SCENARIO_ROSTER.filter((s) => s.injectBeforeRun2 !== undefined).map((s) => s.id); + expect(armed).toEqual([]); + }); +}); diff --git a/scripts/skill-harness.ts b/scripts/skill-harness.ts index 179958b..ec5e783 100644 --- a/scripts/skill-harness.ts +++ b/scripts/skill-harness.ts @@ -25,18 +25,34 @@ import { existsSync, mkdirSync, writeFileSync } from "node:fs"; import { dirname, join, resolve } from "node:path"; import { createRealPort } from "../packages/exec/src/index"; -import { createTempDirs, renderArtifact, renderReport, runHarness, SCENARIO_ROSTER } from "../packages/harness/src/index"; +import { + createTempDirs, + INJECTION_ROSTER, + renderArtifact, + renderReport, + runHarness, + SCENARIO_ROSTER, +} from "../packages/harness/src/index"; const DEFAULT_OUT_RELATIVE = join(".harness", "skill-harness-report.json"); interface ParsedArgs { readonly skillsDir: string; readonly out: string; + /** Case 9 of docs/test-plans/293-plan.md. Selects `INJECTION_ROSTER` + * (five scenarios, one per `FailureMode`, `packages/harness/src/ + * injection-roster.ts`) in place of `SCENARIO_ROSTER`. A SWITCH between + * two frozen constants, never a parameter that shapes either one -- see + * `injection-roster.ts`'s own header for why that does not reopen the + * vacuity hole `scenario-roster.ts`'s "NO FLAG, NO PARAMETERISATION, NO + * INJECTION POINT" forbids. */ + readonly inject: boolean; } function parseArgs(argv: readonly string[], repoRoot: string): ParsedArgs { let skillsDir = join(repoRoot, "skills"); let out = join(repoRoot, DEFAULT_OUT_RELATIVE); + let inject = false; for (let i = 0; i < argv.length; i += 1) { const arg = argv[i]; @@ -60,9 +76,13 @@ function parseArgs(argv: readonly string[], repoRoot: string): ParsedArgs { i += 1; continue; } + if (arg === "--inject") { + inject = true; + continue; + } } - return { skillsDir, out }; + return { skillsDir, out, inject }; } async function main(): Promise { @@ -78,7 +98,7 @@ async function main(): Promise { let exitCode: 0 | 1; try { const verdict = await runHarness({ - roster: SCENARIO_ROSTER, + roster: args.inject ? INJECTION_ROSTER : SCENARIO_ROSTER, skillsDir: args.skillsDir, temps, makePort: () => createRealPort(), @@ -89,7 +109,28 @@ async function main(): Promise { console.log(renderReport(verdict)); console.log(`skill-harness: artifact written to ${args.out}`); + if (args.inject) { + console.log( + "skill-harness: --inject ran the five-scenario INJECTION_ROSTER, not the six-scenario " + + "SCENARIO_ROSTER. A non-zero exit below is the SUCCESS condition of this mode.", + ); + } + // A NON-ZERO EXIT UNDER `--inject` IS THE SUCCESS CONDITION OF THIS MODE + // AND MUST NEVER BE "FIXED". `INJECTION_ROSTER`'s truncated-list + // scenario is BUILT to make run 2 issue a duplicate create (this + // module's own header, and `injection-roster.ts`'s), so `verdict. + // exitCode` for an `--inject` run is EXPECTED to be 1 -- that failing + // exit code IS the proof, required by case 9 of + // docs/test-plans/293-plan.md, that this harness can fail. Inverting it + // (returning 0 when `--inject` "worked"), suppressing it, or special- + // casing `args.inject` anywhere near `exitCode` below would reintroduce + // EXACTLY the branch-on-the-verdict shape case 24 of + // packages/harness/test/runner.test.ts statically forbids + // `decideVerdict` from creating in the first place -- moving that + // branch one file over to this wrapper does not make it not a branch. + // `exitCode` is assigned from `verdict.exitCode` UNCONDITIONALLY, for + // both rosters, exactly as it always was. exitCode = verdict.exitCode; } finally { // `process.exit` below is called AFTER this finally has run -- calling diff --git a/test/ci-workflow.test.ts b/test/ci-workflow.test.ts new file mode 100644 index 0000000..c037c80 --- /dev/null +++ b/test/ci-workflow.test.ts @@ -0,0 +1,297 @@ +// Tests for Task #323 (build target 7 of docs/design/stories/293.md): the +// seam rung's CI job, added but deliberately not yet required. Case 13 of +// docs/test-plans/293-plan.md: "the required-context count is asserted both +// before and after, and each verifier hardcodes the list twice, so all four +// sites must agree. A live job silently promoted to required fails here." +// +// House style copied from test/transcription-gap.test.ts exactly: every +// denominator is asserted non-zero FIRST, cardinality (a Set's size) rather +// than array length wherever duplicates would otherwise hide, and every +// parse asserts its own pattern actually matched before anything is built +// on top of it — a regex that silently matches nothing would make every +// later assertion in this file vacuously true. + +import { describe, expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +const repoRoot = join(import.meta.dir, ".."); +const ciYmlPath = join(repoRoot, ".github", "workflows", "ci.yml"); +const requiredChecksPath = join(repoRoot, "scripts", "verify-required-checks.sh"); +const workflowHygienePath = join(repoRoot, "scripts", "verify-workflow-hygiene.sh"); +const packageJsonPath = join(repoRoot, "package.json"); + +const ciYmlText = readFileSync(ciYmlPath, "utf8"); +const requiredChecksText = readFileSync(requiredChecksPath, "utf8"); +const workflowHygieneText = readFileSync(workflowHygienePath, "utf8"); +const packageJsonText = readFileSync(packageJsonPath, "utf8"); + +// =========================================================================== +// Job-name parse: the top-level keys under `jobs:` in ci.yml. +// =========================================================================== + +interface JobBlock { + readonly name: string; + readonly headerLineIndex: number; + readonly bodyLines: readonly string[]; +} + +function parseCiYmlJobs(text: string): readonly JobBlock[] { + const lines = text.split("\n"); + const jobsIdx = lines.findIndex((l) => l === "jobs:"); + if (jobsIdx === -1) throw new Error("`jobs:` top-level key not found in ci.yml"); + + const headerIndices: number[] = []; + const headerNames: string[] = []; + for (let i = jobsIdx + 1; i < lines.length; i += 1) { + const line = lines[i] as string; + const m = /^ {2}([A-Za-z0-9_-]+):\s*$/.exec(line); + if (m) { + headerIndices.push(i); + headerNames.push(m[1] as string); + } + } + + const blocks: JobBlock[] = []; + for (let i = 0; i < headerIndices.length; i += 1) { + const start = (headerIndices[i] as number) + 1; + const end = i + 1 < headerIndices.length ? (headerIndices[i + 1] as number) : lines.length; + blocks.push({ + name: headerNames[i] as string, + headerLineIndex: headerIndices[i] as number, + bodyLines: lines.slice(start, end), + }); + } + return blocks; +} + +const ciJobs = parseCiYmlJobs(ciYmlText); + +describe("job names parsed from ci.yml's `jobs:` block", () => { + test("0. denominator non-zero first: at least one job was parsed", () => { + expect(ciJobs.length).toBeGreaterThan(0); + }); + + test("job names are unique (cardinality equals count)", () => { + const names = ciJobs.map((j) => j.name); + expect(new Set(names).size).toBe(names.length); + }); +}); + +const ciJobNames = ciJobs.map((j) => j.name); +const ciJobNameSet = new Set(ciJobNames); + +// =========================================================================== +// The FOUR hardcoded lists. Each parse MUST assert it matched before its +// result is used for anything — a silently-empty match would make every +// later "all four agree" assertion vacuous. +// =========================================================================== + +// (1) scripts/verify-required-checks.sh's REQUIRED_CONTEXTS bash array. +const requiredContextsArrayMatch = /REQUIRED_CONTEXTS=\(([^)]*)\)/.exec(requiredChecksText); + +// (2) the same file's inline `required_status_checks.contexts[]=` list, in +// the branch-protection help text shown on a 404. +const inlineContextsMatches = [ + ...requiredChecksText.matchAll(/required_status_checks\.contexts\[\]=([A-Za-z0-9_-]+)/g), +]; + +// (3) scripts/verify-workflow-hygiene.sh's REQUIRED_JOBS bash array. +const requiredJobsArrayMatch = /REQUIRED_JOBS=\(([^)]*)\)/.exec(workflowHygieneText); + +// (4) the Python `required = [...]` list inside that file's `<<'PYEOF'` +// heredoc (run_yaml_parse). This is the site that gets missed in a plain +// grep for REQUIRED_JOBS/REQUIRED_CONTEXTS, because it lives inside a +// heredoc under a different name. +const pythonRequiredListMatch = /required\s*=\s*\[([^\]]*)\]/.exec(workflowHygieneText); + +describe("the four hardcoded required-job/context lists each parse successfully", () => { + test("(i) REQUIRED_CONTEXTS bash array in verify-required-checks.sh matched", () => { + expect(requiredContextsArrayMatch).not.toBeNull(); + expect((requiredContextsArrayMatch as RegExpExecArray)[1]?.trim().length).toBeGreaterThan(0); + }); + + test("(ii) inline `contexts[]=` occurrences in verify-required-checks.sh matched, and there is more than one", () => { + expect(inlineContextsMatches.length).toBeGreaterThan(0); + }); + + test("(iii) REQUIRED_JOBS bash array in verify-workflow-hygiene.sh matched", () => { + expect(requiredJobsArrayMatch).not.toBeNull(); + expect((requiredJobsArrayMatch as RegExpExecArray)[1]?.trim().length).toBeGreaterThan(0); + }); + + test("(iv) Python `required = [...]` list inside the <<'PYEOF' heredoc matched — the site that gets missed", () => { + expect(pythonRequiredListMatch).not.toBeNull(); + expect((pythonRequiredListMatch as RegExpExecArray)[1]?.trim().length).toBeGreaterThan(0); + // Confirm this match actually landed inside the heredoc, not some other + // `required = [...]` that might be added elsewhere in the future: the + // matched text must appear between the `<<'PYEOF'` open and the `PYEOF` + // close markers. + const heredocStart = workflowHygieneText.indexOf("<<'PYEOF'"); + const heredocEnd = workflowHygieneText.indexOf("\nPYEOF", heredocStart); + expect(heredocStart).toBeGreaterThan(-1); + expect(heredocEnd).toBeGreaterThan(heredocStart); + const matchOffset = (pythonRequiredListMatch as RegExpExecArray).index; + expect(matchOffset).toBeGreaterThan(heredocStart); + expect(matchOffset).toBeLessThan(heredocEnd); + }); +}); + +const requiredContextsArray = ((requiredContextsArrayMatch as RegExpExecArray)[1] as string) + .trim() + .split(/\s+/); +const inlineContextsList = inlineContextsMatches.map((m) => m[1] as string); +const requiredJobsArray = ((requiredJobsArrayMatch as RegExpExecArray)[1] as string) + .trim() + .split(/\s+/); +const pythonRequiredList = ((pythonRequiredListMatch as RegExpExecArray)[1] as string) + .split(",") + .map((s) => s.trim().replace(/^["']|["']$/g, "")) + .filter((s) => s.length > 0); + +describe("all four hardcoded lists agree, as ORDERED arrays (case 13 of docs/test-plans/293-plan.md)", () => { + test("0. denominator non-zero first: all four parsed lists are non-empty", () => { + expect(requiredContextsArray.length).toBeGreaterThan(0); + expect(inlineContextsList.length).toBeGreaterThan(0); + expect(requiredJobsArray.length).toBeGreaterThan(0); + expect(pythonRequiredList.length).toBeGreaterThan(0); + }); + + test("REQUIRED_CONTEXTS (bash array) == inline contexts[]= list", () => { + expect(requiredContextsArray).toEqual(inlineContextsList); + }); + + test("REQUIRED_CONTEXTS (bash array) == REQUIRED_JOBS (bash array)", () => { + expect(requiredContextsArray).toEqual(requiredJobsArray); + }); + + test("REQUIRED_CONTEXTS (bash array) == Python required = [...] list", () => { + expect(requiredContextsArray).toEqual(pythonRequiredList); + }); + + test("REQUIRED_JOBS (bash array) == Python required = [...] list", () => { + expect(requiredJobsArray).toEqual(pythonRequiredList); + }); + + test("inline contexts[]= list == Python required = [...] list", () => { + expect(inlineContextsList).toEqual(pythonRequiredList); + }); + + test("the required-context COUNT is still exactly six", () => { + expect(requiredContextsArray.length).toBe(6); + }); +}); + +// =========================================================================== +// THE PARTITION. required-list ∪ NOT_REQUIRED.keys() must equal the ci.yml +// job set EXACTLY, and the two must be DISJOINT. +// +// This is a partition, not a carve-out, and that distinction is the whole +// point of this block. A carve-out ("here are the jobs known not to be +// required") can only ever confirm what it already lists; it has no way to +// notice a brand-new job that was added to ci.yml and classified as +// neither. A partition instead computes the union against the LIVE job set +// parsed from ci.yml at run time and asserts equality — so a new job that +// nobody sorted into either bucket makes this test fail loudly, rather than +// silently passing because nothing here mentions it. +// =========================================================================== + +const NOT_REQUIRED: Readonly> = { + seam: "awaiting promotion; see ci.yml's comment above the `seam` job — not yet required because a never-reported context sits pending forever, and story/47-s2-3-execution-skills' task branches carry no seam job yet", + "smoke-install": + "cannot meaningfully fail until packages/installer/src/cli.ts exists (M8); see ci.yml's comment above the `smoke-install` job", +}; + +describe("THE PARTITION: required ∪ NOT_REQUIRED == ci.yml's job set, and the two are disjoint", () => { + test("0. denominator non-zero first: both the required list and NOT_REQUIRED are non-empty", () => { + expect(requiredContextsArray.length).toBeGreaterThan(0); + expect(Object.keys(NOT_REQUIRED).length).toBeGreaterThan(0); + }); + + test("required list and NOT_REQUIRED's keys are disjoint", () => { + const requiredSet = new Set(requiredContextsArray); + const overlap = Object.keys(NOT_REQUIRED).filter((k) => requiredSet.has(k)); + expect(overlap).toEqual([]); + }); + + test("required ∪ NOT_REQUIRED equals ci.yml's job set EXACTLY — a new, unclassified job fails here", () => { + const unionSet = new Set([...requiredContextsArray, ...Object.keys(NOT_REQUIRED)]); + expect(unionSet.size).toBe(ciJobNameSet.size); + expect([...unionSet].sort()).toEqual([...ciJobNameSet].sort()); + + const missingFromCi = [...unionSet].filter((j) => !ciJobNameSet.has(j)); + const missingFromPartition = ciJobNames.filter((j) => !unionSet.has(j)); + expect(missingFromCi).toEqual([]); + expect(missingFromPartition).toEqual([]); + }); +}); + +// =========================================================================== +// Per-job structural assertions: no job-level `if:` key on any required +// job, and the `seam` job specifically runs skill-harness. +// =========================================================================== + +function jobBlockByName(name: string): JobBlock { + const block = ciJobs.find((j) => j.name === name); + if (!block) throw new Error(`job "${name}" not found in ci.yml`); + return block; +} + +function hasJobLevelIf(block: JobBlock): boolean { + return block.bodyLines.some((l) => /^ {4}if:/.test(l)); +} + +describe("the `seam` job", () => { + test("exists as a top-level job in ci.yml", () => { + expect(ciJobNameSet.has("seam")).toBe(true); + }); + + const seamBlock = jobBlockByName("seam"); + + test("its steps include `bun run skill-harness`", () => { + const runLines = seamBlock.bodyLines + .map((l) => l.trim()) + .filter((l) => l.startsWith("- run:")); + expect(runLines.length).toBeGreaterThan(0); + expect(runLines).toContain("- run: bun run skill-harness"); + }); + + test("its YAML block contains no `if:` key (NEVER-9.8)", () => { + expect(hasJobLevelIf(seamBlock)).toBe(false); + }); + + test("its YAML block contains no `strategy:`/`matrix:` key", () => { + expect(seamBlock.bodyLines.some((l) => /^ {4}(strategy|matrix):/.test(l))).toBe(false); + }); +}); + +describe("no required job carries a job-level `if:` (NEVER-9.8, all six)", () => { + test("0. denominator non-zero first: the required-context list is non-empty", () => { + expect(requiredContextsArray.length).toBeGreaterThan(0); + }); + + for (const jobName of requiredContextsArray) { + test(`"${jobName}" carries no \`if:\` key`, () => { + const block = jobBlockByName(jobName); + expect(hasJobLevelIf(block)).toBe(false); + }); + } +}); + +// =========================================================================== +// The seam job's command must resolve to a real script — a typo here would +// silently no-op (or error) only when the job actually runs in CI. +// =========================================================================== + +describe("`skill-harness` is a real script in the root package.json", () => { + const pkg = JSON.parse(packageJsonText) as { scripts?: Record }; + + test("package.json parses and has a scripts block", () => { + expect(pkg.scripts).toBeDefined(); + expect(Object.keys(pkg.scripts ?? {}).length).toBeGreaterThan(0); + }); + + test("`skill-harness` is one of its scripts", () => { + expect(Object.prototype.hasOwnProperty.call(pkg.scripts ?? {}, "skill-harness")).toBe(true); + }); +}); diff --git a/test/skill-harness.test.ts b/test/skill-harness.test.ts index 4ece32d..b57379d 100644 --- a/test/skill-harness.test.ts +++ b/test/skill-harness.test.ts @@ -48,6 +48,9 @@ interface HarnessArtifact { readonly notVerified: readonly string[]; } +// Explicit 120 s timeout on every test below that spawns a subprocess: each +// spawn runs a real git fixture + fake forge over the whole roster; inheriting +// the 5 s default made the required `test` CI job flaky under parallel load. describe("skill-harness CLI over the real roster", () => { test("1. exits 0, and the artifact is a passing verdict with zero failures", async () => { const outPath = join(temps.create("iai-skill-harness-out-"), "report.json"); @@ -61,7 +64,7 @@ describe("skill-harness CLI over the real roster", () => { expect(artifact.verdict).toBe("pass"); expect(artifact.exitCode).toBe(0); expect(artifact.failures).toEqual([]); - }); + }, 120_000); test("2. the artifact's exitCode and the observed process exitCode agree, and its verdict is pass", async () => { // A verdict that disagreed with its own artifact is the defect this @@ -76,7 +79,7 @@ describe("skill-harness CLI over the real roster", () => { const artifact = JSON.parse(readFileSync(outPath, "utf8")) as HarnessArtifact; expect(artifact.exitCode).toBe(spawned.exitCode); expect(artifact.verdict).toBe("pass"); - }); + }, 120_000); test("3. every denominator in the artifact is non-zero, and any zero-valued one is named", async () => { const outPath = join(temps.create("iai-skill-harness-out-"), "report.json"); @@ -98,7 +101,7 @@ describe("skill-harness CLI over the real roster", () => { // ASSERT THE LIST, NOT THE COUNT. A failure must name the denominator. expect(zero).toEqual([]); - }); + }, 120_000); test("4. notVerified is present, non-empty, and mentions CLAIM-41.8", async () => { // A FUTURE READER WILL WANT TO DELETE THIS ASSERTION AS NOISE. Do not: @@ -111,7 +114,7 @@ describe("skill-harness CLI over the real roster", () => { const artifact = JSON.parse(readFileSync(outPath, "utf8")) as HarnessArtifact; expect(artifact.notVerified.length).toBeGreaterThan(0); expect(artifact.notVerified.some((s) => s.includes("CLAIM-41.8"))).toBe(true); - }); + }, 120_000); }); describe("skill-harness CLI error paths, each with its own distinct message", () => { @@ -124,7 +127,7 @@ describe("skill-harness CLI error paths, each with its own distinct message", () const combined = spawned.stdout + spawned.stderr; expect(combined).toContain("no-skills-on-disk"); expect(combined).not.toContain("empty-roster: the scenario roster passed to runHarness has zero entries"); - }); + }, 120_000); test("6. a nonexistent --skills-dir exits 1 with its own distinct message", async () => { const missing = join(tmpdir(), "iai-skill-harness-does-not-exist-xyz"); @@ -136,7 +139,7 @@ describe("skill-harness CLI error paths, each with its own distinct message", () // Distinct from the empty-directory message (case 5): a directory that // does not exist is not the same defect as one that exists and is empty. expect(combined).not.toContain("no-skills-on-disk"); - }); + }, 120_000); }); // =========================================================================== @@ -157,6 +160,58 @@ const SPAWN_RE = // over a child's stdout or stderr. const STDOUT_RESPONSE_RE = /new Response\(\s*\w+\.(stdout|stderr)/; +// =========================================================================== +// Case 9 of docs/test-plans/293-plan.md, `bun run skill-harness --inject`. +// Task #323, closing CLAIM-293.6 half (ii). +// =========================================================================== + +describe("skill-harness CLI over --inject (the five-scenario INJECTION_ROSTER)", () => { + test("exits NON-ZERO, the artifact's verdict is fail, and failures is non-empty -- THIS NON-ZERO EXIT IS THE SUCCESS CONDITION OF THIS MODE", async () => { + const outPath = join(temps.create("iai-skill-harness-inject-out-"), "report.json"); + const spawned = await spawnHarness(["--inject", "--out", outPath]); + + // THE EXIT CODE FIRST -- and it must be 1, never 0. A `--inject` run + // that exits 0 would mean the injected roster's own duplicate-create + // scenario was not reached, or was reached and not reported: either way + // the one thing case 9 requires ("this is the case that proves the + // harness can fail") would be false. + expect(spawned.exitCode).toBe(1); + + expect(existsSync(outPath)).toBe(true); + const artifact = JSON.parse(readFileSync(outPath, "utf8")) as HarnessArtifact; + expect(artifact.verdict).toBe("fail"); + expect(artifact.exitCode).toBe(1); + expect(artifact.failures.length).toBeGreaterThan(0); // denominator first + }, 120_000); + + test("the artifact's exitCode agrees with the observed process exitCode, both non-zero", async () => { + const outPath = join(temps.create("iai-skill-harness-inject-out-"), "report.json"); + const spawned = await spawnHarness(["--inject", "--out", outPath]); + const artifact = JSON.parse(readFileSync(outPath, "utf8")) as HarnessArtifact; + expect(artifact.exitCode).toBe(spawned.exitCode); + expect(artifact.exitCode).toBe(1); + }, 120_000); + + test("the roster length under --inject is 5, distinct from the default roster's 6", async () => { + const outPath = join(temps.create("iai-skill-harness-inject-out-"), "report.json"); + await spawnHarness(["--inject", "--out", outPath]); + const artifact = JSON.parse(readFileSync(outPath, "utf8")) as HarnessArtifact; + expect(artifact.denominators.rosterLength).toBe(5); + expect(artifact.denominators.scenariosExecuted).toBe(5); + }, 120_000); +}); + +describe("skill-harness CLI without --inject: the default path is unchanged", () => { + test("still exits 0 over the real roster, and SCENARIO_ROSTER.length is still 6", async () => { + const outPath = join(temps.create("iai-skill-harness-default-out-"), "report.json"); + const spawned = await spawnHarness(["--out", outPath]); + expect(spawned.exitCode).toBe(0); + const artifact = JSON.parse(readFileSync(outPath, "utf8")) as HarnessArtifact; + expect(artifact.verdict).toBe("pass"); + expect(artifact.denominators.rosterLength).toBe(6); + }, 120_000); +}); + describe("case 21: neither the wrapper nor the runner spawns a process or reads a child's raw stdout", () => { const scannedFiles = [ join(repoRoot, "scripts", "skill-harness.ts"), diff --git a/test/transcription-gap.test.ts b/test/transcription-gap.test.ts index 36adf45..c2ff7a9 100644 --- a/test/transcription-gap.test.ts +++ b/test/transcription-gap.test.ts @@ -180,6 +180,10 @@ describe("case 16 (P0, NEVER-293.5): mutating a SKILL.md leaves the seam rung gr // distinguishing evidence separately: a copy carrying a FIFTH skill, which // no scenario covers, must turn the harness RED. If it does not, the flag // is not being honoured and every assertion below is worthless. + // + // Explicit 120 s timeout: each spawn runs a real git fixture + fake forge + // over the whole roster; inheriting the 5 s default made the required + // `test` CI job flaky under parallel load. test("0b. control: --skills-dir is load-bearing, so the four runs below are not vacuous", async () => { const controlDir = temps.create("iai-transcription-gap-control-"); cpSync(realSkillsDir, controlDir, { recursive: true }); @@ -198,7 +202,7 @@ describe("case 16 (P0, NEVER-293.5): mutating a SKILL.md leaves the seam rung gr const artifact = JSON.parse(readFileSync(outPath, "utf8")) as HarnessArtifact; expect(artifact.verdict).toBe("fail"); expect(artifact.failures.length).toBeGreaterThan(0); - }); + }, 120_000); // Driven one at a time -- 4 separate `test()` calls -- rather than a // single loop-wrapped assertion, so each skill's pass/fail is its own @@ -253,7 +257,7 @@ describe("case 16 (P0, NEVER-293.5): mutating a SKILL.md leaves the seam rung gr const artifact = JSON.parse(readFileSync(outPath, "utf8")) as HarnessArtifact; expect(artifact.verdict).toBe("pass"); expect(artifact.failures).toEqual([]); - }); + }, 120_000); } }); From 44ee4fdd6e37cb07a86a0eda8d162ee14760b686 Mon Sep 17 00:00:00 2001 From: Prem Mallappa Date: Sat, 12 Sep 2026 15:58:34 +0530 Subject: [PATCH 3/4] #323: cite #334 as the seam rung's promotion tracker The promotion procedure in ci.yml listed its six sites but named no owner, so the next reader had no way to tell whether it was scheduled or forgotten. #334 carries the checklist and both unblocking conditions, and is explicitly not a sub-issue of #293 so the Story's exit does not depend on it. --- .github/workflows/ci.yml | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 31b8ae3..1d05281 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -169,10 +169,14 @@ jobs: # now would permanently block PR #313 and every S2.3 task PR, since # their heads predate this commit and can never report this context. # - # PROMOTION PROCEDURE (a follow-up commit, once the two reasons above no - # longer hold). All FOUR of the following in-tree sites must be edited in - # ONE commit, or the four-way agreement the tests in - # test/ci-workflow.test.ts assert on will go red: + # PROMOTION PROCEDURE — TRACKED AS ISSUE #334, which carries this list as + # a checklist and records both unblocking conditions. It is deliberately + # NOT a sub-issue of #293: the promotion cannot happen while S2.6 is in + # flight, so that Story's exit must not depend on it. + # + # All FOUR of the following in-tree sites must be edited in ONE commit, or + # the four-way agreement the tests in test/ci-workflow.test.ts assert on + # will go red: # (i) scripts/verify-required-checks.sh's REQUIRED_CONTEXTS array # (ii) the same file's inline `required_status_checks.contexts[]=` # list in the branch-protection help text (the `gh api` example From 80bd77d1de24622dbaa2f228526d9ea74f1b8602 Mon Sep 17 00:00:00 2001 From: Prem Mallappa Date: Sat, 12 Sep 2026 16:03:12 +0530 Subject: [PATCH 4/4] #323: record the verification evidence Measured on 44ee4fd, which frontmatter `commit:` pins as the parent -- an evidence artifact cannot carry the hash of the commit that contains it. Chain: 1095 tests / 1081 pass / 0 fail / 14 skip / 3967 expect(); lint 0 across 123 files; claim-lint 121 files, 0 errors, 1 warning; skill-harness exit 0 and --inject exit 1; both verifiers PASS at SIX contexts. Adding this artifact takes the suite to 3969 expect() -- the +2 evidence-corpus delta of #299, reproduced exactly. --- docs/evidence/323-20260912T102956Z.md | 193 ++++++++++++++++++++++++++ 1 file changed, 193 insertions(+) create mode 100644 docs/evidence/323-20260912T102956Z.md diff --git a/docs/evidence/323-20260912T102956Z.md b/docs/evidence/323-20260912T102956Z.md new file mode 100644 index 0000000..151976e --- /dev/null +++ b/docs/evidence/323-20260912T102956Z.md @@ -0,0 +1,193 @@ +--- +issue: 323 +story: 293 +repo: dev-pmallapp/iAI +branch: task/323-seam-rung-and-ci-job +commit: 44ee4fdd6e37cb07a86a0eda8d162ee14760b686 +run: 2026-09-12T10:29:56Z +verdict: PASS +--- + +# Evidence: #323 — the seam rung, its CI job, and the transcription audit + +Build target 7 of `docs/design/stories/293.md`. Cases **16, 17 and 18**, plus +case **9**, which #322 left open as `CLAIM-293.6` half (ii). + +`commit:` pins the **parent** of the commit that carries this artifact. An +evidence artifact cannot carry the hash of the commit that contains it; every +figure below was measured on `44ee4fd`. + +## Chain + +``` +bun run build exit 0 +bun test 1095 tests / 1081 pass / 0 fail / 14 skip / 3967 expect() +bun run lint 0 violations across 123 files +bun run typecheck exit 0 +bun run skill-lint 4 SKILL.md + 12 contract files, 0 errors, 0 warnings +bun run claim-lint 121 files, 0 errors, 1 warning +bun run skill-harness exit 0, verdict pass, six scenarios +bun run skill-harness --inject exit 1, verdict fail <- the success condition of this mode +IAI_PURITY_CHILD=1 … 9 pass +IAI_BINDING_SWEEP_CHILD=1 … 5 pass +scripts/verify-required-checks.sh PASS, SIX contexts +scripts/verify-workflow-hygiene.sh PASS +``` + +The 1 claim-lint warning is `docs/design/stories/9.md`, which predates the +metadata convention (ruling G1b of #41). **It must stay a warning.** + +Deltas against `story/293` @ `6e05328`: **+57 tests, +190 `expect()`, +2 lint +files, +1 claim-lint file.** The claim-lint file is +`docs/audits/293-transcription-audit.md`; this artifact makes it +2 on the next +measurement, per #299's evidence-corpus delta. + +**Required contexts are still SIX, deliberately.** See case 13 below. + +## Case results + +| Case | Claim | Verdict | +|---|---|---| +| 9 | `CLAIM-293.6` (ii) | **PASS** — the truncated-list lie makes `goal-create`'s run 2 issue a duplicate create, and the harness reports it | +| 16 | `NEVER-293.5` | **PASS** — 4 of 4 bodies reordered, harness stays green, count asserted first | +| 17 | `NEVER-293.5` | **PASS** — `CLAIM-41.8`'s class still reads `model-judged` | +| 18 | `CLAIM-293.11` | **PASS** — 16 rows audited against a 16-row run-time denominator | +| 13 | `NEVER-293.10` | **not claimed here** — the four hardcoded lists agree at six; #324 owns the before/after | + +### Case 9 — the duplicate create, on the argv and not the exit code + +`--inject` selects `INJECTION_ROSTER`: five frozen scenarios, one per +`FailureMode`. Run 1 POSTs a milestone; the lie drops the last element of the +one-element list; run 2 reads absence and POSTs the identical title. The fake +has no duplicate check on any identity key, so it lands: + +``` +run 1 gh api --method POST repos/OWNER/REPO/milestones -f title=M9 — universal lifecycle … +run 2 gh api --method POST repos/OWNER/REPO/milestones -f title=M9 — universal lifecycle … + +harness: FAIL run-2-mutated: scenario "goal-create/injected-truncated-list" + made 1 mutation(s) on run 2. +harness: verdict: fail (exit 1) +``` + +The assertion is on the **recorded argv**, never the exit code — #320's M9/M9b +pair measured that a double which refuses and a double which accepts return the +same verdict and differ only in **state**. + +**The lie is armed between the runs, never before run 1.** All five are lies +about what run 2 *sees*. Arming one for run 1 leaves nothing genuine for run 2's +create to duplicate, and the harness would have proven nothing. + +**A non-zero exit under `--inject` is the success condition of the mode.** The +wrapper still assigns `exitCode = verdict.exitCode` unconditionally. Inverting +it under `--inject` would move the branch-on-the-verdict one file over from +`decideVerdict`, which case 24 statically forbids; a branch does not stop being +a branch by relocating. + +### Two findings from case 9, reported and not forced + +- **Three of the five modes collapse to one signature.** The read-lie trio — + zeroed budget, header-absent 403, transient connection error — all crash run 2 + identically on `JSON.parse` of empty stdout, because no transcription checks a + read's exit code before parsing. This is **#320's finding reproduced one layer + up**, and it belongs to **#331**. At the fake's own response level the three + remain distinguishable, so the collapse is the transcriptions' handling, not + the fake's modelling. +- **`silent-no-op-mutation` is armed but never engaged.** All four + transcriptions are correctly idempotent under honest reads, so run 2 attempts + no mutation for the lie to intercept. Verified at `createFakeForge`'s own seam + instead. No bug was invented to force it. + +### Cases 16 and 18 — the audit and the gap are the same fact + +`docs/audits/293-transcription-audit.md` carries 16 rows, one per `## Re-entry` +row, each naming the transcription step, the mutation it gates, and the +reasoning. **It is not an evidence artifact**: `evidence-upsert.test.ts:33-50` +requires every file under `docs/evidence/` to match `{issue}-{ts}.md`, and an +artifact is immutable once pushed — the wrong lifetime for a record that must be +amended whenever a table or a transcription changes. + +The audit's finding: the ordering obligation the bodies state is **per-row** — +*"every mutating step is preceded by the read that decides it"* — never +table-wide. **Three of the four transcriptions execute rows out of table order**, +each for a recorded reason, and all three are faithful. That is precisely why +case 16 passes when the harness fails to notice a reordered table: a harness +that went red on a reorder would assert an ordering the bodies never claimed, +and would go red on three of four skills as shipped. + +Two authorised mutations are **dead in the roster** — `goal-create`'s milestone +`PATCH` and `story-create`'s `--milestone` reassign. Both are the fake's +unmodelled shapes, not transcription defects. The audit **records** them; only +the live rung (#324) can close them. This discharges the handoff's *"needs an +issue if #323's audit or #324's live rung does not absorb it"* as a record. + +### Case 13 — required contexts stand at six, and that is the ruling + +The `seam` job ships **not required**, by decision, for two independently +sufficient measured reasons: + +1. A required context that has never reported sits `pending` **forever** (#204). +2. `story/47` and its six open task issues carry **no seam job on their heads**. + Promoting now would permanently block **PR #313** and every S2.3 task PR. + +Both verifiers therefore still PASS at six, and all four hardcoded lists agree. +The promotion is **#334**, deliberately not a sub-issue of #293 so the Story's +exit does not depend on it. + +## Mutation testing + +**Twelve injected across three surfaces. One survived; the gap it found is +closed and re-killed.** + +| # | Mutation | Result | +|---|---|---| +| M1 | `CLAIM-41.8` → `tool-checked` | killed | +| M2 | one audit row deleted | killed | +| M3 | audit row **duplicated** | killed by cardinality; a length check would have passed | +| M4 | row renumbered 4→9, count still 16 | killed | +| M5 | summary table folded back under `## The audit` | **SURVIVED** → closed → re-killed | +| M6 | the row swap made a no-op | killed by the pattern-matched guard | +| M7 | no row anchors `CLAIM-41.8` | killed by the denominator | +| M8 | a seventh ci.yml job in neither list | killed by the partition | +| M9 | `seam` removed from `NOT_REQUIRED` | killed | +| M10 | `if: always()` on `seam` | killed | +| M11 | `claim-lint` dropped from the Python list only | killed by four-way agreement | +| M12 | `seam`'s run line → a command not in `package.json` | killed | + +**M5 is the one worth carrying forward.** The audit parser took *"the first +contiguous run of `|` lines"*, which read the correct rows **by accident** and +would have silently read the **wrong** table had one ever been inserted above. +It now asserts that `## The audit` holds exactly one table. A parser that +tolerates the structure it depends on cannot report a violation of it. + +**A control was added that no case asked for.** Four green runs against mutated +skill copies prove nothing unless `--skills-dir` is load-bearing — and since a +pure reorder is invisible in the artifact, *no* green run can distinguish the +two directories. A fifth uncovered skill in the copy must turn the harness red; +it does. Without it, case 16 was one ignored flag away from being vacuous. + +## Corrections made to the tree + +- **`ci.yml`'s `claim-lint` comment was false in both halves.** It said + claim-lint was in neither branch protection nor ruleset `21656025`; it is in + **both**, verified live. Corrected, and the promotion commit `15e5a36` is now + cited so the next reader can check it rather than trust it. **#304's class, + found in the very comment the new `seam` comment is modelled on.** +- **Every subprocess-spawning test now declares an explicit 120 s timeout.** + They inherited bun's 5 s default and one had already flaked under parallel + load. The `test` job is required, so that flake breaks merges. + +## What this does NOT establish + +- **Nothing here tests the model reading a `SKILL.md`.** The population is the + transcription. Gate ruling G2 forbids treating any of this as licence to + upgrade `CLAIM-41.8` or `CLAIM-47.4`. The runner still prints three + `NOT VERIFIED` lines saying so on every run; they must not be deleted. +- **The audit catches a row added or removed, never a cell edited.** That is the + residual gap `CLAIM-293.11` exists to bound, and why it is `model-judged`. +- **The seam rung is not yet a required check.** #334. +- **#330 is still unpaid.** No `gh` argv this repository builds has ever been + executed; the injection roster answers from #320's fake, so it cannot pay the + bill either. **Six tasks have now deferred it.** #324's live rung is the first + thing that can. +- **#331 is untouched**, and case 9 produced fresh evidence for it.