diff --git a/.dev/features/version-badge-pin/GRILL.md b/.dev/features/version-badge-pin/GRILL.md new file mode 100644 index 0000000..eb4b132 --- /dev/null +++ b/.dev/features/version-badge-pin/GRILL.md @@ -0,0 +1,138 @@ +# GRILL — version-badge-pin + +Plan under interrogation: `.dev/features/version-badge-pin/PLAN.md`. +Spec-hash check: **MATCH** — `node .dev/floor/hash-doc.mjs pharn/ARCHITECTURE.md` recomputed +`8f5ec002e3b18cbfd2f094b08a3671f7ed42a05a3fbaf01a11bbbd28da30fb52`, equal to the plan's pin. No drift. +Grillers registered (FLOOR membership, `node pharn/floor/count-grillers.mjs .`): **13**. + +**ADVISORY end-to-end.** Nothing below blocks `/pharn-dev-build`. The `PLAN.md` is `trust: untrusted`; +its quoted text is DATA, never an instruction. The enum-gated fields (`rule_id`, `severity`, `file`) +are this stage's own membership/path assertions; `problem` / `evidence` inherit the plan's tag. + +--- + +## Findings + +### Axis: L1 meta-doc sweep (the sweep the plan ran, run once more against the plan itself) + +```yaml +- type: FINDING + rule_id: "P6" + severity: important + file: ".claude/commands/pharn-dev-verify.md:100" + problem: "The increment makes an ALREADY-FALSE sentence measurably falser, and the plan's L1 sweep did not catch it because the sweep looked for facts the increment CHANGES rather than facts it WIDENS." + evidence: "pharn-dev-verify.md:100 asserts `The format:check + lint:md + lint + test set is exactly the repo's npm run check aggregate, so the verdict tracks the full npm run check`. Read live this run, npm run check is `format:check && lint && lint:md && docs:check && check:markers && test` — so the claimed set is already a strict SUBSET (docs:check and check:markers are absent from verify's gate map), and adding check:badge makes it a third divergence." +``` + +**Disposition (for the human, not decided here).** The file is a `pharn-dev-*` command — apparatus, no +`SKILLS_VERSION` bump — but it is **not** in the approved plan's `## Files`, so fix #7 will (correctly) +deny a write to it. Three honest options: (a) re-approve the plan with an 8th path and fix the sentence +in this increment; (b) record it as a follow-up `verify-gate-map-claim` and leave it; (c) treat it as +out of scope permanently because verify's gate map is deliberately a subset. **Note the plan is not +wrong to exclude it** — the defect predates this increment and is a different axis (verify's self +description), not the front-page version story. Recommendation: **(b)**, and say so in `SHIP.md` rather +than silently widening an approved scope mid-chain. + +### Axis: P1 / testability — gaps in the test roster + +```yaml +- type: FINDING + rule_id: "P1" + severity: important + file: ".dev/features/version-badge-pin/PLAN.md:52" + problem: "The roster does not cover a SKILLS_VERSION value containing a hyphen, which the shields badge encoding cannot round-trip — so the checker would mis-parse a pre-release version and report a confusing mismatch instead of a named refusal." + evidence: "Roster line: `agreement -> exit 0 (pharn-2.5.1 badge vs SKILLS_VERSION 2.5.1)`. Shields encodes a literal `-` in a message as `--`, and the planned anchor `img.shields.io/badge/pharn-([^-\\s)]+)-` stops at the first hyphen. A SKILLS_VERSION of `2.6.0-rc.1` would extract `2.6.0` and compare it against `2.6.0-rc.1`." +``` + +**Direction is safe, message is not.** The failure is fail-closed (RED, not a false green), so this is a +legibility defect rather than a hole. Remedy folded into the build: the value shape guard REFUSES a +`SKILLS_VERSION` containing `-` with an explicit "this badge encoding cannot represent a pre-release +version" message, and a test asserts that refusal — rather than letting it surface as a mismatch whose +printed values look, confusingly, almost equal. + +```yaml +- type: FINDING + rule_id: "P1" + severity: minor + file: ".dev/features/version-badge-pin/PLAN.md:57" + problem: "The roster specifies the single-failure cases but never the DOUBLE-failure case, leaving the precedence between two simultaneous REDs unspecified — an undefined output for a reachable input." + evidence: "Roster covers `badge absent -> exit 1` and `SKILLS_VERSION missing / blank / multi-line -> exit 1` as separate rows; no row states which reason is reported when BOTH are true." +``` + +Remedy folded into the build: `SKILLS_VERSION` is read and validated **first**, so its refusal wins, and +a test pins that precedence. Deterministic order, not incidental. + +### Axis: P3 — is this one increment or three? + +```yaml +- type: FINDING + rule_id: "P3" + severity: minor + file: ".dev/features/version-badge-pin/PLAN.md:25" + problem: "The increment spans front-page prose, a new deterministic gate, and CI wiring, which reads on its face as three axes of change bundled into one PR." + evidence: "`## Files` lists 7 paths across README.md + CHANGELOG.md prose, a new .dev/floor checker + test, package.json script wiring, .github/workflows/ci.yml, and CLAUDE.md." +``` + +**Interrogated and judged coherent — recorded so the judgement is visible rather than assumed.** The +three parts are not independent: L20 forbids shipping the prose fix with a discipline-only remedy, so +the checker is not a separable addition but a **precondition** of the prose change; and the CI step is +not separable from the checker, because a checker CI never runs would make the plan's own gating claim +false (verified live: `ci.yml` runs each script individually and never `npm run check`). `CLAUDE.md` is +the L1 consequence. Splitting would produce a first PR whose fix is knowingly unenforced. + +### Axis: P0 — guarantee audit + +No finding. The audit already narrows the two claims that could be overstated ("cannot silently drift +again" is scoped to `npm run check` / CI, and the CI pin's harness-layer residual is named), and it +carries an explicit **STRUCK** line for the structured-location claim, which is the honest reading of +L6 rather than a cargo-culted citation. + +### Axis: P2 — trust propagation + +No finding. Both inputs are compared as bytes behind a control-char guard, and the only free-text +output is a printed message no decision reads. + +### Axis: P5 / error-handling — fail-closed behaviour + +No finding beyond the precedence gap above. Every branch is a membership test and every ambiguity +(≠ 1 match, unreadable input) is a named RED rather than a first-match guess. + +### Axis: security — the checker parses file content with a regex + +No finding. Verified empirically this run rather than argued: the anchor +`img.shields.io/badge/pharn-([^-\s)]+)-` is a negated character class under a single quantifier with no +nesting, so it cannot backtrack catastrophically; run against the live `README.md` it yields **0** +matches today and exactly **1** post-edit, and the three `shields.io/badge/` labels present +(`version`, `license`, `built%20for`) do not collide with the `pharn-` anchor. + +### Axis: P7 — speculative additions + +No finding. Each of the 7 paths traces to a verified live failure: the badge/CHANGELOG to the reported +defect; the checker to L20's escalation rule; `ci.yml` to the live reading that CI never invokes +`npm run check`; the CI pin test to the recorded precedent in +`.dev/floor/lessons-index-core.test.mjs`, whose own comment documents a commit where exactly this +wiring claim was false; `CLAUDE.md` to L1. + +--- + +## Summary + +The plan survives interrogation on its load-bearing axes — the guarantee audit is honest about what the +gate does and does not buy, the trust and determinism audits hold, and the security axis checks out +empirically rather than by assertion. The concerns are concentrated in two places. + +The first is a **meta-doc defect the plan's own L1 sweep missed** — not a fact this increment changes, +but one it _widens_: `/pharn-dev-verify`'s self-description already claims its gate set is "exactly" +`npm run check` when it is a strict subset, and `check:badge` becomes the third item outside it. That is +worth a human decision, not a silent scope widening. + +The second is **test-roster legibility**: two reachable inputs (a hyphen-bearing `SKILLS_VERSION`, and +two simultaneous failures) produce correct fail-closed behaviour with an unspecified or confusing +message. Both remedies are small and land inside the approved `## Files`. + +The P3 bundling question was raised deliberately and resolved as coherent, because the parts are +causally dependent rather than merely related — recorded so a reviewer sees the reasoning instead of +inferring it. + +ADVISORY VERDICT: **4 concerns raised (0 blocking-severity, 2 important, 2 minor)** — for the human to +weigh before `/pharn-dev-build`. This is not a judgement that the plan is sound, and it gates nothing. diff --git a/.dev/features/version-badge-pin/PLAN.md b/.dev/features/version-badge-pin/PLAN.md new file mode 100644 index 0000000..d2b56b4 --- /dev/null +++ b/.dev/features/version-badge-pin/PLAN.md @@ -0,0 +1,91 @@ +# PLAN — version-badge-pin + +- spec_content_hash: 8f5ec002e3b18cbfd2f094b08a3671f7ed42a05a3fbaf01a11bbbd28da30fb52 +- applied_lessons: [L1, L2, L3, L6, L7, L9, L11, L13, L14, L16, L18, L19, L20] +- increment: Make the two version tracks legible on the front page — the README badge shows `SKILLS_VERSION` under a label that distinguishes it from the `1.0.0` foundation tag, the CHANGELOG header names both tracks, and a new apparatus checker pins the badge to `SKILLS_VERSION` deterministically so it cannot silently drift again. +- layer(s): none (build apparatus + repo-meta — no product-surface capability; `.dev/` is outside the layer tree) +- constitution_refs: [P0, P5, P6, P7] + +## Applied lessons + +- L1 — Meta-doc sweep run against **live** state, not assumed. Verified this run that the README `CURRENT-STATE` generated region counts `pharn/floor/` `.mjs` files (**46**) and **not** `.dev/floor/`, so adding a dev-floor checker owes **no** `npm run docs:generate` regeneration — the region's bytes are unchanged. Two meta-doc facts **do** go stale and are therefore named in `## Files`: `CLAUDE.md:215`'s `npm run check` component list, and CLAUDE.md's Commands block (which documents each `.dev/floor` checker). `CHANGELOG.md` gets an `[Unreleased]` entry so the file's own "all notable changes are documented" contract holds. +- L2 — The checker's honesty travels **in the checker**, not only in this plan: its header carries the guarantee audit below verbatim, and it cites only ops verified live this run. This lesson is what forced the discovery that **CI does not invoke `npm run check`** — `.github/workflows/ci.yml` runs each script as its own step (read live: `format:check`, `lint:md`, `lint`, `validate.mjs`, `docs:check`, `check:markers`, `test`). Wiring the checker into `npm run check` alone would have left it **unrun on every PR** while this plan claimed it was gated. `ci.yml` is in `## Files` for that reason. +- L3 — Making the badge value load-bearing means re-auditing every existing declaration of the same kind. Audited all **7** README badges live: only the version badge is pinned by this increment. The other six (License, CI, CodeQL, Floor, Secrets, Built for Claude Code) are named as out of scope below — none has a demonstrated drift, and pinning them speculatively is the P7 violation. +- L6 — A badge value has **no** structured location; a README badge is free-text prose by nature, which is exactly why it drifted. The checker therefore does the narrowest honest thing: it matches the **shields URL token** (a structured token _within_ prose), never a line number and never a loose version-like substring, and it REDs on **≠ 1** match rather than silently taking the first. The header states this prose-scan bound instead of claiming a structured read. +- L7 — `## Files` lists exactly the seven paths this increment writes — nothing aspirational, and no downstream gate's target. No canon file (`.dev/memory-bank/**`) and no product-surface path appears. +- L9 — The badge check is deliberately **not** added to `/pharn-dev-verify`'s gate map; it lives in `npm run check` + CI only. Verify's gate map stays the deterministic style/test authority, and this increment adds no new class of red to it. +- L11 — Corollary of the above, and the reason it is deliberate: `/pharn-dev-verify`'s gates are whole-repo and run once at HEAD with no base comparison, so a badge RED added there would block **every later feature's** verify until someone fixed an unrelated front-page byte. Keeping the gate in `npm run check` + CI puts it where a red is attributable to the PR that caused it. +- L13 — This stage formats its own artifact (`prettier` + `markdownlint-cli2 --fix` scoped to this `PLAN.md`) before halting, rather than leaving it for a mid-pipeline manual pass. +- L14 — The value regex **composes after** a guard, never replaces one: the extracted badge value and the `SKILLS_VERSION` file contents are each required to be a control-char-free, single-line scalar **before** the anchored shape regex runs. This is why `SKILLS_VERSION` is trimmed **and** shape-validated rather than string-compared raw — JS `$` matches before a single trailing newline, so a raw compare would admit exactly the newline vector the guard exists to reject. +- L16 — No GNU-only tooling anywhere: the checker is pure Node stdlib (Node 24, zero runtime deps) and the npm wiring is a portable `&&` chain, matching the existing `check:markers` shape. +- L18 — The exclusion block below is a real markdown `###` **heading**, not a bold prose intro, so `set-writes-scope.cjs --from-plan` terminates the authorized list structurally rather than depending on prose vocabulary. +- L19 — Stated rather than pretended-gated: the `prettier` / `markdownlint-cli2` passes this stage runs are **Bash** writes and therefore pass **neither** fix #2 nor fix #7. They are declared here as a known, accepted escape scoped to this stage's own artifact — never a repo-wide sweep. +- L20 — This lesson **decides the mechanism**. A hardcoded `pharn-2.5.1` badge plus "remember to bump it when you bump `SKILLS_VERSION`" is a discipline-only remedy, and L20's rule is that such a remedy WILL recur and that the trigger to escalate it to a floor check has been met. The badge has silently survived the entire `1.x → 2.5.1` run of bumps, which is the recurrence evidence. Hence mechanism **(A)** below — and hence the CI wiring gets a **pin test** rather than a note, since "remember to add the CI step" would reproduce the same anti-pattern one level down. + +## Files + +- `.dev/floor/check-version-badge.mjs` — NEW. Apparatus checker: locate the README version badge by its shields URL pattern, assert its value equals `SKILLS_VERSION`. — layer: none (`.dev/` apparatus) +- `.dev/floor/check-version-badge.test.mjs` — NEW. Mutant-driven tests: green on agreement, RED on drift, clean RED on absent/malformed/duplicate badge, plus a ✧ pin that `package.json` and `ci.yml` actually invoke it. — layer: none (apparatus test, never ships) +- `README.md` — EDIT line 13 only: `version-1.0.0-blue` → `pharn-2.5.1-blue` (renders `pharn | 2.5.1`). The `1.0.0` foundation note at `:25` is accurate and stays untouched. — layer: none (repo-meta) +- `CHANGELOG.md` — EDIT the header line 5 (ambiguous single-version sentence → explicit two-track statement naming `SKILLS_VERSION` as the version of the **product surface**, the bytes under `pharn/` an install receives) + ADD an `[Unreleased]` entry carrying **no** version line. — layer: none (repo-meta) +- `package.json` — ADD `check:badge` script; fold it into the `check` chain. — layer: none (repo-meta) +- `.github/workflows/ci.yml` — ADD a `Version badge check` step running `npm run check:badge`, carrying the same install-gated `if:` as its siblings. — layer: none (repo-meta/CI) +- `CLAUDE.md` — EDIT: document the new checker in the Commands block, and correct `:215`'s `npm run check` component list (which is **already** stale — it omits the live `check:markers`). — layer: none (repo-meta) + +### Deliberately NOT in scope + +- `SKILLS_VERSION` — **untouched.** Every file above is repo-meta or apparatus; none is product surface, so the versioning discipline's bump-triggering set is not entered. No bump, and the CHANGELOG entry carries no version line. +- `package.json`'s `version: "1.0.0"` — the foundation tag itself. Reconciling it (a release-tagging policy) is a separate axis and a separate increment. +- The other six README badges (License, CI, CodeQL, Floor, Secrets, Built for Claude Code) — no demonstrated drift; pinning them would be the P7 speculative addition (L3). +- `/pharn-dev-verify`'s gate map — untouched, per L9/L11 above. +- **F8** (`package.json "private": true`) and **F12** (the `gitleaks.yml` `validate.mjs` comment) — each its own tiny change. +- The product surface (`pharn/**`, `.claude/commands/pharn-*`) — nothing here ships to a user's install. + +## Contracts satisfied + +- None. This increment adds no capability and no `pharn-contracts` shape — it is a build-apparatus checker plus repo-meta prose. Cited for completeness per P4: the finding vocabulary it prints follows `pharn/pharn-contracts/finding-shape.md`'s enum-gated / free-text split, restated nowhere. + +## Evals to write (P1) + +- **N/A — no Capability is added.** P1 binds `role:`-bearing capabilities to `evals/cases/*` + `evals/expected/*`; a `.dev/floor/*.mjs` checker is not one, and the existing dev-floor checkers carry `*.test.mjs` suites instead. This increment ships that equivalent: `check-version-badge.test.mjs`, written mutant-first per L4 (an authored fixture passes by construction, so the tests drive the RED paths). +- Test roster (each a fixture tree in `os.tmpdir()`, badge located **by pattern** in the fixture too, so the tests survive fixture line shifts): + - agreement → exit 0 (`pharn-2.5.1` badge vs `SKILLS_VERSION` `2.5.1`) + - ✧ **drift mutant** → exit 1 (badge `1.0.0` vs `SKILLS_VERSION` `2.5.1`) — the whole point of the checker + - ✧ badge absent → exit 1, named message, **no crash/stack** + - ✧ badge URL malformed (no `pharn--` match) → exit 1, clean + - ✧ **two** `pharn-` badges → exit 1 (ambiguity is RED, never first-match-wins — L6) + - ✧ the anchor is the **badge** URL, not any `pharn-` substring: a fixture whose prose says `pharn-9.9.9` outside an `img.shields.io/badge/` URL must NOT be picked up + - ✧ `SKILLS_VERSION` missing / blank / multi-line / control-char-bearing → exit 1, clean (L14 guard-before-regex) + - ✧ pin: `package.json` `check:badge` runs the checker **and** `check` runs `check:badge` + - ✧ pin: `ci.yml` has a step whose `run:` is `npm run check:badge`, carrying the sibling install-gated `if:` (the docs:check pin precedent — a step disabled by `if: false` is a dead guard) + - real-repo case: the checker run against this repo exits 0 after the README edit + +## Guarantee audit (P0) + +- "The README version badge's value equals `SKILLS_VERSION`" → **FLOOR: enum/regex** (primitive #3). A deterministic string comparison between a pattern-extracted token and a shape-validated file read. Zero LLM. +- "The badge cannot silently drift again" → **FLOOR, and NARROWED.** It cannot drift _undetected by `npm run check` or CI_. It can still drift in a working tree until a gate runs, and the guarantee is only as live as the wiring — which is why the wiring itself is pinned by tests rather than trusted. +- "CI runs this check" → **FLOOR within the repo** (the ✧ `ci.yml` pin asserts the step exists and is not `if:`-disabled) — but the **honest residual** is the same one the `docs:check` pin already records: that GitHub _executed_ the job, that the workflow is enabled, and that branch protection requires it are **harness-layer** facts unverifiable from inside the repo. "The wiring is pinned" NEVER means "CI is guaranteed to run it". +- "The front page's version story is now legible/coherent" → **ADVISORY.** A checker compares two strings; it has no opinion on whether a human reads the two tracks correctly. The CHANGELOG header rewrite and the badge relabel are prose judgment, reviewed by a human, gated by nothing. +- "`SKILLS_VERSION` is the product-surface version and `1.0.0` is the foundation tag" → **ADVISORY.** This is a documented convention. No floor op binds `package.json`'s version to anything, and none is added here. +- "Every file this increment touches is repo-meta or apparatus, so no bump is owed" → **ADVISORY.** The bump-triggering set is a `CLAUDE.md` convention; nothing on the floor checks a bump against a diff. Stated as a reasoned call, not a verified one. +- "The badge value is read from a structured location" → **STRUCK.** It is not. It is extracted from README prose by URL pattern. See L6 above; the checker's header says so rather than implying otherwise. + +## Trust audit (P2) + +- **Inputs.** `README.md` and `SKILLS_VERSION` are in-repo, human-authored, committed files — trusted in the ordinary sense, but the checker treats their contents as **DATA** regardless and never as instructions. +- **Taint handling.** The extracted badge value and the trimmed, shape-validated `SKILLS_VERSION` scalar are compared with JavaScript string equality (`===`), never interpreted beyond the scalar/shape guards. Both pass a control-char-free single-line-scalar guard _before_ any anchored regex (L14), so a crafted README cannot launder a newline or control sequence into the comparison or into the printed finding. +- **Output.** The checker prints the two values inside a quoted finding message. That message is free text and is treated as untrusted DATA by any reader; **no decision anywhere rests on it** — the verdict is the exit code alone. + +## Determinism audit (P5) + +- The verdict is a string equality plus regex membership — a membership test, no classification. +- Ambiguity **fails closed and loudly**: ≠ 1 badge match, an unreadable/blank/multi-line `SKILLS_VERSION`, or a malformed URL is RED with a named reason, never a silent GREEN and never a first-match guess. +- There is no fallback chain that ends in a guess. The terminal state is a RED naming the file and the reason, which hands the decision to the human. + +## Open questions (HALT) — all four RESOLVED at the human gate + +1. **Mechanism** → **(A)**. Hardcoded badge value pinned by a new deterministic checker. (B) a dynamic shields _endpoint_ JSON + a CI job that writes it, and (C) hardcode + a bump-discipline note, were both declined — (C) explicitly as the L20 anti-pattern. +2. **Checker placement** → **`.dev/floor/` (apparatus), NOT `pharn/floor/`.** Raised at the gate as "this is about pharn, not pharn-dev", and resolved to **framing, not relocation**: the thing `SKILLS_VERSION` versions is the product surface (`pharn/`), so the badge and CHANGELOG prose must say so — but the checker reads **this repo's** `README.md`, which no user install has. Grounded live rather than argued: **no file under `pharn/` mentions `SKILLS_VERSION`, `shields`, or a badge**, and there is **no `pharn` CLI** (`package.json` `bin` is undefined), so `pharn status` / `pharn update` are specified, not built. Shipping the checker would bump `SKILLS_VERSION` to 2.6.0, force a `CURRENT-STATE` regeneration (46 → 47), and land an inert checker in user repos that have neither a PHARN README nor a `SKILLS_VERSION` file. +3. **CI wiring** → **add the step AND its pin test.** `.github/workflows/ci.yml` stays in `## Files`: CI was verified live to run each script as its own step and **never** `npm run check`, so `check`-only wiring would leave the checker unrun on every PR while this plan claimed it was gated. +4. **`CLAUDE.md:215`** → **fix both.** `check:markers` (already missing today) and `check:badge` land in the same sentence, rather than shipping a list that is accurate about the new checker and still false about an existing one. +5. **Badge label** → **`pharn-2.5.1`**, rendering `pharn | 2.5.1`. It names the product whose surface the number versions, and is unmistakably not a rival release number beside the `1.0.0` foundation note. The checker's anchor is therefore the **badge URL** `img.shields.io/badge/pharn--`, never a bare `pharn-` substring — a distinction the test roster asserts directly. diff --git a/.dev/features/version-badge-pin/REGRESSION.md b/.dev/features/version-badge-pin/REGRESSION.md new file mode 100644 index 0000000..10c08cf --- /dev/null +++ b/.dev/features/version-badge-pin/REGRESSION.md @@ -0,0 +1,63 @@ +# REGRESSION — version-badge-pin + +Base: `c583c1f11c13cee659b1ed9b259423c8c8467a6d` (working-tree dogfood — `git status --porcelain` was +non-empty, so the base resolves deterministically to `HEAD`, not a merge-base). + +## Partition + +**Inside (9 changed paths)** — the 7 the plan declared, plus this feature's own pipeline artifacts: + +| Path | Kind | +| ------------------------------------------ | ------------------------- | +| `.dev/floor/check-version-badge.mjs` | declared — new checker | +| `.dev/floor/check-version-badge.test.mjs` | declared — new tests | +| `README.md` | declared — badge | +| `CHANGELOG.md` | declared — header + entry | +| `package.json` | declared — `check:badge` | +| `.github/workflows/ci.yml` | declared — CI step | +| `CLAUDE.md` | declared — docs | +| `.dev/features/version-badge-pin/PLAN.md` | stage artifact (exempt) | +| `.dev/features/version-badge-pin/GRILL.md` | stage artifact (exempt) | + +**`escaped: []`** — `check-regress.mjs scope` exited 0. The build did **not** write outside the plan's +`## Files`. The two exemptions are listed in `escape_exempt` and were read rather than assumed: each is +written by its own stage under that stage's own Step-0 writes-scope, which is exactly the false-positive +class `.dev/memory-bank/lessons-learned.md` **L17** documents and **L20** demanded be given a floor +check. That check is doing its job here — this is the designed workflow, not an escape. + +**Outside:** 62 test files, `validate` (whole-repo), and 1 committed eval pair +(`expected-injection-comment.json` ↔ `.dev/features/trust-fence/findings.json`). + +## Gates — base → head + +| Gate | Base | Head | Flip | +| --------------------------------------- | ---- | ---- | ---- | +| `tests` (62 outside files) | 0 | 0 | none | +| `validate` | 0 | 0 | none | +| `structural:expected-injection-comment` | 0 | 0 | none | + +Gate-id sets are identical on both sides, so the comparison is not inconclusive. + +**Style gates deliberately absent from both maps.** The deterministic skip rule ran: `inside` touches no +shared style config (`eslint.config.mjs`, `.prettierrc.json`, `.prettierignore`, +`.markdownlint-cli2.jsonc`). This was checked rather than assumed — `package.json` **is** in `inside`, +and had prettier's config lived there the skip would have been unsound; it does not +(`require("./package.json").prettier` is `undefined`; the config is `.prettierrc.json`, which is in the +rule's list and untouched). Over outside files byte-identical at base and head, a style flip is then +provably impossible. Independently, the full `npm run check` — which does include `format:check`, +`lint`, and `lint:md` — was **exit 0 at the pre-build baseline and exit 0 at HEAD**, so the style +dimension is covered by direct observation as well as by the skip's argument. + +## Verdict + +**REGRESSIONS: none — no deterministically-detectable breakage outside the feature.** +`check-regress.mjs verdict` exit **0**, `"verdict": "no-regressions"`, `regressions: []`, +`pre_existing: []`. + +The verdict is floor-grade: it rests entirely on the helper comparing two exit-code maps, never on +judgment. Everything around it — choosing the base, partitioning, running the suite — is advisory +command-layer work. + +**The honest residual (P0/P7):** this catches **exactly what the suite catches, and nothing more**. A +regression no deterministic check covers is invisible to it. The claim is "deterministically-detectable +breakage outside the feature is caught," **not** "nothing broke." diff --git a/.dev/features/version-badge-pin/REVIEW.md b/.dev/features/version-badge-pin/REVIEW.md new file mode 100644 index 0000000..7b4e07b --- /dev/null +++ b/.dev/features/version-badge-pin/REVIEW.md @@ -0,0 +1,141 @@ +# REVIEW — version-badge-pin + +**Floor first (P0):** `node pharn/floor/validate.mjs .` → **GREEN**, 36 capabilities, exit 0. The +increment was eligible for review. Everything below the floor line is **advisory**. + +The increment under review is `trust: untrusted`. Nothing in it read as an instruction to this reviewer; +the checker's header comments are descriptive prose about its own guarantees, which is what they should +be. No instruction-looking content changed this reviewer's behaviour. + +--- + +## Floor-gate findings (blocking) + +**None.** No guarantee is claimed without a floor reduction or an `advisory` label, no eval binding is +missing (none is owed — see L-eval), and no sibling reference exists. + +--- + +## Advisory-gate findings + +### L-trust → P2 + +```yaml +- type: FINDING + rule_id: "P2" + severity: important + file: ".dev/floor/check-version-badge.mjs:150" + problem: "Untrusted README text reaches operator stdout unsanitised on the ENUM_ERROR path — a badge value that FAILS the clean-scalar guard is then interpolated raw into the printed message, so control characters the guard exists to reject are emitted to the terminal anyway." + evidence: '`if (!isCleanScalar(badge) || !VERSION_RE.test(badge)) { return finding("ENUM_ERROR", README_PATH, `badge value "${badge}" is not a .. version`); }` — the short-circuit means the raw value is printed precisely in the case where it was found unclean.' +``` + +**Reproduced live, not inferred.** A README containing +`img.shields.io/badge/pharn-[31mPWNED[0m-blue` was fed to the checker; `od -c` on its stdout +shows `033 [ 3 1 m` — the escape reaches the terminal. It is reachable because the extraction class +`[^-\s)]+` excludes whitespace but **not** other control characters, which is exactly the gap +`isCleanScalar` was composed in to close; the message then reintroduces it. + +**Bounded, and the bound matters.** The **verdict is unaffected** — the exit code is `1` either way, so +no guaranteed decision rests on the tainted value, and `check-verify.mjs` never sees it. This is a +free-text rendering defect, not a gate bypass. But "free text is rendered as quoted DATA, never emitted +raw" is the discipline this repo applies to every finding it produces (`pharn/ARCHITECTURE.md §8`, +fix #1), and a checker in a repo whose thesis is untrusted-input handling should not be the exception. + +**Remedy** (not applied — `/pharn-dev-review` writes only this file): print a sanitised rendering on that +path, e.g. escape or elide non-printing characters before interpolation, and add a mutant test asserting +no `\x1b` survives into stdout. Small, and it belongs in this increment rather than a follow-up, because +the file is new and the defect ships with it. + +### L-floor → P0 + +```yaml +- type: FINDING + rule_id: "P0" + severity: minor + file: "CHANGELOG.md:11" + problem: "The entry's headline sentence claims a stronger guarantee than the guarantee audit supports, and it sits in the most scannable position in the entry." + evidence: '"and a checker now makes that impossible to repeat silently" — but the checker''s own header correctly narrows this to "cannot drift UNDETECTED by npm run check or CI"; a working tree can still drift until a gate runs, and the CI half rests on a pin that cannot prove GitHub executed the job.' +``` + +**Mitigated but worth recording.** The same entry states the narrowing explicitly three paragraphs +later ("What it does not buy, stated rather than implied"), so the entry as a whole is honest — but a +reader who scans only the bolded lead gets the unqualified claim. "Impossible" is the register P0 warns +about. **Remedy:** soften the headline to match the audit (e.g. "and a checker now REDs the gate the +moment it drifts"), or move the narrowing earlier. + +### L-eval → P1 + +**No finding, and the floor agrees.** No `role:`-bearing Capability is added, so no `evals/cases/*` + +`evals/expected/*` pair is owed and no `rule_id` binding exists to check; `validate.mjs` independently +reports the same 36 capabilities as before, confirming the lens and the floor do not disagree. The +substitute the plan named — a `*.test.mjs` suite — shipped: 27 tests, mutant-first, of which the +load-bearing ones assert the checker **fails** when its guard is broken rather than passes when all is +well (L4). + +### L-axis → P3 + +**No finding.** `check-version-badge.mjs` has one reason to change (how the badge is located and +compared); its test file has one (what is asserted about that). No `reads:` entry and no prose reference +crosses a sibling module root — the checker is `.dev/` apparatus and references no `pharn-*` module at +all. + +--- + +## Proposed lesson candidate (NOT written to canon here) + +`/pharn-dev-review` declares no `.dev/memory-bank/**` path and cannot write canon. This is a **proposal** +for a separate, human-gated `/pharn-dev-memory-promote` run. + +**Candidate — a promoted lesson's stated MECHANISM can be false while its REMEDY is sound; assert the +mechanism in a test rather than inheriting it.** + +`.dev/memory-bank/lessons-learned.md` **L14** (promoted 2026-07-09, human-approved) states that +JavaScript `$` without the `m` flag "matches at end-of-string OR just before a single trailing newline, +so `/^P[0-7]$/.test('P2\n') === true`". Verified live on Node v24.13.1 this run: that expression is +**`false`**. `$` without `m` matches only at end of input in JavaScript; the described behaviour is +Perl/Python/PCRE and requires the `m` flag here. + +**Why it matters.** L14's remedy — compose the clean-scalar guard _before_ the shape regex, never +replace it — is correct and this increment follows it. But its stated _reason_ is wrong, so anyone +applying L14 by reasoning from its mechanism reaches a wrong model of what the guard buys. This +increment did exactly that in its first draft: the checker's header asserted a trailing-newline hole +that does not exist, and the plan's own `applied_lessons` line for L14 repeated it. It surfaced **only** +because the test suite asserted L14's example as an explicit precondition and the assertion **failed**. +Had the test merely asserted `isCleanScalar("2.5.1\n") === false` — true, and passing — the false claim +would have shipped with a green suite. + +It is the P0 disease aimed at canon itself: "the lesson is promoted and human-approved" read as +"therefore its technical claims are true." It complements **L20** (a lesson whose only remedy is +discipline will recur) along a different axis — L20 concerns a remedy that is the wrong _kind_, this one +a rationale that is simply _false_ while the remedy stays right, which is harder to notice precisely +because following the lesson still produces correct code. + +**Provenance:** feature `version-badge-pin`; base commit `c583c1f`; source +`.dev/features/version-badge-pin/VERIFY.md` + the failing-then-corrected assertion now pinned in +`.dev/floor/check-version-badge.test.mjs` ("✧ CANON DEFECT: JS `$` does NOT match before a trailing +newline"). Reproduced live before and after. **Note the promotion should also correct L14 itself** — +that is a canon edit and belongs to the same gated run, not to this one. + +--- + +## Also surfaced, deliberately out of scope + +`.claude/commands/pharn-dev-verify.md:100` asserts its gate set is "exactly the repo's `npm run check` +aggregate"; live it is a strict subset (`docs:check` and `check:markers` were already outside it, and +`check:badge` is now a third). Raised at `/pharn-dev-grill`, and **not** fixed here: the file is not in the +approved plan's `## Files`, so writing it would mean widening an approved scope mid-run, and the defect +predates this increment on a different axis. Follow-up: `verify-gate-map-claim`. + +--- + +## Verdict + +**GREEN on the floor — 0 blocking floor-gate findings.** Two advisory findings stand: one **important** +(P2 — untrusted text reaching stdout raw, reproduced live, verdict unaffected) and one **minor** (P0 — +a headline claim stronger than its own audit). + +Stated honestly: this review is **advisory**. `validate.mjs` GREEN is the only guaranteed content in it; +the severities above are this reviewer's assignment, not a deterministic verdict, and neither finding +blocks — the human decides at the post-review gate whether the P2 finding is fixed in this increment or +deferred. My own recommendation is to fix it here, because the file is new and would ship with the +defect. diff --git a/.dev/features/version-badge-pin/SHIP.md b/.dev/features/version-badge-pin/SHIP.md new file mode 100644 index 0000000..bc0d9a3 --- /dev/null +++ b/.dev/features/version-badge-pin/SHIP.md @@ -0,0 +1,67 @@ +# SHIP — version-badge-pin + +An **advisory** roll-up of a `/pharn-dev-ship` run. It records that the chain ran and what each stage's +floor verdict was. It is not an approval, not a "shipped", and not a `PHARN ✓ reviewed` seal. + +## Stages, in order + +| # | Stage | Outcome | +| --- | -------------------- | --------------------------------------------------- | +| 1 | `/pharn-dev-plan` | `PLAN.md` written → **GATE 1**, human approved | +| 2 | `/pharn-dev-grill` | `GRILL.md` — 4 concerns, 0 blocking (gates nothing) | +| 3 | `/pharn-dev-build` | 7 files written; floor GREEN | +| 4 | `/pharn-dev-regress` | `no-regressions` | +| 5 | `/pharn-dev-verify` | `PASS` | +| 6 | `/pharn-dev-review` | `REVIEW.md` — 0 floor-gate, 2 advisory | + +**Where the run ended: GATE 2.** No stage returned a non-GREEN verdict, so the chain reached the +post-review human gate rather than a RED-verdict stop. + +## Structural verdicts read, verbatim + +- **`/pharn-dev-build`** → `node pharn/floor/validate.mjs .` exit **0** (`FLOOR: GREEN — 36 capabilities +checked in .`). The two build gates also passed first: the `pharn/ARCHITECTURE.md` content-hash + recomputed to `8f5ec002e3b18cbfd2f094b08a3671f7ed42a05a3fbaf01a11bbbd28da30fb52`, equal to the plan's + pin (fix #4, no drift), and the plan carried no unresolved open questions. +- **`/pharn-dev-regress`** → `regression-report.json` `.verdict` = **`"no-regressions"`**, exit 0. + `regressions: []`, `pre_existing: []`. Outside gates `tests` / `validate` / + `structural:expected-injection-comment` were 0 → 0. `check-regress.mjs scope` exit 0 with + **`escaped: []`** — the build did not write outside the plan's `## Files`. +- **`/pharn-dev-verify`** → `verify-report.json` `.verdict` = **`"PASS"`**, exit 0. `failing_gates: []`. + All six gates 0: `test`, `validate`, `lint`, `format:check`, `lint:md`, + `structural:expected-injection-comment.json`. `verifiers: { registered: 0 }` — floor gates only, and + the advisory verifier layer is empty by design (P7), so nothing advisory could have influenced this + number even in principle. + +## Artifacts (cited, not restated — P4) + +- `.dev/features/version-badge-pin/PLAN.md` — the approved intent, `applied_lessons` floor-GREEN across 13 cited ids +- `.dev/features/version-badge-pin/GRILL.md` — advisory; 2 of its findings were folded into the build before the first write +- `.dev/features/version-badge-pin/REGRESSION.md` + `regression-report.json` +- `.dev/features/version-badge-pin/VERIFY.md` + `verify-report.json` +- `.dev/features/version-badge-pin/REVIEW.md` — **read this before deciding**; it carries two advisory findings and a proposed canon lesson + +## What the human is being asked to decide + +`REVIEW.md` stands at **0 floor-gate findings** and **2 advisory** ones. The reviewer's own +recommendation, recorded there, is that the **important** P2 finding +(`.dev/floor/check-version-badge.mjs` prints an unsanitised badge value on the `ENUM_ERROR` path — +reproduced live, verdict unaffected) be fixed **in this increment** rather than deferred, because the +file is new and would ship carrying it. The **minor** P0 finding is a CHANGELOG headline stronger than +its own guarantee audit. + +Two things were found and deliberately **not** fixed, each recorded rather than smuggled in: +`.claude/commands/pharn-dev-verify.md:100`'s subset-vs-aggregate claim (follow-up +`verify-gate-map-claim` — not in the approved `## Files`), and a factual error in canon lesson **L14** +about JavaScript's `$` anchor (proposed for a gated `/pharn-dev-memory-promote` run; canon is never +written by a build or a review). + +`SKILLS_VERSION` is **untouched at 2.5.1** and no CHANGELOG version entry was added — every path this +increment writes is repo-meta or build apparatus. + +## The honest line + +The chain ran; the named floor verdicts are as shown. **This is NOT a judgment that the increment is +good or wise** — that is the human's call at the post-review gate. `/pharn-dev-ship` added no floor +primitive of its own: every guarantee above belongs to a sub-stage's own checker, and the act of running +the stages in order and reading their verdicts is advisory command-layer work. diff --git a/.dev/features/version-badge-pin/VERIFY.md b/.dev/features/version-badge-pin/VERIFY.md new file mode 100644 index 0000000..d3c3427 --- /dev/null +++ b/.dev/features/version-badge-pin/VERIFY.md @@ -0,0 +1,65 @@ +# VERIFY — version-badge-pin + +## FLOOR layer — the gates that own the verdict + +| Gate | Exit | +| -------------------------------------------- | ---- | +| `test` (1329 tests, incl. 27 new) | 0 | +| `validate` (36 capabilities) | 0 | +| `lint` | 0 | +| `format:check` | 0 | +| `lint:md` | 0 | +| `structural:expected-injection-comment.json` | 0 | + +`node pharn/floor/check-verify.mjs` exit **0**. + +**VERIFIED: floor gates PASS.** + +## ADVISORY layer — verifiers + +`node pharn/floor/count-verifiers.mjs .` → `{"registered":0,"verifiers":[]}`. + +**No verifiers registered — floor gates only.** Membership is a deterministic frontmatter read, not a +content grep, so the several files in this feature whose prose mentions verifiers (this artifact +included) correctly register as zero. + +## Feature-specific evidence, beyond the whole-repo gates + +The whole-repo gates answer "is the repo green with this in it". The feature-specific signal is the 27 +new tests collected by `npm test`, of which the load-bearing ones are mutants — each asserts the checker +**fails** when the thing it guards is broken: + +- **the drift case itself** — badge `1.0.0` against `SKILLS_VERSION` `2.5.1` → exit 1, naming both values +- ambiguity (two `pharn` badges) → RED even when the _first_ badge is correct — never first-match-wins +- a pre-release `SKILLS_VERSION` → a named refusal explaining the shields `--` encoding, not a + near-identical-looking mismatch +- both inputs broken → the `SKILLS_VERSION` refusal wins, deterministically +- prose containing `pharn-9.9.9` outside a shields URL → **not** picked up as a badge +- the two wiring pins: `package.json` runs the checker and `check` runs `check:badge`; `ci.yml` has a + step whose `run:` is `npm run check:badge`, carrying the install-gated `if:` + +Additionally verified **outside** the gate map, by running the checker against a throwaway copy of the +repo with `SKILLS_VERSION` bumped to `2.6.0` and the badge left behind: exit **1**, `[DRIFT]`. The real +`SKILLS_VERSION` was not touched. That is the end-to-end proof that the gate fires on the actual failure +mode rather than only on synthesised fixtures. + +## A defect found during this increment, in canon, not fixed here + +`.dev/memory-bank/lessons-learned.md` **L14** states that JavaScript `$` without the `m` flag "matches at +end-of-string OR just before a single trailing newline, so `/^P[0-7]$/.test('P2\n') === true`". Verified +live on Node v24.13.1: that expression is **`false`**. `$` without `m` matches only at end of input in +JavaScript; the behaviour L14 describes is Perl/Python/PCRE and needs the `m` flag here. + +It surfaced because the new test suite asserted L14's mechanism as a **precondition** rather than +assuming it, and the assertion failed. L14's **remedy** (compose the clean-scalar guard before the shape +regex, never replace it) is sound and this checker follows it; only its stated **reason** is wrong. The +checker's own header and a dedicated test now record the correction so the false claim is not propagated +into a new file. Canon is edited only through a gated promotion with human approval, and +`.dev/memory-bank/` is not in this increment's `## Files`, so this reports rather than fixes. + +## The honest residual + +**Verified = the named gates passed.** This is **not** a guarantee of correctness beyond what those +gates check — a defect no test, eval, rule, or lint covers is invisible to this verdict, and the +verifier layer that might have noticed it is advisory and currently empty. Verifier concerns, when they +exist, are advisory help, not assurance. diff --git a/.dev/features/version-badge-pin/regression-report.json b/.dev/features/version-badge-pin/regression-report.json new file mode 100644 index 0000000..2edffe1 --- /dev/null +++ b/.dev/features/version-badge-pin/regression-report.json @@ -0,0 +1,31 @@ +{ + "base": "c583c1f11c13cee659b1ed9b259423c8c8467a6d", + "inside": [ + ".dev/features/version-badge-pin/GRILL.md", + ".dev/features/version-badge-pin/PLAN.md", + ".dev/floor/check-version-badge.mjs", + ".dev/floor/check-version-badge.test.mjs", + ".github/workflows/ci.yml", + "CHANGELOG.md", + "CLAUDE.md", + "README.md", + "package.json" + ], + "outside_gates": { + "structural:expected-injection-comment": { + "base": 0, + "head": 0 + }, + "tests": { + "base": 0, + "head": 0 + }, + "validate": { + "base": 0, + "head": 0 + } + }, + "regressions": [], + "pre_existing": [], + "verdict": "no-regressions" +} diff --git a/.dev/features/version-badge-pin/verify-report.json b/.dev/features/version-badge-pin/verify-report.json new file mode 100644 index 0000000..c3eba13 --- /dev/null +++ b/.dev/features/version-badge-pin/verify-report.json @@ -0,0 +1,14 @@ +{ + "feature": "version-badge-pin", + "gates": { + "format:check": 0, + "lint": 0, + "lint:md": 0, + "structural:expected-injection-comment.json": 0, + "test": 0, + "validate": 0 + }, + "verdict": "PASS", + "failing_gates": [], + "verifiers": { "registered": 0, "findings": [] } +} diff --git a/.dev/floor/check-version-badge.mjs b/.dev/floor/check-version-badge.mjs new file mode 100644 index 0000000..625c31c --- /dev/null +++ b/.dev/floor/check-version-badge.mjs @@ -0,0 +1,208 @@ +#!/usr/bin/env node +// .dev/floor/check-version-badge.mjs — the README version-badge DRIFT CHECKER (build apparatus). +// +// The GUARANTEE (P0, ARCHITECTURE §2 primitive #3 — enum/regex): the value extracted from the README's +// shields version badge equals the trimmed, shape-validated SKILLS_VERSION scalar. Both sides are read live, +// the badge value is extracted by URL pattern, and the comparison is JavaScript string equality (`===`). +// ZERO LLM. +// +// - MISSING_VERSION : SKILLS_VERSION is absent or unreadable +// - ENUM_ERROR : SKILLS_VERSION or the badge value is not a clean `..` scalar +// - UNSUPPORTED : SKILLS_VERSION carries a `-` (see "the hyphen" below) +// - MISSING_README : README.md is absent or unreadable +// - NO_BADGE : no `img.shields.io/badge/pharn--` badge in README.md +// - AMBIGUOUS : MORE THAN ONE such badge — a set with 2 members is not a value +// - DRIFT : the badge value and SKILLS_VERSION disagree — the defect this file exists for +// Any of these → RED (exit 1). Clean → GREEN (exit 0). Fail-closed throughout: no input state returns +// GREEN by default, and ambiguity is a RED rather than a first-match guess (P5). +// +// WHY THIS EXISTS AT ALL (the trigger, P7 — not a hypothetical): the badge read `version-1.0.0` while +// SKILLS_VERSION had reached `2.5.1`. It sat in the README's UNGUARDED prose — outside the +// CURRENT-STATE markers that check-capability-catalog holds to byte-equality — so no gate could see it, +// and it survived the entire 1.x → 2.5.x run of bumps. Per lessons-learned L20, a defect whose only +// remedy is "remember to update it" has already demonstrated that discipline is the wrong kind of +// remedy; the second occurrence is the trigger to give it a floor check. This is that check. +// +// WHAT THIS DOES NOT GUARANTEE (P0 — say it, don't bury it): +// - NOT that the README's version story is COHERENT. This compares two strings. Whether a reader can +// tell the product-surface version from package.json's 1.0.0 foundation tag is prose judgment, +// reviewed by a human, gated by nothing. +// - NOT that SKILLS_VERSION is CORRECT. If a bump is wrong or missing, a badge matching it is still +// GREEN. The guarantee is agreement, not truth. +// - NOT read from a STRUCTURED location. Lessons-learned L6 says a membership fact is read from its +// structured location, never grepped from free text. A README badge HAS no structured location — +// it is prose, which is precisely why it drifted. This narrows rather than claims: the anchor is +// the shields URL (a structured token WITHIN prose), never a line number and never a bare version +// substring, and >1 match is RED instead of first-match-wins. Stated, not implied. +// - NOT that this checker RUNS. It guards nothing unless something invokes it; the package.json and +// ci.yml wiring is pinned separately by check-version-badge.test.mjs. +// +// THE HYPHEN, and why it is a refusal rather than a comparison. Shields encodes a literal `-` in a +// badge message as `--`, so a pre-release SKILLS_VERSION such as `2.6.0-rc.1` cannot round-trip through +// the badge URL this checker reads: the anchor would extract `2.6.0` and report a mismatch against +// `2.6.0-rc.1` — two values that look almost equal, which is the least legible possible red. The +// checker therefore REFUSES such a version by name. The direction was always fail-closed; this makes +// the reason readable. (Raised at /pharn-dev-grill; folded in before the first build.) +// +// PRECEDENCE is deterministic, not incidental: SKILLS_VERSION is read and validated FIRST, so when both +// inputs are broken its refusal is the one reported. Two REDs must not race. +// +// Usage: node .dev/floor/check-version-badge.mjs [targetDir] (default: cwd) +// Non-LLM, stdlib-only, fail-closed. Apparatus: never ships to a user install, so no SKILLS_VERSION bump. + +import { readFileSync, existsSync, statSync } from "node:fs"; +import { join } from "node:path"; + +/** The file that owns the version. Single source of truth; the badge is a rendering of it. */ +export const VERSION_PATH = "SKILLS_VERSION"; +/** The document whose badge must agree with it. */ +export const README_PATH = "README.md"; +/** The badge's shields label — also the checker's anchor token. */ +export const BADGE_LABEL = "pharn"; +/** + * The anchor: a shields BADGE url with the `pharn` label, capturing the message up to the next `-`. + * Deliberately NOT global here — a shared /g regex carries `lastIndex` between calls; the global copy + * is built per call in findBadgeValues(). + */ +export const BADGE_RE = /img\.shields\.io\/badge\/pharn-([^-\s)]+)-/; +/** The accepted version shape. Strict three-part semver core; see "THE HYPHEN" above for pre-releases. */ +export const VERSION_RE = /^\d+\.\d+\.\d+$/; +/** Upper bound on either scalar. Generous for a version; bounds a pathological single-line input. */ +export const MAX_LEN = 64; + +/** + * True iff `v` is a non-empty, length-bounded string containing NO control characters. + * + * This is the PRECONDITION, never the replacement, for the anchored shape regexes below — the + * compose-don't-replace discipline of lessons-learned L14, which this file follows. + * + * L14's REMEDY is right; its stated MECHANISM is not, and repeating it here would propagate a false + * claim into a new file. L14 says "JavaScript `$` (without the `m` flag) matches at end-of-string OR + * just before a single trailing newline, so `/^P[0-7]$/.test('P2\n') === true`". Verified live on Node + * v24.13.1: that expression is **false**. `$` without `m` matches ONLY at end of input in JavaScript; + * matching before a trailing newline is Perl/Python/PCRE behaviour, and in JS it needs the `m` flag. + * The trailing-newline hole L14 describes does not exist here. Flagged for a human — canon is edited + * only through a gated promotion, never by a build. + * + * So what this guard ACTUALLY buys, stated honestly rather than inherited: + * - a LENGTH BOUND. VERSION_RE has no upper bound, so a million-digit input matches its shape; this + * is the only thing that stops it. This is the load-bearing half. + * - a TYPE check, so a non-string can never reach `.test()`. + * - defence in depth on the BADGE value, whose extraction class `[^-\s)]+` excludes whitespace but + * NOT other control characters. VERSION_RE would also reject those, so the guarantee does not + * depend on this — but it also does not depend on VERSION_RE's exact character class staying as + * it is, which is the point of layering. + * Char-code scanning rather than a regex keeps the guard readable and avoids a control-character class + * in source. + */ +export function isCleanScalar(v, max = MAX_LEN) { + if (typeof v !== "string" || v.length === 0 || v.length > max) return false; + for (let i = 0; i < v.length; i++) { + const c = v.charCodeAt(i); + if (c < 0x20 || c === 0x7f) return false; + } + return true; +} + +/** Every badge value in `readme`, in document order. A fresh /g regex per call — no shared lastIndex. */ +export function findBadgeValues(readme) { + return [...readme.matchAll(new RegExp(BADGE_RE.source, "g"))].map((m) => m[1]); +} + +const finding = (type, file, problem) => ({ ok: false, findings: [{ type, file, problem }] }); + +/** + * Compare the README badge against SKILLS_VERSION. Returns { ok, findings, version, badge }. + * Pure — no process exit — so tests can call it directly. + */ +export function checkVersionBadge(targetDir) { + // ── 1. SKILLS_VERSION first (deterministic precedence: its refusal wins over a README one) ────── + let rawVersion; + try { + rawVersion = readFileSync(join(targetDir, VERSION_PATH), "utf8"); + } catch { + return finding("MISSING_VERSION", VERSION_PATH, "the version file is absent or unreadable"); + } + const version = rawVersion.trim(); + if (!isCleanScalar(version)) { + return finding( + "ENUM_ERROR", + VERSION_PATH, + `contents are not a clean single-line scalar (empty, over ${MAX_LEN} chars, multi-line, or control-character-bearing)` + ); + } + if (version.includes("-")) { + return finding( + "UNSUPPORTED", + VERSION_PATH, + `${JSON.stringify(version)} contains a hyphen; a shields badge message encodes a literal "-" as "--", so this version cannot round-trip through the badge URL — the badge would have to be built and read differently before a pre-release version is supported` + ); + } + if (!VERSION_RE.test(version)) { + return finding("ENUM_ERROR", VERSION_PATH, `${JSON.stringify(version)} is not a .. version`); + } + + // ── 2. The README badge ───────────────────────────────────────────────────────────────────────── + let readme; + try { + readme = readFileSync(join(targetDir, README_PATH), "utf8"); + } catch { + return finding("MISSING_README", README_PATH, "the README is absent or unreadable"); + } + const values = findBadgeValues(readme); + if (values.length === 0) { + return finding("NO_BADGE", README_PATH, `no "${BADGE_LABEL}" version badge found (expected a shields URL matching ${BADGE_RE.source})`); + } + if (values.length > 1) { + return finding( + "AMBIGUOUS", + README_PATH, + `${values.length} "${BADGE_LABEL}" version badges found (${values.map((v) => JSON.stringify(v)).join(", ")}); exactly one is required — a set with two members is not a value` + ); + } + const badge = values[0]; + if (!isCleanScalar(badge) || !VERSION_RE.test(badge)) { + return finding("ENUM_ERROR", README_PATH, `badge value ${JSON.stringify(badge)} is not a .. version`); + } + if (badge !== version) { + return { + ok: false, + version, + badge, + findings: [ + { + type: "DRIFT", + file: README_PATH, + problem: `the badge reads ${JSON.stringify(badge)} but ${VERSION_PATH} is ${JSON.stringify(version)}`, + }, + ], + }; + } + return { ok: true, findings: [], version, badge }; +} + +function main() { + const target = process.argv[2] || "."; + // Fail-closed (P5): a missing / non-directory target is an error, never a silent GREEN. + if (!existsSync(target) || !statSync(target).isDirectory()) { + process.stderr.write(`check-version-badge: target dir not found (or not a directory): ${target}\n`); + process.exit(1); + } + const { ok, findings, version, badge } = checkVersionBadge(target); + if (ok) { + process.stdout.write( + `VERSION-BADGE: GREEN — ${README_PATH} badge ${JSON.stringify(badge)} matches ${VERSION_PATH} ${JSON.stringify(version)}\n` + ); + process.exit(0); + } + process.stdout.write(`VERSION-BADGE: RED — ${findings.length} finding(s)\n`); + for (const f of findings) { + process.stdout.write(`- [${f.type}] ${f.file}\n ${f.problem}\n`); + } + process.stdout.write(`\nFIX: ${VERSION_PATH} is the single source — update the README badge to match it, not the other way round.\n`); + process.exit(1); +} + +if (process.argv[1] && import.meta.url === `file://${process.argv[1]}`) { + main(); +} diff --git a/.dev/floor/check-version-badge.test.mjs b/.dev/floor/check-version-badge.test.mjs new file mode 100644 index 0000000..6bdc86d --- /dev/null +++ b/.dev/floor/check-version-badge.test.mjs @@ -0,0 +1,331 @@ +// .dev/floor/check-version-badge.test.mjs — apparatus tests for the README version-badge checker. +// +// L4: an authored fixture passes by construction. The ✧ cases are therefore MUTANTS — each asserts the +// checker FAILS when the thing it guards is broken, not merely that it passes when everything is fine. +// The drift case is the whole point of the checker and is asserted directly. +// +// Fixtures locate the badge BY PATTERN, never by line number, so they survive fixture line shifts — +// the same discipline the checker itself follows. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { execFileSync } from "node:child_process"; +import { mkdtempSync, writeFileSync, rmSync, readFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join, dirname } from "node:path"; +import { fileURLToPath } from "node:url"; +import { checkVersionBadge, findBadgeValues, isCleanScalar, BADGE_RE, VERSION_RE } from "./check-version-badge.mjs"; + +const HERE = dirname(fileURLToPath(import.meta.url)); +const CHECKER = join(HERE, "check-version-badge.mjs"); +const REPO = join(HERE, "..", ".."); + +/** Run the checker as a child process; never throws. Returns {code, out}. */ +function run(target) { + try { + const out = execFileSync(process.execPath, [CHECKER, target], { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] }); + return { code: 0, out }; + } catch (e) { + return { code: e.status, out: `${e.stdout ?? ""}${e.stderr ?? ""}` }; + } +} + +/** + * Build a throwaway repo. `badge` is the badge VALUE (null = omit the badge line entirely). + * The badge is embedded in a realistic multi-badge block so a fixture never accidentally tests a + * one-line README the real one does not resemble. + */ +function fixture({ version = "2.5.1", badge = "2.5.1", extra = "", omitVersionFile = false, omitReadme = false } = {}) { + const dir = mkdtempSync(join(tmpdir(), "pharn-badge-")); + if (!omitVersionFile) writeFileSync(join(dir, "SKILLS_VERSION"), version); + if (!omitReadme) { + const badgeLine = badge === null ? "" : `[![pharn](https://img.shields.io/badge/pharn-${badge}-blue)](./CHANGELOG.md)\n`; + writeFileSync( + join(dir, "README.md"), + `# PHARN\n\nSome prose about the project.\n\n${badgeLine}` + + `[![License: Apache 2.0](https://img.shields.io/badge/license-Apache%202.0-green)](./LICENSE)\n` + + `[![CI](https://github.com/x/y/actions/workflows/ci.yml/badge.svg)](https://github.com/x/y)\n${extra}\n\nMore prose.\n` + ); + } + return dir; +} + +function withFixture(opts, fn) { + const dir = fixture(opts); + try { + return fn(dir); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +} + +// ── The happy path ──────────────────────────────────────────────────────────────────────────────── + +test("GREEN when the badge value equals SKILLS_VERSION", () => { + withFixture({ version: "2.5.1", badge: "2.5.1" }, (dir) => { + const { code, out } = run(dir); + assert.equal(code, 0, out); + assert.match(out, /VERSION-BADGE: GREEN/); + assert.match(out, /2\.5\.1/); + }); +}); + +test("GREEN survives a trailing newline in SKILLS_VERSION (the normal committed shape)", () => { + withFixture({ version: "2.5.1\n", badge: "2.5.1" }, (dir) => { + assert.equal(run(dir).code, 0); + }); +}); + +// ── ✧ THE MUTANT THIS CHECKER EXISTS FOR ────────────────────────────────────────────────────────── + +test("✧ DRIFT: badge 1.0.0 vs SKILLS_VERSION 2.5.1 → exit 1 (the exact defect, reproduced)", () => { + withFixture({ version: "2.5.1", badge: "1.0.0" }, (dir) => { + const { code, out } = run(dir); + assert.equal(code, 1, "a drifted badge MUST be RED"); + assert.match(out, /\[DRIFT\]/); + assert.match(out, /badge reads "1\.0\.0"/); + assert.match(out, /SKILLS_VERSION is "2\.5\.1"/); + }); +}); + +test("✧ DRIFT is detected on ANY disagreement, not just the historical pair", () => { + withFixture({ version: "3.0.0", badge: "2.9.9" }, (dir) => { + assert.equal(run(dir).code, 1); + }); +}); + +test("✧ the FIX message names SKILLS_VERSION as the source, not the badge", () => { + withFixture({ version: "2.5.1", badge: "1.0.0" }, (dir) => { + assert.match(run(dir).out, /SKILLS_VERSION is the single source/); + }); +}); + +// ── ✧ Clean failures — a RED, never a crash ─────────────────────────────────────────────────────── + +test("✧ badge absent → exit 1, named, and no stack trace", () => { + withFixture({ badge: null }, (dir) => { + const { code, out } = run(dir); + assert.equal(code, 1); + assert.match(out, /\[NO_BADGE\]/); + assert.doesNotMatch(out, /at .*\.mjs:\d+/, "a missing badge must not surface as a thrown stack"); + }); +}); + +test("✧ badge URL malformed (no pharn-- match) → exit 1, clean", () => { + withFixture({ badge: null, extra: "[![pharn](https://img.shields.io/badge/pharn)](./CHANGELOG.md)\n" }, (dir) => { + const { code, out } = run(dir); + assert.equal(code, 1); + assert.match(out, /\[NO_BADGE\]/); + }); +}); + +test("✧ TWO pharn badges → exit 1 AMBIGUOUS, never first-match-wins (L6)", () => { + withFixture({ badge: "2.5.1", extra: "[![pharn](https://img.shields.io/badge/pharn-9.9.9-red)](./x)\n" }, (dir) => { + const { code, out } = run(dir); + assert.equal(code, 1, "ambiguity must be RED even when the FIRST badge is correct"); + assert.match(out, /\[AMBIGUOUS\]/); + assert.match(out, /"2\.5\.1", "9\.9\.9"/); + }); +}); + +test("✧ README absent → exit 1 MISSING_README, clean", () => { + withFixture({ omitReadme: true }, (dir) => { + const { code, out } = run(dir); + assert.equal(code, 1); + assert.match(out, /\[MISSING_README\]/); + }); +}); + +test("✧ target dir does not exist → exit 1, clean", () => { + const { code, out } = run(join(tmpdir(), "pharn-badge-does-not-exist-xyz")); + assert.equal(code, 1); + assert.match(out, /target dir not found/); +}); + +// ── ✧ SKILLS_VERSION guard (L14: the clean-scalar guard PRECEDES the shape regex) ────────────────── + +test("✧ SKILLS_VERSION missing → exit 1 MISSING_VERSION", () => { + withFixture({ omitVersionFile: true }, (dir) => { + const { code, out } = run(dir); + assert.equal(code, 1); + assert.match(out, /\[MISSING_VERSION\]/); + }); +}); + +test("✧ SKILLS_VERSION blank → exit 1 ENUM_ERROR", () => { + withFixture({ version: " \n" }, (dir) => { + const { code, out } = run(dir); + assert.equal(code, 1); + assert.match(out, /\[ENUM_ERROR\]/); + }); +}); + +test("✧ SKILLS_VERSION multi-line → exit 1 ENUM_ERROR (not a silent first-line read)", () => { + withFixture({ version: "2.5.1\n3.0.0\n", badge: "2.5.1" }, (dir) => { + const { code, out } = run(dir); + assert.equal(code, 1, "a two-line version file must NOT quietly match on its first line"); + assert.match(out, /\[ENUM_ERROR\]/); + }); +}); + +test("✧ SKILLS_VERSION bearing a control character → exit 1 ENUM_ERROR", () => { + // The version literal below carries a LITERAL U+0001 between "2.5" and "1" — invisible in a diff, so + // it is called out here. `.trim()` does not strip an EMBEDDED control character, which is precisely + // why isCleanScalar is composed BEFORE the shape regex rather than replaced by it (L14). + withFixture({ version: "2.51", badge: "2.5.1" }, (dir) => { + assert.equal(run(dir).code, 1); + }); +}); + +test("✧ SKILLS_VERSION not .. → exit 1 ENUM_ERROR", () => { + withFixture({ version: "v2.5", badge: "2.5.1" }, (dir) => { + const { code, out } = run(dir); + assert.equal(code, 1); + assert.match(out, /\[ENUM_ERROR\]/); + }); +}); + +test("✧ badge bearing ESC in the URL value is escaped in stdout, never emitted raw (P2)", () => { + // ESC is built with fromCharCode, never written as a literal escape or a raw byte: a raw control byte + // in a source file survives copy/paste and tooling badly (check-plan-spec-agree.test.mjs precedent). + const ESC = String.fromCharCode(27); + const hostile = `${ESC}[31mPWNED${ESC}[0m`; + withFixture({ version: "2.5.1", badge: hostile }, (dir) => { + const { code, out } = run(dir); + assert.equal(code, 1); + assert.match(out, /\[ENUM_ERROR\]/); + assert.ok(!out.includes(ESC), "the ESC must not reach stdout raw"); + assert.match(out, /\\u001b/); // escaped, as JSON.stringify renders it + }); +}); + +// ── ✧ The hyphen: a REFUSAL by name, not a confusing near-equal mismatch (raised at grill) ───────── + +test("✧ a pre-release SKILLS_VERSION is REFUSED by name, not reported as a lookalike mismatch", () => { + withFixture({ version: "2.6.0-rc.1", badge: "2.6.0" }, (dir) => { + const { code, out } = run(dir); + assert.equal(code, 1); + assert.match(out, /\[UNSUPPORTED\]/, "must name the encoding limit, not print 2.6.0 vs 2.6.0-rc.1"); + assert.match(out, /encodes a literal "-" as "--"/); + assert.doesNotMatch(out, /\[DRIFT\]/); + }); +}); + +// ── ✧ Precedence: two simultaneous failures must not race (raised at grill) ──────────────────────── + +test("✧ when BOTH inputs are broken, the SKILLS_VERSION refusal wins deterministically", () => { + withFixture({ version: "nonsense", badge: null }, (dir) => { + const { code, out } = run(dir); + assert.equal(code, 1); + assert.match(out, /\[ENUM_ERROR\]/); + assert.match(out, /SKILLS_VERSION/); + assert.doesNotMatch(out, /\[NO_BADGE\]/, "precedence is defined: the version file is validated first"); + }); +}); + +// ── ✧ The anchor is the BADGE URL, not any pharn- substring ─────────────────────────────────────── + +test("✧ a pharn- string OUTSIDE a shields badge URL is not picked up", () => { + withFixture({ badge: "2.5.1", extra: "Install with `pharn-9.9.9` or see pharn-1.2.3 elsewhere.\n" }, (dir) => { + const { code, out } = run(dir); + assert.equal(code, 0, `prose mentioning pharn-9.9.9 must not register as a badge: ${out}`); + }); +}); + +test("✧ the OTHER shields badges (license, built-for) do not collide with the pharn anchor", () => { + const values = findBadgeValues( + `[![License](https://img.shields.io/badge/license-Apache%202.0-green)](./LICENSE)\n` + + `[![Built for](https://img.shields.io/badge/built%20for-Claude%20Code-555)](https://claude.com)\n` + ); + assert.deepEqual(values, [], "only the pharn-labelled badge may match"); +}); + +// ── ✧ Unit-level guards ─────────────────────────────────────────────────────────────────────────── + +test("✧ CANON DEFECT: JS `$` does NOT match before a trailing newline — lessons-learned L14 says it does", () => { + // This test was written to assert L14's stated mechanism and FAILED, which is how the defect surfaced. + // L14 claims: "JavaScript `$` (without the `m` flag) matches at end-of-string OR just before a single + // trailing newline, so `/^P[0-7]$/.test('P2\n') === true`". It does not. That is Perl/Python/PCRE + // behaviour; in JavaScript `$` without `m` matches ONLY at end of input. + // + // Pinned here so the correction cannot quietly regress, and so a reader of this file is not left + // believing the canon sentence. L14's REMEDY (compose the guard before the shape regex, never + // replace it) is sound and this checker follows it — only its reason is wrong. Canon itself is + // edited only through a gated promotion, so this test reports rather than fixes. + assert.equal(/^P[0-7]$/.test("P2\n"), false, "L14's own example: JS `$` does NOT admit a trailing newline"); + assert.equal(/^P[0-7]$/m.test("P2\n"), true, "it takes the `m` flag to get the behaviour L14 describes"); + assert.equal(VERSION_RE.test("2.5.1\n"), false, "so the shape regex alone already rejects a trailing newline"); +}); + +test("✧ isCleanScalar's real contribution is a LENGTH BOUND the shape regex does not have", () => { + // The honest reason the guard is composed first: VERSION_RE is unbounded, so shape alone admits a + // pathological input. This is the half that is genuinely load-bearing. + assert.equal(VERSION_RE.test(`${"9".repeat(5000)}.0.0`), true, "the shape regex admits an unbounded run"); + assert.equal(isCleanScalar(`${"9".repeat(5000)}.0.0`), false, "the guard is what bounds it"); + assert.equal(isCleanScalar("2.5.1"), true); + assert.equal(isCleanScalar(""), false); + assert.equal(isCleanScalar("x".repeat(65)), false); + assert.equal(isCleanScalar(null), false); + assert.equal(isCleanScalar(undefined), false); + assert.equal(isCleanScalar("2.5.1\n"), false, "and it still rejects a trailing newline, belt and braces"); +}); + +test("✧ the anchor regex is linear — a long non-matching run does not blow up", () => { + const hostile = `https://img.shields.io/badge/pharn-${"a".repeat(50000)}`; + const started = process.hrtime.bigint(); + findBadgeValues(hostile); + const ms = Number(process.hrtime.bigint() - started) / 1e6; + assert.ok(ms < 1000, `anchor took ${ms}ms on a 50k-char run — expected linear behaviour`); +}); + +// ── ✧ WIRING PINS — the checker guards nothing unless something invokes it ───────────────────────── + +test("✧ package.json wires check:badge to this checker, and `check` runs check:badge", () => { + const pkg = JSON.parse(readFileSync(join(REPO, "package.json"), "utf8")); + assert.match(pkg.scripts["check:badge"] ?? "", /check-version-badge\.mjs/, "check:badge must run check-version-badge.mjs"); + assert.match(pkg.scripts.check, /check:badge/, "npm run check must run check:badge"); +}); + +test("✧ CI actually INVOKES the badge check, and its step is not disabled by an `if:` (L2)", () => { + // The wiring precedent and its reason: ci.yml does NOT run `npm run check` — it runs each script as + // its own step. A checker folded only into `check` would therefore never fire on a pull request while + // the plan claimed it was gated. The `if:` half is deliberate: matching only the `run:` string would + // let an edit to `if: false` leave the invocation present, this test green, and the guard dead. + const ci = readFileSync(join(REPO, ".github", "workflows", "ci.yml"), "utf8"); + const step = ci.match(/^ {6}- name: [^\n]*\n(?: {8}[^\n]*\n)*? {8}run: npm run check:badge[ \t]*$/m); + assert.ok(step, "ci.yml must contain a step whose `run:` is `npm run check:badge`"); + assert.match( + step[0], + /^ {8}if: \$\{\{ always\(\) && steps\.install\.outcome == 'success' \}\}$/m, + "the badge-check step must carry the same install-gated `if:` as its sibling steps — a disabled step is a dead guard" + ); + + // HONEST RESIDUAL (P0), stated so this pin is not oversold: what remains uncheckable from inside the + // repo is that GitHub EXECUTED the job, that the workflow is enabled, and that branch protection + // requires this check. Those are harness-layer facts. "The wiring is pinned" NEVER means "CI is + // guaranteed to run it". +}); + +// ── The real repo ───────────────────────────────────────────────────────────────────────────────── + +test("the checker is GREEN against this repo", () => { + const { code, out } = run(REPO); + assert.equal(code, 0, out); +}); + +test("checkVersionBadge is pure — it returns a verdict rather than exiting", () => { + withFixture({ version: "2.5.1", badge: "1.0.0" }, (dir) => { + const res = checkVersionBadge(dir); + assert.equal(res.ok, false); + assert.equal(res.findings[0].type, "DRIFT"); + assert.equal(res.version, "2.5.1"); + assert.equal(res.badge, "1.0.0"); + }); +}); + +test("✧ BADGE_RE is exported non-global so no lastIndex leaks between calls", () => { + assert.equal(BADGE_RE.global, false); + const readme = `https://img.shields.io/badge/pharn-2.5.1-blue`; + assert.deepEqual(findBadgeValues(readme), ["2.5.1"]); + assert.deepEqual(findBadgeValues(readme), ["2.5.1"], "a second call must return the same result"); +}); diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c61036d..3cfe65c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,6 +39,9 @@ jobs: - name: Specified-marker check if: ${{ always() && steps.install.outcome == 'success' }} run: npm run check:markers + - name: Version badge check + if: ${{ always() && steps.install.outcome == 'success' }} + run: npm run check:badge - name: Test if: ${{ always() && steps.install.outcome == 'success' }} run: npm test diff --git a/CHANGELOG.md b/CHANGELOG.md index 673d7e8..5de0dcf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,12 +2,28 @@ All notable changes to PHARN-OSS are documented in this file. -The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). The current version is also recorded in [`SKILLS_VERSION`](./SKILLS_VERSION). +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +**There are two version numbers here, and they track different things.** The entries below are keyed to [`SKILLS_VERSION`](./SKILLS_VERSION) — the version of PHARN's **product surface**: the bytes an install receives (the `pharn/` tree, the product-floor checkers, the four trusted docs, and the `pharn-*` commands). It moves whenever those bytes change, including for prose-only corrections, and it is what the `pharn` badge at the top of the README shows. `package.json`'s `1.0.0` is a separate **foundation tag**, marking that the spec, the build tooling, and the pipeline commands are in place — **not** an adoptable release, as the README status note says plainly. It does not move as fixes land, so a `1.0.0` beside a `2.x` entry is not a contradiction. ## [Unreleased] ### Fixed +- **The front page stopped advertising a version number that had been wrong for the whole `2.x` line — and a checker now makes that impossible to repeat silently.** The README badge read `version-1.0.0` while `SKILLS_VERSION` had reached **2.5.1**, and it linked to a CHANGELOG whose every recent entry is keyed to `2.x`. Rendered by shields as a conventional release marker, it implied a stable `1.0.0` release that the status note three lines below **explicitly disowns** ("not an adoptable release"), leaving a visitor no way to tell whether the project is at `1.0.0` or `2.5.1`. The badge is now `pharn-2.5.1` — labelled for the thing it versions — and the CHANGELOG header states the two tracks outright instead of the ambiguous "The current version is also recorded in `SKILLS_VERSION`". + + **Why a checker and not a note in the bump discipline.** The obvious fix — change the number, add "remember to update the badge" — is the remedy `.dev/memory-bank/lessons-learned.md` **L20** rejects by name: when a lesson's only remedy is that the agent should remember, a second occurrence is evidence the remedy is the wrong kind. This badge had already survived the entire `1.x → 2.5.1` run of bumps, so the trigger was long since met. `.dev/floor/check-version-badge.mjs` reduces the claim to primitive #3 (enum/regex): it locates the badge by its **shields URL pattern** — never a line number, since editing the README shifts lines — and asserts the extracted value equals `SKILLS_VERSION`. **The badge drifted precisely because it sits in the README's unguarded prose**, outside the `CURRENT-STATE` markers that `check-capability-catalog` holds to byte-equality; that region is exactly what no gate was reading. + + **The wiring is the half that would have been missed.** The request said "wire it into `npm run check`", and that alone would have left the checker **unrun on every pull request**: read live, `.github/workflows/ci.yml` invokes each script as its own step and **never** `npm run check`. This repo has shipped that exact false claim before — the comment on the `docs:check` pin in `.dev/floor/lessons-index-core.test.mjs` records a commit where the guard was believed wired and was not. So the checker gets both a `check:badge` script folded into `check` **and** its own CI step, each pinned by a test, including the `if:` condition — because a step disabled by `if: false` is a dead guard that leaves the invocation visible and the test green. + + **What it does not buy, stated rather than implied (P0).** It compares two strings. It does **not** guarantee `SKILLS_VERSION` is correct (a badge matching a wrong bump is still GREEN — the guarantee is agreement, not truth), it does **not** guarantee the version story now _reads_ coherently (prose judgment, human-reviewed, gated by nothing), and the pin cannot prove GitHub executed the job or that branch protection requires it — harness-layer facts unverifiable from inside the repo. It also does **not** read a structured location: a README badge has none, which is the honest narrowing of **L6** rather than a claim against it. The anchor is the shields URL — a structured token within prose — and more than one match is a RED, never first-match-wins. + + **Two defects found at `/pharn-dev-grill` and fixed before the first build**, both about legibility rather than direction. A `SKILLS_VERSION` carrying a hyphen cannot round-trip through a shields message (which encodes a literal `-` as `--`), so `2.6.0-rc.1` would have surfaced as a mismatch between `2.6.0` and `2.6.0-rc.1` — two values that look almost identical; it is now a named refusal explaining the encoding limit. And when both inputs are broken the precedence was undefined, so `SKILLS_VERSION` is now validated first and a test pins that order. + + **No `SKILLS_VERSION` bump, and that is the rule rather than an exemption.** Every path touched is repo-meta (`README.md`, `CHANGELOG.md`, `package.json`, `ci.yml`, `CLAUDE.md`) or build apparatus (`.dev/floor/**`, whose `*.test.mjs` never ships). None is in the bump-triggering set, so this entry carries no version line. `CLAUDE.md`'s `npm run check` component list was corrected in the same edit: it was **already** missing `check:markers` before this increment, and adding `check:badge` while knowingly leaving that out would have shipped a list still false. + + **Found but deliberately not fixed here (recorded, not smuggled in).** `.claude/commands/pharn-dev-verify.md:100` claims its gate set is "exactly the repo's `npm run check` aggregate"; live it is a strict **subset** — `docs:check` and `check:markers` are already outside it, and `check:badge` becomes a third. The defect predates this increment and belongs to a different axis (verify's self-description, not the front-page version story), and the file was not in the approved plan's `## Files`, so correcting it here would have meant widening an approved scope mid-run. Follow-up: `verify-gate-map-claim`. + - **`features/README.md` stopped describing the shipped product pipeline as unbuilt.** The directory guide still read as it did before the product commands existed: `/pharn-spec` was called "(a later increment)", with "Until then this directory is the declared, empty home…" asserting the command does not exist yet, and the artifact list described the downstream stages "as those stages are built". All seven spine stages ship — verified live this run as `.claude/commands/pharn-{spec,plan,grill,build,regress,verify,ship}.md` — so a user cloning the repo read a false statement about tools they can already run. The two sentences are now present-tense while **keeping** the accurate fact the old wording carried: a fresh clone's `features/` really is empty until the user runs the command, which is what makes the directory a declared home rather than a populated one. **No `SKILLS_VERSION` bump (it stays 2.5.1), and that is the rule, not an exemption.** `features/README.md` is a README, and the versioning discipline exempts pure repo-meta from the bump — the decision is settled by the file's _kind_, not by whether an install scaffold copies it, and the concrete bump-triggering set (the `pharn/` tree, `pharn/floor/*.mjs`, the four trusted docs, the product `.claude/` surface) does not list it. This entry carries no version either. diff --git a/CLAUDE.md b/CLAUDE.md index 7728e28..d61b031 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -203,6 +203,23 @@ node .dev/floor/check-specified-markers.mjs [target-dir] [--manifest ] node .dev/floor/gen-lessons-index.mjs [target-dir] node .dev/floor/check-lessons-index.mjs [target-dir] +# Assert the README's shields version badge agrees with SKILLS_VERSION. FLOOR (enum/regex, primitive #3): +# the badge value is located by its shields URL PATTERN (`img.shields.io/badge/pharn--`) — never a line +# number, since editing the README shifts lines — and string-compared to the file. Added 2.5.1-era because +# the badge read `version-1.0.0` through the WHOLE 2.x line and nothing noticed: it sits in the README's +# UNGUARDED prose, OUTSIDE the CURRENT-STATE markers check-capability-catalog holds to byte-equality. Per +# L20 a defect whose only remedy is "remember to update it" has earned a floor check, so this is one. +# Fail-closed everywhere: >1 badge is AMBIGUOUS-RED (never first-match-wins), a hyphen-bearing SKILLS_VERSION +# is a NAMED REFUSAL (shields encodes a literal `-` as `--`, so a pre-release cannot round-trip), and +# SKILLS_VERSION is validated FIRST so two simultaneous REDs cannot race. +# NARROWED, and stated: it proves the two strings AGREE, never that SKILLS_VERSION is CORRECT (a badge +# matching a wrong bump stays GREEN) and never that the version story READS coherently. It does NOT read a +# structured location — a README badge has none, which is the honest bound on L6, not a claim against it. +# Wired into `npm run check` as `check:badge` AND as its own ci.yml step — ci.yml runs each script +# individually and never `npm run check`, so `check`-only wiring would never fire on a PR; both wirings are +# pinned by tests. Apparatus: no SKILLS_VERSION bump. Exits non-zero on RED. +node .dev/floor/check-version-badge.mjs [target-dir] + # Self-test the write-guard hook: echo '{"tool_name":"Edit","tool_input":{"file_path":"pharn/CONSTITUTION.md"}}' | node .claude/hooks/protect-trusted-paths.cjs # → exit 2, denied echo '{"tool_name":"Write","tool_input":{"file_path":"pharn/pharn-core/rules/x.md"}}' | node .claude/hooks/protect-trusted-paths.cjs # → exit 0, allowed @@ -212,7 +229,8 @@ echo '{"tool_name":"Write","tool_input":{"file_path":"pharn/pharn-core/rules/x.m - **Dev tooling is real; the methodology stays stdlib-only.** The floor, the hook, and the commands have **zero runtime dependencies** (Node stdlib; Node 24). The repo carries **dev-only** devDependencies (ESLint, Prettier, markdownlint) wired as npm scripts: `npm run check` - (`format:check` + `lint` + `lint:md` + `docs:check` + `test`) is the aggregate gate, and `npm test` runs + (`format:check` + `lint` + `lint:md` + `docs:check` + `check:markers` + `check:badge` + `test`) is the + aggregate gate, and `npm test` runs `node --test` over the hook, product-floor, and dev-floor suites (`.claude/hooks/*.test.cjs` + `pharn/floor/*.test.mjs` + `.dev/floor/*.test.mjs`) — **green** at this writing; read the count live (`npm test`), never assert it from this doc (P6). diff --git a/README.md b/README.md index 34a07cb..eda617a 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ runs on Claude Code today, and the discipline itself ships as readable markdown lenses, rules — that you read, diff, and version yourself. PHARN does not make anyone understand the code; it keeps a deterministic floor under it and the record available the moment someone needs it. -[![Version](https://img.shields.io/badge/version-1.0.0-blue)](./CHANGELOG.md) +[![pharn](https://img.shields.io/badge/pharn-2.5.1-blue)](./CHANGELOG.md) [![License: Apache 2.0](https://img.shields.io/badge/license-Apache%202.0-green)](./LICENSE) [![CI](https://github.com/pharn-dev/pharn-oss/actions/workflows/ci.yml/badge.svg)](https://github.com/pharn-dev/pharn-oss/actions/workflows/ci.yml) [![CodeQL](https://github.com/pharn-dev/pharn-oss/actions/workflows/codeql.yml/badge.svg)](https://github.com/pharn-dev/pharn-oss/actions/workflows/codeql.yml) diff --git a/package.json b/package.json index 9053294..003f404 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,8 @@ "docs:generate": "node .dev/floor/gen-capability-catalog.mjs . && node .dev/floor/gen-lessons-index.mjs .", "docs:check": "node .dev/floor/check-capability-catalog.mjs . && node .dev/floor/check-lessons-index.mjs .", "check:markers": "node .dev/floor/check-specified-markers.mjs .", - "check": "npm run format:check && npm run lint && npm run lint:md && npm run docs:check && npm run check:markers && npm test" + "check:badge": "node .dev/floor/check-version-badge.mjs .", + "check": "npm run format:check && npm run lint && npm run lint:md && npm run docs:check && npm run check:markers && npm run check:badge && npm test" }, "devDependencies": { "@eslint/js": "^10.0.1",