Skip to content

fix(hooks): the HOOKS check reads what Claude Code reads and prints what can be pasted back - #222

Merged
PrzemekGalarowicz merged 1 commit into
mainfrom
claude/optimistic-heisenberg-8rw8ke
Sep 25, 2026
Merged

PrzemekGalarowicz merged 1 commit into
mainfrom
claude/optimistic-heisenberg-8rw8ke

Conversation

@PrzemekGalarowicz

Copy link
Copy Markdown
Contributor

What this changes

Plan D of the second batch from the PHARN-01..18 review, shipped through /pharn-dev-ship. It covers findings F15, F16, F17 and F18, plus the status --strict half of F8.

  • F15: local settings. The check now reads the union of .claude/settings.json and .claude/settings.local.json, which is what Claude Code merges. User-level settings are never read.
    • Hooks wired only locally get a new local-only status. The note names them ("they run for you, not for teammates or CI"), but they do not fail --strict. status.ts now gates on hookWiringFails(diff) instead of on whether a note exists.
    • An unreadable project file is never guessed around: the result is unreadable, and the note names the file.
    • The false "is absent — no PHARN hook is wired" wording is gone.
  • F8, status half: symlinks. A symlinked project settings.json file (dotfiles) is now read through, as Claude Code does. A symlinked .claude/ directory and any upstream symlink are still refused.
  • F18: FIFOs. Every settings file is opened O_NONBLOCK, and fstat refuses non-regular files. A FIFO can no longer block status or update forever. The test runs in a child with a hard timeout, so a regression fails fast instead of hanging CI.
  • F17: paste-ready lines. Each missing hook prints as the hook's own JSON under Event [matcher]:
    Stop: {"type":"command","command":"node","args":["${CLAUDE_PROJECT_DIR}/.claude/hooks/require-loop-record.cjs"]}
    
    Exec form and shell form now read differently, and a pasted line satisfies the check. The round trip is tested, and was verified against the real upstream settings.json.
  • F16: terminal safety. C0, C1 and Unicode-format characters, plus U+2028/2029, become \uXXXX JSON escapes, so the JSON still parses back to the exact upstream string. Each line is capped at 300 characters.

Type of change

  • feat — new stack option, wizard step, or command capability
  • fix — bug fix
  • docs — docs-only change
  • chore / refactor — tooling or internal restructure, no behavior change

Area(s) touched

src/lib/hook-wiring.ts · src/commands/status.ts · tests · docs/commands/status.md, docs/commands/update.md · CLAUDE.md · CHANGELOG · .dev/features/hook-wiring-truth/

Checklist

  • Read the existing file(s) before editing; followed the ESM .js-extension import convention.
  • Updated the matching tests. 15 of the new cases fail on the base.
  • Updated the relevant docs/ pages.
  • Preserved the security invariants: reads are size-capped, regular-file-only and non-blocking; upstream symlinks are refused; project content is never printed; upstream text is only printed escaped.

Quality gates

  • npm run check passes locally (format:check + lint + typecheck + test) — 1568 tests.
  • npm run build succeeds.
  • npm run test:coverage passes (coverage thresholds met).

Notes for the reviewer

  • Pipeline results: validate exit 0, regress no-regressions, verify PASS, review GREEN with 2 minor advisory findings (.dev/features/hook-wiring-truth/REVIEW.md).
  • The GATE 1 decisions applied here: local-only hooks count as wired, with a note; only a symlinked leaf is followed.
  • The init half of F8 (a symlinked settings.json refused by init) ships with plan B.

🤖 Generated with Claude Code

https://claude.ai/code/session_0199owRmYfskqYQVQrVP679o


Generated by Claude Code

…hat can be pasted back

- Reads the union of .claude/settings.json and .claude/settings.local.json
  (what Claude Code merges; user-level settings are never read). Hooks
  wired only locally are `local-only`: named in the note, not failing
  --strict (status.ts now gates on hookWiringFails, not on the note's
  presence). An unreadable project file is never guessed around. The
  "no PHARN hook is wired" claim is gone.
- A symlinked project settings.json FILE is read through, as Claude Code
  does; a symlinked .claude/ and any upstream symlink are still refused.
- Every settings file is opened O_NONBLOCK: a FIFO is refused instead of
  blocking status/update forever (test runs in a child with a timeout).
- Each missing hook prints as the hook's own JSON under `Event [matcher]`,
  so exec form (command + args) and shell form differ and a pasted line
  satisfies the check (round trip tested, and verified on the real
  upstream settings.json). C0/C1/Cf characters and U+2028/2029 become
  \u JSON escapes; each line is capped at 300 characters.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0199owRmYfskqYQVQrVP679o
@coderabbitai

coderabbitai Bot commented Sep 25, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 15c4e9ea-6f49-4eaa-ad30-b3baaf99d3b8


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@PrzemekGalarowicz
PrzemekGalarowicz merged commit 5b63e31 into main Sep 25, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants