diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 10926b1..1d05281 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,69 @@ 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 — 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 + # 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/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/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. 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 new file mode 100644 index 0000000..c2ff7a9 --- /dev/null +++ b/test/transcription-gap.test.ts @@ -0,0 +1,396 @@ +// 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. + // + // 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 }); + 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); + }, 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 + // 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([]); + }, 120_000); + } +}); + +// =========================================================================== +// 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)); + }); +});