diff --git a/.dev/features/ci-matrix-os-node/GRILL.md b/.dev/features/ci-matrix-os-node/GRILL.md new file mode 100644 index 0000000..1fde852 --- /dev/null +++ b/.dev/features/ci-matrix-os-node/GRILL.md @@ -0,0 +1,109 @@ +# GRILL — ci-matrix-os-node (M7) — **ADVISORY, gates nothing** + +Plan under interrogation: `.dev/features/ci-matrix-os-node/PLAN.md`. +**Spec-hash check:** recomputed `sha256(ARCHITECTURE.md)` = `bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e` — **matches** the plan's pin. No drift. (The computation is content-hash floor-grade; the *block* on drift is `/pharn-dev-build`'s gate, not this one.) +**Griller discovery (FLOOR — enum membership):** `node .dev/floor/count-grillers.mjs .` → `{"registered":0,"grillers":[]}`. **No griller capabilities exist in this repo**, so Step 2b contributed nothing and the inline axes carry this entire grill. Stated so the coverage is not overread: the `testability` griller the stage doc names lives in pharn-oss, not here. + +The plan is treated as `trust: untrusted`. Its self-claims are tested, not believed. + +--- + +## Findings + +### Axis: guarantee-audit completeness (P0) + +```yaml +- type: FINDING + rule_id: "P0" + severity: important + file: ".dev/features/ci-matrix-os-node/PLAN.md:106" + problem: "The plan fuses two DIFFERENT claims about `.gitattributes` into one row, and the only proof it schedules (Phase C's renormalize check) verifies the weaker one; the claim that actually matters is verified nowhere but the probe's Windows cell." + evidence: "\"Source bytes are LF on every OS at checkout\" → FLOOR — git's, declared … | \"The renormalization is a no-op today\" → git add --renormalize . && git diff --cached --stat must be empty" +``` + +Sharpened: `git add --renormalize .` run on **this macOS working tree** proves the *index does not change* — a statement about the repo's current bytes. It says nothing whatsoever about **what a Windows runner checks out under `core.autocrlf=true`**, which is the entire red-class `.gitattributes` is being shipped to kill. The two are separate claims with separate evidence, and only the second one matters. Recommendation: keep both proofs but stop presenting the renormalize output as evidence for the checkout claim — the Windows cell going green **is** that evidence, which is precisely the probe's job. + +```yaml +- type: FINDING + rule_id: "P0" + severity: minor + file: ".dev/features/ci-matrix-os-node/PLAN.md:103" + problem: "The plan labels the soft-tier gate FLOOR without naming HOW it executes in CI — no workflow step runs it, and floor.yml is a declared non-goal, so its CI enforcement rides entirely on its own test's live-repo block." + evidence: "\"No workflow in this repo declares a soft tier\" | FLOOR — primitive #3 (enum/regex), NEW in this increment." +``` + +Not a defect — it is the exact mechanism `check-action-pins` and `check-run-pins` already rely on, and the plan read that precedent correctly. But the reduction is only honest when stated end-to-end: **the gate runs in CI because `floor.yml`'s `node --test "**/*.test.mjs"` collects `check-soft-tier.test.mjs`, whose `★ LIVE REPO-CONSISTENCY` block invokes the checker against the real tree.** Delete that `★` block and the gate becomes decorative while still looking like floor. Recommendation: say so in the script header, and treat the `★` block as load-bearing rather than as a nicety. + +### Axis: eval coverage (P1) + +```yaml +- type: FINDING + rule_id: "P1" + severity: important + file: ".dev/features/ci-matrix-os-node/PLAN.md:41" + problem: "The new floor gate is NOT exercised by /pharn-dev-build's own floor — validate.mjs does not call check-soft-tier.mjs and npm run check does not collect .mjs tests — so a build that goes GREEN proves nothing about the gate this increment adds." + evidence: "`.dev/floor/check-soft-tier.mjs` — new, deterministic scanner … layer: floor (primitive #3)" +``` + +Concrete and checkable: `/pharn-dev-build`'s verdict is `node .dev/floor/validate.mjs .`, and `validate.mjs` walks markdown frontmatter — it never invokes the `check-*` scanners. `npm run check` = `format:check + lint + typecheck + vitest`, and vitest globs `tests/*.test.ts`, not `.dev/**/*.test.mjs`. So **both** of this stage's automatic verdicts are blind to the new gate. Recommendation (process, not plan-shape): run `node --test ".dev/**/*.test.mjs"` explicitly during build and paste the result — otherwise the increment's only genuinely new floor primitive ships unexecuted locally, and its first real execution is on a runner. + +### Axis: determinism / honest scope (P5, P7) + +```yaml +- type: FINDING + rule_id: "P7" + severity: important + file: ".dev/features/ci-matrix-os-node/PLAN.md:34" + problem: "Coverage-on-every-cell interacts with a fact the plan never records: vitest.config.ts enforces four coverage thresholds, so all five cells now gate on them — converting any platform-specific test skip into a red cell that reports a coverage table instead of a test name." + evidence: "3. **Coverage runs on EVERY cell** (draft proposed canonical-cell-only; overruled). Consequences, all simplifications:" +``` + +**This is the finding I would most want a human to see before the probe runs.** Measured this run: `vitest.config.ts:13-18` sets `thresholds: { statements: 90, branches: 82, functions: 95, lines: 92 }`, described in its own comment as *"Conservative floors set just below current measured coverage so CI stays green."* Those floors were calibrated against **exactly one platform**. Two consequences the plan calls "all simplifications" but are not: + +1. If any test skips or short-circuits on Windows/macOS — a `symlinkSync` fixture hitting `EPERM`, a platform-gated branch — coverage **drops below a floor tuned to have no headroom**, and the cell goes red on a *coverage table*, not a test name. That is a materially harder triage than a failing assertion, and it is a **new** red-class not in the brief's §3 taxonomy. +2. `coverage.include: ['src/**/*.ts']` is a glob evaluated against paths on the runner. A glob/separator mismatch on Windows would not fail loudly — it would yield an **empty include set**, and empty-set threshold behavior is a silent-catastrophe shape rather than a clean red. + +I am **not** recommending reversing the human's decision — coverage everywhere has real value, and this is advisory. I am recommending that HALT 1's triage treat *"red on coverage threshold"* as its own named class, per §3's `Unclassified` rule, rather than filing it under a test failure it is not. + +```yaml +- type: FINDING + rule_id: "P7" + severity: minor + file: ".dev/features/ci-matrix-os-node/PLAN.md:33" + problem: "The increment now bundles two independent concerns — a measurement (the matrix) and an enforcement (the soft-tier gate) — after the plan's own draft argued they should be separate tickets." + evidence: "2. **The soft-tier fence IS built in this increment** (draft recommended deferring; overruled)." +``` + +Raised for the record, not to relitigate: the human weighed this and chose to bundle, which is legitimate. The residual cost is concrete — a red probe cell and a soft-tier-gate bug are now independent failure modes landing in one PR, so a revert of one is a revert of both. Worth one sentence in the PR description so the coupling is visible to a future bisect. + +### Axis: trust propagation (P2) + +No finding. The increment ingests no untrusted artifact, and the plan's fix-#1 discipline for the new scanner (verdict = integer count; offending line copied to output as data only) matches `check-action-pins.mjs`'s stated contract. Recorded as checked, not skipped. + +### Axis: one axis of change / no sibling imports (P3) + +No finding. Each planned file has one reason to change, and the plan's R3 explicitly **refuses** to share an enumerator module between floor scripts — preserving the no-floor-script-imports-another isolation rather than "DRY-ing" it into a shared dependency. That is the right call and the plan already gives the reason. + +--- + +## Axis checked and DE-RISKED (reported so the absence of a finding is evidence, not silence) + +I suspected the single most common first-red cause for this exact change — **platform-specific optional deps missing from `package-lock.json`**, the `@esbuild/*` / `@rollup/*` class that makes `npm ci` fail on a platform the lock was not generated on. Measured rather than assumed: + +- `@esbuild/*` — **26** platform variants present, including `darwin-arm64`, `linux-x64`, `win32-x64` +- `@rolldown/binding-*` — **11** variants, including `win32-x64-msvc` and `darwin-arm64` +- `lightningcss-*` — all platforms including `win32-x64-msvc` + +The lockfile is fully populated across the matrix's three platforms. Residual, stated: *the lock containing a variant is not proof `npm ci` selects and links it correctly* — only the probe proves that. But this axis is healthier than expected and should **not** be the first place to look at HALT 1. + +--- + +## Summary + +The plan is unusually explicit about what it does **not** guarantee, and its two strongest moves are both defensible under interrogation: refusing to write a tautology-test over `ci.yml`, and refusing to share an enumerator between floor scripts. Its guarantee audit is genuinely complete in structure — every row carries a reduction or an `advisory` label. + +The concerns are concentrated in one place: **things the GATE-1 amendments changed that the plan then re-described as simplifications without re-checking their consequences.** Coverage-on-every-cell is the sharp one — it silently recruits four calibrated-with-no-headroom thresholds into the gating set of four never-before-exercised platforms, and produces a red shape the brief's taxonomy does not have a row for. The other cluster is verification blindness: the increment's one genuinely new floor primitive is invisible to both `/pharn-dev-build`'s floor and `npm run check`, so nothing automatic in this pipeline will execute it before it reaches a runner. + +Nothing here argues against building. Two items (the coverage-threshold class, the explicit `node --test` run) are cheap enough to absorb during the build without a plan change. + +**ADVISORY VERDICT: 5 concerns raised (0 blocking-severity, 3 important, 2 minor), plus 1 axis checked and de-risked — for the human to weigh before `/pharn-dev-build`. This is not a pass, not a gate, and not a judgment that the plan is sound; `/pharn-dev-grill` gates nothing.** diff --git a/.dev/features/ci-matrix-os-node/PLAN.md b/.dev/features/ci-matrix-os-node/PLAN.md new file mode 100644 index 0000000..a876eba --- /dev/null +++ b/.dev/features/ci-matrix-os-node/PLAN.md @@ -0,0 +1,157 @@ +# PLAN — ci-matrix-os-node (M7) + +- spec_content_hash: bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e # fix #4 +- increment: Expand `ci.yml` from one cell (`ubuntu-latest` × node 20) to a 5-cell OS×Node matrix, pin source bytes to LF, turn "no soft tier" from discipline into a floor gate, and **measure** the result by pushing a draft PR and reading the real cells before deciding the final shape. +- layer(s): repo tooling / CI definitions + `.dev/floor` (repo infrastructure — the same layer as `floor.yml` and the `check-*-pins` gates that fence it; outside `ARCHITECTURE.md §4`'s product tree) +- constitution_refs: [P0, P1, P5, P6, P7] +- status: **APPROVED at GATE 1** (2026-08-11) with three amendments — recorded in "GATE 1 decisions" below. + +## Live state read this run (P6 — nothing asserted from memory) + +| Fact | Measured | How | +| --- | --- | --- | +| Base | `21db522`, `origin/main` synced (identical SHAs) | `git rev-parse HEAD origin/main` | +| Brief's base `dd8af18` | **is an ancestor** of HEAD; 6 intervening commits | `git merge-base --is-ancestor` | +| Drift in `.github/` since `dd8af18` | **none** — `git diff --stat dd8af18..HEAD -- .github/` is empty | `git diff` | +| Intervening commits | 5 dependency bumps (#83–#87) + `3d5052e` M5 lint-gate (#90) + its loop artifacts | `git log` | +| `ci.yml` shape | ONE job `check`, `ubuntu-latest`, `node-version: 20`, six gates via `always() && steps.install.outcome == 'success'`, `Test` = `npm run test:coverage` | read `.github/workflows/ci.yml` | +| Action digests | `actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1`, `actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0`, `persist-credentials: false` | read `ci.yml:15-21` | +| CRLF in tracked files | **zero** — `git grep -I -l $'\r'` returns empty | `git grep` | +| `.gitattributes` | **absent** | `ls` | +| `continue-on-error` in `.github/**` | **zero occurrences** — the new gate starts from a clean tree | `grep -rn .github/` (exit 1) | +| `.github/actions/` | **does not exist** — so the gate's composite-action walk is *vacuous on this repo today* (covered by hermetic fixtures, not by the live tree) | `ls -d` | +| `engines` | `{ "node": ">=20" }`; version `0.4.0` | read `package.json` | +| Workflows present | `ci.yml`, `codeql.yml`, `floor.yml`, `gitleaks.yml`, `publish.yml` | `ls .github/workflows/` | + +**Base drift verdict: NOT material.** The brief's §0 description of `ci.yml`, the six-gate pattern, `engines`, and the `.gitattributes` absence all match byte-for-byte. The CRLF check is empty, so the `.gitattributes` renormalization is still a verified no-op. No HALT condition from §5 is tripped by the base. + +**One fact the brief predates** (landed in `3d5052e`, after `dd8af18`): `tests/lint-gate.test.ts` resolves the repo root from `import.meta.url` **and** spawns eslint against scratch files. It is both a repo-bytes reader and a spawn-based self-test — i.e. it sits in *two* of the risk classes this matrix exists to measure, and no runner has ever executed it outside `ubuntu × 20`. Recorded as an added reason the probe is worth running, not as a predicted failure. + +## GATE 1 decisions (human, 2026-08-11 — these amend the pre-approval draft) + +1. **Approved as written**, and the outward-facing step is authorized: create branch → push → open a **draft** PR. Nothing further — no merge, no ready-for-review flip, no branch-protection change. +2. **The soft-tier fence IS built in this increment** (draft recommended deferring; overruled). The whitelist widens by one named item — `.dev/floor/check-soft-tier.mjs` + its `.test.mjs`. "No soft tier" stops being a promise and becomes floor primitive #3. +3. **Coverage runs on EVERY cell** (draft proposed canonical-cell-only; overruled). Consequences, all simplifications: no `matrix.coverage` key, all six steps uniform across cells, and **`CLAUDE.md:23` is dropped from the whitelist** — `(CI runs this)` stays true unedited. +4. **`fail-fast: false` is permanent**, not probe-only. + +## Files + +- `.github/workflows/ci.yml` — replace the single `check` job with a 5-cell `strategy.matrix` (`fail-fast: false`), explicit `name: ${{ matrix.os }} / node ${{ matrix.node }}`, six gates reused verbatim — layer: repo CI definition +- `.gitattributes` — **new**, one line `* text=auto eol=lf` — layer: repo-wide byte policy +- `.dev/floor/check-soft-tier.mjs` — **new**, deterministic scanner: no `continue-on-error` key anywhere in `.github/workflows/*.{yml,yaml}` or `.github/actions/**/action.{yml,yaml}` — layer: floor (primitive #3) +- `.dev/floor/check-soft-tier.test.mjs` — **new**, hermetic fixtures + a `★ LIVE REPO-CONSISTENCY` block against the real tree (P1) — layer: floor test +- `CHANGELOG.md` — one entry under `## [Unreleased] → ### Added`, following the `:279` "Repo-health tooling" precedent — layer: docs +- `src/lib/apply-update.ts` — **HALT-1 line-item R1, approved 2026-08-11.** Make `readDiskState`'s absent-vs-unreadable split errno-free, fixing the Windows red cell — layer: `lib/` + +### The proposed `ci.yml` shape + +```yaml +jobs: + check: + name: ${{ matrix.os }} / node ${{ matrix.node }} + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + include: + - { os: ubuntu-latest, node: 20 } + - { os: ubuntu-latest, node: 22 } + - { os: ubuntu-latest, node: 24 } + - { os: windows-latest, node: 24 } + - { os: macos-latest, node: 24 } +``` + +The six steps stay **byte-identical in body** to today's — `npm ci`, then `format:check` / `lint` / `lint:md` / `typecheck` / `test:coverage` / `build`, each behind `if: ${{ always() && steps.install.outcome == 'success' }}`. Only `runs-on` and `node-version` become expressions. With coverage now uniform (decision 3), the "six-gate pattern preserved verbatim" claim is exactly true — per cell **and** per file; the draft's seventh-step caveat is gone. + +**The explicit `name:` is load-bearing, not decoration.** Without it GitHub derives each check's name from *every* matrix key — so any future key added to `include` would silently rename all five required checks and re-break branch protection. Pinning `name:` to `os` + `node` fixes the check identity against that. + +### The soft-tier gate — shape, enforcement, and honest residuals + +`check-soft-tier.mjs` follows the house style of its two neighbours exactly: Node stdlib only, no network, no `child_process`, no dynamic import; a **line scanner**, not a YAML parser; unrecognised shapes fail **toward flagging**. + +- **Enumerates** the same two sets `check-action-pins.mjs` does, for the same reason: `.github/workflows/*.{yml,yaml}` (non-recursive, mirroring GitHub) **and** `.github/actions/**/action.{yml,yaml}` (recursive) — a composite action is a laundering path if only call sites are scanned. +- **Violation = the presence of a `continue-on-error:` key, value-blind.** `false` is rejected along with `true`. This is deliberate: reading the value would make the verdict depend on untrusted file content, whereas `violations.length > 0` is an integer test (the same P2/fix-#1 discipline `check-action-pins.mjs` states for its `ref` field). It also catches the experimental-cell laundering shape `continue-on-error: ${{ matrix.experimental }}` without needing to know what `matrix.experimental` is. +- **Enforcement (outside the scanned workflow surface):** `floor.yml` runs `node .dev/floor/check-soft-tier.mjs .` directly (not only via its test file), and the `main protection` ruleset requires the `floor` status check. The `★ LIVE REPO-CONSISTENCY` block in `check-soft-tier.test.mjs` is retained as a third anti-vacuity residual on top of both wires. +- **Named residuals** (stated, never claimed closed): + - **R1 — other required checks are still a REPO-SETTINGS property.** This gate guarantees no workflow *asks* to be soft, and `floor` itself is now required on `main`; a *different* job left out of the required list (e.g. a matrix cell) remains invisible to this scanner. The operational note for those names still goes to the human by hand. + - **R2 — SHELL-LEVEL SWALLOWING.** `run: npm test || true`, `set +e`, a trailing `|| exit 0` — a soft tier written in bash rather than YAML. Detecting it is classification, which P5 forbids. Out of contract by construction. + - **R3 — ENUMERATOR DUPLICATION.** `collectFiles`/`isYaml`/`safeLstat` are duplicated from `check-action-pins.mjs`, because **no floor script imports another** and that isolation is a safety property (one module's bug cannot take down two gates) — the identical trade `check-run-pins.mjs` documents as its R2. Mitigated the same way: the test cross-checks that this scanner's `files[]` equals `check-action-pins.mjs`'s for this repo, so the two walkers cannot drift silently. + +## Contracts satisfied + +- None in `pharn-contracts`. This increment adds no Capability, no `rule_id`, and no inter-layer contract — it changes where the **existing** gates execute, and adds one floor gate. Recorded explicitly so the absence is a finding, not an omission (P4). + +## Evals to write (P1) + +The matrix and `.gitattributes` add no product behavior — `src/**` is untouched, and every gate the matrix runs is an existing, already-tested gate. **The soft-tier gate does add behavior, so P1 binds it**, and it is tested in the `.mjs` floor-test style its two neighbours established (collected by `floor.yml`'s `node --test`, not by vitest — matching precedent, not inventing a home): + +- clean workflow fixture → exit 0, `violations: []` +- `continue-on-error: true` on a step → exit 1, violation carries `{file, line}` +- `continue-on-error: false` → exit 1 — **pins the value-blind choice**, so a later "helpful" relaxation is a test failure, not a silent drift +- `continue-on-error: ${{ matrix.experimental }}` → exit 1 — the experimental-cell laundering shape +- quoted-key and leading-whitespace variants → flagged (line-scanner tolerance) +- a violation inside `.github/actions/x/action.yml` → flagged — pins that the composite-action laundering path is walked +- a non-YAML file, and a file in a *subdirectory* of `.github/workflows/` → skipped (mirrors GitHub's own non-recursive rule) +- `★ LIVE REPO-CONSISTENCY` — run against the **real repo**: exit 0, and `files[]` enumerates the live workflow directory (an anti-vacuity **lower bound**, so adding a workflow later never silently shrinks coverage) +- `★` cross-check — `files[]` is deep-equal to `check-action-pins.mjs`'s for this repo (the R3 anti-drift) + +**What is deliberately NOT written:** a vitest test asserting `ci.yml` contains `"windows-latest"`. That asserts the file says what the file says — a tautology wearing a test's clothes, and precisely the "written in the config" mistaken for "guaranteed" disease P0 exists to prevent. **The execution proof is the probe**: five green cells on a real PR. That is why Phase A is empirical and HALT 1 is a results table. + +## Guarantee audit (P0) + +| Claim | Reduction | +| --- | --- | +| "The tests execute on Windows, macOS, and node 20/22/24" | **FLOOR — the runner's, not this repo's.** GitHub executes the matrix; the proof is five green cells on the probe PR, an observed artifact. Until the probe returns this claim is **unmeasured** and is stated nowhere. | +| "No workflow in this repo declares a soft tier" | **FLOOR — primitive #3 (enum/regex), NEW in this increment.** `check-soft-tier.mjs`, verdict = `violations.length > 0`, an integer test. Bounded by R1/R2/R3 above. | +| "Both action digests stay pinned by 40-hex + full semver" | **FLOOR** — `check-action-pins.mjs`, executed against the real tree by its own `★` self-test. | +| "No `run:` line I add installs a floating package" | **FLOOR** — `check-run-pins.mjs`, same `★` mechanism. Both new-shape `run:` lines are `npm run