[kyle-sexton]

Refs: #7613

PR B of the check-placement rollout; it implements the git-hook registration contract that #7544 records. #7613 holds two review findings deferred to later phases (foreign shared entries before hooks-path; pre-push blob reads).

Summary

Plugins had no way to put their checks in a git hook, so report-only findings could only reach the model per edit. This PR ships the shared installer and check library every check plugin vendors, and moves typos-format, the pilot plugin, from a per-edit report to a pre-commit entry on changed lines, with an opt-in end-of-turn check. A three-arm pilot (30 headless runs) found no regression and set the end-of-turn mode to off by default.

Fix

• lib/git-hooks/install.mjs (detect, list, plan, copy, decline, verify; lefthook, husky and plain adapters) and lib/git-hooks/check-lib.sh (staged files, changed lines, push range, checks.skipmissing, agent detection, partly staged refusal), vendored per ADR 0019. Scripts only do mechanical steps; edits to user-owned hook-manager config come back as decisions the setup agent applies.
• Repository contents are untrusted: copy refuses a target that is or passes through a symlink (unsafe-path), writes by temp file and rename, reads repository files through one bounded, no-follow, regular-file-only reader, and treats only the exact dispatcher as its own (any other hook carrying the sentinel is modified).
• typos-format: per-edit PostToolUse row and its write mode removed; pre-commit entry typos-check with the bundled config as an asset (a file staged then deleted from the work tree is read from the index); typos_format_end_of_turn Stop row, default off; setup gains install-git-hooks and a drift check in check; jq is no longer required.
• The SessionStart prerequisites probe reports git-hook drift (stale, modified, missing) once per session, so no new SessionStart row; this changes the vendored prerequisites.mjs in 69 plugins (68 fragments, user-experience bumped to 0.1.8).

Verification

• bash lib/git-hooks/install.test.sh (runs node --test lib/git-hooks/*.test.mjs; the plan's node --test lib/git-hooks/ form does not resolve a directory on Node 24), check-lib.test.sh, typos-check.test.sh, typos-format.test.sh, end-of-turn: all pass; real lefthook 2.2.1, husky 9.1.7 and plain-hook commits block on a new-line typo and pass an untouched one.
• Review fixes: each new test (symlinked targets, symlinked directory, /dev/zero and oversize reads, sentinel-only hook, staged-then-deleted file, missing jq) fails against the pre-fix code and passes on the head.
• scripts/check-affected.sh --tests passes on the head; sync-shared-copies.sh --check and --check-bump origin/main, check-fixture-git-isolation.sh, check-silent-skips.sh, check-pipefail-grep-q.sh, generate-catalog.mjs --check, ratchet.py check: all pass.
• Fresh-context code and security reviews of the merged diff; findings fixed above or deferred to git-hooks installer: surface foreign shared entries before hooks-path #7613.
• Left out: run inside a Claude Code session, actionlint-check.test.sh Case 9 fails on main too, because the session's CLAUDE_PLUGIN_DATA/CLAUDE_CODE_SESSION_ID leak into the test; it passes with them unset. Windows host timing not measured.

Related

• chore(testing): move the test judge off hooks and record check placement #7544 (merged) records the rubric and this contract.
• git-hooks installer: surface foreign shared entries before hooks-path #7613: deferred review findings.
• The other check plugins and guardrails follow in later PRs of this rollout.

🤖 Generated with Claude Code

https://claude.ai/code/session_011Wn4wZmSz8fVYJLNgFG8Zz

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-16T15:34:45.378886Z	095fde0	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[claude]

Claude finished @kyle-sexton's task in 5m 2s —— View job

---

Security review in progress

• Gather context (PR diff, changed files, custom instructions)
• Identify request: /review:security-review invocation per custom instructions
• Read core security-relevant code (lib/git-hooks/install.mjs, lib/git-hooks/check-lib.sh, lib/prerequisites.mjs, typos-check.sh, typos-format.sh)
• Check instruction-surface deletions (docs/conventions changes, deleted schema file) — none found; doc edits track removed code, not removed guardrails
• Fan out hunters across diff slices with adversarial verification
• Post inline findings (if any)
• Final summary

[claude]

Claude finished @kyle-sexton's task in 8m 35s —— View job

---

Code review complete

• Gather context (PR body, diff, custom instructions)
• Resolve project standards / REVIEW.md criteria (a security lane runs on this repo, so security findings are left to it per REVIEW.md)
• Review lib/git-hooks/install.mjs and check-lib.sh
• Review lib/prerequisites.mjs drift-check changes
• Review typos-format plugin changes (pre-commit hook, config asset, Stop row)
• Spot-check vendored copies and doc updates across plugins
• Verify candidate findings adversarially (review:lane-verifier)
• Post findings

I focused on the hand-written logic — lib/git-hooks/install.mjs, lib/git-hooks/check-lib.sh, the typos-format pre-commit/Stop hooks, and lib/prerequisites.mjs's new gitHookDrift — plus the accompanying doc/changelog updates, since the other ~90 changed files are either generated vendored copies (sync-shared-copies.sh --check is author-claimed and unverified by me) or changelog fragments.

The symlink/unsafe-path handling, the staged-then-deleted-file fix, the partly-staged checks, and the checks.skipmissing propagation all read correctly and match their stated test coverage. One doc inconsistency surfaced:

🟡 Stale cross-reference left behind in evidence-packet.md. This PR removes typos-format's in-session write capability entirely (hooks/hooks.json drops its only PostToolUse/Write|Edit row; hooks/typos-format.sh shrinks to a report-only --end-of-turn path). The author correctly swept most references to this in the same PR — docs/conventions/hook-observability/README.md, docs/conventions/hook-precision/README.md, docs/catalog.md, and even two other paragraphs in this exact file (plugins/plugin-quality/skills/audit/reference/evidence-packet.md lines ~188–201 and plugins/plugin-quality/scripts/packet-seal.sh) were updated to stop describing typos-format as an in-place rewriter. But one sentence a few lines below the edited hunk in the same file was missed:

claude-code-plugins/plugins/plugin-quality/skills/audit/reference/evidence-packet.md

Lines 222 to 224 in 095fde0

	These escapes do not hold: a non-`.md` extension evades `markdown-format` but not
	`typos-format` when the project-dir gate holds, and it breaks closed-set basenames; a
	`typos` allowlist / `markdownlint` opt-out is unreliable on the `$HOME`-rooted residual;

"These escapes do not hold: a non-.md extension evades markdown-format but not typos-format when the project-dir gate holds..."

This is no longer true: typos-format has no write/rewrite path left at all (not just outside .md), so it can't serve as the backstop this sentence claims. Since this PR's own change elsewhere in the file is what made the sentence wrong, and it demonstrably swept the rest of the file for the same issue, this isn't a pre-existing/out-of-scope item — worth a follow-up edit to drop or rephrase the typos-format half of that escape.

Fix this →

No other findings met the bar for this lane. Nothing here is a correctness bug in the shipped installer, check library, or hook scripts.

claude-lane-summary: lane=claude-review sha=095fde024d7a950f3ccecc306a85b9d62376cd31 run=38924358258

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 095fde024d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

claude-review has reviewed this pull request through d2fdb67; a later push is reviewed from there.

[claude]

Claude finished @kyle-sexton's task in 4m 32s —— View job

---

Code review complete

• Gather context (incremental diff, custom instructions, REVIEW.md)
• Review lib/git-hooks/install.mjs (and its byte-identical plugin copies): marker-content wiring check, stage() failure propagation, stage-failed status
• Review plugins/typos-format/git-hooks/typos-check.sh: run_typos verdict-exit-code check, staged_mirror nested-config fix
• Review accompanying tests (install.test.mjs, typos-check.test.sh, typos-format-end-of-turn.test.sh)
• Review doc/registry updates (check-placement/README.md, evidence-packet.md, setup/SKILL.md, cross-plugin-source-registry.txt)
• Verify candidate findings adversarially (none met the bar, so no fan-out was needed)
• Post findings

This incremental slice is a tightly scoped fix-up of the three review findings raised against 095fde0:

1. typos-check.sh's run_typos now captures typos's own exit status outside a pipe and fails closed (exit 1) on anything other than 0 (clean) or 2 (findings) — fixing the earlier bug where a config typos couldn't read produced an empty, unflagged "pass" (the pipe's exit status previously came from sed, which always succeeds). New tests cover both the whole-file and partly-staged paths.
2. staged_mirror replaces the old stdin-based typos_excluded/pipe approach: it now materializes the staged blob at its real relative path inside a scratch tree that also carries every nested typos.toml/_typos.toml/etc. on the way down, so a nested allow-list or exclude governs the staged copy exactly as it would the working-tree file. The new partly-staged/staged-then-deleted test under a nested _typos.toml exercises this directly.
3. install.mjs wiring() (and all vendored copies) now compares the plain marker's content against the expected <plugin>/<script> text instead of only checking existence, so a marker edited to point at a different or missing script is correctly reported as drift (verify → missing/"installed but not wired") rather than a false "wired: true". Covered by the new "marker naming another script reads as drift" test.
4. stage() now returns whether git add succeeded, and copy() returns a new stage-failed status (propagated to the CLI's exit code) instead of silently reporting installed/refreshed when staging fails (e.g. an ignored path or a locked index). Covered by the new index.lock test.
5. evidence-packet.md drops the now-false claim that typos-format still backstops a non-.md extension escaping markdown-format, consistent with this PR's removal of typos-format's in-session write path elsewhere in the same file.

All five fixes read correctly, match their stated contracts in docs/conventions/check-placement/README.md, and are exercised by a new test apiece. The cross-plugin-source-registry.txt additions for lib/git-hooks/install.mjs / check-lib.sh follow the file's existing path-within-plugin convention. I looked for a resulting correctness gap in the stage(info, exec, true) && stage(info, [...]) short-circuit (a first-batch staging failure skips attempting the second batch), but on inspection git add fails atomically per invocation and either outcome is already surfaced as the same stage-failed status, so there's no observable incorrect result from it — not reportable at this lane's bar.

No findings to report for this incremental slice.

claude-lane-summary: lane=claude-review sha=63d57e7e8ed3ef983c57a49e5e3c5fbf8ae1509d run=38926179168

[claude]

Claude finished @kyle-sexton's task in 3m 48s —— View job

---

Security review complete

• Gather context (PR diff at HEAD 63d57e7, custom instructions)
• Check skip gate (prior reviews existed but were against an earlier commit; new commits landed since, so a fresh pass was needed)
• Review lib/git-hooks/install.mjs and check-lib.sh security-relevant logic
• Review typos-check.sh and prerequisites drift-check changes
• Check deleted/modified docs and conventions for instruction-surface deletions
• Fan out hunters with adversarial verification
• Post inline findings (if any)
• Final summary

This PR was already through one round of review (Codex + claude-review on commit 095fde0), and the two commits added since then — a0fe7ec (refuse symlinked targets and bound repository reads) and 63d57e7 (block on a typos run with no verdict, report failed staging) — are fix commits for that round's findings, so I focused on verifying those fixes actually close the gaps and checked for anything new they introduce.

Prior findings, verified fixed at 63d57e7:

• Codex P1 (typos-check.sh): a typos exit status outside the documented clean (0) / findings (2) range now exits the hook with 1 instead of silently passing — run_typos checks rc explicitly.
• Codex P2 (typos-check.sh, nested config): staged_mirror now writes the index blob to its own relative path inside the scratch mirror (via git show ":$f" >"$root/$f") instead of piping through stdin, so path-anchored nested typos.toml configs apply — L43-L60.
• Codex P2 (install.mjs, marker validation): wiring() now compares the marker's full content against the expected <plugin>/<script> text rather than just checking existence, so an edited/retargeted marker reads as drift — L378-L382.
• Codex P2 (install.mjs, staging failure): stage() now returns whether git add succeeded, and copy() reports a new stage-failed status instead of silently reporting installed/refreshed — L625-L666.

All four fixes carry regression tests added in the same commits (install.test.mjs: marker-drift and stage-failure cases; typos-check.test.sh: no-verdict and nested-config cases), and each test is written to fail against the pre-fix code per the PR's own verification notes.

Fresh pass over the current head: the symlink defenses in install.mjs (symlinkOnPath/unsafeTarget, checked before every write; writes go through a temp-file-then-rename beside the target) correctly cover every file copy() touches, including the marker, dispatcher, and .gitattributes. readText's bounded, O_NOFOLLOW-protected reads are applied consistently wherever repository content is read. The typos-check.sh partial/whole-file split and checks_filter_findings line-range gating still only report on lines the commit actually adds. I didn't find anything new introduced by the two latest commits, and no instruction-surface deletions (the deleted hook-telemetry/data/typos-format.schema.json and doc edits track the removed PostToolUse write hook, not a removed guardrail with nothing replacing it).

No new security findings to report.

claude-lane-summary: lane=claude-security-review sha=63d57e7e8ed3ef983c57a49e5e3c5fbf8ae1509d run=38926179446

[github-actions]

claude-security-review has reviewed this pull request through d2fdb67; a later push is reviewed from there.

[claude]

Claude finished @kyle-sexton's task in 28s —— View job

---

Security review complete

• Gather context (incremental diff, custom instructions)
• Read the one changed file: plugins/typos-format/hooks/typos-format.test.sh
• Assess security relevance of the change
• Post findings
• Final summary

This lane's scope for this pass was narrowed to exactly one file changed since the last review at 63d57e7: plugins/typos-format/hooks/typos-format.test.sh. The only diff is:

   PG_WORK="$(mktemp -d)"
+  trap 'rm -rf "$PG_WORK"' EXIT
   mkdir -p "$PG_WORK/sysbin" "$PG_WORK/cwd"

This adds an EXIT trap that removes the scratch directory the probe-gate test case creates (a symlink farm mirroring system binaries, used to simulate the SessionStart probe's PATH). $PG_WORK comes from mktemp -d immediately above, so it's not attacker-influenced, and the trap only runs inside this test script's own process — it doesn't touch the shipped hook, the installer, or anything executed outside the test harness. This is test hygiene (leftover scratch-directory cleanup), not a change to any trust boundary, input-handling path, or shipped artifact.

No security findings for this incremental slice.

claude-lane-summary: lane=claude-security-review sha=d2fdb679257e484a7ba11538481c23d7bf40b19b run=38927390232

[claude]

Claude finished @kyle-sexton's task in 52s —— View job

---

Code review complete

• Gather context (incremental diff, custom instructions)
• Review the one changed file: plugins/typos-format/hooks/typos-format.test.sh
• Check for trap conflicts / scratch dir reuse elsewhere in the script
• Post findings

This incremental slice only adds one line to the probe-gate block in plugins/typos-format/hooks/typos-format.test.sh:

claude-code-plugins/plugins/typos-format/hooks/typos-format.test.sh

Lines 88 to 89 in d2fdb67

	PG_WORK="$(mktemp -d)"
	trap 'rm -rf "$PG_WORK"' EXIT

   PG_WORK="$(mktemp -d)"
+  trap 'rm -rf "$PG_WORK"' EXIT
   mkdir -p "$PG_WORK/sysbin" "$PG_WORK/cwd"

This is the only trap in the script, so there's no conflict with a prior handler, and it's set after PG_WORK is assigned, so it never references an unset variable. Since the script runs under set -u (not -e), an unbound-variable reference or external interruption anywhere in the probe-gate block (lines 90-138) could previously have exited the script before reaching the explicit rm -rf "${PG_WORK:?}" at line 139, leaking the scratch directory; the new trap now covers that path. The existing explicit cleanup at line 139 isn't made redundant by this — it still frees the directory immediately once the probe-gate subtests finish, while the trap is purely the safety net for early-exit paths, so keeping both is correct, not leftover duplication.

No correctness or maintainability issues found in this slice.

claude-lane-summary: lane=claude-review sha=d2fdb679257e484a7ba11538481c23d7bf40b19b run=38927390291