diff --git a/docs/evidence/320-20260908T074833Z.md b/docs/evidence/320-20260908T074833Z.md new file mode 100644 index 0000000..aad9a36 --- /dev/null +++ b/docs/evidence/320-20260908T074833Z.md @@ -0,0 +1,231 @@ +--- +issue: 320 +story: 293 +repo: dev-pmallapp/iAI +branch: task/320-stateful-fake-forge +commit: b2979326f21b053478800f4e2170ee29eedc20e9 +run: 2026-09-08T07:48:33Z +verdict: PASS +--- + +# Evidence: #320 — The stateful fake forge: able to lie, and non-enforcing + +**Verdict: PASS.** Build target 4 of `docs/design/stories/293.md`. Case **8** +PASS and `NEVER-293.7` met; case **20** PASS; case **9 PARTIAL** and +**`CLAIM-293.6` NOT met** — see *What this does NOT establish*, which is the +load-bearing section of this artifact. + +Measured at `b297932`, before this artifact was committed. Per #299 the +evidence artifact itself adds **+2 `expect()`** via +`packages/core/test/evidence-upsert.test.ts:33-50`, so the count after this +commit is **3391**, not 3389. + +## Chain + +| Check | Result | +|---|---| +| `bun run build` | clean | +| `bun test` | **959 tests / 945 pass / 0 fail / 14 skip / 3389 `expect()`** | +| `bun run lint` | **0 violations across 107 files** | +| `bun run typecheck` | clean | +| `bun run skill-lint` | 4 `SKILL.md` + 12 contract files, **0 errors, 0 warnings** | +| `bun run claim-lint` | **116 files, 0 errors, 1 warning** | + +The 1 warning is `docs/design/stories/9.md`, which predates the metadata +convention — ruling G1b of #41. It must stay a warning. + +Deltas from `story/293-s2-6-skill-execution-harness` @ `8747a96`: +**934 → 959 tests** (+25), **3286 → 3389 `expect()`** (+103), **lint 105 → 107 +files**. No new package: the fake is a module inside `iai-harness`, so the +`"bun": "./src/index.ts"` export condition that broke CI on #319 is already +present and unchanged. + +## Case results + +| Case | Claim | Result | +|---|---|---| +| 8 | `NEVER-293.7` | **PASS** — 4 of 4 identity key classes, driven directly, all accepted | +| 20 | cross-cutting | **PASS** — three rate-limit states, each asserted against the other two | +| 9 (i) | `CLAIM-293.6` | **PASS** — 5 of 5 modes injectable, 5 distinct observable signatures | +| 9 (ii) | `CLAIM-293.6` | **NOT MET** — needs a transcribed skill and a runner; closes at #323 | + +### Case 8 — the four identity keys are four key CLASSES + +The plan names four keys. Enumerating the `## Re-entry` "Read first" column off +disk yields **sixteen rows**, not four — `docs/test-plans/{n}-plan.md` is a +fifth path-shaped key the plan's prose does not list. Hardcoding 4 while +claiming it was read off disk would have been a lie the plan's own wording +invites. + +Resolved as four key **classes** over a **partition of all sixteen rows**: +`forge-object-title` (1), `forge-list-row` (1), `sentinel-identity` (2), +`worktree-path` (2) = **6 identity rows**; `precondition` (3), +`amend-in-place` (5), `report-only` (2) = **10 non-identity rows**; 6 + 10 = 16, +asserted. A row matching no class is reported **by name**, so a new Re-entry row +fails loudly rather than being silently ignored. This is #307's ruling applied: +a carve-out cannot detect an unclassified new member; a partition must. Mutation +M7 proves it fires. + +The fourth key is on the **other surface**, and that is why it does not collapse +into the third. Expressed as a `gh` argv the Design path would be +`gh issue comment --body ""` — the same argv shape as the sentinel +case, differing only in the string inside `--body`, which +`references/verification.md:127-129` rules is one argv and not two. Its +`argv[0]` is `git`, which `packages/exec/src/port.ts:58` admits, so it is still +driven directly through the port exactly as `NEVER-293.7` words it. + +## Mutation testing — 10 injected, 8 killed, 1 survivor by design, 1 pair that is the whole argument + +| # | Mutation | Result | +|---|---|---| +| M1 | The fake **enforces** the duplicate milestone title, the way GitHub's 422 does | **KILLED** (2 fail) | +| M2 | `truncate()` becomes a no-op: the fake can no longer lie by omission | **KILLED** (2 fail) | +| M3 | The header-absent 403 emits headers too: the **third state** collapses into the first | **KILLED** (3 fail) | +| M4 | The silent no-op actually writes: verdict and state agree again | **KILLED** (1 fail) | +| M5 | The fake **upserts** the sentinel comment, enforcing the skill's own contract for it | **KILLED** (1 fail) | +| M6 | The fake gains `import { execFile } from "node:child_process"` | **KILLED** (3 fail, across `packages/exec` **and** `packages/harness`) | +| M7 | A fifth `## Re-entry` row appears in a `SKILL.md` and no class claims it | **KILLED** (2 fail) | +| M8 | The mixed-case header fixture is lowercased | **KILLED** (1 fail) | +| M10 | The transient error never spends itself: a fatal error wearing a transient one's name | **KILLED** (1 fail) | +| M9 | Case 8's state-growth assertion replaced by `expect(first.exitCode).toBe(0)` | **SURVIVED by design of the mutation** | +| M9b | **M9 *and* M1 together** — the weakened case 8 against the enforcing fake | **KILLED, but not by case 8** | + +Every mutation asserted its pattern matched **exactly once** before being +applied, and asserted the file actually changed before the run was trusted — a +no-op mutation reports the result you were hoping for. **Every verdict is the +runner's exit code.** Nothing in the harness greps stdout, which is finding 37's +requirement and finding 37's recorded owner is *"any future harness"* — this one. + +### M9/M9b is the most valuable result in this artifact + +M9 alone weakens a test rather than injecting a defect, so its survival proves +nothing. The pair is the argument: + +> With case 8's state-growth assertion replaced by an exit-code assertion **and** +> the fake made to enforce the 422, **all four of case 8's tests pass.** The kill +> comes from **case 9 (ii)** — a different case, anchored to a different claim. + +So `NEVER-293.7`'s own case, written the obvious way, **scores the exact defect +it exists to detect as a pass**. The exit code is the verdict; the state is the +reason. This is the S1.5 shadowing trap arriving through the test rather than +through the fake, and it is why every branch of case 8 reads its result back +through a **list argv** and never through a private field or an exit code. + +## Six findings + +### 1. Case 8 asserted on an exit code cannot detect an enforcing fake + +Recorded above as M9/M9b. Generalises to every test of a permissive double: a +double that refuses and a double that accepts return the same verdict on the +**first** call, and differ only in state on the second. **Owner: pattern — +applies to #321, #322 and #323, all of which will assert against this fake.** + +### 2. Exit code and row counts collapse **three of the five** failure modes + +Measured while writing `case 9 (i)`. On `read.exit` + `read.rows` + +`create.exit` + `state.rows` the five modes produce **three** distinct +signatures: the zeroed budget, the header-absent 403 and the transient error are +all *"the read exited 1 and returned no rows"*. Adding `classifyRateLimit`'s +answer to the signature separates all five. + +This is not a defect in the fake. It is the measurement that justifies case 20 +existing at all: **the five lies are not distinguishable by what they do, only +by why.** A harness branching on the verdict would treat a rate limit, a +permission failure and a dropped connection as one event. + +### 3. `shouldRetryResponse` answers `retry: false` for a transient connection error + +Measured directly: + +``` +classifyExitCode(1) -> { code: 1, classification: "fatal", provenance: "observed" } +classifyRateLimit(..) -> "not-rate-limited" +shouldRetryResponse -> { retry: false, reason: 'exit code 1 classifies fatal (observed) and + rate limit classification is "not-rate-limited"' } +``` + +`references/gh-error-handling.md:120-131` records **three** `gh` calls failing +with a connection error in a single session, **each succeeding on immediate +retry**, and instructs *"Retry once before believing a failure."* The kernel's +own retry-decision function does not retry the failure mode the reference +document exists to tell you to retry. **Needs an issue — not yet raised.** + +### 4. The kernel's own `reason` strings differ only in the value they quote + +`packages/core/src/gh/errors.ts:270` returns +`rate limit classification is "rate-limited"` and, for the third state, +`...is "possibly-rate-limited"`. Two messages that differ only in the value they +quote are one message — `references/verification.md:127-129`, the rule this +repository has now paid for three times. + +It is inside the kernel, in a `reason` field whose purpose is to be read by a +human deciding why a batch stopped. **This is why none of case 20's distinctness +assertions use that string**; they assert the classification **value** and the +**absence** of the other two. Recorded, not fixed: changing a kernel message is +not this task's. **Owner: whoever next edits `errors.ts`.** + +### 5. `git clean -fd` destroyed this task's uncommitted work, and the handoff recommends it + +The handoff records that `rm` is denied by tool policy and directs the reader to +`git clean -fd` instead. **`-fd` removes untracked files, and a task in progress +before its first commit consists entirely of untracked files.** Both new files +were destroyed by a cleanup of a stray probe script and had to be rewritten. + +The recommended remedy is not the recommended verb. **Commit before any cleanup +verb, or use `git stash -u`**, which is recoverable. This is the same class as +the handoff's existing warning that `git checkout ` reverts to the index — +a recovery verb that destroys the thing being recovered. **Owner: the handoff.** + +### 6. The `## Re-entry` column yields 16 keys, not the 4 the plan names + +Recorded under *Case 8* above. The plan's four are read as classes. The fifth +literal key — `docs/test-plans/{n}-plan.md` — is honestly classified +`worktree-path` rather than pretended away. + +## What this does NOT establish + +- **Case 9 is NOT closed and `CLAIM-293.6` is NOT met.** Half (i) — all five + modes injectable, each distinct — is met and mutation-tested (M2, M3, M4, M8, + M10). Half (ii) — *"the truncated-list mode makes at least one **skill's** + run 2 issue a duplicate create, and the harness reports it"* — is **not** met: + no skill has been executed, and **`bun run skill-harness`, which case 9's own + Command column names, does not exist in any `package.json` in this tree.** + Half (ii) closes at **#323**, once a transcribed skill body runs under the + runner #322 builds. +- **The read-then-create driver in `packages/harness/test/fake-forge.test.ts` is + not a transcription of `goal-create` and must not be cited as one.** It + implements one sentence — `skills/goal-create/SKILL.md:107-110` — and is + confined to the test file, deliberately not exported from the barrel, so that + #323's transcription (which `CLAIM-293.11` and case 18 require to be audited + one row per Re-entry row) remains the only transcription in the repository. + Reusing it would be Problem 4's category error at its sixth occurrence. +- **Nothing here establishes that the fake forge is behaviourally equivalent to + GitHub.** `docs/design/stories/293.md:460-462` ruled that claim unwritable. + `NEVER-293.7` is established only over the four named identity key classes, + driven directly; it says nothing about any other invariant. +- **THE FIRST-CONTACT BILL IS STILL UNPAID, AND #320 CANNOT PAY IT.** + `docs/evidence/318-20260907T152554Z.md` and `docs/evidence/319-20260908T022709Z.md` + both deferred it here. A fake forge answers `gh` argv **from memory** — that is + its purpose — so a `gh` family whose *real* argv is wrong passes here exactly + as it passed at #318 and #319. **No `gh` argv this repository builds has ever + been executed.** The S2.1 precedent is a required check that shipped with sixty + passing tests and rejected **7 of 9 real verbs** on first contact. Build targets + 1–10 now contain **no owner for this**. It must be raised as its own issue + before the seam rung is called done. +- **The fake is deliberately more permissive than GitHub on the milestone + title**, where GitHub returns 422 (`docs/design/stories/293.md:150-153`). + `NEVER-293.7` requires exactly that. The consequence is that **no skill's + handling of a real 422 is exercised anywhere in this repository.** +- **The fake's 403 stderr strings carry provenance `assumed`, not `observed`.** + `packages/core/src/gh/errors.ts:199-206` records that a 403 was never + reproduced live. Authoring the fake's 403 to satisfy `HTTP_403_RE` and then + testing against what the fake emits means **the fake and the regex are tuned to + each other and can both be wrong about real GitHub together**. #324, the live + rung, is the first thing that will meet a real 403 and can confirm or refute + it. +- **The header side channel models a capture path no adapter in this repository + has.** `gh api --include` appears nowhere in the tree, asserted over a non-zero + denominator, and `createRealPort`'s result is asserted to carry exactly + `exitCode`/`stdout`/`stderr` and no header key. That divergence is published as + a passing absence-test rather than hidden — and **the first reader will be + tempted to delete it as vacuous.** Owner: **#324**. diff --git a/packages/harness/src/fake-forge.ts b/packages/harness/src/fake-forge.ts new file mode 100644 index 0000000..b1e5522 --- /dev/null +++ b/packages/harness/src/fake-forge.ts @@ -0,0 +1,579 @@ +// The stateful fake forge: GitHub semantics over in-memory state, able to lie, +// and deliberately NOT enforcing. +// +// Build target 4 of docs/design/stories/293.md. Anchors CLAIM-293.6 and +// NEVER-293.7. Ruled YES at the gate (G4), 2026-09-07, as TWO SEPARATE +// requirements -- Decision 5, docs/design/stories/293.md:558-571. +// +// WHY IT MUST BE ABLE TO LIE. +// +// 4 of 4 skill bodies name a rate-limited read treated as absence as their +// damaging failure mode, and skills/story-create/SKILL.md:119-123 calls it +// "the single most damaging failure mode of this skill" -- because ABSENCE IS +// WHAT EVERY CREATE STEP KEYS ON. A fixture forge that always answers +// correctly cannot exercise the failure the skills were written to survive, +// and .github/workflows/ci.yml:137-146 names that disqualifier itself: a check +// that cannot meaningfully fail is equivalent to one that is always skipped. +// +// WHY IT MUST NOT ENFORCE, WHICH IS THE SUBTLER HALF AND FAILS BY PASSING. +// +// GitHub's milestone endpoint returns 422 on a duplicate title and +// `goal-create` keys identity on that title (skills/goal-create/SKILL.md:79). +// If the fake enforces that, run 2 makes no mutation because THE FAKE refused +// it -- not because the skill declined to try. That is the S1.5 shadowing trap +// one layer out: a skill shadowed by its own test double. +// +// So this fake is MORE PERMISSIVE THAN GITHUB on the four identity keys, on +// purpose. NEVER-293.7 (docs/design/stories/293.md:373-376) says so in terms. +// The property that makes the permissiveness safe rather than sloppy is not in +// this file at all -- it is packages/exec/src/recording.ts:10-16, which records +// the ATTEMPT and not the success. Acceptance and refusal are therefore both +// visible, and the harness never has to infer a skill's behaviour from the +// fake's verdict. +// +// WHY THERE IS NO PROCESS HERE, AND WHY THAT IS LOAD-BEARING. +// +// This module answers argv from memory. It imports no launcher, which is what +// keeps CLAIM-293.1 true: case 12 of packages/exec/test/port.test.ts asserts +// exactly ONE production module in the workspace launches a process and names +// packages/exec/src/real.ts, and packages/harness/test/fixture-repo.test.ts +// independently asserts packages/harness/src imports no launcher at all. A +// `node:child_process` import here would fail both and falsify CLAIM-293.1. +// +// WHAT THIS FILE CANNOT DO, STATED HERE SO IT IS NOT INFERRED LATER. +// +// A fake forge answers `gh` argv from memory. It therefore cannot pay the +// first-contact bill of Problem 2 (docs/design/stories/293.md:70-75): a `gh` +// argv that is WRONG against the real GitHub passes here exactly as it passed +// against golden-argv tests. #318 and #319 exercised `git`, never `gh`. That +// bill is still unpaid and this task cannot pay it. + +import type { GhResponse } from "iai-core"; +import { checkArgv, type ExecResult, type Port } from "iai-exec"; + +// --------------------------------------------------------------------------- +// The five lies +// --------------------------------------------------------------------------- + +/** The five injectable failure modes, per references/gh-error-handling.md and + * docs/design/stories/293.md:368-372. + * + * A CLOSED SET, and its size is a denominator. Case 9 of + * docs/test-plans/293-plan.md asserts 5 of 5 are injectable, so a sixth mode + * added without a case is a visible edit here rather than a silent widening. */ +export type FailureMode = + | "rate-limit-zeroed-budget" + | "forbidden-no-budget-header" + | "truncated-list" + | "silent-no-op-mutation" + | "transient-connection-error"; + +export const FAILURE_MODES: readonly FailureMode[] = [ + "rate-limit-zeroed-budget", + "forbidden-no-budget-header", + "truncated-list", + "silent-no-op-mutation", + "transient-connection-error", +]; + +/** Emitted when the fake is asked for an operation it does not model. + * + * DISTINCT FROM EVERY OTHER NON-ZERO CODE ON PURPOSE. `REFUSED_EXIT_CODE` + * (packages/exec/src/real.ts:34) means the port refused to launch; 1 means + * the forge answered with a failure; this means the fake has no answer at + * all. Collapsing the three would let "the fake does not model this" be read + * as "GitHub said no", which is the shadowing failure NEVER-293.7 exists to + * prevent, arriving through a different door. */ +export const FAKE_UNMODELLED_EXIT_CODE = 125; + +/** The budget headers a rate-limited response carries when they were captured. + * + * MIXED CASE DELIBERATELY. packages/core/src/gh/errors.ts:153-158 records + * that `getHeaderCaseInsensitive` exists because a literal + * `headers["x-ratelimit-remaining"]` would silently miss + * `X-RateLimit-Remaining`. A fake that only ever emitted the lowercase form + * would leave that function unexercised, and a mutation collapsing it to a + * literal lookup would survive. */ +export const RATE_LIMIT_HEADERS_MIXED_CASE: Readonly> = { + "X-RateLimit-Limit": "5000", + "X-RateLimit-Remaining": "0", + "X-RateLimit-Used": "5000", +}; + +/** The same budget, lowercased. Not emitted by the fake -- it exists so a test + * can exercise the other half of the case-insensitive lookup against the same + * classifier. */ +export const RATE_LIMIT_HEADERS_LOWER_CASE: Readonly> = { + "x-ratelimit-limit": "5000", + "x-ratelimit-remaining": "0", + "x-ratelimit-used": "5000", +}; + +// `gh`'s own error format for a 403 is ASSUMED, NOT OBSERVED, and this comment +// is the second home of that provenance -- packages/core/src/gh/errors.ts +// :199-206 is the first, and it says a 403 "was not reproduced live (provoking +// a real rate limit was not attempted...)". +// +// THE LOOP THAT MUST NOT BE CLOSED SILENTLY: if the string below is authored +// to satisfy HTTP_403_RE (errors.ts:206) and the test is then written against +// whatever this fake emits, the fake and the regex are tuned to each other and +// CAN BOTH BE WRONG ABOUT REAL GITHUB TOGETHER. That is +// references/verification.md:79-88's non-representative-corpus failure in its +// purest form. The first real 403 -- which is #324, the live rung -- is what +// confirms or refutes it. Until then both strings carry provenance `assumed`. + +/** Rate limited, budget header captured and zero. */ +const RATE_LIMITED_STDERR = + "gh: HTTP 403: API rate limit exceeded for user ID 0 " + + "(https://api.github.com/repos/OWNER/REPO/milestones)"; + +/** The THIRD STATE: a 403 with no budget header captured. + * + * This is the one most likely to be forgotten, and + * references/gh-error-handling.md:56-60 explains why it is also the COMMON + * case: `gh api --include` appears nowhere in this repository, so the usual + * situation is that no headers were captured at all. + * + * IT MUST NOT CONTAIN THE SECONDARY-LIMIT PHRASE. errors.ts:226-228 tests + * SECONDARY_RATE_LIMIT_RE first and short-circuits, so a stray "secondary + * rate limit" here would classify as `rate-limited` and collapse this mode + * into the one above -- two modes that differ only in the value they quote + * are one mode. */ +const FORBIDDEN_NO_HEADER_STDERR = + "gh: HTTP 403: Resource not accessible by integration " + + "(https://api.github.com/repos/OWNER/REPO/milestones)"; + +/** references/gh-error-handling.md:122-125 -- observed three times in a single + * session, each succeeding on immediate retry. */ +const TRANSIENT_STDERR = "gh: error connecting to api.github.com: connection reset by peer"; + +// --------------------------------------------------------------------------- +// State +// --------------------------------------------------------------------------- + +export interface FakeMilestone { + readonly number: number; + readonly title: string; + readonly description: string; +} + +export interface FakeIssue { + readonly number: number; + readonly title: string; + readonly body: string; + readonly milestone: string | null; + readonly labels: readonly string[]; +} + +export interface FakeComment { + readonly id: number; + readonly issue: number; + readonly body: string; +} + +/** One answered call, in the envelope the kernel already declares. + * + * TYPED AS `GhResponse` (packages/core/src/gh/types.ts:65-70) AND NOT AS A + * NEW SHAPE. That interface was designed in S1.3 for exactly this handoff -- + * "a VALUE handed in by the caller, never a lookup, because a lookup is I/O" + * -- and `classifyRateLimit` (errors.ts:217) takes it directly. Five Stories + * later this is the first adapter able to hand it one. + * + * `headers` IS NOT ON `ExecResult` AND MUST NOT BE ADDED TO IT. + * `createRealPort` can never populate it, because nothing in this repository + * passes `gh api --include`; widening packages/exec/src/port.ts:34-38 would + * be a type that lies to every implementation for one test's benefit. */ +export interface FakeGhResponse extends GhResponse { + readonly argv: readonly string[]; +} + +export interface FakeForge extends Port { + /** Arm a lie, or `null` to answer honestly. */ + inject(mode: FailureMode | null): void; + readonly injected: FailureMode | null; + /** Every answer given, with the headers the kernel's classifier wants. */ + readonly responses: readonly FakeGhResponse[]; + /** Drop the answer log. State is NOT cleared: a forge that forgot its + * milestones between run 1 and run 2 would make every skill idempotent by + * construction, which is the result the harness exists to disprove. */ + reset(): void; +} + +export interface FakeForgeOptions { + /** Seeded milestones, in order. */ + readonly milestones?: readonly Pick[]; + /** Anything the fake does not model as `gh` -- notably every `git` argv -- + * is handed here. The fixture repository runs its git through the SAME port + * the fake occupies, so a harness wiring both surfaces passes a real port in + * and gets one seam for packages/exec/src/recording.ts to wrap. */ + readonly delegate?: Port; +} + +// --------------------------------------------------------------------------- +// argv classification +// --------------------------------------------------------------------------- + +/** Whether a `gh` argv is a MUTATION rather than a read. + * + * #321 owns the two-surface mutation recorder and must REUSE this rather than + * build a second classifier -- the handoff's standing rule, and the same one + * that kept #49's git layer from becoming a second port. Two classifiers + * disagreeing about whether `gh issue edit` mutates is a disagreement no test + * would surface until a run 2 quietly scored zero. */ +export function isMutatingGhArgv(argv: readonly string[]): boolean { + if (argv[0] !== "gh") return false; + const verb = argv[1]; + if (verb === "api") { + const method = flagValue(argv, "--method"); + return method === "POST" || method === "PATCH" || method === "PUT" || method === "DELETE"; + } + if (verb === "issue" || verb === "pr" || verb === "label") { + const action = argv[2]; + return ( + action === "create" || + action === "edit" || + action === "close" || + action === "comment" || + action === "ready" || + action === "delete" + ); + } + return false; +} + +function flagValue(argv: readonly string[], flag: string): string | undefined { + const index = argv.indexOf(flag); + if (index === -1) return undefined; + return argv[index + 1]; +} + +/** All values of a repeatable flag, e.g. every `--label`. */ +function flagValues(argv: readonly string[], flag: string): string[] { + const values: string[] = []; + for (const [index, part] of argv.entries()) { + if (part !== flag) continue; + const value = argv[index + 1]; + if (value !== undefined) values.push(value); + } + return values; +} + +/** A `-f key=value` field, as `gh api` takes them. */ +function fieldValue(argv: readonly string[], key: string): string | undefined { + const prefix = `${key}=`; + for (const [index, part] of argv.entries()) { + if (part !== "-f") continue; + const pair = argv[index + 1]; + if (pair !== undefined && pair.startsWith(prefix)) return pair.slice(prefix.length); + } + return undefined; +} + +// --------------------------------------------------------------------------- +// The fake +// --------------------------------------------------------------------------- + +export function createFakeForge(options: FakeForgeOptions = {}): FakeForge { + const milestones: FakeMilestone[] = []; + const issues: FakeIssue[] = []; + const comments: FakeComment[] = []; + const responses: FakeGhResponse[] = []; + + let nextMilestone = 1; + let nextIssue = 1; + let nextComment = 1000; + + let injected: FailureMode | null = null; + // The transient error is TRANSIENT: it spends itself on the first call after + // it is armed and the identical argv then succeeds. A mode that failed for + // ever would be indistinguishable from a fatal one, and + // references/gh-error-handling.md:130 -- "retry once before believing a + // failure" -- would be untestable against it. + let transientSpent = false; + + for (const seed of options.milestones ?? []) { + milestones.push({ + number: nextMilestone++, + title: seed.title, + description: seed.description, + }); + } + + function record(argv: readonly string[], response: GhResponse): ExecResult { + responses.push({ argv: [...argv], ...response }); + return { exitCode: response.exitCode, stdout: response.stdout, stderr: response.stderr }; + } + + function fail( + argv: readonly string[], + stderr: string, + headers?: Readonly>, + ): ExecResult { + return record( + argv, + headers === undefined + ? { exitCode: 1, stdout: "", stderr } + : { exitCode: 1, stdout: "", stderr, headers }, + ); + } + + function ok(argv: readonly string[], stdout: string): ExecResult { + return record(argv, { exitCode: 0, stdout, stderr: "" }); + } + + /** The lie applied to a READ, if one is armed. `null` means answer honestly. */ + function lieOnRead(argv: readonly string[]): ExecResult | null { + if (injected === "rate-limit-zeroed-budget") { + return fail(argv, RATE_LIMITED_STDERR, RATE_LIMIT_HEADERS_MIXED_CASE); + } + if (injected === "forbidden-no-budget-header") { + return fail(argv, FORBIDDEN_NO_HEADER_STDERR); + } + if (injected === "transient-connection-error" && !transientSpent) { + transientSpent = true; + return fail(argv, TRANSIENT_STDERR); + } + return null; + } + + /** The lie applied to a MUTATION, if one is armed. */ + function lieOnMutation(argv: readonly string[]): ExecResult | null { + if (injected === "transient-connection-error" && !transientSpent) { + transientSpent = true; + return fail(argv, TRANSIENT_STDERR); + } + if (injected === "silent-no-op-mutation") { + // Exit 0, nothing written. `gh pr ready` has done this for real + // (references/gh-error-handling.md:126), and it is the reason case 8 + // may not assert acceptance from an exit code: a silent no-op and a + // real acceptance are the same verdict and different reasons. + return ok(argv, ""); + } + return null; + } + + /** Truncate a list the way an unpaginated read truncates it: return a + * PREFIX and say nothing about the remainder. + * + * The last element is dropped because the newest object is the one a + * re-entrant skill just created and is about to look for. Dropping it turns + * "present" into "absent", and absence is what every create step keys on. + * + * IT TRUNCATES DESPITE `--paginate`. packages/core/src/gh/milestones.ts + * :34-36 passes that flag precisely because "the milestone is missing" is + * indistinguishable from "the milestone is on page two" without it -- so a + * fake that honoured the flag could never reproduce the hazard the flag was + * added to prevent. */ + function truncate(items: readonly T[]): readonly T[] { + return injected === "truncated-list" ? items.slice(0, -1) : items; + } + + function unmodelled(argv: readonly string[]): ExecResult { + return record(argv, { + exitCode: FAKE_UNMODELLED_EXIT_CODE, + stdout: "", + // No "403", no "secondary rate limit": an unmodelled operation is a gap + // in the fake, never a forge refusal, and classifyRateLimit must read it + // as `not-rate-limited`. + stderr: + `the fake forge does not model this operation: ${argv.join(" ")}. ` + + "It is a gap in the double, not an answer from a forge, and it is exit " + + `${FAKE_UNMODELLED_EXIT_CODE} so it can never be read as either a refusal or a rejection`, + }); + } + + function milestonesJson(): string { + return JSON.stringify( + truncate(milestones).map((m) => ({ + number: m.number, + title: m.title, + description: m.description, + state: "open", + })), + ); + } + + function issuesJson(): string { + return JSON.stringify( + truncate(issues).map((i) => ({ + number: i.number, + title: i.title, + body: i.body, + labels: i.labels.map((name) => ({ name })), + milestone: i.milestone === null ? null : { title: i.milestone }, + state: "open", + })), + ); + } + + function handleApi(argv: readonly string[]): ExecResult { + const method = flagValue(argv, "--method"); + const path = argv.find( + (part, index) => + index > 1 && + !part.startsWith("-") && + argv[index - 1] !== "--method" && + argv[index - 1] !== "-f", + ); + if (path === undefined) return unmodelled(argv); + + // Milestone list: GET .../milestones?... + if (method === undefined && /\/milestones\?/.test(path)) { + const lie = lieOnRead(argv); + return lie ?? ok(argv, milestonesJson()); + } + + // Milestone create: POST .../milestones + if (method === "POST" && /\/milestones$/.test(path)) { + const lie = lieOnMutation(argv); + if (lie) return lie; + const title = fieldValue(argv, "title"); + if (title === undefined) return unmodelled(argv); + // NO DUPLICATE CHECK, AND THIS ABSENCE IS THE CLAIM. GitHub answers 422 + // here; NEVER-293.7 requires the fake to accept, so that run 2's silence + // is the SKILL's decision and never the double's. + const created: FakeMilestone = { + number: nextMilestone++, + title, + description: fieldValue(argv, "description") ?? "", + }; + milestones.push(created); + return ok(argv, JSON.stringify({ number: created.number, title: created.title })); + } + + // Comment edit: PATCH .../issues/comments/{id} + const commentEdit = /\/issues\/comments\/(\d+)$/.exec(path); + if (method === "PATCH" && commentEdit) { + const lie = lieOnMutation(argv); + if (lie) return lie; + const id = Number(commentEdit[1]); + const index = comments.findIndex((c) => c.id === id); + if (index === -1) return unmodelled(argv); + const existing = comments[index] as FakeComment; + comments[index] = { ...existing, body: fieldValue(argv, "body") ?? existing.body }; + return ok(argv, JSON.stringify({ id })); + } + + return unmodelled(argv); + } + + function handleIssue(argv: readonly string[]): ExecResult { + const action = argv[2]; + + if (action === "list") { + const lie = lieOnRead(argv); + return lie ?? ok(argv, issuesJson()); + } + + if (action === "view") { + const lie = lieOnRead(argv); + if (lie) return lie; + const number = Number(argv[3]); + const issue = issues.find((i) => i.number === number); + if (issue === undefined) return unmodelled(argv); + const own = comments.filter((c) => c.issue === number); + return ok( + argv, + JSON.stringify({ + number: issue.number, + title: issue.title, + body: issue.body, + comments: truncate(own).map((c) => ({ id: c.id, body: c.body })), + }), + ); + } + + if (action === "create") { + const lie = lieOnMutation(argv); + if (lie) return lie; + const title = flagValue(argv, "--title"); + if (title === undefined) return unmodelled(argv); + // Again no duplicate check: the feature row's description text is + // `story-create`'s identity key (skills/story-create/SKILL.md:93) and the + // fake must not be the thing that notices. + const created: FakeIssue = { + number: nextIssue++, + title, + body: flagValue(argv, "--body") ?? "", + milestone: flagValue(argv, "--milestone") ?? null, + labels: flagValues(argv, "--label"), + }; + issues.push(created); + return ok(argv, `https://github.com/OWNER/REPO/issues/${created.number}`); + } + + if (action === "comment") { + const lie = lieOnMutation(argv); + if (lie) return lie; + const number = Number(argv[3]); + const body = flagValue(argv, "--body"); + if (!Number.isInteger(number) || body === undefined) return unmodelled(argv); + // And again: a second `## iai-design` sentinel is accepted. "One + // sentinel, one comment" (skills/story-design/SKILL.md:79) is the + // SKILL's contract, and a fake that upserted on its behalf would score + // every skill compliant including one that never read the comment list. + const created: FakeComment = { id: nextComment++, issue: number, body }; + comments.push(created); + return ok( + argv, + `https://github.com/OWNER/REPO/issues/${number}#issuecomment-${created.id}`, + ); + } + + if (action === "edit") { + const lie = lieOnMutation(argv); + if (lie) return lie; + const number = Number(argv[3]); + const index = issues.findIndex((i) => i.number === number); + if (index === -1) return unmodelled(argv); + const existing = issues[index] as FakeIssue; + issues[index] = { ...existing, body: flagValue(argv, "--body") ?? existing.body }; + return ok(argv, ""); + } + + return unmodelled(argv); + } + + return { + get injected() { + return injected; + }, + get responses() { + return responses; + }, + inject(mode: FailureMode | null) { + injected = mode; + transientSpent = false; + }, + reset() { + responses.length = 0; + }, + async run(argv: readonly string[]): Promise { + // The port's own validation, reused rather than restated -- port.ts:72-73 + // says checkArgv is pure "so it is testable without a process and + // reusable by the fake". A fake that accepted an argv the real port + // refuses would be green over inputs production rejects. + const check = checkArgv(argv); + if (!check.ok) { + return record(argv, { + exitCode: FAKE_UNMODELLED_EXIT_CODE, + stdout: "", + stderr: check.refusal.message, + }); + } + + if (argv[0] !== "gh") { + const delegate = options.delegate; + if (delegate === undefined) return unmodelled(argv); + // Delegated calls are NOT recorded in `responses`: this log is the + // forge's answers, and a git result carrying a forge envelope would + // put the two surfaces of CLAIM-293.8 back into one list. + return delegate.run(argv); + } + + if (argv[1] === "api") return handleApi(argv); + if (argv[1] === "issue") return handleIssue(argv); + return unmodelled(argv); + }, + }; +} diff --git a/packages/harness/src/index.ts b/packages/harness/src/index.ts index 44b271d..8dbe222 100644 --- a/packages/harness/src/index.ts +++ b/packages/harness/src/index.ts @@ -7,3 +7,21 @@ export type { FixtureFile, FixtureRepo, FixtureRepoSpec } from "./fixture-repo"; export { GOALS_FIXTURE_CORPUS_DECLARATION, renderGoalsFixture } from "./goals-fixture"; export type { GoalsFixtureOptions } from "./goals-fixture"; + +export { + createFakeForge, + FAILURE_MODES, + FAKE_UNMODELLED_EXIT_CODE, + isMutatingGhArgv, + RATE_LIMIT_HEADERS_LOWER_CASE, + RATE_LIMIT_HEADERS_MIXED_CASE, +} from "./fake-forge"; +export type { + FailureMode, + FakeComment, + FakeForge, + FakeForgeOptions, + FakeGhResponse, + FakeIssue, + FakeMilestone, +} from "./fake-forge"; diff --git a/packages/harness/test/fake-forge.test.ts b/packages/harness/test/fake-forge.test.ts new file mode 100644 index 0000000..873d698 --- /dev/null +++ b/packages/harness/test/fake-forge.test.ts @@ -0,0 +1,782 @@ +// #320, build target 4 of docs/design/stories/293.md. +// Cases 8, 9 and 20 of docs/test-plans/293-plan.md. +// +// CASE 9 IS PARTIAL AT THIS COMMIT AND CLAIM-293.6 IS NOT MET. Case 9's +// "Passes when" is two independent conjuncts and its Command column names +// `bun run skill-harness --inject`, a script that does not exist in any +// package.json in this tree. Half (i) -- all five modes injectable, each +// distinct -- is this task's, and is met below. Half (ii) -- "the truncated +// list mode makes at least one SKILL's run 2 issue a duplicate create" -- +// needs a transcribed skill body and a runner, which are build targets 5, 6 +// and 7 (#321, #322, #323). Recording case 9 as PASS here would be the G2 +// mechanism (docs/design/stories/293.md:504-511) one layer down: a green read +// off a rung that never ran the thing the claim is about. +// +// Corpus: `synthetic` throughout for the forge -- a real forge cannot be made +// to rate-limit on demand, which is case 9's own stated reason. `real` for the +// git in case 8's fourth identity key: that is a real repository with real +// commits. + +import { afterAll, describe, expect, test } from "bun:test"; +import { mkdirSync, readdirSync, readFileSync, statSync, writeFileSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { classifyRateLimit, shouldRetryResponse } from "iai-core"; +import { createRealPort, createRecordingPort, REFUSED_EXIT_CODE } from "iai-exec"; +import { + createFakeForge, + createFixtureRepo, + createTempDirs, + FAILURE_MODES, + FAKE_UNMODELLED_EXIT_CODE, + isMutatingGhArgv, + RATE_LIMIT_HEADERS_LOWER_CASE, + RATE_LIMIT_HEADERS_MIXED_CASE, + type FailureMode, +} from "../src/index"; + +const repoRoot = join(import.meta.dir, "../../.."); +const skillsDir = join(repoRoot, "skills"); +const temps = createTempDirs(); +afterAll(() => temps.cleanup()); + +/** Pinned, and asserted equal to what is on disk. Both halves are needed: + * reading the directory alone would score 0 of 0 as "all of them". */ +const PINNED_SKILL_COUNT = 4; + +function skillBodies(): { readonly name: string; readonly text: string }[] { + return readdirSync(skillsDir) + .filter((name) => statSync(join(skillsDir, name)).isDirectory()) + .map((name) => ({ name, text: readFileSync(join(skillsDir, name, "SKILL.md"), "utf8") })) + .sort((a, b) => a.name.localeCompare(b.name)); +} + +// =========================================================================== +// The denominators, read off disk +// =========================================================================== + +describe("the skill corpus is read off disk and pinned", () => { + test("the skill count is non-zero and equals the pinned count", () => { + const bodies = skillBodies(); + expect(bodies.length).toBeGreaterThan(0); + expect(bodies.length).toBe(PINNED_SKILL_COUNT); + }); +}); + +// The phrase every one of the four opens the hazard sentence with. Detecting +// the SENTENCE and not the word "rate" is the same posture +// packages/core/src/gh/errors.ts:194-196 takes about the secondary-limit +// phrase: "a whole documented phrase, not a heuristic on the word 'rate'". +const HAZARD_RE = /A rate-limited \*?read\*?/; + +describe("4 of 4 skill bodies name the rate-limited-read-as-absence hazard", () => { + test("the count is read off disk, is non-zero, and is 4 of 4", () => { + const bodies = skillBodies(); + expect(bodies.length).toBeGreaterThan(0); + + const naming = bodies.filter((b) => HAZARD_RE.test(b.text)).map((b) => b.name); + // ASSERT THE LIST, NOT THE COUNT. `toBe(4)` would pass if one body lost + // the sentence and a fifth skill gained it, which is precisely the + // regression CLAIM-293.6's denominator exists to catch. + expect(naming).toEqual(bodies.map((b) => b.name)); + expect(naming.length).toBe(PINNED_SKILL_COUNT); + }); + + test("every one of the four ties the hazard to absence keying a create", () => { + const bodies = skillBodies(); + expect(bodies.length).toBeGreaterThan(0); + // The hazard is not "reads can fail". It is that a failed read is read as + // ABSENCE, and absence is the thing every create step branches on. A body + // that named the first without the second would not motivate this fake. + const tying = bodies.filter((b) => /absen(ce|t)/i.test(b.text) && HAZARD_RE.test(b.text)); + expect(tying.map((b) => b.name)).toEqual(bodies.map((b) => b.name)); + }); +}); + +// =========================================================================== +// Case 8's key list, enumerated from the four bodies as a PARTITION +// =========================================================================== + +// Case 8 names four identity keys. Enumerating the `## Re-entry` "Read first" +// column off disk yields SIXTEEN rows, not four -- `docs/test-plans/{n}-plan.md` +// alone is a fifth path-shaped key the plan's prose does not list. So the four +// are read as KEY CLASSES, and every row is mapped to exactly one class. +// +// THIS IS A PARTITION AND NOT A CARVE-OUT. The tempting shape was +// `if (!isIdentityRow(row)) continue;`, which is the derived, uncounted +// exemption #307 spent a bug removing from this repository. Both populations +// are named, both are counted, and their sum is asserted equal to the whole -- +// so a NEW Re-entry row that fits no class fails loudly instead of being +// silently ignored. + +const IDENTITY_CLASSES = [ + "forge-object-title", + "forge-list-row", + "sentinel-identity", + "worktree-path", +] as const; +type IdentityClass = (typeof IDENTITY_CLASSES)[number]; + +const NON_IDENTITY_CLASSES = ["precondition", "amend-in-place", "report-only"] as const; +type NonIdentityClass = (typeof NON_IDENTITY_CLASSES)[number]; + +/** Every `## Re-entry` "Read first" cell in the repository, mapped by its + * exact text. Keyed on the literal so a reworded row fails rather than + * drifting into the wrong class. */ +const ROW_CLASS: Readonly> = { + // goal-create + "Does a milestone with this title exist?": "forge-object-title", + "Does its description already carry the feature table?": "amend-in-place", + "Did the goal resolve at all?": "precondition", + // story-create + "Does a Story already exist for this row?": "forge-list-row", + "Does the declared domain resolve to a binding?": "precondition", + "Is the existing Story assigned to this milestone?": "amend-in-place", + "Are there Stories with no row?": "report-only", + // story-design + "Does `docs/design/stories/{n}.md` exist?": "worktree-path", + "Does the Story already carry a design sentinel?": "sentinel-identity", + "Does the pinned commit resolve on the remote?": "precondition", + "Do the registered claims already exist?": "amend-in-place", + // story-test-plan + "Does the plan file exist?": "worktree-path", + "Which claims have no case?": "amend-in-place", + "Which cases anchor to a claim that is gone?": "report-only", + "Does every case already declare a `Corpus`?": "amend-in-place", + "Is a test-plan sentinel already posted?": "sentinel-identity", +}; + +const PINNED_REENTRY_ROWS = 16; + +function reentryRows(): { readonly skill: string; readonly read: string }[] { + const rows: { skill: string; read: string }[] = []; + for (const body of skillBodies()) { + const lines = body.text.split("\n"); + const start = lines.findIndex((l) => l.trim() === "## Re-entry"); + if (start === -1) continue; + let end = lines.length; + for (let i = start + 1; i < lines.length; i += 1) { + if ((lines[i] as string).startsWith("## ")) { + end = i; + break; + } + } + const tableLines = lines.slice(start, end).filter((l) => l.startsWith("|")); + // Drop the header row and the `|---|---|` separator. + for (const line of tableLines.slice(2)) { + const cell = (line.split("|")[1] ?? "").trim(); + if (cell.length > 0) rows.push({ skill: body.name, read: cell }); + } + } + return rows; +} + +describe("case 8: the identity keys are enumerated from the four bodies", () => { + test("every skill body carries a Re-entry table, and the row total is pinned", () => { + const bodies = skillBodies(); + const withTable = bodies.filter((b) => b.text.includes("\n## Re-entry\n")); + expect(withTable.map((b) => b.name)).toEqual(bodies.map((b) => b.name)); + + const rows = reentryRows(); + expect(rows.length).toBeGreaterThan(0); + expect(rows.length).toBe(PINNED_REENTRY_ROWS); + }); + + test("the mapping is a partition: zero unmapped rows, and the halves sum to the whole", () => { + const rows = reentryRows(); + expect(rows.length).toBeGreaterThan(0); + + // Name the offenders. A count would report "15 of 16" and leave the reader + // grepping for which row a `SKILL.md` edit just introduced. + const unmapped = rows + .filter((r) => ROW_CLASS[r.read] === undefined) + .map((r) => `${r.skill}: ${r.read}`); + expect(unmapped).toEqual([]); + + const identity = rows.filter((r) => + IDENTITY_CLASSES.includes(ROW_CLASS[r.read] as IdentityClass), + ); + const other = rows.filter((r) => + NON_IDENTITY_CLASSES.includes(ROW_CLASS[r.read] as NonIdentityClass), + ); + + expect(identity.length).toBeGreaterThan(0); + expect(other.length).toBeGreaterThan(0); + expect(identity.length + other.length).toBe(rows.length); + }); + + test("there are exactly four identity key classes and each has at least one row", () => { + const rows = reentryRows(); + expect(rows.length).toBeGreaterThan(0); + + const present = IDENTITY_CLASSES.filter((cls) => rows.some((r) => ROW_CLASS[r.read] === cls)); + // Four, and each one actually occupied -- a class listed but never walked + // is mutation N2's shape, where a scope entry existed and nothing read it. + expect(present).toEqual([...IDENTITY_CLASSES]); + expect(present.length).toBe(4); + }); +}); + +// =========================================================================== +// Case 8: driven directly, the fake ACCEPTS the duplicate +// =========================================================================== + +const REPO_FLAG = ["--repo", "OWNER/REPO"] as const; +const MILESTONE_LIST_ARGV = [ + "gh", + "api", + "--paginate", + "repos/OWNER/REPO/milestones?state=all&per_page=100", +] as const; + +function parse(stdout: string): unknown { + return JSON.parse(stdout) as unknown; +} + +describe("case 8: the fake accepts a duplicate-creating argv, per identity key class", () => { + // ACCEPTANCE IS NOT AN EXIT CODE. `silent-no-op-mutation` returns exit 0 and + // writes nothing, so a test asserting `exitCode === 0` would score a fake + // that dropped every mutation on the floor as fully compliant. Each case + // below asserts THREE things: not refused, two attempts recorded, and the + // state grew -- read back through a list argv, never through a private + // field. references/verification.md:125, assert the reason not the verdict. + + test("forge-object-title: two milestones with the same title", async () => { + const forge = createFakeForge(); + const port = createRecordingPort(forge); + const title = "M9 — the universal lifecycle"; + const argv = [ + "gh", + "api", + "--method", + "POST", + "repos/OWNER/REPO/milestones", + "-f", + `title=${title}`, + ]; + + const first = await port.run(argv); + const second = await port.run(argv); + + for (const result of [first, second]) { + expect(result.exitCode).not.toBe(REFUSED_EXIT_CODE); + expect(result.exitCode).not.toBe(FAKE_UNMODELLED_EXIT_CODE); + } + expect(port.calls.filter((c) => c.argv.join(" ") === argv.join(" ")).length).toBe(2); + + const listed = await port.run([...MILESTONE_LIST_ARGV]); + const rows = parse(listed.stdout) as { title: string }[]; + // GitHub answers 422 here (docs/design/stories/293.md:150-153). The fake + // must not, or run 2's silence would be the double's decision. + expect(rows.filter((m) => m.title === title).length).toBe(2); + }); + + test("forge-list-row: two issues with the same feature-row description", async () => { + const forge = createFakeForge(); + const port = createRecordingPort(forge); + const title = "S9.1 — the seam rung"; + const argv = ["gh", "issue", "create", ...REPO_FLAG, "--title", title, "--body", "Refs #293"]; + + await port.run(argv); + await port.run(argv); + + expect(port.calls.filter((c) => c.argv.join(" ") === argv.join(" ")).length).toBe(2); + for (const call of port.calls) expect(call.exitCode).not.toBe(REFUSED_EXIT_CODE); + + const listed = await port.run(["gh", "issue", "list", ...REPO_FLAG, "--json", "number,title"]); + const rows = parse(listed.stdout) as { title: string }[]; + expect(rows.filter((i) => i.title === title).length).toBe(2); + }); + + test("sentinel-identity: two comments carrying the same sentinel", async () => { + const forge = createFakeForge(); + const port = createRecordingPort(forge); + await port.run(["gh", "issue", "create", ...REPO_FLAG, "--title", "S9.1", "--body", "b"]); + + const sentinel = "## iai-" + "design\n\nPinned at 0123456789abcdef"; + const argv = ["gh", "issue", "comment", "1", ...REPO_FLAG, "--body", sentinel]; + await port.run(argv); + await port.run(argv); + + expect(port.calls.filter((c) => c.argv.join(" ") === argv.join(" ")).length).toBe(2); + + const viewed = await port.run(["gh", "issue", "view", "1", ...REPO_FLAG, "--json", "comments"]); + const view = parse(viewed.stdout) as { comments: { body: string }[] }; + // "One sentinel, one comment" is skills/story-design/SKILL.md:79 -- the + // SKILL's contract. A fake that upserted on the skill's behalf would score + // a skill that never read the comment list as compliant. + expect(view.comments.filter((c) => c.body === sentinel).length).toBe(2); + }); + + test("worktree-path: two commits touching the same Design path", async () => { + // THE FOURTH KEY IS ON THE OTHER SURFACE, and that is why it does not + // collapse into the third. Expressed as a `gh` argv it would be + // `gh issue comment --body ""` -- byte-identical + // in shape to the sentinel case above and differing only in the string + // inside --body. Two argv that differ only in the value they quote are one + // argv (references/verification.md:127-129). Its argv[0] is `git`, which + // packages/exec/src/port.ts:58 admits, so it is still "driven directly + // through the port" exactly as NEVER-293.7 words it. + const real = createRealPort(); + const port = createRecordingPort(real); + const root = temps.create("iai-fx-320-design"); + const repo = await createFixtureRepo(port, root, { + name: "design-path", + files: [{ path: "docs/milestones/M9.md", contents: "# M9\n" }], + }); + + const designPath = "docs/design/stories/9999.md"; + const full = join(root, designPath); + let commits = 0; + for (const contents of ["# Story 9999\n\n## Problem\n", "# Story 9999\n\n## Problem\n\n## Goal\n"]) { + mkdirSync(dirname(full), { recursive: true }); + writeFileSync(full, contents, "utf8"); + const add = await repo.run(["git", "-C", root, "add", "-A"]); + const commit = await repo.run([ + "git", + "-C", + root, + "commit", + "-q", + "-m", + `#320: write ${designPath}`, + ]); + expect(add.exitCode).toBe(0); + expect(commit.exitCode).toBe(0); + commits += 1; + } + expect(commits).toBe(2); + + // Read the state back out of git, not out of a counter this test kept. + // The tree hash cannot be the measure here: case 10 requires an + // identical-bytes rewrite to count as a mutation, and that leaves the hash + // unchanged. + const counted = await repo.run([ + "git", + "-C", + root, + "rev-list", + "--count", + "HEAD", + "--", + designPath, + ]); + expect(counted.exitCode).toBe(0); + expect(Number(counted.stdout.trim())).toBe(2); + + // Both attempts are on the ONE seam, alongside every `gh` attempt above. + const commitAttempts = port.calls.filter((c) => c.argv.includes("commit")); + expect(commitAttempts.length).toBe(3); // the seed commit, plus the two writes + }); +}); + +// =========================================================================== +// Case 9, half (i): five modes, injectable, and each lie is causally a lie +// =========================================================================== + +describe("case 9 (i): all five failure modes are injectable and distinct", () => { + test("the mode roster is non-zero, five, and has no duplicates", () => { + expect(FAILURE_MODES.length).toBeGreaterThan(0); + expect(FAILURE_MODES.length).toBe(5); + // Cardinality, not length: a roster listing one mode twice would satisfy + // `length === 5` and inject four things. + expect(new Set(FAILURE_MODES).size).toBe(FAILURE_MODES.length); + }); + + test("each mode changes an observable the honest fake does not produce", async () => { + let exercised = 0; + const observed = new Map(); + + for (const mode of FAILURE_MODES) { + const forge = createFakeForge({ milestones: [{ title: "M9", description: "" }] }); + const honest = await forge.run([...MILESTONE_LIST_ARGV]); + expect(honest.exitCode).toBe(0); + + forge.inject(mode); + const read = await forge.run([...MILESTONE_LIST_ARGV]); + const create = await forge.run([ + "gh", + "api", + "--method", + "POST", + "repos/OWNER/REPO/milestones", + "-f", + "title=M8", + ]); + forge.inject(null); + const after = await forge.run([...MILESTONE_LIST_ARGV]); + + // The signature is what the mode actually changed, not the name of the + // mode -- so two modes wired to the same branch collapse visibly. + // + // THE CLASSIFICATION IS IN THE SIGNATURE BECAUSE WITHOUT IT THIS + // ASSERTION FAILS, AND THAT FAILURE IS THE POINT. Measured while writing + // this file: on exit code and row counts alone the five modes produce + // THREE distinct signatures, not five -- the zeroed budget, the + // header-absent 403 and the transient error are indistinguishable, + // because all three are "the read exited 1 and returned no rows". They + // differ only in the REASON, which is what `classifyRateLimit` exists to + // extract and what case 20 requires be asserted. A caller branching on + // the verdict cannot tell them apart; one branching on the + // classification can. + const readResponse = forge.responses[1]; + observed.set( + mode, + [ + `read.exit=${String(read.exitCode)}`, + `read.rows=${read.exitCode === 0 ? String((parse(read.stdout) as unknown[]).length) : "n/a"}`, + `read.class=${readResponse === undefined ? "none" : classifyRateLimit(readResponse)}`, + `read.retry=${readResponse === undefined ? "none" : String(shouldRetryResponse(readResponse).retry)}`, + `create.exit=${String(create.exitCode)}`, + `state.rows=${String((parse(after.stdout) as unknown[]).length)}`, + ].join(" "), + ); + exercised += 1; + } + + expect(exercised).toBe(FAILURE_MODES.length); + // 5 of 5 injectable means 5 DISTINCT observable effects. A `length` check + // counts what you collected; a cardinality check counts what is distinct. + expect(new Set(observed.values()).size).toBe(FAILURE_MODES.length); + }); + + test("the truncated-list mode omits an object that is in the fake's own state", async () => { + const forge = createFakeForge({ + milestones: [ + { title: "M8", description: "" }, + { title: "M9", description: "" }, + ], + }); + + const honest = await forge.run([...MILESTONE_LIST_ARGV]); + const honestTitles = (parse(honest.stdout) as { title: string }[]).map((m) => m.title); + expect(honestTitles).toEqual(["M8", "M9"]); + + forge.inject("truncated-list"); + const lied = await forge.run([...MILESTONE_LIST_ARGV]); + const liedTitles = (parse(lied.stdout) as { title: string }[]).map((m) => m.title); + + // THE LIE IS CAUSALLY A LIE: exit 0, well-formed JSON, and an object the + // fake is holding is simply not in it. That is the shape a caller cannot + // tell from absence, which is the whole hazard. + expect(lied.exitCode).toBe(0); + expect(liedTitles).toEqual(["M8"]); + expect(liedTitles).not.toContain("M9"); + + // AND IT TRUNCATES DESPITE `--paginate`. + // packages/core/src/gh/milestones.ts:34-36 added that flag because "the + // milestone is missing" is indistinguishable from "the milestone is on + // page two". A fake that honoured the flag could never reproduce the + // hazard the flag exists to prevent. + expect(MILESTONE_LIST_ARGV).toContain("--paginate"); + }); + + test("the transient error spends itself: the identical argv succeeds on retry", async () => { + const forge = createFakeForge({ milestones: [{ title: "M9", description: "" }] }); + forge.inject("transient-connection-error"); + const first = await forge.run([...MILESTONE_LIST_ARGV]); + const second = await forge.run([...MILESTONE_LIST_ARGV]); + + expect(first.exitCode).toBe(1); + expect(second.exitCode).toBe(0); + // references/gh-error-handling.md:130 -- "retry once before believing a + // failure". A mode that failed for ever would be a fatal error wearing a + // transient one's name, and that instruction would be untestable. + expect(first.stderr).not.toBe(second.stderr); + }); + + test("the silent no-op returns success and writes nothing", async () => { + const forge = createFakeForge(); + forge.inject("silent-no-op-mutation"); + const create = await forge.run([ + "gh", + "api", + "--method", + "POST", + "repos/OWNER/REPO/milestones", + "-f", + "title=M9", + ]); + forge.inject(null); + const listed = await forge.run([...MILESTONE_LIST_ARGV]); + + expect(create.exitCode).toBe(0); + // The verdict says it worked and the state says it did not. `gh pr ready` + // has done exactly this for real (references/gh-error-handling.md:126), + // and it is why case 8 above may not read acceptance off an exit code. + expect(parse(listed.stdout)).toEqual([]); + }); +}); + +// =========================================================================== +// Case 9, half (ii): the hazard, demonstrated -- and NOT a skill +// =========================================================================== + +describe("case 9 (ii): a truncated read makes a second run create a duplicate", () => { + // THIS IS NOT A TRANSCRIPTION OF `goal-create` AND MUST NOT BE CITED AS ONE. + // + // It implements exactly one sentence -- skills/goal-create/SKILL.md:107-110, + // "a rate-limited read ... must never be treated as 'no such milestone' ... + // because absence is what the create step keys on" -- in six lines, to show + // the fake's truncation reaches a create. Problem 4 of + // docs/design/stories/293.md calls a hand-written procedure standing in for + // a skill "the category error at its fifth occurrence", and CLAIM-293.11 + // requires every transcription to be audited one row per Re-entry row. + // + // So it is deliberately NOT exported from packages/harness/src/index.ts and + // lives only in this file. #322 and #323 must build their runner on a + // transcription that goes through that audit, not on this. + async function readThenCreate(driver: { run(argv: readonly string[]): Promise<{ exitCode: number; stdout: string }> }, title: string): Promise { + const listed = await driver.run([...MILESTONE_LIST_ARGV]); + const rows = listed.exitCode === 0 ? (parse(listed.stdout) as { title: string }[]) : []; + if (!rows.some((m) => m.title === title)) { + await driver.run([ + "gh", + "api", + "--method", + "POST", + "repos/OWNER/REPO/milestones", + "-f", + `title=${title}`, + ]); + } + } + + test("run 1 creates, run 2 is silent, and run 2 under the lie is not", async () => { + const title = "M9 — the universal lifecycle"; + const forge = createFakeForge(); + const port = createRecordingPort(forge); + + await readThenCreate(port, title); + const run1Mutations = port.calls.filter((c) => isMutatingGhArgv(c.argv)); + // THE `> 0` HALF, EVALUATED FIRST. docs/design/stories/293.md:424-426: + // "run 2 made zero mutations" is meaningless alone. + expect(run1Mutations.length).toBeGreaterThan(0); + + port.reset(); + await readThenCreate(port, title); + expect(port.calls.filter((c) => isMutatingGhArgv(c.argv))).toEqual([]); + + port.reset(); + forge.inject("truncated-list"); + await readThenCreate(port, title); + forge.inject(null); + + const lieMutations = port.calls.filter((c) => isMutatingGhArgv(c.argv)); + // The harness sees it because the recorder records the ATTEMPT + // (packages/exec/src/recording.ts:10-16) -- no second capture mechanism. + expect(lieMutations.length).toBe(1); + expect(lieMutations[0]?.argv).toContain(`title=${title}`); + + const listed = await forge.run([...MILESTONE_LIST_ARGV]); + expect( + (parse(listed.stdout) as { title: string }[]).filter((m) => m.title === title).length, + ).toBe(2); + }); +}); + +// =========================================================================== +// Case 20: the reasons are pairwise distinct, asserted by ABSENCE +// =========================================================================== + +describe("case 20: the two rate-limit lies differ by REASON, not by verdict", () => { + async function respond(mode: FailureMode | null) { + const forge = createFakeForge({ milestones: [{ title: "M9", description: "" }] }); + forge.inject(mode); + await forge.run([...MILESTONE_LIST_ARGV]); + const last = forge.responses.at(-1); + if (last === undefined) throw new Error("the fake recorded no response"); + return last; + } + + test("both lies share the verdict, so the verdict cannot be the discriminator", async () => { + const zeroed = await respond("rate-limit-zeroed-budget"); + const headerless = await respond("forbidden-no-budget-header"); + expect(zeroed.exitCode).toBe(headerless.exitCode); + expect(zeroed.exitCode).not.toBe(0); + }); + + test("the classifier separates them into three states, each asserted against the other two", async () => { + const zeroed = await respond("rate-limit-zeroed-budget"); + const headerless = await respond("forbidden-no-budget-header"); + // NEGATIVE CONTROL. Without it, a classifier returning "rate-limited" + // whenever ANY header key is present satisfies the two-way distinction + // perfectly. An unmodelled operation is a genuine failure that is not a + // rate limit, so it is the honest third fixture. + const forge = createFakeForge(); + await forge.run(["gh", "workflow", "run", "nothing.yml"]); + const control = forge.responses.at(-1); + if (control === undefined) throw new Error("the fake recorded no response"); + + expect(classifyRateLimit(zeroed)).toBe("rate-limited"); + expect(classifyRateLimit(zeroed)).not.toBe("possibly-rate-limited"); + expect(classifyRateLimit(zeroed)).not.toBe("not-rate-limited"); + + expect(classifyRateLimit(headerless)).toBe("possibly-rate-limited"); + expect(classifyRateLimit(headerless)).not.toBe("rate-limited"); + expect(classifyRateLimit(headerless)).not.toBe("not-rate-limited"); + + expect(classifyRateLimit(control)).toBe("not-rate-limited"); + expect(shouldRetryResponse(control).retry).toBe(false); + expect(control.exitCode).toBe(FAKE_UNMODELLED_EXIT_CODE); + }); + + test("the third state is reached by the ABSENCE of the header, not by its value", async () => { + const headerless = await respond("forbidden-no-budget-header"); + // references/gh-error-handling.md:56-60: the third state exists because + // `gh api --include` appears nowhere in this repository, so the common + // case is that no headers were captured at all. + expect(headerless.headers).toBeUndefined(); + + const zeroed = await respond("rate-limit-zeroed-budget"); + expect(zeroed.headers).toBeDefined(); + }); + + test("the header-absent fixture does not contain the secondary-limit phrase", async () => { + const headerless = await respond("forbidden-no-budget-header"); + // packages/core/src/gh/errors.ts:226-228 tests SECONDARY_RATE_LIMIT_RE + // FIRST and short-circuits. A stray "secondary rate limit" in this string + // would classify it `rate-limited` and silently collapse the two modes + // into one -- and both assertions above would still pass. + expect(/secondary rate limit/i.test(headerless.stderr)).toBe(false); + expect(/secondary rate limit/i.test(headerless.stdout)).toBe(false); + }); + + test("the modes produce pairwise-distinct stderr, each excluding the others' phrase", async () => { + // The phrase that belongs to exactly one mode. Asserting only that each + // message CONTAINS its own phrase is the pair of positive assertions + // mutation M3 of #319 walked straight through; each must also assert the + // ABSENCE of the others'. + const OWN_PHRASE: Readonly> = { + "rate-limit-zeroed-budget": "API rate limit exceeded", + "forbidden-no-budget-header": "Resource not accessible by integration", + "transient-connection-error": "error connecting to api.github.com", + }; + const modes = Object.keys(OWN_PHRASE); + expect(modes.length).toBeGreaterThan(0); + + let checked = 0; + for (const mode of modes) { + const response = await respond(mode as FailureMode); + const own = OWN_PHRASE[mode] as string; + expect(response.stderr).toContain(own); + for (const other of modes) { + if (other === mode) continue; + expect(response.stderr).not.toContain(OWN_PHRASE[other] as string); + } + checked += 1; + } + expect(checked).toBe(modes.length); + }); + + test("both header casings classify identically", () => { + const base = { exitCode: 1, stdout: "", stderr: "gh: HTTP 403: API rate limit exceeded" }; + // packages/core/src/gh/errors.ts:153-158 records that + // getHeaderCaseInsensitive exists because a literal lowercase lookup + // "would silently miss X-RateLimit-Remaining". Emitting only one casing + // anywhere in the corpus leaves a mutation collapsing it to a literal + // lookup alive. + expect(classifyRateLimit({ ...base, headers: RATE_LIMIT_HEADERS_MIXED_CASE })).toBe( + "rate-limited", + ); + expect(classifyRateLimit({ ...base, headers: RATE_LIMIT_HEADERS_LOWER_CASE })).toBe( + "rate-limited", + ); + const mixedKeys = Object.keys(RATE_LIMIT_HEADERS_MIXED_CASE); + expect(mixedKeys.length).toBeGreaterThan(0); + // The two fixtures must actually differ in case, or this proves nothing. + expect(mixedKeys).not.toEqual(Object.keys(RATE_LIMIT_HEADERS_LOWER_CASE)); + expect(mixedKeys.map((k) => k.toLowerCase())).toEqual( + Object.keys(RATE_LIMIT_HEADERS_LOWER_CASE), + ); + }); +}); + +// =========================================================================== +// The header-capture gap, published rather than hidden +// =========================================================================== + +function sourceFilesUnder(dir: string): string[] { + const out: string[] = []; + for (const entry of readdirSync(dir, { withFileTypes: true })) { + if (entry.name === "node_modules" || entry.name === "dist" || entry.name.startsWith(".")) { + continue; + } + const full = join(dir, entry.name); + if (entry.isDirectory()) out.push(...sourceFilesUnder(full)); + else if (entry.name.endsWith(".ts")) out.push(full); + } + return out; +} + +describe("the header-capture gap is a published measurement, not a hidden divergence", () => { + // READ THIS BEFORE "FIXING" THE TEST BELOW. + // + // The fake hands out a structured `headers` map. `createRealPort` never can: + // nothing in this repository passes `gh api --include`, which is the exact + // reason the third rate-limit state exists at all + // (packages/core/src/gh/errors.ts:212-216). That divergence is real, and the + // choice made here is the one NEVER-293.5 makes -- turn it into a PUBLISHED + // NUMBER instead of a silence. The PASS condition below is an ABSENCE, so + // the first person to read it will be tempted to delete it as vacuous, and + // deleting it deletes the only measurement of the gap. + // + // OWNER: #324, the live rung -- the first thing in this repository that will + // meet a real 403 and can confirm or refute both the capture gap and the + // `assumed` 403 format at errors.ts:199-206. + + test("no argv anywhere in the tree captures headers, so production never has them", () => { + const files = sourceFilesUnder(join(repoRoot, "packages")); + expect(files.length).toBeGreaterThan(0); // denominator first + + const capturing = files + .map((f) => f.slice(repoRoot.length + 1)) + .filter((rel) => /["']--include["']/.test(readFileSync(join(repoRoot, rel), "utf8"))); + expect(capturing).toEqual([]); + }); + + test("the real port's result shape has no header channel at all", async () => { + const port = createRealPort(); + const result = await port.run(["git", "--version"]); + expect(result.exitCode).toBe(0); + // Not `expect(result.headers).toBeUndefined()` -- a missing property and a + // property set to undefined read the same that way. The key must be + // absent, because the point is that packages/exec/src/port.ts:34-38 was + // never widened for the fake's benefit. + expect(Object.keys(result).sort()).toEqual(["exitCode", "stderr", "stdout"]); + }); +}); + +// =========================================================================== +// The mutation classifier is single, and shared +// =========================================================================== + +describe("isMutatingGhArgv is the one classifier, and it is not vacuous", () => { + const MUTATIONS: readonly (readonly string[])[] = [ + ["gh", "api", "--method", "POST", "repos/OWNER/REPO/milestones", "-f", "title=M9"], + ["gh", "api", "--method", "PATCH", "repos/OWNER/REPO/issues/comments/1000", "-f", "body=x"], + ["gh", "issue", "create", "--repo", "OWNER/REPO", "--title", "t", "--body", "b"], + ["gh", "issue", "comment", "1", "--repo", "OWNER/REPO", "--body", "b"], + ["gh", "issue", "edit", "1", "--repo", "OWNER/REPO", "--body", "b"], + ["gh", "issue", "close", "1", "--repo", "OWNER/REPO"], + ["gh", "pr", "ready", "1", "--repo", "OWNER/REPO"], + ["gh", "label", "create", "iai", "--repo", "OWNER/REPO", "--color", "24292f"], + ]; + const READS: readonly (readonly string[])[] = [ + ["gh", "api", "--paginate", "repos/OWNER/REPO/milestones?state=all&per_page=100"], + ["gh", "issue", "list", "--repo", "OWNER/REPO", "--json", "number"], + ["gh", "issue", "view", "1", "--repo", "OWNER/REPO", "--json", "comments"], + ["gh", "pr", "list", "--repo", "OWNER/REPO", "--json", "number"], + ["gh", "label", "list", "--repo", "OWNER/REPO", "--limit", "200"], + ["git", "-C", "/tmp/x", "commit", "-m", "#0: not a gh argv"], + ]; + + test("both populations are non-empty and classified correctly, named on failure", () => { + expect(MUTATIONS.length).toBeGreaterThan(0); + expect(READS.length).toBeGreaterThan(0); + // A classifier answering `true` to everything passes the first list and + // fails the second; one answering `false` to everything does the reverse. + // Neither direction alone is a check. + expect(MUTATIONS.filter((a) => !isMutatingGhArgv(a)).map((a) => a.join(" "))).toEqual([]); + expect(READS.filter((a) => isMutatingGhArgv(a)).map((a) => a.join(" "))).toEqual([]); + }); +});