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
33 changes: 33 additions & 0 deletions .dev/features/hook-wiring-drift/GRILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# GRILL — hook-wiring-drift

Plan: `.dev/features/hook-wiring-drift/PLAN.md` · spec-hash `bca940a5…d729d3c4e` matches live
`ARCHITECTURE.md`. Registered grillers: `{"registered":0,"grillers":[]}` → inline axes only.

## Findings

```yaml
- type: FINDING
rule_id: 'P5'
severity: important
file: '.dev/features/hook-wiring-drift/PLAN.md:25'
problem: 'Upstream 6.12.0 ships an EXEC-form hook (`command` + `args[]`, no shell). A key built from `command` alone would make every exec-form hook with the same binary (`node`) collide; the normalized entry must include `args`.'
evidence: 'normalizes `hooks.<Event>[].{matcher, hooks[].command}`'
- type: FINDING
rule_id: 'P7'
severity: important
file: '.dev/features/hook-wiring-drift/PLAN.md:31'
problem: 'Making `status --strict` exit 1 on missing upstream hooks turns currently-green CI gates red for every install predating 6.1.0/6.12.0 until the user edits settings.json by hand — intended by the finding, but a user-visible behavior change that belongs in the docs and CHANGELOG.'
evidence: '`--strict` exits 1 when upstream hooks are missing'
- type: FINDING
rule_id: 'P0'
severity: minor
file: '.dev/features/hook-wiring-drift/PLAN.md:57'
problem: 'The equality is purely textual: a user who rewrote the same hook with different quoting is reported as missing. The plan labels this advisory; the note text should say so.'
evidence: 'a differing command string is reported as missing — advisory to the human'
```

## Summary

Report-only design keeps the settings.json invariant. Key normalization must cover exec-form `args`.

ADVISORY VERDICT: 3 concerns raised (0 blocking-severity, 3 advisory) — for the human to weigh before /pharn-dev-build.
71 changes: 71 additions & 0 deletions .dev/features/hook-wiring-drift/PLAN.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
# PLAN — hook-wiring-drift (PHARN-04: upstream hook-wiring changes must be reported, not silent)

- spec_content_hash: bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e # fix #4
- increment: a pure `diffHookWiring` compares the `hooks` block of the project's `.claude/settings.json`
with the fetched upstream one and returns the upstream hook entries (event + matcher + command) the
project lacks; `update` prints a `HOOKS` note naming them, and `status` prints a `HOOKS` section and
counts missing upstream hooks toward `--strict`. `settings.json` itself is still NEVER written.
- layer(s): the CLI itself (`src/lib/hook-wiring.ts` new, `src/commands/{update,status}.ts`)
- constitution_refs: [P0, P1, P2, P3, P4, P5, P7]

## Discovery — verified this run (P6)

Reproduced in the review (`repro/hook-wiring`): `init` at upstream 6.0.0 then `update --yes` to 6.17.1 →
"updated 35 · restored 33 · skipped 0", `.claude/settings.json` byte-identical (relative commands, no
`Stop` hook although `require-loop-record.cjs` was installed), and `status --strict` exit 0 "No drift".
Upstream changed the wiring twice (6.1.0 `CLAUDE_PROJECT_DIR`-anchored commands; 6.12.0 new `Stop` hook).
Live code: `install-capabilities.ts` copies `settings.json` only when absent; `install-manifest.ts`
excludes it, so neither `update` nor `status` ever looks at it. Not overwriting it is documented and stays.

## Files

- `src/lib/hook-wiring.ts` — `diffHookWiring(repoDir, projectRoot)`: reads both `.claude/settings.json`
(`safeJoin` + `findSymlinkComponent` + regular-file + 256 KB cap, JSON parse, all failures → a named
`unreadable`/`absent` status, never a throw); normalizes `hooks.<Event>[].{matcher, hooks[].command}` into
`Event · matcher · command` entries; returns `{ status: 'match' | 'missing' | 'no-upstream' |
'project-absent' | 'unreadable', missing: HookEntry[] }`. Command strings are display-sanitized
(control characters replaced). Pure of `process`/console — layer CLI/lib
- `src/commands/update.ts` — after a successful apply, compute it against the clone and print a
`HOOKS` note when `missing` is non-empty (or the project file is unreadable), naming the entries and
that `settings.json` is never written by pharn — layer CLI/command
- `src/commands/status.ts` — `HOOKS` note in the drift path; `--strict` exits 1 when upstream hooks are
missing (extra user hooks never count) — layer CLI/command
- `tests/hook-wiring.test.ts` — match; missing Stop hook; changed command string counts as missing;
extra user hooks ignored; project absent / unparseable / symlinked / oversized; upstream absent;
control characters sanitized
- `tests/update.test.ts` — old wiring → HOOKS note printed, settings.json byte-identical
- `tests/status.test.ts` — old wiring → HOOKS section; `--strict` exits 1; matching wiring → exit 0
- `docs/commands/update.md` — the protected-files section: `update` reports hook-wiring differences (P4)
- `docs/commands/status.md` — HOOKS section + `--strict` semantics (P4)
- `CLAUDE.md` — status/update paragraphs (P4)

## Contracts satisfied

- `docs/commands/update.md` "What is protected by default": `settings.json` stays user-owned (unchanged);
the defect was the silence, now closed by a report.

## Evals to write (P1)

- per test file above; the lib cases and the `status --strict` exit must fail on the base source.

## Guarantee audit (P0)

- "a missing upstream hook is reported by `status` and fails `--strict`" → floor: set membership over
normalized `Event·matcher·command` strings (exact string compare).
- "pharn never writes `.claude/settings.json` on update" → unchanged; asserted byte-identical in tests.
- What the wiring MEANS (whether a user's differing command is equivalent) → not judged; a differing
command string is reported as missing — advisory to the human, named in the note text.

## Trust audit (P2)

- Upstream `settings.json` is untrusted remote content; the project's is local user content. Both are
parsed as DATA only (never executed), size-capped, symlink-refused, and only command strings reach the
terminal — sanitized of control characters.

## Determinism audit (P5)

- Exact-string set difference; an unparseable file is a named status, never a guess.

## Open questions (HALT)

- none
30 changes: 30 additions & 0 deletions .dev/features/hook-wiring-drift/REGRESSION.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# REGRESSION — hook-wiring-drift

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

## Base and partition

- **base:** `71bc375143fb9666ea6b1725ecbd7612fe774f9e` (`origin/main` at build time; the build is an uncommitted working tree on top of it).
- **inside** (each declared in `PLAN.md` `## Files`): `CLAUDE.md`, `docs/commands/status.md`, `docs/commands/update.md`, `src/commands/status.ts`, `src/commands/update.ts`, `src/lib/hook-wiring.ts`, `tests/hook-wiring.test.ts`, `tests/status.test.ts`, `tests/update.test.ts`.
- **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 stdlib `*.test.mjs` / `*.test.cjs` files + 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.

## 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.
45 changes: 45 additions & 0 deletions .dev/features/hook-wiring-drift/REVIEW.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# REVIEW — hook-wiring-drift

Floor first: `node .dev/floor/validate.mjs .` → exit 0 (GREEN). Everything below is **advisory**.

## Floor-gate findings (blocking)

None.

- **L-floor (P0):** "a missing upstream hook is reported and fails `status --strict`" reduces to an
exact-string set difference; "update never writes settings.json" is asserted byte-identical in a test.
Textual equality is labeled in the user-facing note.
- **L-eval (P1):** 13 lib cases (6.0.0→6.17.1 wiring, exec-form `args`, extra user hooks, absent /
invalid JSON / oversized / symlinked project file, no usable upstream, control-char sanitizing, note
text) + 3 `status` cases + 2 `update` cases; the command cases fail on the base source.
- **L-trust (P2):** both files are size-capped (256 KB), symlink-refused, parsed as data; only command
strings reach the terminal, with C0/C1 control characters replaced.
- **L-axis (P3):** one new lib module with one axis (hook-wiring comparison); `status`/`update` only
consume it.

## Advisory findings

```yaml
- type: FINDING
rule_id: 'P7'
severity: important
file: 'src/commands/status.ts'
problem: "`status --strict` now exits 1 for every install whose settings.json predates upstream's 6.1.0/6.12.0 re-wiring, until the user merges the hooks by hand — the intended fix, but CI pipelines using --strict will turn red on upgrade. Needs a CHANGELOG line."
evidence: 'hookLines !== null'
- type: FINDING
rule_id: 'P5'
severity: minor
file: 'src/commands/update.ts'
problem: "`update`'s same-version early return never clones, so the HOOKS note only appears on runs that actually fetch; `status` is the continuous check."
evidence: 'return { ...applied, hookWiring: diffHookWiring(repo.dir, cwd) };'
- type: FINDING
rule_id: 'P4'
severity: minor
file: 'CHANGELOG.md:8'
problem: "No CHANGELOG `[Unreleased]` entry (not in the plan's `## Files`)."
evidence: '## [Unreleased]'
```

## Verdict

**GREEN** — 0 floor-gate findings, 3 advisory findings. No lesson proposed for canon.
17 changes: 17 additions & 0 deletions .dev/features/hook-wiring-drift/SHIP.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# SHIP — hook-wiring-drift

Stages run, in order: `/pharn-dev-plan` → GATE 1 (human: **Approve as written**) → `/pharn-dev-grill` →
`/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)
- Run ended at **GATE 2**. The human's standing instruction for this batch: open a PR and merge it only if
its CI checks are green.

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.
27 changes: 27 additions & 0 deletions .dev/features/hook-wiring-drift/VERIFY.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# VERIFY — hook-wiring-drift

## FLOOR layer (owns the verdict)

Gates run over the whole repo with the feature present, as a non-root user on node 22 with the session
proxy variables unset (as root with the proxy set, 5 pre-existing tests in `init.test.ts` /
`update.test.ts` fail for environmental reasons, identically at the baseline).

| gate | exit |
| -------------- | ---- |
| `format:check` | 0 |
| `lint` | 0 |
| `lint:md` | 0 |
| `test` | 0 |
| `typecheck` | 0 |
| `validate` | 0 |

No `structural:*` gate — the increment ships no eval-actual pair.

**VERDICT: PASS** (`.dev/floor/check-verify.mjs`, `failing_gates: []`).

## ADVISORY layer

`node .dev/floor/count-verifiers.mjs .` → `{"registered":0,"verifiers":[]}` — floor gates only.

Residual (P0/P7): "verified" means the named gates passed — not that the feature is correct in any sense
the suite does not encode.
27 changes: 27 additions & 0 deletions .dev/features/hook-wiring-drift/regression-report.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{
"base": "71bc375143fb9666ea6b1725ecbd7612fe774f9e",
"inside": [
"CLAUDE.md",
"docs/commands/status.md",
"docs/commands/update.md",
"src/commands/status.ts",
"src/commands/update.ts",
"src/lib/hook-wiring.ts",
"tests/hook-wiring.test.ts",
"tests/status.test.ts",
"tests/update.test.ts"
],
"outside_gates": {
"tests": {
"base": 0,
"head": 0
},
"validate": {
"base": 0,
"head": 0
}
},
"regressions": [],
"pre_existing": [],
"verdict": "no-regressions"
}
17 changes: 17 additions & 0 deletions .dev/features/hook-wiring-drift/verify-report.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"feature": "hook-wiring-drift",
"gates": {
"format:check": 0,
"lint": 0,
"lint:md": 0,
"test": 0,
"typecheck": 0,
"validate": 0
},
"verdict": "PASS",
"failing_gates": [],
"verifiers": {
"registered": 0,
"findings": []
}
}
4 changes: 2 additions & 2 deletions .pharn/writes-scope.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"scope": [
".dev/features/lock-read-modify-write/SHIP.md"
".dev/features/hook-wiring-drift/SHIP.md"
],
"set_by": ".claude/commands/pharn-dev-ship.md",
"set_at": "2026-09-24T08:06:32.625Z"
"set_at": "2026-09-24T08:17:20.630Z"
}
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ ESM-only (`"type": "module"`, NodeNext). **Relative imports must use `.js` exten

**`pharn update` (`commands/update.ts`) is drift-safe by default.** It re-resolves the recorded archetypes and **unions** the result with the user's manual adds (`lib/merge-capabilities.ts` — the pure 9-row membership table: `next = resolve(archetypes) ∪ manual`; sticky manual; a manual entry gone from the index is dropped, its files left alone; a `source`-less legacy entry is inferred ONCE at merge time — in the resolved set → `auto`, outside it → `manual` — which is the ONLY place absence may be resolved). Every membership change is NAMED in a `CAPABILITIES` note (`added` / `dropped-unselected` / `dropped-gone` / `kept-manual`); zero changes print nothing. Resurrection of a removed capability is **reported, not prevented** (no tombstones). It then decides **per file** instead of re-copying wholesale: `lib/install-records.ts` holds `pharn.records.json` (a sha256 of every file an install wrote, hashed at the DEST, stamped with the config's `skillsVersion`/`commit` so a store left by another tool is detected and ignored); `lib/update-decision.ts` is the PURE 6-row table (`decideFileAction` + `planUpdate` — missing→restore, identical→no-op, equals-record→upgrade, else SKIP `modified`/`unrecorded`/`unverifiable`); `lib/apply-update.ts` executes the writes (dest-symlink refusal, parent `mkdir`, and an `ApplyError` carrying what was already written so those files are still recorded on a partial failure); `lib/backup.ts` copies every `--force` casualty to `.pharn-backup/<ts>/` BEFORE any original is touched. Records are written BEFORE the config, and a run that skipped anything **withholds** the `skillsVersion`/`commit` bump so the recorded version stays true and the next run still has work. `--force` overwrites the skip buckets and bypasses the same-version early-return. Update **never deletes** and never touches `.claude/settings.json`. `CONSTITUTION.md` (from `paths.docs` in `lib/install-manifest.ts` — flat: root; pharn layout: `pharn/CONSTITUTION.md`) is in the expected trusted-doc set and follows the same per-file table: missing→restore, still-at-recorded-hash→upgrade, locally modified→skip (`modified`); `add`/`remove` never touch it. It records the layout detected in the CLONE (closing the latent drift where bytes landed at `pharn/` paths while the config still said `flat`).

**`pharn status` (`commands/status.ts`) is strictly read-only** — it never writes, deletes, or overwrites (fixing is `update`/`add`). Two sections: a **version** check (installed `skillsVersion` vs the upstream `SKILLS_VERSION` file, plus an archetype + capability-count summary) and a **drift** check. Default clones `@main` once and reuses it for both; `--no-drift` skips the clone and uses `fetchRemoteSkillsVersion` for the version section only; `--strict` exits 1 on any outdated/modified/missing (CI gate, default exit 0). Cleanup runs in a `finally`, and every `process.exit` happens _after_ it. The pure (no I/O) engine is **`lib/diff.ts` → `diffInstalledCapabilities`**: it mirrors `installCapabilities` to derive the **expected** file set — the selected capability dirs + the fixed product surfaces, at the recorded `layout` — then `sha256`-compares each against the project root, returning `{modified, missing, okCount}`. Every read is `safeJoin`-guarded (from `lib/validate.ts`). `.claude/settings.json` is user-owned (preserved at install) and excluded; the copied-verbatim trusted docs (including `CONSTITUTION.md` / `pharn/CONSTITUTION.md`), hooks, contracts, and floor checkers ARE compared. Drift is derived live from the clone, always against `@main`, never the pinned `commit`. `status` does NOT read `pharn.records.json`, so it is a report, not a preview: a file it lists as differing may be cleanly upgraded OR skipped — only `update` can tell those apart.
**`pharn status` (`commands/status.ts`) is strictly read-only** — it never writes, deletes, or overwrites (fixing is `update`/`add`). Two sections: a **version** check (installed `skillsVersion` vs the upstream `SKILLS_VERSION` file, plus an archetype + capability-count summary) and a **drift** check. Default clones `@main` once and reuses it for both; `--no-drift` skips the clone and uses `fetchRemoteSkillsVersion` for the version section only; `--strict` exits 1 on any outdated/modified/missing (CI gate, default exit 0). Cleanup runs in a `finally`, and every `process.exit` happens _after_ it. The pure (no I/O) engine is **`lib/diff.ts` → `diffInstalledCapabilities`**: it mirrors `installCapabilities` to derive the **expected** file set — the selected capability dirs + the fixed product surfaces, at the recorded `layout` — then `sha256`-compares each against the project root, returning `{modified, missing, okCount}`. Every read is `safeJoin`-guarded (from `lib/validate.ts`). `.claude/settings.json` is user-owned (preserved at install) and excluded from the file diff — but its `hooks` block is compared by `lib/hook-wiring.ts` (`diffHookWiring`: exact-string set difference over `Event · matcher · command · args`, both files size-capped + symlink-refused, command strings control-char-sanitized for display); upstream hooks the project does not wire print a `HOOKS` note in `status` (and fail `--strict`) and in `update` (report only — `update` still never writes the file); the copied-verbatim trusted docs (including `CONSTITUTION.md` / `pharn/CONSTITUTION.md`), hooks, contracts, and floor checkers ARE compared. Drift is derived live from the clone, always against `@main`, never the pinned `commit`. `status` does NOT read `pharn.records.json`, so it is a report, not a preview: a file it lists as differing may be cleanly upgraded OR skipped — only `update` can tell those apart.

## Testing

Expand Down
Loading
Loading