docs(conventions): add hook-precision convention (false-positive discipline) by kyle-sexton · Pull Request #761 · melodic-software/claude-code-plugins

kyle-sexton · 2026-07-20T21:51:50Z

No linked issue — lands the ratified Option A gap of #547 (the umbrella stays open while member fixes ride their own issues). Convention doc at docs/conventions/hook-precision/, sibling to hook-telemetry, structure mirroring seam-phrasing (discipline doc, README-only — versioning is git history per that precedent); one registry row added to PLUGIN-PHILOSOPHY.

Worked-example claims verified against source at the branch base (quoted-span stripping in block-hook-bypass, buffered stdin in cli-flag-verify, single-marker gate in flag-commit-pr-skill-bypass); rules not yet landed everywhere (diff-scoping, toplevel gate) are stated as rules, not claimed as adopters. markdownlint 0 errors; relative links resolve.

Follow-up (deferred to avoid a wave-3 collision on the guardrails README): a one-line plugin-side pointer from plugins/guardrails/ to the convention.

…ipline) Guardrails hooks repeatedly shipped false-positive over-fires, each found live by a babysit worker, hand-filed, and fixed in isolation. This codifies the prevention layer the umbrella ratified: five precision rules generalized from the observed over-fire classes (diff-scoping, structural matching over token co-occurrence, bounded stdin, either/or canonicality markers, toplevel-based path gating) plus the two-half discipline — every filed production false positive becomes a repro-first MUST-stay-quiet case in the hook's co-located contract test. No new harness: the existing per-hook test files are the corpus. Registered in the PLUGIN-PHILOSOPHY convention registry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

claude · 2026-07-20T21:52:08Z

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

PR Review — docs(conventions): add hook-precision convention

Gather context
Read changed files
Review convention doc for correctness and completeness
Verify PLUGIN-PHILOSOPHY.md registry entry
Check alignment with CLAUDE.md guidelines

Summary

Docs-only PR. The structure, discipline framing, and "what this is not" scoping are solid. Two issues found: one factual error in the adopter attribution that contradicts the PR body's own "verified against source" claim, and one minor ordering nit.

Finding 1 — Rule 4 adopter is the wrong hook (moderate)

docs/conventions/hook-precision/README.md L71–73:

- `flag-commit-pr-skill-bypass` gates on the canonical `-F -` marker alone, having dropped the `--trailer`
  conjunct it once required (rule 4).

This is factually wrong. flag-commit-pr-skill-bypass.sh no longer handles git commit at all — that logic moved to block-noncanonical-commit.sh. The hook that now gates on the -F - marker (and dropped the --trailer conjunct) is block-noncanonical-commit. The flag-commit-pr-skill-bypass.sh file itself says so in its header (lines 24–26):

"That hook [block-noncanonical-commit] also drops the --trailer conjunct this one used to require"

And block-noncanonical-commit.sh lines 6–15 state the rule explicitly:

"WHAT IT ENFORCES: git commit -F -… WHY NOT --trailer: the trailer is POLICY, not mechanic."

The correct adopter row should read:

- `block-noncanonical-commit` gates on the `-F -` stdin-form marker alone (rule 4), having shed the
  `--trailer` conjunct that `flag-commit-pr-skill-bypass` once required — the trailer is policy, not
  mechanic, and gating on it silenced the skill's own canonical output under `trailer_policy: none`.

The PR body explicitly says "Worked-example claims verified against source at the branch base" — this is one of the three verification targets and it maps to the wrong hook.

Fix this →

Finding 2 — Registry row order (nit)

docs/PLUGIN-PHILOSOPHY.md L281:

The new row is inserted after "Hook telemetry", but alphabetically "Hook precision" (p) should sort before "Hook telemetry" (t). The table isn't globally strict, but the two Hook rows being transposed within their own group stands out. Worth swapping unless there's a precedent-over-alpha reason to keep "telemetry" first (e.g., temporal adoption order).

What looks good

All three relative links resolve — ../../PLUGIN-PHILOSOPHY.md, ../../../plugins/guardrails/hooks/guardrails-test-helpers.sh, and ../../../plugins/guardrails/hooks/hardcoded-path-check.test.sh all exist at the referenced paths.
Rule 2 claim verified: block-hook-bypass.sh has a strip_literals() function at line 90 that strips single- and double-quoted spans before the executable-token scan. ✅
Rule 3 claim verified: cli-flag-verify.sh line 53 uses hook::buffer_stdin. ✅
"What this convention is not" section correctly scopes out the golden-fixture harness non-starter, true-positive tuning, and deference to the philosophy's posture rules — all accurate boundaries.
Discipline section is unambiguous: repro-first, stay-quiet cases committed before the fix, not after. The "already green before the fix guards nothing" sentence closes the most common escape hatch.
Conformance section's CI claim is accurate — the *.test.sh files are present next to each hook.
README-only structure, git-history versioning — consistent with seam-phrasing precedent.
Registry entry text and path are correct.

Finding 1 is a factual error in a doc the PR explicitly claims was verified. I'd want that corrected before merge. Finding 2 is a nit and can go either way.

The -F - single-marker gate lives in block-noncanonical-commit since the commit-handling moved out of flag-commit-pr-skill-bypass; the doc named the predecessor hook as the adopter. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

claude · 2026-07-20T21:58:42Z

Claude finished @kyle-sexton's task in 1m 22s —— View job

PR Review — docs(conventions): add hook-precision convention (follow-up)

Gather context (prior review findings, latest commit)
Read current state of docs/conventions/hook-precision/README.md
Read current state of docs/PLUGIN-PHILOSOPHY.md
Verify Finding 1 fix (rule-4 adopter correction)
Check Finding 2 status (registry row order)

Finding 1 — Fixed ✅

docs/conventions/hook-precision/README.md L75–76:

- `block-noncanonical-commit` gates on the canonical `-F -` marker alone, dropping the `--trailer`
  conjunct its advisory predecessor required (rule 4).

The correct hook is now named. Verified against block-noncanonical-commit.sh lines 10–15, which explicitly states "WHY NOT --trailer: the trailer is POLICY, not mechanic." The phrasing "its advisory predecessor required" accurately refers to flag-commit-pr-skill-bypass's former conjunction requirement. Description is factually correct.

Finding 2 — Acknowledged nit, still open

docs/PLUGIN-PHILOSOPHY.md L280–281:

"Hook precision" (p) remains after "Hook telemetry" (t) in the registry. The prior review flagged this as a nit and noted it "can go either way" — not a blocker.

Verdict

The one factual error the prior review flagged as a blocker is corrected and verified against source. All three worked-example claims now hold: quoted-span stripping in block-hook-bypass (rule 2), buffered stdin in cli-flag-verify (rule 3), and single-marker gate in block-noncanonical-commit (rule 4). The doc is ready to merge.

kyle-sexton · 2026-07-20T22:11:50Z

Babysit pass (safe tier) — classification of the review findings on this docs-only PR:

#	Finding	Classification	Evidence
1	Rule 4 adopter named the wrong hook (`flag-commit-pr-skill-bypass` → `block-noncanonical-commit`)	VALID — already fixed	Corrected at head `1dcf226` (README L75–76 now names `block-noncanonical-commit`); the follow-up `review / review` run verified it against `block-noncanonical-commit.sh` L10–15.
2	Registry rows: "Hook precision" (p) sorts after "Hook telemetry" (t)	INCORRECT (decline) — table is adoption-order, not alphabetical	The registry is not globally alphabetical: it runs Topic-docs → Lifecycle → Shared-hook-util → Cross-plugin → Consumer-config → Ecosystem → Hook telemetry → … → Seam phrasing (last). Rows append in adoption order, and the new row correctly lands directly after the related `hook-telemetry` row. Alphabetizing would break the tables actual convention. The follow-up review already noted this exemption ("temporal adoption order") and marked it a non-blocking nit.

No branch change required — CI is green and there are no inline threads to resolve.

## Summary Part of #836 (epic #830, sub-item 6) — this PR does not close it (fleet adoption is a separate follow-up, see Next). Lands the `hook-observability` owner doc — the first of two PRs for #836, following the `hook-precision` precedent (`d0805dc8fc`, PR #761: doc-only commit, fleet adoption deferred to a follow-up) and `docs/PLUGIN-PHILOSOPHY.md`'s own registry rule: *"A new cross-plugin convention lands in an owner doc before a second plugin adopts it."* ## Design decisions - **Corrects the epic brief's framing.** Brief item 6 says hooks "emit `statusMessage`." Fresh fetch of <https://code.claude.com/docs/en/hooks> (2026-07-22) shows `statusMessage` is a static `hooks.json` handler-config field (sibling of `type`/`command`/`timeout`), not a runtime JSON-output field. The doc states this correction explicitly. - **`systemMessage` scope, precisely bounded.** Required only for a missing-runtime-prerequisite silent-skip (the existing doctrine at `lib/hook-utils.sh:26-30`, now generalized fleet-wide). Explicitly *not* required for exit-2 blocking paths (already user-visible via Claude Code's own permission-denial UI) or for legitimate agent-only advisory findings (`additionalContext` is correct there). - **Telemetry scope, precisely bounded.** Required for every meaningful outcome (a check that ran and returned ok/blocked/skipped-for-cause), not for pure inapplicability short-circuits (wrong tool, excluded path, missing prerequisite) — verified empirically against all 8 existing telemetry-emitting guardrails hooks, all of which already follow this shape. - **Grounds the "local envelope, not real OTel export" design** in the documented fact that Claude Code strips `OTEL_*` exporter env vars from every hook subprocess — a hook cannot emit real OTel even if it tried. - **`prompt_id` correlation deferred**, not included — it's a `hook-telemetry` schema change (`schema_version` 1.0 → 1.1) touching ~25 producer call sites, not a `hook-observability` concern. Filed as #930. - **Documents the `check-silent-skips.sh` gate correction**, and the `statusMessage`/`systemMessage`/telemetry rollout, as explicitly pending work for the follow-up PR — the doc states current state honestly rather than describing not-yet-landed adoption as done. ## Review history - Codex (round 1, 2 findings, fixed in `c0105d3a37`): the doc described the `statusMessage` rollout and the `check-silent-skips.sh` gate correction in present tense as already complete — neither has landed yet. Reworded both as explicitly pending. - Codex (round 2, 1 finding, fixed in `72aa0a37ad`): "24 wired producer hooks" was wrong — actual count via `grep -rc '"type": "command"' plugins/*/hooks/hooks.json` is 27 across the 12 touched plugins. Fixed in the doc and the plan. - Codex (round 3, 3 findings, fixed in `06602d3c27`): (1) the same 24→27 count reappeared in a not-yet-resolved thread — confirmed already fixed; (2) "systemMessage already implemented fleet-wide" conflated the composing helpers existing fleet-wide with actual adoption at every skip site — reworded; (3) "telemetry on every exit path" doesn't match the fleet's actual (and correct) shape — verified empirically across all 8 telemetry-emitting guardrails hooks that pre-`emit_tel` exits are pure inapplicability short-circuits, corrected the rule to "every meaningful outcome." - Codex (round 4, 1 finding, fixed in `4c57feb6f5`): `docs/topics/836-hook-observability/PLAN.md` staying tracked through both this PR and the follow-up violates `docs/conventions/topic-docs/README.md`'s contract-tier rule — "committed on the task branch only; pruned before merge." Pruned in this PR; the follow-up recreates its own scoped `PLAN.md` on its own branch and prunes it before its own merge, same pattern. ## Next Fleet adoption (statusMessage across 27 wired producer hooks in 12 plugins, systemMessage fixes for 11 genuine gaps, 1 telemetry gap, and the silent-skip-gate correction) lands in a follow-up PR that closes #836, branched off main once this merges. ## Related - #836 — epic sub-item this PR is part of (not closed by this PR) - #930 — deferred `prompt_id`-correlation follow-up - #761 (`d0805dc8fc`) — `hook-precision` convention, the doc-first-then-adopt precedent this PR's structure matches <details> <summary>Final PLAN.md (topic doc, pruned from the tree in this PR's last commit — preserved here per convention)</summary> # Plan: #836 — hook-observability fleet convention + fleet adoption ## Brief Issue #836 (epic #830, sub-item 6 of `docs/topics/lint-static-analysis-gaps/PLAN.md`, lines 37-41): > Hook-observability fleet convention — every fleet hook emits `statusMessage` (during run), > `systemMessage` (failure/notable action), and the hook-telemetry OTel envelope. Grounded in > current official hooks docs at authoring time (no native user-visible hook UI exists as of > 2026-07-21; OTel events + author-emitted messages are the sanctioned surfaces). Optional > sub-item: upstream feature request for a native verbose-hooks UI toggle. Acceptance criteria (PLAN.md lines 64-65): *"Hook-observability convention documented as an owner doc (convention registry row) and adopted by every fleet hook; conformance audited."* ## Brief-said-X / docs-say-Y / so-we-did-Z (mandatory correction) The brief says every hook "emits `statusMessage`". Fresh fetch of <https://code.claude.com/docs/en/hooks> (2026-07-22) shows `statusMessage` is a static field on the hooks.json handler object — sibling of `type`/`command`/`timeout`/`if`/`once` — not a runtime JSON-output field a hook script emits on stdout. It is "a custom spinner message displayed while the hook runs," declared once at config time. So we corrected the mechanism: fleet adoption of `statusMessage` is a `hooks.json` config edit, not a shell-script change. This does not change the acceptance criterion's intent (a live status label during hook execution) — only the implementation surface. ## Research findings (fresh, cited) Source: <https://code.claude.com/docs/en/hooks>, fetched 2026-07-22. 1. **`statusMessage`** — handler-object config field (`hooks.json`), optional, no default. Spinner label shown while the hook process runs. 2. **`systemMessage`** — JSON output field (exit 0), "warning message shown to the user," 10,000 char cap, immediate effect. The composing helpers (`hook::emit_channels` / `hook::emit_skip_notice`, `lib/hook-utils.sh:58,74`) exist fleet-wide and are already callable by every hook — adoption at every missing-prerequisite skip site is not yet complete; see "systemMessage — 11 genuine gaps" below for the sites still on stderr-only or `additionalContext`-only. 3. **Exit-code display semantics** (load-bearing for scoping "notable action" below): - Exit 0: stdout parsed as JSON if present; stderr is ignored — never shown to user or agent on exit 0. - Exit 2: stderr fed to Claude as an error / shown to user depending on event; for `PreToolUse` this blocks the tool call — the block itself is the user-visible surface (via Claude Code's own permission-denial UI), independent of any `systemMessage`. 4. **OTel correlation** — hook input JSON carries `prompt_id` (v2.1.196+), which matches the `prompt.id` attribute on real OpenTelemetry events, enabling external correlation. Not adopted in this lane — see "Deferred: prompt_id correlation" below. 5. **Why the envelope is local-file, not real OTel export** — Claude Code strips all `OTEL_*` exporter environment variables from every hook subprocess it spawns (documented at `/docs/en/monitoring-usage#administrator-configuration`). A hook process cannot emit real OTel telemetry even if it wanted to; `hook::emit_telemetry`'s file-sink envelope is the only surface available to a hook. This convention doc states that rationale explicitly so it reads as a grounded design choice, not an oversight. ## Deferred: prompt_id correlation Adding `prompt_id` to the telemetry envelope (`hook::emit_telemetry`'s `data` object, or a new schema field) is a genuine improvement — it would let external tooling correlate a hook's local telemetry with the same turn's real OTel events. It requires either a new parameter on `hook::emit_telemetry` (`lib/hook-utils.sh`, the synced SSOT) or updating every producer's `data_json` construction (25 call sites) to extract and pass it. Bundling it into #836 would: - Be a `hook-telemetry` schema change (bump `schema_version` 1.0 → 1.1), not a `hook-observability` concern — different owner doc, different issue. - Force either an inconsistent partial rollout (some producers populate `prompt_id`, others don't — indistinguishable from "genuinely absent, pre-first-input" per the docs) or a 25-file sweep unrelated to this issue's three-surface scope. Filed separately, not fixed here. `prompt_id`-correlation stays out of #836; tracked as #930. ## Two-PR structure (precedent-matched) Per `docs/PLUGIN-PHILOSOPHY.md:272-274`: "A new cross-plugin convention lands in an owner doc before a second plugin adopts it." Confirmed via git history: `hook-precision` (`d0805dc8fc`, PR #761) landed as a doc-only commit (README + one registry row), with fleet adoption explicitly deferred to follow-up work ("member fixes ride their own issues"). Matching that precedent: - **PR A — convention doc** (this PR). `docs/conventions/hook-observability/README.md` (new, owner doc) + one row in `docs/PLUGIN-PHILOSOPHY.md`'s Convention registry table. Body: "Part of #836" (not "Closes" — the issue's acceptance criteria require adoption too). - **PR B — fleet adoption.** Branches off main after PR A merges, so hooks.json/scripts can cite the merged doc. Closes #836. Recreates its own scoped `PLAN.md` on its own branch, pruned before its own merge (topic-docs contract-tier rule — see Review history). ## PR B scope (second lane, after PR A merges) Zero `lib/hook-utils.sh` (SSOT) edits — verified: every fix uses an existing helper (`hook::emit_skip_notice`, `hook::emit_telemetry`, `hook::require_jq`). This collapses the collision risk against open PR #903 (also touches `lib/hook-utils.sh`) to zero for this lane. ### statusMessage — mechanical, all 27 wired producer hooks, 12 plugins Add a `statusMessage` field to every `command`-type handler object in each plugin's `hooks.json`. One line per handler, present-tense gerund wording. Plugins touched (hooks/ present): actionlint, bash-format, biome-format, claude-ops, desktop-notification, eol-normalizer, go-format, guardrails, markdown-format, powershell-format, ruff-format, typos-format. Each gets a patch-level `plugin.json` version bump + CHANGELOG entry. ### systemMessage — 11 genuine gaps All 9 use `hook::require_jq <event> "guardrails" "$INPUT"` (not raw `emit_skip_notice` — needs the once-per-session gate `require_jq` wraps, or a broad matcher spams the notice on every invocation): - `plugins/guardrails/hooks/block-dangerous-git.sh:47-50` - `plugins/guardrails/hooks/block-hook-bypass.sh:45-48` - `plugins/guardrails/hooks/block-no-verify.sh:45-48` - `plugins/guardrails/hooks/block-noncanonical-commit.sh:72-75` - `plugins/guardrails/hooks/cli-flag-verify.sh:38-41` (jq-missing) and `:78` (bundled-verifier missing — currently fully silent; paired with manual `hook::notice_once` since `require_jq` doesn't fit a non-jq prerequisite) - `plugins/guardrails/hooks/flag-commit-pr-skill-bypass.sh:56-59` - `plugins/guardrails/hooks/hardcoded-path-check.sh:40-43` - `plugins/guardrails/hooks/secret-pattern-detection.sh:33-36` - `plugins/guardrails/hooks/workflow-resilience-check.sh:24-27` (secondary gap, bundled with its telemetry fix below) Convert agent-only skip branches to dual-channel: - `plugins/claude-ops/hooks/skill-usage-audit.sh:42-43,58-59` - `plugins/claude-ops/hooks/skill-usage-expansion-audit.sh:53-54,71-72` No change to the 6 pure-telemetry claude-ops emitters or guardrails' exit-2 block paths (already correct per the doc's scoping rules). Bundled: tighten `scripts/check-silent-skips.sh`'s `is_visible()` to drop bare `>&2` as a sanctioned signal (verified safe — the only 9 fleet sites relying on that leniency are the 9 converted above) and flip its corresponding test fixture. ### Telemetry — 1 genuine gap, precisely scoped `plugins/guardrails/hooks/workflow-resilience-check.sh` has zero telemetry calls anywhere, including its meaningful outcomes (fan-out detected, throttle applied, advisory issued) — unlike every sibling guardrails hook. Add a `hook::emit_telemetry` call at each meaningful exit; its pure-inapplicability exits correctly need none, matching the sibling pattern. ### Housekeeping bundled into PR B Reconcile `docs/conventions/hook-telemetry/README.md`'s stale Implementers table (omits actionlint, biome-format, eol-normalizer, powershell-format, several guardrails hooks, skill-usage-expansion-audit). ### Test plan (PR B) - `claude plugin validate --strict` per touched plugin (statusMessage acceptance already smoke- tested against go-format — passes). - Each converted hook: existing `*.test.sh` extended with a jq-missing-path case asserting `systemMessage` is now non-empty. - `scripts/validate-plugin-contracts.mjs` full run. - Local CI-equivalent gate sweep: hygiene, changelog-parity-gate, hook-utils-sync (expect no diff — confirms zero-SSOT-edit empirically), silent-skip-gate (post-tightening), skill-quality-gate. ## Open decisions / not yet locked - Exact `statusMessage` wording per hook — draft during PR B implementation (27 short strings). - Whether the upstream native-verbose-hooks-UI feature request (brief's optional sub-item) gets filed externally or noted as declined — decide at PR B close-out. </details> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

kyle-sexton mentioned this pull request Jul 20, 2026

guardrails: cli-flag-verify PostToolUse hook scans whole file, warns on pre-existing lines outside the edited hunk #543

Closed

kyle-sexton merged commit d0805dc into main Jul 20, 2026
20 checks passed

kyle-sexton deleted the docs/547-hook-precision-convention branch July 20, 2026 23:24

This was referenced Jul 20, 2026

ops: loop-lane telemetry — running per-lane status (babysit / triage / work) #502

Closed

docs(conventions): add hook-observability owner doc #931

Merged

kyle-sexton mentioned this pull request Jul 23, 2026

conventions: audit provenance/ratification of docs/conventions/* seams (human vs agent-as-kyle-sexton) #1187

Closed

Navigation Menu

Uh oh!

docs(conventions): add hook-precision convention (false-positive discipline) - #761

docs(conventions): add hook-precision convention (false-positive discipline)#761
kyle-sexton merged 2 commits into
mainfrom
docs/547-hook-precision-convention

kyle-sexton commented Jul 20, 2026

Uh oh!

claude Bot commented Jul 20, 2026 •

edited

Loading

Uh oh!

claude Bot commented Jul 20, 2026 •

edited

Loading

Uh oh!

kyle-sexton commented Jul 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

kyle-sexton commented Jul 20, 2026

Related

Uh oh!

claude Bot commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review — docs(conventions): add hook-precision convention

Summary

Finding 1 — Rule 4 adopter is the wrong hook (moderate)

Finding 2 — Registry row order (nit)

What looks good

Uh oh!

claude Bot commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review — docs(conventions): add hook-precision convention (follow-up)

Finding 1 — Fixed ✅

Finding 2 — Acknowledged nit, still open

Verdict

Uh oh!

kyle-sexton commented Jul 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

claude Bot commented Jul 20, 2026 •

edited

Loading

claude Bot commented Jul 20, 2026 •

edited

Loading