Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions .dev/features/hook-wiring-truth/GRILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# GRILL — hook-wiring-truth

Plan: `.dev/features/hook-wiring-truth/PLAN.md`. Spec hash recomputed:
`bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e` — **matches**. Registered
grillers: `{"registered":0,"grillers":[]}` → inline axes only. The plan is `trust: untrusted`;
nothing in it read as an instruction.

## Findings

### Honest scope / completeness (P7, P3)

```yaml
- type: FINDING
rule_id: 'P3'
severity: important
file: '.dev/features/hook-wiring-truth/PLAN.md:84'
problem: 'status.ts fails --strict on `hookLines !== null` — on the PRESENCE of a HOOKS note (status.ts:139-147). A local-only note that must NOT fail --strict (GATE 1 answer 1 → a) therefore needs the strict test to read the diff status instead, and src/commands/status.ts is not in ## Files. Declare it, or the build cannot land the approved behaviour without escaping scope.'
evidence: '`tests/status.test.ts` — layer tests. `--strict` exits 0 when the hooks are wired only in'
```

### Determinism (P5)

```yaml
- type: FINDING
rule_id: 'P5'
severity: minor
file: '.dev/features/hook-wiring-truth/PLAN.md:62'
problem: 'The union does not say what an UNREADABLE file contributes. If settings.json is malformed and settings.local.json wires everything, is that "match" or "unreadable"? Keep the conservative answer: any unreadable project file makes the result unreadable, naming WHICH file, because pharn cannot know what it wires. Decide it in the table rather than leaving it to the build.'
evidence: "The diff reads the project's `settings.json` AND `settings.local.json`; the set of wired hooks"
```

### Guarantee audit (P0)

```yaml
- type: FINDING
rule_id: 'P0'
severity: minor
file: '.dev/features/hook-wiring-truth/PLAN.md:67'
problem: 'U+2028 / U+2029 (LINE and PARAGRAPH SEPARATOR, category Zl/Zp, not Cf) pass both terminalSafe and JSON.stringify raw. Escape them too, so the "nothing raw that moves the cursor" claim holds for the whole line.'
evidence: 'It is built as JSON, then every character the shared sanitizer would'
```

### Checked, no finding

- **Trust (P2).** Upstream strings reach the terminal only escaped and capped. Following a
project-side symlinked leaf reads the user's own target as data. Its content is never printed:
only upstream entries are.
- **Eval coverage (P1).** Each finding has a failing-on-base case listed. The FIFO case needs no
child process: with `O_NONBLOCK` the open returns at once, and on the base it hangs past the
vitest timeout, which is the failure.

## Summary

Right idea, one scope gap. The approved "local-only counts as wired, with a note" cannot be built
without touching the strict gate in `status.ts`, so the plan must declare that file. The union also
needs an explicit rule for an unreadable file. The escape set should cover the two line separators.

**ADVISORY VERDICT: 3 concerns raised (0 blocking-severity, 1 important, 2 minor) — for the human to
weigh before /pharn-dev-build.**
142 changes: 142 additions & 0 deletions .dev/features/hook-wiring-truth/PLAN.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
# PLAN — hook-wiring-truth (the HOOKS check reads what Claude Code reads, and prints what can be pasted back)

- spec_content_hash: bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e # fix #4
- increment: `lib/hook-wiring.ts` changes in five ways.
1. The project side also reads `.claude/settings.local.json` (F15).
2. A symlinked `.claude/settings.json` LEAF on the project side is followed for reading (the
`status --strict` half of F8).
3. Every file is opened non-blocking, so a FIFO is refused instead of hanging (F18).
4. Each missing hook prints as the exact JSON object that satisfies the check, exec form and shell
form kept distinct (F17).
5. That rendering is terminal-safe and length-capped through the shared sanitizer (F16).

The note's heading only says what is true.

- layer(s): the CLI itself (`src/lib/hook-wiring.ts`), docs
- constitution_refs: [P0, P1, P2, P5, P6]

## Discovery — verified this run (P6)

Read on HEAD `abb274a`. `hook-wiring.ts` is unchanged since the review range.

- **F18.** `readSettings` (`:62-110`) calls `openSync(…, 'r')`, with no `O_NONBLOCK`. On a FIFO,
`open` blocks before `fstat` can refuse it. The hardened readers already use
`O_RDONLY | O_NONBLOCK` (`capability-index.ts:167-173`, `detect-archetype.ts:180-184`).
- **F8, status half.** `readSettings` walks `findSymlinkComponent`, and any symlinked component,
leaf included, makes it `unreadable` → a HOOKS note → `--strict` exit 1 (`status.ts:137-147`). That
happens even when the linked file wires every hook. The project-side content is never printed:
only UPSTREAM entries reach the terminal. The upstream side is a clone written by our own
extractor, and that side keeps its refusal.
- **F15.** Only `.claude/settings.json` is read. Claude Code merges hooks from `settings.json`,
`settings.local.json` (gitignored, per-user) and user-level `~/.claude/settings.json`. Hooks wired
in `settings.local.json` are reported missing, and with `settings.json` absent the heading says
"is absent — no PHARN hook is wired" (`:185-186`), which is false. None of this is documented
(`docs/commands/status.md:78-80`).
- **F16.** `displayHook` (`:170-174`) strips `[\x00-\x1f\x7f-\x9f]` with its own regex. It keeps
`\p{Cf}` (U+202E, U+2066, U+200B) and has no length cap. `terminalSafe` (`terminal-safe.ts:27-39`)
is the shared sanitizer and does both.
- **F17.** `displayHook` joins `[command, ...args]` with spaces. `entryKey` (`:143-145`) compares
`[event, matcher, command, args]`, so exec and shell forms are different keys. Upstream's real
`Stop` hook is exec form, and the other two are shell form:

```json
{
"type": "command",
"command": "node",
"args": ["${CLAUDE_PROJECT_DIR}/.claude/hooks/require-loop-record.cjs"],
"timeout": 10
}
```

(from a git-archive of pharn-oss). The exec-form hook prints as `Stop: node ${CLAUDE_PROJECT_DIR}/…`.
Pasted back as `command`, that is shell form, so it stays "missing".

## Files

- `src/lib/hook-wiring.ts` — layer CLI/lib. Changes:
- Every settings read opens with `O_RDONLY | O_NONBLOCK`, and the upstream side adds
`O_NOFOLLOW`. `fstat` still refuses anything but a regular file, so a FIFO is refused instead of
blocking forever (F18).
- Project side: a symlinked LEAF is followed; a symlinked intermediate directory is still refused
(GATE 1 answer 2 → a). Upstream side: the refusal stays as it is.
- The diff reads the project's `settings.json` AND `settings.local.json`; the set of wired hooks
is their union (F15). Each missing entry is still upstream data. The result names the upstream
hooks wired ONLY in `settings.local.json` (GATE 1 answer 1 → a: they count as wired, with a
note). An UNREADABLE project file makes the whole result unreadable, naming which file — pharn
cannot know what it wires (grill finding 2).
- The display line is `<event> [<matcher>]: {"type":"command","command":…,"args":[…]}`, args
only when present (F17). It is built as JSON, then every character the shared sanitizer would
strip is replaced by its backslash-u escape: still valid JSON, parsing back to the exact
upstream string, with nothing raw reaching the terminal (F16). U+2028 / U+2029 are escaped too
(grill finding 3). The event and matcher get the same escaping. Each line is capped at 300 characters with an ellipsis.
- Headings say "not wired in `.claude/settings.json` or `.claude/settings.local.json`". The
"absent" heading is used only when neither file exists, and makes no claim about other scopes.
- `tests/hook-wiring.test.ts` — layer tests. FAIL on base:
- FIFO at `settings.json` → unreadable, within a bounded runtime (on base it hangs).
- Hooks only in `settings.local.json` → match.
- Symlinked leaf to a file wiring every hook → match.
- U+202E / U+200B in an upstream command → the line holds their escape text and no raw Cf
character.
- A 200 000-character command → a line of at most 301 characters.
- Exec-form Stop hook → the printed JSON, parsed and pasted into a project `settings.json`, makes
the diff return match (the round trip F17 lacked).
- Guards: a symlinked `.claude` dir is still unreadable; the upstream-side symlink is still
refused.
- `src/commands/status.ts` — layer CLI/commands. `--strict` fails on the diff STATUS (missing or
unreadable), no longer on the mere presence of a HOOKS note, so the local-only note prints without
failing it (grill finding 1).
- `tests/status.test.ts` — layer tests. `--strict` exits 0 when the hooks are wired only in
`settings.local.json` (FAILS on base), and the note names them as local-only.
- `tests/update.test.ts` — layer tests. Only if a heading assertion there changes.
- `docs/commands/status.md` — layer docs. The HOOKS check reads `settings.json` +
`settings.local.json`, never user-level settings; it notes local-only wiring and prints
paste-ready JSON.
- `docs/commands/update.md` — layer docs. The same, for update's HOOKS note.
- `CLAUDE.md` — layer docs. The `lib/hook-wiring.ts` sentence ("both files size-capped +
symlink-refused…") becomes the new read rules.
- `CHANGELOG.md` — `[Unreleased]` → `### Fixed` (local settings, symlinked settings, paste-able
lines, FIFO) and `### Security` (format characters and the cap in the HOOKS note)

## Contracts satisfied

- PHARN-17's "one display sanitizer" (`terminal-safe.ts` header) now covers the HOOKS note, and
PHARN-14/15's non-blocking open rule now covers this reader too (cited, P4).

## Evals to write (P1)

- Listed under Files. Seven cases FAIL on the base.

## Guarantee audit (P0)

- "no Cf, C0 or C1 character from upstream reaches the HOOKS note" → floor: an escape pass over the
same `\p{Cf}`/C0/C1 class `terminalSafe` uses, plus a test.
- "a printed line satisfies the check when pasted" → floor: the round-trip test. Advisory beyond
the tested shapes.
- "a FIFO cannot hang `status`/`update`" → floor: `O_NONBLOCK` + `fstat`, plus a test.
- "local-only wiring passes `--strict`" → floor: a test. Whether that is the right policy is the
human's call (open question 1).

## Trust audit (P2)

- Upstream `settings.json` is untrusted. Its strings reach the terminal only escaped and capped.
- The project files are the user's own, read as data and never printed. Following their symlinked
leaf reveals nothing: a target that is not JSON is `unreadable`, and its content never reaches
output.

## Determinism audit (P5)

- Exact-string set membership, as today. Union is set union. No fallback guess.

## Open questions (HALT)

None open. Resolved at GATE 1 (human, 2026-09-25): every question below → **(a)**, the
recommended answer. Kept for the record:

1. Hooks wired only in `settings.local.json`. (a) They count as wired, so `--strict` passes, and the
note says they apply to you only, not to teammates or CI — recommended. CI never has that file,
so CI's answer is unchanged. (b) They are shown as local-only but `--strict` still fails, keeping
local runs at parity with CI. (c) Keep reading `settings.json` only, and fix just the false
wording and the docs.
2. A symlinked project `.claude/settings.json`. (a) Follow a symlinked LEAF only (the dotfiles case
reported); a symlinked `.claude/` dir stays refused — recommended. (b) Follow any project-side
symlink, since this is a read-only path. (c) Keep refusing, and say so in the note.
41 changes: 41 additions & 0 deletions .dev/features/hook-wiring-truth/REGRESSION.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# REGRESSION — hook-wiring-truth

The verdict below is computed by `.dev/floor/check-regress.mjs`, not by this stage's judgment.

## Base and partition

- **base:** `36ecf7003aa4fa9d3d9d755c73e15f20a0715f90` (`HEAD` — `origin/main` after #221; the
build is an uncommitted working tree on top of it).
- **inside** (each declared in `PLAN.md` `## Files`):
- `src/lib/hook-wiring.ts`, `src/commands/status.ts`
- `tests/hook-wiring.test.ts`, `tests/status.test.ts`, `tests/update.test.ts`
- `docs/commands/status.md`, `docs/commands/update.md`
- `CLAUDE.md`, `CHANGELOG.md`
- **scope partition:** `check-regress.mjs scope` exited **0**, `escaped: []`. `.pharn/` (hook
scratch) and this feature's own stage artifacts are not build output.
- **outside gates:** the 46 stdlib `*.test.mjs` / `*.test.cjs` files `scope` returned (754 tests) +
whole-repo `validate`; 0 committed eval pairs.
- **style-gate skip:** `inside` touches no shared style config, so `lint` / `format:check` /
`lint:md` are absent from both maps.
- **environment:** both sides ran with no proxy variables and as root **without**
`CAP_DAC_OVERRIDE` / `CAP_DAC_READ_SEARCH` / `CAP_FOWNER` (`setpriv`), the CI-equivalent of this
root sandbox.

## Per-gate exit codes

| gate | base | head | flipped? |
| ---------- | ---- | ---- | -------- |
| `tests` | 0 | 0 | no |
| `validate` | 0 | 0 | no |

- `regressions[]`: **empty**
- `pre_existing[]`: **empty**

## Verdict

**REGRESSIONS: none — no deterministically-detectable breakage outside the feature.**
(`regression-report.json` `.verdict` = `no-regressions`.)

Residual (P0/P7): this catches exactly what its suite catches. The vitest suite exercising `src/**`
is owned by `/pharn-dev-build`'s floor and `/pharn-dev-verify`. This certifies the comparison, never
the increment.
64 changes: 64 additions & 0 deletions .dev/features/hook-wiring-truth/REVIEW.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# REVIEW — hook-wiring-truth

Increment:

- `src/lib/hook-wiring.ts`:
- reads the UNION of `.claude/settings.json` and `.claude/settings.local.json`;
- adds a `local-only` status and a `hookWiringFails` predicate;
- opens every file `O_NONBLOCK` (upstream also `O_NOFOLLOW`);
- follows a symlinked project FILE but not a symlinked `.claude/`;
- prints each missing hook as its own JSON, escaped over the shared sanitizer's class plus
U+2028/2029, capped at 300 characters.
- `src/commands/status.ts`: `--strict` reads the diff status instead of the note's presence.
- Tests in three files, `docs/commands/status.md`, `docs/commands/update.md`, CLAUDE.md and
CHANGELOG.

Treated as `trust: untrusted`; nothing in it read as an instruction.

## Floor first (P0)

`node .dev/floor/validate.mjs .` → `FLOOR: GREEN` (exit 0). `/pharn-dev-build`'s `npm run check`
passed (1568 tests), `/pharn-dev-regress` returned `no-regressions`, and `/pharn-dev-verify`
returned `PASS`.

## Floor-gate findings (blocking)

None.

- **L-floor (P0)** — each claim has a test:
- "no format, line-separator or control character from upstream reaches the note" → the escape
class plus tests over C0, C1, Cf and U+2028/2029;
- "a printed hook satisfies the check when pasted" → the round-trip test over the NEW fixture;
VERIFY repeated it on the real upstream file;
- "a FIFO cannot hang status/update" → `O_NONBLOCK` + `fstat`, plus a child-process test with a
hard timeout;
- "local-only passes `--strict`" → the `status` test;
- "an unreadable file is never guessed around" → a test that names the file.
- **L-eval (P1)** — 15 cases failed on the base. The rest are guards: a symlinked `.claude/`, a
symlinked upstream file, C0 escaping, and split wiring.
- **L-trust (P2)** — upstream strings reach the terminal only escaped and capped. Project files are
data and never printed. Following a symlinked project FILE reads the user's own target; what
reaches output from it is at most the status and a fixed reason string.
- **L-axis (P3)** — the reading and display rules stay in `hook-wiring.ts`. `status.ts` gains one
predicate call, and its gate now asks the module that owns the rule.

## Advisory findings (warn — severity is this reviewer's judgment, fix #3)

```yaml
- type: FINDING
rule_id: 'P5'
severity: minor
file: 'src/lib/hook-wiring.ts:224'
problem: 'The two casts narrow `ReadResult` to ok|absent after the loop that returns on `unreadable`. They are correct today, but the compiler no longer checks the invariant; a later edit that drops the early return would pass typecheck. A small narrowing helper would keep it checked.'
evidence: "const inShared = wiredKeys(shared as ReadResult & { kind: 'ok' | 'absent' });"
- type: FINDING
rule_id: 'P2'
severity: minor
file: 'src/lib/hook-wiring.ts:254'
problem: "The escape class is built from code points so no raw U+2028/2029 sits in the source. It duplicates terminal-safe.ts's class with two additions. If that class ever widens, this one will not follow unless edited too. A shared export would single-source it; kept local to avoid widening terminal-safe.ts's API in this increment."
evidence: 'const ESCAPED = new RegExp('
```

## Verdict

**GREEN — 0 floor-gate findings, 2 advisory (minor).** The standing decision is the human's (GATE 2).
29 changes: 29 additions & 0 deletions .dev/features/hook-wiring-truth/SHIP.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# SHIP — hook-wiring-truth

Stages run, in order:

1. `/pharn-dev-plan` → GATE 1 (human: plans A–F accepted with every recommended answer).
2. `/pharn-dev-grill`.
3. The plan's `## Files` was rewritten for the writes-scope parser. Raw U+202E/U+200B characters had
crept in where escape text was meant, and they were removed. The grill findings were folded in:
- `src/commands/status.ts` declared, since the approved local-only behaviour needs the strict
gate to read the diff status;
- the unreadable-file rule;
- U+2028/2029 escaping.

Intent unchanged.

4. `/pharn-dev-build` → `/pharn-dev-regress` → `/pharn-dev-verify` → `/pharn-dev-review` → GATE 2.

| stage | structural verdict (verbatim) |
| -------------------- | ------------------------------------------------------ |
| `/pharn-dev-build` | `node .dev/floor/validate.mjs .` exit `0` |
| `/pharn-dev-regress` | `regression-report.json` `.verdict` = `no-regressions` |
| `/pharn-dev-verify` | `verify-report.json` `.verdict` = `PASS` |

- Review: [`REVIEW.md`](REVIEW.md) · Grill (advisory): [`GRILL.md`](GRILL.md)
- The run ended at **GATE 2**. The human's standing instruction for this batch: after each
increment, open a pull request and merge it once its checks are green, then start the next plan.

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