pharn-dev · PrzemekGalarowicz · Aug 12, 2026 · Aug 12, 2026 · coderabbitai · Aug 12, 2026
@@ -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();
 }
 

@@ -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. ---

@@ -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`.**
@@ -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 `<placeholder>` 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.
@@ -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).