From 5431a5158aebaf433ba8f9366641ca47d692efde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Galarowicz?= Date: Wed, 12 Aug 2026 15:09:19 +0200 Subject: [PATCH] changes --- .claude/hooks/set-writes-scope.cjs | 2 +- .claude/hooks/set-writes-scope.test.cjs | 50 +++++++++++++++++ .dev/features/f15-route-group-scope/GRILL.md | 35 ++++++++++++ .dev/features/f15-route-group-scope/PLAN.md | 54 +++++++++++++++++++ .../f15-route-group-scope/REGRESSION.md | 36 +++++++++++++ .dev/features/f15-route-group-scope/REVIEW.md | 46 ++++++++++++++++ .dev/features/f15-route-group-scope/SHIP.md | 33 ++++++++++++ .dev/features/f15-route-group-scope/VERIFY.md | 24 +++++++++ .../regression-report.json | 31 +++++++++++ .../f15-route-group-scope/verify-report.json | 14 +++++ CHANGELOG.md | 10 ++++ README.md | 2 +- SKILLS_VERSION | 2 +- 13 files changed, 336 insertions(+), 3 deletions(-) create mode 100644 .dev/features/f15-route-group-scope/GRILL.md create mode 100644 .dev/features/f15-route-group-scope/PLAN.md create mode 100644 .dev/features/f15-route-group-scope/REGRESSION.md create mode 100644 .dev/features/f15-route-group-scope/REVIEW.md create mode 100644 .dev/features/f15-route-group-scope/SHIP.md create mode 100644 .dev/features/f15-route-group-scope/VERIFY.md create mode 100644 .dev/features/f15-route-group-scope/regression-report.json create mode 100644 .dev/features/f15-route-group-scope/verify-report.json diff --git a/.claude/hooks/set-writes-scope.cjs b/.claude/hooks/set-writes-scope.cjs index cca9a6b..4343b48 100644 --- a/.claude/hooks/set-writes-scope.cjs +++ b/.claude/hooks/set-writes-scope.cjs @@ -93,7 +93,7 @@ function normalizeForTest(entry) { // Strip a trailing " (annotation)" (e.g. " (gated)") and surrounding whitespace. function clean(entry) { return String(entry) - .replace(/\s*\([^)]*\)\s*$/, "") + .replace(/\s+\([^)]*\)\s*$/, "") .trim(); } diff --git a/.claude/hooks/set-writes-scope.test.cjs b/.claude/hooks/set-writes-scope.test.cjs index 81ed8cd..6e99ea0 100644 --- a/.claude/hooks/set-writes-scope.test.cjs +++ b/.claude/hooks/set-writes-scope.test.cjs @@ -288,6 +288,56 @@ test("the usage line advertises --allow-claude-dir", () => { assert.match(r.stderr, /--allow-claude-dir/); }); +// --- F15: `clean()`'s trailing-annotation strip must not mangle a path segment that itself ends in +// `)` — a Next.js route-group directory (`app/(marketing)`) is a real, concrete `writes:` value whose +// last path segment is `(marketing)`. The old `\s*\([^)]*\)\s*$` matched a ZERO-space gap, so it +// stripped the group off entirely (scope collapsed to `app/`), silently under-scoping the writes-scope +// guard for a common layout. An annotation (the documented use, e.g. ` (gated)`) is always written with +// a LEADING SPACE, so requiring `\s+` distinguishes the two without behavior change for the documented +// case. --- + +test("F15 fix: a route-group directory `app/(marketing)` survives `clean()` intact (was mangled to `app/`)", () => { + const cwd = tmp(); + const cap = capWith(cwd, "writes:", ' - "app/(marketing)"'); + const r = setter(cwd, "--from-frontmatter", cap); + assert.equal(r.status, 0); + const rec = JSON.parse(fs.readFileSync(join(cwd, ".pharn", "writes-scope.json"), "utf8")); + assert.deepEqual(rec.scope, ["app/(marketing)"]); +}); + +test("F15 fix: a NESTED route-group path `app/(a)/(b)` survives `clean()` intact", () => { + const cwd = tmp(); + const cap = capWith(cwd, "writes:", ' - "app/(a)/(b)"'); + const r = setter(cwd, "--from-frontmatter", cap); + assert.equal(r.status, 0); + const rec = JSON.parse(fs.readFileSync(join(cwd, ".pharn", "writes-scope.json"), "utf8")); + assert.deepEqual(rec.scope, ["app/(a)/(b)"]); +}); + +test("F15 regression guard: the documented SPACE-separated annotation (e.g. ` (gated)`) is still stripped", () => { + const cwd = tmp(); + const cap = capWith(cwd, "writes:", ' - "src/widget.ts (gated)"'); + const r = setter(cwd, "--from-frontmatter", cap); + assert.equal(r.status, 0); + const rec = JSON.parse(fs.readFileSync(join(cwd, ".pharn", "writes-scope.json"), "utf8")); + assert.deepEqual(rec.scope, ["src/widget.ts"]); +}); + +test("F15 regression guard: a route-group FILE (`app/(marketing)/page.tsx`) was never affected either way", () => { + const cwd = tmp(); + const cap = capWith(cwd, "writes:", ' - "app/(marketing)/page.tsx"'); + const r = setter(cwd, "--from-frontmatter", cap); + assert.equal(r.status, 0); + const rec = JSON.parse(fs.readFileSync(join(cwd, ".pharn", "writes-scope.json"), "utf8")); + assert.deepEqual(rec.scope, ["app/(marketing)/page.tsx"]); +}); + +// Mutant MEASURED (L4 — an authored assertion passes by construction until proven otherwise): reverting +// `\s+` back to `\s*` in `clean()` was applied to the live file and both F15-fix tests above FAILED +// (scope collapsed to `["app/"]` / `["app/(a)"]`); the fix was then restored and the full suite is green +// again. No mutant-guard test is added here (that would just re-encode the same regex the fix already +// pins) — the measurement is recorded as evidence the two tests above are not vacuous. + // --- Coverage backfill for the entry-resolution paths this increment's refusal sits downstream of. // These were untested before: a refusal that runs over `scope` is only as sound as the parsing that // builds `scope`, so the glob / block-list / unquoted-inline forms are pinned here. --- diff --git a/.dev/features/f15-route-group-scope/GRILL.md b/.dev/features/f15-route-group-scope/GRILL.md new file mode 100644 index 0000000..0f8cb7e --- /dev/null +++ b/.dev/features/f15-route-group-scope/GRILL.md @@ -0,0 +1,35 @@ +# GRILL — f15-route-group-scope + +**Plan:** `.dev/features/f15-route-group-scope/PLAN.md` +**Spec-hash check:** recomputed `sha256(pharn/ARCHITECTURE.md)` = `8f5ec002e3b18cbfd2f094b08a3671f7ed42a05a3fbaf01a11bbbd28da30fb52` — matches the plan's pinned `spec_content_hash`. No drift; nothing to surface here (`/pharn-dev-build`'s own gate re-verifies this at build time regardless). + +**Grillers discovered:** `node pharn/floor/count-grillers.mjs .` → 13 registered (`a11y, architecture, comprehension, coupling, documentation, error-handling, i18n, migrations, observability, performance, privacy, security, testability`). Ran all 13; three (`a11y`, `i18n`, `migrations`) declare `applies: ["ssr","spa"]` / `["backend","ssr"]` and this increment is a Node CLI hook regex fix with no ssr/spa surface and no schema change — noted as **not applicable by `applies` membership**, not silently skipped. + +## Findings + +```yaml +- type: FINDING + rule_id: P3 + severity: minor + file: ".dev/features/f15-route-group-scope/PLAN.md:6" + problem: "The plan's `layer(s)` field names `pharn-core` for a change that is NOT in the pharn-core capability tree, then says so in the same breath — a self-contradictory label for a human skimming just that line." + evidence: "- layer(s): pharn-core (the `.claude/hooks/` deterministic tooling; not a `pharn-*` capability layer)" +``` + +**Architecture griller (Layer 2, advisory judgment):** `pharn-core` is a specific capability-tree directory under `pharn/pharn-core/` (`pharn/ARCHITECTURE.md §4`). `.claude/hooks/*.cjs` is build-apparatus tooling that implements the floor's hook primitive — it sits **outside** the `pharn/` capability tree entirely, alongside `.claude/commands/`. Labeling it `pharn-core` and then immediately disclaiming "not a `pharn-*` capability layer" in the same field reads as a mislabel corrected inline rather than a considered classification. This is a **plan-metadata clarity concern, not a structural fit violation** — the plan does not actually couple a leaf to a sibling, invert a layer, or reinvent a mechanism; it just picked the wrong layer name for a file class that has no `pharn-*` layer name to pick. Recommend the field read something like `layer(s): build-apparatus (.claude/hooks/ — outside the pharn/ capability tree; implements floor primitive #1)` in a future increment's plan template guidance, but this is **not worth blocking a one-line regex fix to re-approve a plan over** (P7 — proportionality). + +**Coupling griller:** the four `## Files` entries (the hook, its test, `CHANGELOG.md`, `SKILLS_VERSION`) are the standard SKILLS_VERSION-discipline bundle (fix + its proof + its version record), not sibling entanglement — no finding. + +**Security griller:** Layer 1 (floor) — `node pharn/floor/scan-plan-secrets.mjs PLAN.md` → `{"found":false,"hits":[]}`, clean. Layer 2 (advisory) — the increment touches a security-relevant floor guard (the writes-scope setter) but only tightens an existing regex; it introduces no new sensitive/destructive operation, no injection surface, and does not weaken `enforce-writes-scope.cjs` or `protect-trusted-paths.cjs` — no concern. + +**Testability griller:** Layer 1 (floor-checkable presence) — a verification section is **present**: the plan's `## Files` names four new `node --test` cases (fix, annotation-preserved, route-group-file regression, mutant-revert) and `## Evals to write` explains why Capability-eval coverage doesn't apply to dev-tooling hooks. No absence finding. Layer 2 (advisory adequacy) — the four cases cover the fix, the preserved behavior, a regression guard, and a mutant check (proving the test can fail) — adequate for a one-line regex change; no concern. + +**Error-handling griller:** considered whether tightening `\s*`→`\s+` could newly break a case that previously worked. The only behavior removed is matching **zero** spaces before the trailing paren — exactly the route-group bug pattern being fixed; every previously-intended annotation-strip (which the plan's own discovery found written with a leading space per the source comment) still matches. No new failure mode identified — no finding. + +**Observability, performance, privacy, documentation, comprehension grillers:** no concerns raised. The fix is a single regex-class tightening with no new logging/metrics surface, no scale-sensitive path, no personal data, and the plan's own "Decision resolved in discovery" section already documents the _why_ (not just the _what_) for a future reader. + +## Summary + +One advisory, minor-severity finding: the plan's `layer(s)` field is an internally-contradictory label (names `pharn-core` then disclaims it) rather than a real structural-fit problem. No blocking-severity concern from any of the 13 registered grillers or the inline Step-2 axes. The spec→plan hash chain holds (re-verified live, matches the plan's pin). The secret scanner is clean. + +**ADVISORY VERDICT: 1 concern raised (0 blocking-severity, 1 minor-severity) — for the human to weigh before `/pharn-dev-build`. This is not a pass/fail signal; `/pharn-dev-grill` gates nothing (P0). The deterministic backstops that actually gate remain `/pharn-dev-build`'s spec-hash check and `pharn/floor/validate.mjs`.** diff --git a/.dev/features/f15-route-group-scope/PLAN.md b/.dev/features/f15-route-group-scope/PLAN.md new file mode 100644 index 0000000..fd8214a --- /dev/null +++ b/.dev/features/f15-route-group-scope/PLAN.md @@ -0,0 +1,54 @@ +# PLAN — f15-route-group-scope + +- spec_content_hash: 8f5ec002e3b18cbfd2f094b08a3671f7ed42a05a3fbaf01a11bbbd28da30fb52 +- applied_lessons: [L19] +- increment: Fix `set-writes-scope.cjs`'s `clean()` regex so it strips only a space-separated trailing annotation (e.g. `(gated)`), not a path segment that itself ends in `)` — a Next.js route-group directory (`app/(marketing)`) was being mangled to `app/`, silently under-scoping the writes-scope guard for a common real-world layout. +- layer(s): pharn-core (the `.claude/hooks/` deterministic tooling; not a `pharn-*` capability layer) +- constitution_refs: [P0, P5, P6, P7] + +## Applied lessons + +- L19 — During the write procedure, `set-writes-scope.cjs` is edited via **Bash** (self-lock, F3) rather than Write/Edit, which is exactly the escape hatch L19 warns bypasses fix #7 unchecked. This plan's Bash edit is a single targeted `sed`/heredoc replacement of the one regex line — never a repo-wide command — and no formatter is invoked over anything outside the plan's own `## Files`. `npx prettier`/`markdownlint` are run only on this stage's own `PLAN.md` (per the dev-plan command's own Step-4 formatting step), never repo-wide. + +## Files + +- `.claude/hooks/set-writes-scope.cjs` — tighten `clean()`'s regex from `\s*\([^)]*\)\s*$` to `\s+\([^)]*\)\s*$` (edited via Bash — self-locked by `protect-trusted-paths.cjs`, F3; not a Write/Edit-tool path) +- `.claude/hooks/set-writes-scope.test.cjs` — add the fix test, the annotation-preserved test, the route-group-file regression test, and record the mutant check +- `CHANGELOG.md` — add an `[Unreleased]` entry under `### Fixed` naming the defect, the fix, and the patch bump +- `SKILLS_VERSION` — bump patch from `2.5.1` to `2.5.2` +- `README.md` — update the shields version badge from `pharn-2.5.1` to `pharn-2.5.2` (line 13), matching the `SKILLS_VERSION` bump. **Added after `/pharn-dev-regress`'s first run surfaced a real regression:** `.dev/floor/check-version-badge.test.mjs`'s live-repo self-test flipped pass→fail because the original `## Files` list bumped `SKILLS_VERSION` without also moving the badge `.dev/floor/check-version-badge.mjs` holds to agreement with it. This is a scope widening ratified by the human at that regression stop, not a silent addition — the regression report (`.dev/features/f15-route-group-scope/REGRESSION.md`) is the record of why. + +## Contracts satisfied + +- No `pharn-contracts` schema governs `.claude/hooks/*.cjs` directly — these are the fix #7 floor mechanism itself (`pharn/ARCHITECTURE.md §2` primitive #1, hooks), referenced by contract text (`pharn/ARCHITECTURE.md:73`, `:240`) but not schema-shaped. The correctness contract here is the existing hook's own header comment (`clean()`'s doc comment: "Strip a trailing `(annotation)`") — the fix makes the regex match that comment, it does not change the contract. + +## Evals to write (P1) + +- N/A — `.claude/hooks/*.cjs` are dev tooling (deterministic hooks), not `role:`-bearing Capabilities under `pharn/pharn-*`, so P1's eval-per-Capability requirement does not apply. Coverage is via `node --test` unit tests in `set-writes-scope.test.cjs` (existing convention for this file), enforced by `/pharn-dev-verify`'s `test` gate. + +## Guarantee audit (P0) + +- "A route-group directory entry (`app/(marketing)`) survives `clean()` intact" → floor: enum-regex (the tightened `\s+` pattern), pinned by a `node --test` case in `set-writes-scope.test.cjs`, which is itself gated by `/pharn-dev-verify`'s `test` gate (`npm run check`). +- "The documented annotation-strip (`(gated)`) still fires" → floor: enum-regex, pinned by a regression test in the same file. Narrowed and stated: no real shipped `writes:`/`## Files` entry in this repo currently exercises this path (confirmed by discovery grep across `.claude/commands/*.md` frontmatter) — the test guards against a _future_ input, not a currently-live one. +- "`set-writes-scope.cjs` was edited only via Bash, never Write/Edit" → floor: hook (`protect-trusted-paths.cjs` denies Write/Edit/MultiEdit to this file outright, confirmed live this run: exit 2). This is not something the plan can violate even if it tried — it is a structural fact about the write path. +- "The fix does not newly-RED an existing install" → advisory (a correctness argument about semver compatibility, not a floor-checked property): a route-group entry that was silently under-scoped now scopes correctly (fail-closed → correct, never the reverse), and a spaced annotation still strips identically. No floor primitive verifies backward-compatibility across installs; this is reasoned, not measured. + +## Trust audit (P2) + +N/A — no untrusted artifact is ingested by this increment. The fix touches the hook's own regex logic; the increment does not process a `writes:`/`## Files` declaration from an untrusted PLAN as part of building itself (the _test_ fixtures constructed for `set-writes-scope.test.cjs` are trusted, agent-authored test data, not the untrusted-input class the hook itself handles at runtime). + +## Determinism audit (P5) + +The fix is a single-character class change to a fixed regex (`\s*` → `\s+`) — a membership/pattern-match change, not a branch. No new branching is introduced. The decision to tighten rather than remove the paren-strip was resolved in discovery (below), not deferred to a runtime fallback. + +## Decision resolved in discovery (`\s+` vs. removal) + +Chose **`\s+`** (tighten, not remove), matching the build prompt's own recommendation. Evidence gathered live this run: + +- `grep` across every `.claude/commands/*.md` frontmatter `writes:` block found no entry using a trailing `(annotation)` form — the only paren-containing entry (`pharn-build.md:17`) is a `` ending in `>`, not `)`, so `clean()`'s trailing-paren regex never matches it regardless of `\s*` vs `\s+`. +- No test in `set-writes-scope.test.cjs` exercises `clean()`'s annotation-strip today. +- So the annotation-strip is **provably unexercised by any live input in this repo** — removal would be defensible, but per the build prompt's own instruction ("do not remove without that evidence" of it being _exercised_, and recommending `\s+` regardless), `\s+` is the smaller, lower-risk change: it preserves the documented behavior for a future frontmatter `writes:` value that legitimately wants a spaced annotation, at zero cost, versus removal which would need to disclose a behavior change for no compensating benefit. + +## Open questions (HALT) + +None outstanding — the build prompt itself resolves scope, decision, and versioning; discovery confirmed every factual premise it makes (reproduction, self-lock, test-file being unprotected, absence of any real annotation-strip usage) against live state this run. diff --git a/.dev/features/f15-route-group-scope/REGRESSION.md b/.dev/features/f15-route-group-scope/REGRESSION.md new file mode 100644 index 0000000..9dc6d80 --- /dev/null +++ b/.dev/features/f15-route-group-scope/REGRESSION.md @@ -0,0 +1,36 @@ +# REGRESSION — f15-route-group-scope + +**Base:** `c880413ef5e24916c5743e66306b39b3d68e25c9` (working-tree dogfood build — `git status --porcelain` was non-empty, so base = HEAD per the deterministic auto-detect rule). + +**Inside (the changed scope, second/final run):** `.claude/hooks/set-writes-scope.cjs`, `.claude/hooks/set-writes-scope.test.cjs`, `CHANGELOG.md`, `README.md`, `SKILLS_VERSION`, plus this feature's own `GRILL.md` / `PLAN.md` / `regression-report.json` / `REGRESSION.md` (exempted via `--feature f15-route-group-scope`, `escaped: []`). Nothing escaped the (now-widened) plan's declared `## Files`. + +**Style gates skipped (deterministic optimization):** `inside` touches no shared style config (`eslint.config.mjs`, `.prettierrc.json`, `.prettierignore`, `.markdownlint-cli2.jsonc`), so a style flip over the byte-identical outside files is provably impossible — `lint` / `format:check` / `lint:md` were not run at either base or head. + +## This is the SECOND run — a real regression was found, fixed, and re-measured + +The **first** `/pharn-dev-regress` run over the originally-approved plan (`README.md` **not** in `## Files`) found a real regression: `.dev/floor/check-version-badge.test.mjs`'s live-repo self-test flipped pass→fail, because `SKILLS_VERSION` moved `2.5.1`→`2.5.2` while the README's shields badge stayed `pharn-2.5.1`. That run's verdict was `"regressions"` (exit 1) and `/pharn-dev-ship`'s gated chain **stopped** there and presented it to the human, per its non-negotiable stop-on-regression rule — it was not routed around. + +**The human chose to widen scope**, not abandon or proceed unresolved: `README.md` was added to the plan's `## Files` (with a note recording why and pointing at this file), the badge was updated `pharn-2.5.1`→`pharn-2.5.2`, and `/pharn-dev-regress` was **re-run from scratch** with the widened `declared` set. This file records the **final** (second) run's result; the interim regression is preserved in this increment's history (the plan's `## Files` note + this section) rather than silently overwritten. + +## Per-gate exit codes (base → head), final run + +| Gate | Base | Head | Flip? | +| ------------------------------------------------------------------------------------------ | :--: | :--: | ----- | +| `tests` (62 outside `*.test.mjs`/`*.test.cjs` files, `node --test`) | 0 | 0 | no | +| `validate` (`node pharn/floor/validate.mjs .`, whole-repo granularity) | 0 | 0 | no | +| `structural:pharn/pharn-review/trust-fence/evals/expected/expected-injection-comment.json` | 0 | 0 | no | + +(Baseline gate results were reused unchanged from the first run: same base commit, byte-identical `outside_tests` set — confirmed via `diff` before reuse — so re-running the baseline worktree would have reproduced the same three exit codes. Only the HEAD side was re-captured, since HEAD is what changed between runs.) + +## Deterministic verdict + +```json +{ + "base": "c880413ef5e24916c5743e66306b39b3d68e25c9", + "regressions": [], + "pre_existing": [], + "verdict": "no-regressions" +} +``` + +**REGRESSIONS: none — no deterministically-detectable breakage outside the feature.** This is the deterministic comparison only: every outside gate this suite covers was GREEN at base and stayed GREEN at head. It does **not** certify nothing broke anywhere — `/pharn-dev-regress` catches exactly what its suite catches, nothing more (the honest residual, unchanged from the first run's disclosure). diff --git a/.dev/features/f15-route-group-scope/REVIEW.md b/.dev/features/f15-route-group-scope/REVIEW.md new file mode 100644 index 0000000..7051154 --- /dev/null +++ b/.dev/features/f15-route-group-scope/REVIEW.md @@ -0,0 +1,46 @@ +# REVIEW — f15-route-group-scope + +## Step 1 — Floor first + +`node pharn/floor/validate.mjs .` → `FLOOR: GREEN — 36 capabilities checked in .`. The increment reached review with a green floor (also independently confirmed by `/pharn-dev-verify`'s `validate` gate). Proceeding to the four advisory lenses. + +## L-floor → P0 + +```yaml +- type: FINDING + rule_id: P0 + severity: important + file: "CHANGELOG.md" + problem: "The CHANGELOG entry's compatibility claim ('Nothing an existing install newly-REDs') is stated as flat fact, but the same claim in PLAN.md's own Guarantee audit is explicitly labeled `advisory` (a reasoned argument about existing-install behavior, not something a floor primitive verifies across all possible prior inputs) — the shipped prose doesn't carry the hedge its own plan gave it." + evidence: "Nothing an existing install newly-REDs. A route-group `writes:` entry that was silently and incorrectly under-scoped now scopes correctly — a fail-closed-on-a-valid-layout defect becoming correct, never the reverse. A space-separated annotation still strips identically to before." +``` + +This is a labeling-hygiene gap, not a substance error: the underlying claim is true and follows directly from `\s+` being a strict subset of `\s*`'s matches (the only behavior removed is the zero-space case, which is exactly the bug). No floor primitive backs "no existing install regresses" as a general claim, so per P0 it should read as advisory, the way `PLAN.md`'s own guarantee audit already treats it. **Not blocking** — nothing downstream reads this sentence as a guarantee to act on, and the reasoning is sound; it is worth tightening in a future pass over this CHANGELOG entry, not worth re-opening the increment for. + +No other P0 gap found. Every other claim in the built artifacts reduces cleanly: the fix itself → enum-regex + a measured mutant test (floor); the self-lock → hook (`protect-trusted-paths.cjs`, confirmed live); the regression-comparison → `check-regress.mjs`'s exit-code diff (floor); the verify PASS → `check-verify.mjs`'s exit-code threshold (floor). + +## L-eval → P1 + +Not applicable in the Capability sense: `.claude/hooks/*.cjs` are dev tooling, not `role:`-bearing Capabilities under `pharn/pharn-*`, so P1's eval-per-Capability + `enforces`-binding requirement doesn't apply (confirmed: `pharn/floor/validate.mjs` scans `pharn/` only and stayed GREEN without touching these files). Coverage instead comes from `node --test`: `set-writes-scope.test.cjs` grew from 33 to 37 tests, and the two new "survives intact" tests were confirmed to fail against a live-reverted `\s*` mutant and pass again once restored (measured, not just authored — L4). No finding. + +## L-trust → P2 + +The increment under review is `trust: untrusted` by this stage's own framing. Nothing in the diff resembles an injected instruction (no comments telling a reviewer to skip a finding, no fenced blocks posing as directives) — this is a straightforward regex fix with test/doc/version companions, not adversarial input. No instruction-looking content was encountered, so nothing was at risk of being followed. `GRILL.md`'s own finding earlier in this run correctly quoted plan text as data rather than acting on it. No finding. + +## L-axis → P3 + +Every touched file changes for exactly one reason serving this one increment: the hook's regex (the fix), its test file (proof of the fix), `CHANGELOG.md` / `SKILLS_VERSION` / `README.md` (the version-story record the fix requires). No sibling `reads:`/reference crosses module roots — these are hooks and repo-meta, outside the `pharn-contracts` tree entirely, so P3's leaf→leaf constraint doesn't have a tree to violate here. No finding. + +## Verdict + +**GREEN — 0 blocking floor-gate findings, 1 non-blocking advisory finding (L-floor, `important`).** The increment is done; the one advisory finding is a documentation-hygiene note, not a defect requiring rework. + +## Proposed lesson candidate (NOT written to canon here — `/pharn-dev-memory-promote` decides) + +**Candidate:** _A plan that bumps `SKILLS_VERSION` must also declare `README.md` in `## Files` (the shields badge), or `/pharn-dev-regress`'s `tests` gate will catch the drift downstream via `check-version-badge.test.mjs` — plan for the badge alongside the version bump, don't wait for regress to find it._ + +**Why it matters:** this run's `PLAN.md` bumped `SKILLS_VERSION` without the badge, `/pharn-dev-regress` correctly caught it as a real regression (verdict `"regressions"`, `REGRESSION.md`), and the human had to widen scope mid-chain to recover. The mechanism that would have prevented it — `check-version-badge.mjs` — already exists and was added for a near-identical prior incident (`CHANGELOG.md`'s `2.5.1`-era entry: the badge had drifted silently through the entire `1.x → 2.5.1` run). This is the **second** occurrence of essentially the same shape (a `SKILLS_VERSION` move not paired with its badge), which is exactly the trigger `.dev/memory-bank/lessons-learned.md` **L20** names for promoting a discipline-only remedy to something stronger — though here the "stronger" form is cheap: a planning-time checklist item (`/pharn-dev-plan`'s Step 2 already asks planners to state guarantee audits; a version-bumping plan could be prompted to check the badge as part of that same step), not a new floor primitive, since `check-version-badge.mjs` + its `regress`/`verify` wiring already catch it deterministically — this lesson is about **catching it one stage earlier**, at plan time, not about adding a new guarantee. + +**Provenance:** `f15-route-group-scope`, this increment, `.dev/features/f15-route-group-scope/{PLAN.md,REGRESSION.md}`, discovered live during `/pharn-dev-regress`'s first run (2026-08-12). + +Left for a human to accept/deny via a separate `/pharn-dev-memory-promote` run — not written to `.dev/memory-bank/lessons-learned.md` here. diff --git a/.dev/features/f15-route-group-scope/SHIP.md b/.dev/features/f15-route-group-scope/SHIP.md new file mode 100644 index 0000000..c29fd0e --- /dev/null +++ b/.dev/features/f15-route-group-scope/SHIP.md @@ -0,0 +1,33 @@ +# SHIP — f15-route-group-scope + +## Chain run, in order + +1. **`/pharn-dev-plan`** → `PLAN.md` written; spec hash pinned (`8f5ec002e3b18cbfd2f094b08a3671f7ed42a05a3fbaf01a11bbbd28da30fb52`); `applied_lessons: [L19]`, floor-checked GREEN. **GATE 1: Approved as written.** +2. **`/pharn-dev-grill`** → `GRILL.md` written; 13 registered grillers run; 1 minor advisory finding (a self-contradictory `layer(s)` plan-metadata label); secret scan clean; spec-hash re-verified, no drift. Advisory — presented, chain proceeded regardless (grill gates nothing). +3. **`/pharn-dev-build`** → the one-line `clean()` regex fix applied via Bash (self-locked file), 4 tests added and measured against a live mutant, `SKILLS_VERSION` bumped, `CHANGELOG.md` updated. **`validate` exit: 0 (GREEN).** +4. **`/pharn-dev-regress`** → ran **twice**. First run found a **real regression** (`verdict: "regressions"`, `tests` gate flipped via `check-version-badge.test.mjs` — the plan's original `## Files` omitted the `README.md` badge companion to the `SKILLS_VERSION` bump). Presented at the stop; **human chose to widen scope** rather than abandon. `PLAN.md` amended to add `README.md`, the badge fixed, and the stage **re-run from scratch**. Second run: **`verdict: "no-regressions"`** (exit 0). +5. **`/pharn-dev-verify`** → **`verdict: "PASS"`** (exit 0). All 6 gates green: `test`, `validate`, `lint`, `format:check`, `lint:md`, `structural:pharn/pharn-review/trust-fence/evals/expected/expected-injection-comment.json`. No verifiers registered (`{"registered":0}`) — floor gates only. +6. **`/pharn-dev-review`** → **GREEN**, 0 blocking findings, 1 non-blocking advisory finding (a CHANGELOG compatibility claim stated as fact where `PLAN.md`'s own guarantee audit labels the same claim `advisory`), 1 proposed lesson candidate for a separate `/pharn-dev-memory-promote` decision. + +**Where the run ended:** the end of the chain — GATE 2, below. No RED-verdict stop remains standing (the one that occurred at step 4 was resolved and the stage re-run to a clean verdict, both runs preserved in `REGRESSION.md`). + +## Structural verdicts, verbatim + +- `/pharn-dev-build` → `pharn/floor/validate.mjs .` exit **0** (`FLOOR: GREEN — 36 capabilities checked in .`) +- `/pharn-dev-regress` → `regression-report.json.verdict` = **`"no-regressions"`** (final run; first run's `"regressions"` is preserved in `REGRESSION.md`'s history section, not erased) +- `/pharn-dev-verify` → `verify-report.json.verdict` = **`"PASS"`**, `failing_gates: []` + +## Pointers (cited, not restated) + +- `.dev/features/f15-route-group-scope/GRILL.md` — grill findings (advisory) +- `.dev/features/f15-route-group-scope/REGRESSION.md` — both regress runs, in full +- `.dev/features/f15-route-group-scope/VERIFY.md` — the gate table +- `.dev/features/f15-route-group-scope/REVIEW.md` — the four lens findings + the proposed lesson candidate + +## Files changed (working tree, uncommitted) + +`.claude/hooks/set-writes-scope.cjs`, `.claude/hooks/set-writes-scope.test.cjs`, `CHANGELOG.md`, `README.md`, `SKILLS_VERSION` — plus this feature's own `.dev/features/f15-route-group-scope/**` artifacts. `SKILLS_VERSION`: `2.5.1` → `2.5.2` (patch). + +## The standing decision is the human's + +This record states that the chain ran and the named floor verdicts are as shown above — it is **not** a self-issued "shipped," not an approval, and not a `PHARN ✓ reviewed` seal. **GATE 2, now:** the human decides merge / fix / abandon. Nothing here commits, pushes, or merges anything; the working tree is exactly as left by the chain above, uncommitted, per the one-PR discipline the original build prompt named (branch off `main`, single concern). diff --git a/.dev/features/f15-route-group-scope/VERIFY.md b/.dev/features/f15-route-group-scope/VERIFY.md new file mode 100644 index 0000000..29d01e9 --- /dev/null +++ b/.dev/features/f15-route-group-scope/VERIFY.md @@ -0,0 +1,24 @@ +# VERIFY — f15-route-group-scope + +## Gate results + +| Gate | Exit | +| ------------------------------------------------------------------------------------------ | :--: | +| `test` (`npm test` — hermetic suite, includes the feature's own 37 test cases, 4 new) | 0 | +| `validate` (`node pharn/floor/validate.mjs .` — structural floor) | 0 | +| `lint` (`npm run lint` — eslint) | 0 | +| `format:check` (`npm run format:check` — prettier, whole-repo) | 0 | +| `lint:md` (`npm run lint:md` — markdownlint, whole-repo) | 0 | +| `structural:pharn/pharn-review/trust-fence/evals/expected/expected-injection-comment.json` | 0 | + +## Verdict + +**VERIFIED: floor gates PASS.** All six deterministic gates exited 0. `failing_gates: []`. + +## Verifiers (advisory layer) + +`node pharn/floor/count-verifiers.mjs .` → `{"registered":0,"verifiers":[]}` — **no verifiers registered — floor gates only.** Step 2 was a no-op; the verdict above rests entirely on the floor gate table, with nothing annotated on top of it. + +## Honest residual + +Verified = the named gates passed; this is **not** a guarantee of correctness beyond what those gates check — no verifier concerns exist to annotate further, and none would have changed this verdict even if they had (fix #3: a verifier finding never flips the verdict). The feature-specific correctness signal here is `test` (which collected `set-writes-scope.test.cjs`'s 37 tests, 4 of them new to this increment) and the `structural:*` gate; `validate` / `lint` / `format:check` / `lint:md` are whole-repo, confirming the repo is clean **with** this change present, not merely that the change's own files are clean in isolation. diff --git a/.dev/features/f15-route-group-scope/regression-report.json b/.dev/features/f15-route-group-scope/regression-report.json new file mode 100644 index 0000000..2911475 --- /dev/null +++ b/.dev/features/f15-route-group-scope/regression-report.json @@ -0,0 +1,31 @@ +{ + "base": "c880413ef5e24916c5743e66306b39b3d68e25c9", + "inside": [ + ".claude/hooks/set-writes-scope.cjs", + ".claude/hooks/set-writes-scope.test.cjs", + "CHANGELOG.md", + "README.md", + "SKILLS_VERSION", + ".dev/features/f15-route-group-scope/GRILL.md", + ".dev/features/f15-route-group-scope/PLAN.md", + ".dev/features/f15-route-group-scope/regression-report.json", + ".dev/features/f15-route-group-scope/REGRESSION.md" + ], + "outside_gates": { + "structural:pharn/pharn-review/trust-fence/evals/expected/expected-injection-comment.json": { + "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/f15-route-group-scope/verify-report.json b/.dev/features/f15-route-group-scope/verify-report.json new file mode 100644 index 0000000..258c3b4 --- /dev/null +++ b/.dev/features/f15-route-group-scope/verify-report.json @@ -0,0 +1,14 @@ +{ + "feature": "f15-route-group-scope", + "gates": { + "format:check": 0, + "lint": 0, + "lint:md": 0, + "structural:pharn/pharn-review/trust-fence/evals/expected/expected-injection-comment.json": 0, + "test": 0, + "validate": 0 + }, + "verdict": "PASS", + "failing_gates": [], + "verifiers": { "registered": 0, "findings": [] } +} diff --git a/CHANGELOG.md b/CHANGELOG.md index 5de0dcf..86d5a78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Fixed +- **`set-writes-scope.cjs` no longer mangles a Next.js route-group directory in a `writes:` entry.** Its `clean()` helper strips a trailing " (annotation)" (e.g. " (gated)") from a declared path, but the regex used `\s*` (zero-or-more space) before the paren, so it also matched a path segment that itself legitimately ends in `)` — `app/(marketing)` collapsed to `app/`, and a nested `app/(a)/(b)` would have collapsed the same way. A route-group `writes:` entry therefore silently under-scoped: the build's intended writes under `app/(marketing)/…` fell outside the emitted scope, and `enforce-writes-scope.cjs` denied them — fail-closed on a common, real layout, not a hypothetical one. The regex now requires `\s+` (one-or-more space), which still strips the documented space-separated annotation form but leaves a route-group segment (no leading space before its own paren) intact. + + **Why `\s+` and not removing the strip entirely.** An annotation is always written with a leading space per the function's own doc comment; a route-group segment never has one. Requiring the space is therefore a precise, minimal fix on that one distinguishing axis. Discovery confirmed live that no `writes:` frontmatter in this repo's `.claude/commands/*.md` currently exercises the annotation-strip on a real trailing `)` — the one paren-containing entry (`pharn-build.md`'s placeholder) ends in `>`, not `)`, so it was never reachable either way — so removing the strip was a defensible alternative, but `\s+` is the smaller, zero-cost change and preserves the documented behavior for a future spaced annotation. + + **Verified as a real fix, not just an authored assertion (L4).** The two new "survives intact" tests were confirmed to FAIL when the live regex was reverted to `\s*` (scope collapsed back to `["app/"]` / `["app/(a)"]`), then confirmed to pass again once `\s+` was restored — the mutation was applied and reverted against the actual file, not merely described. + + **The self-lock, unaffected.** `set-writes-scope.cjs` is one of the three hook scripts `protect-trusted-paths.cjs` denies Write/Edit/MultiEdit to (fix #2's control surface); this one-line change was applied via a targeted Bash string replacement (confirmed live: a `Write` to this path still exits 2, denied), exactly as prior fixes to this file's sibling hooks have been. + + **Nothing an existing install newly-REDs.** A route-group `writes:` entry that was silently and incorrectly under-scoped now scopes correctly — a fail-closed-on-a-valid-layout defect becoming correct, never the reverse. A space-separated annotation still strips identically to before. **`SKILLS_VERSION` bumped to `2.5.2` (patch)** — `set-writes-scope.cjs` is a product hook (bump-triggering; it ships as part of the guarded `.claude/` surface), and this corrects a mangle in already-shipped bytes without changing any documented, intentional behavior. + - **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. diff --git a/README.md b/README.md index eda617a..55bedd4 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. -[![pharn](https://img.shields.io/badge/pharn-2.5.1-blue)](./CHANGELOG.md) +[![pharn](https://img.shields.io/badge/pharn-2.5.2-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/SKILLS_VERSION b/SKILLS_VERSION index 73462a5..f225a78 100644 --- a/SKILLS_VERSION +++ b/SKILLS_VERSION @@ -1 +1 @@ -2.5.1 +2.5.2