fix(claude-config): repair the inert Too slow gate and drop the unsourced hook threshold - #4152
Conversation
…rced hook threshold (#4145) The `audit-automation-gaps` quality gate required a tool to exceed 30s AND the hook to block. `PostToolUse` can never block, so a `PostToolUse` candidate cleared the timing gate at any latency, including this repository's own measured 13,225 ms in-repo `PostToolUse` Write. The gate now judges each event against its actual cost: a blocked call where the event can block, turn latency where it cannot. The 15-30 second threshold it cited has no upstream basis. No official Claude Code page documents a hook latency budget, and the figure was 15-30x this repository's own ceiling of 1s typical / 2s worst case per tool call. Timing now resolves the consuming repository's documented budget first and labels any fallback this skill supplies as a house rule. The stale figure is removed from all three files that carried it, including the eval that would otherwise grade against a rule the skill no longer states. The new `context/hook-timing.md` spoke carries the budget-resolution ladder, the per-event cost table, the documented levers, and dated upstream-fact records for the 600-second command-hook `timeout` default and the `PostToolUse` no-block behavior, per the upstream-drift convention. Closes #4145 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0114NEwLXgKoQfaPCEcKU8R3
Base moved 57 commits. Two conflicts, both in claude-config: - plugin.json: main rewrote the description (em-dash purge) and advanced the version to 0.44.0. Took main's description and re-based this branch's bump on top as 0.44.1. - CHANGELOG.md: main shipped its own 0.40.41 (the resolve-convention-home BOM fix), colliding with the entry this branch had added at that number. Main's 0.40.41 is restored unchanged and this branch's entry moved to 0.44.1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0114NEwLXgKoQfaPCEcKU8R3
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Claude finished @kyle-sexton's task in 5m 0s —— View job
|
|
Claude finished @kyle-sexton's task in 55s —— View job Security review complete
Skip gate: PR is open, but the change has no security-relevant surface. All six changed files are documentation/config for the
None of these introduce or touch command execution, authorization/access control, injection surfaces (command/SQL/path/template), token/secret handling, or GitHub Actions workflow configuration. This is purely documentation content advising a skill's own guidance text — no logic, trust-boundary, or data-flow change to review. No security issues found. Nothing to report. |
|
Last security-reviewed head: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f34310b7a3
ℹ️ 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".
|
Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
…he absence-claim sources Two findings from the Codex review on #4152, both confirmed against the file. The three-rung ladder could not produce a verdict in its most common case. Phase 2.1 always measures, so a repository with no documented budget landed on rung 2, which supplied a measurement but no threshold; rung 3's one-second ceiling was reachable only when BOTH the documentation and the measurement were absent, which never happens after a successful timing run. The `Too slow` gate compares a measured cost against a resolved budget, so it had nothing to compare against. The ladder is now two rungs: the measurement is named as the left side of the comparison rather than a rung, and the house-rule fallback applies whenever the repository documents no ceiling. The absence claim named four documentation pages in prose with no pointers, so a reader could not re-fetch the corpus that backs the central premise. Each page is now linked, the check performed is stated, and the two nearest event-scoped statements are named so the claim's boundary is visible. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0114NEwLXgKoQfaPCEcKU8R3
|
Note on the red That run was the contract-only No lane failed an assertion. Nothing was skipped or disabled to get past it. The current head Generated by Claude Code |
The lint lane's purged-em-dashes gate declares plugins/claude-config/skills/*/context/*.md already purged, so the new hook-timing spoke's H1 was a regression the moment it landed. Rewrite it to the colon form its sibling gap-analysis.md already uses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0114NEwLXgKoQfaPCEcKU8R3
…ngs, and guard the posture (#4159) Closes #4146 ## Summary Implements the design batch for `/claude-config:audit-automation-gaps`: an inventory that undercounted by two orders of magnitude, findings that nothing persisted, gates driven by keyword counts mistaken for frequencies, and a refusal gate that ran in the context which generated the proposals. All nine items are addressed. The three rows marked needs sign-off got researched verdicts rather than ratification, and one ships as a refusal. ## Fix **Inventory.** `scripts/inventory.sh` read only the project `.claude` tree, reporting `Hook scripts: 3 / Skills: 0 / Agents: 0 / MCP servers: 0 / Plugins enabled: 1` for a repository carrying 77 plugin roots, 271 skills and 93 wired handlers. That output is injected as pre-computed context, so the model anchored on it before the audit began. It now emits a per-scope table over all seven documented hook locations: a status vocabulary where a scope it could not read never reads as zero, a standing versus conditional split, enablement inputs plus a pointer rather than a computed verdict, managed policy probed rather than guessed, and a cloud-session note. **Findings persistence.** New `scripts/findings-state.sh` stores the verdict table, evidence and plans so `--implement` has something to read later, keyed per project through the shared `lib/state-key.sh` so one checkout never reads another's verdicts. The audit writes twice, once when verdicts are presented and once after the human selects, so `--implement` acts on what was chosen rather than on every candidate the skill happened to pass. **Incident-count discipline.** A `git log --grep` count drove two gates directly. Both rows now treat it as a ceiling: a frequency claim needs a sample reported with its denominator and sample size, while a ceiling already under 5 percent still settles YAGNI without one. **Shift-left carve-out** ships with three falsifiable conjuncts and the hook budget as a hard gate, so a consumer documenting a budget with no headroom keeps the REJECT. **Hook enumerator: does not ship.** Recording the `/hooks` registry verdict is the stated prerequisite, and this repository's `native-references` convention puts that verdict in the human-gated category. The body records the deliberate absence instead of a registry row this change is not entitled to write. Also: presence-gated routing to `overengineering:audit` both ways, a routing line for the declined `ci` category, a Gotchas surface, a `## Next` section, and a fresh-context judge for the refusal gate. ## Verification Three independent review passes found sixteen findings, all closed. Four were blocking and every repository gate passed them: a symlinked `.claude/skills` reported `absent` and `0`; an unusable project root printed this repository's counts under the requested root's name; five concurrent writers on one run id produced five successes and one surviving file; and a multi-document payload validated in full while persisting only its first document. The most useful finding was about the tests. Mutation testing showed `inventory.test.sh` staying green against a script pinned to print 7, 999 or 0, so a suite that could not detect a confident wrong number was guarding a script whose entire purpose is not producing one. inventory.test.sh 34 -> 125 checks, 32 mutations, all caught findings-state.test.sh 101 -> 156 checks, 16 mutations, all caught check-skill PASS, 0 errors, 0 warnings (was 2 warnings) em-dash, changelog parity/order/bump, evals schema and quality clean shellcheck, shfmt, portability clean CI on the merged head green Keying is proved rather than asserted: two repositories writing the same run id into one plugin-data root land in different keys and each reads back its own, and a mutation removing the key from the path makes one repository serve the other's artifact, so the suite fails if the keying ever stops carrying weight. Three figures in the issue did not survive checking: the `overengineering` description is 1202 codepoints rather than 1217, the 200-line soft target it cites is not what the gate enforces, and it names a documentation path renamed before this work began. ## Related Refs #4145 (the factual-correction half, merged as #4152) 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_0114NEwLXgKoQfaPCEcKU8R3
Closes #4145
Summary
The
audit-automation-gapsskill'sToo slowquality gate could not fire for the most common hookclass, and the timing threshold it cited had no upstream basis and contradicted this repository's
own hook budget by more than an order of magnitude. Both were found by running the skill against
this repository and then verifying every claim against the official Claude Code documentation.
Fix
The gate was structurally inert for
PostToolUse. It requiredTool exceeds 30s AND the hook must block. The hooks reference statesPostToolUsecannot block, because the tool already ran, sothe conjunct was never satisfiable and a
PostToolUseformatter cleared the gate at any latency.This repository has measured an in-repo
PostToolUseWrite at 13,225 ms. The gate now judges eachevent against its actual cost: a blocked call where the event can block, turn latency where it
cannot.
The threshold was unsourced. No official page documents a hook latency budget; there is no
performance section in the hooks documentation at all. Meanwhile
docs/conventions/hook-budget/README.mdsets 1 s typical and 2 s worst case per tool call and states the budget never relaxes. Timing now
resolves the consuming repository's documented budget first, and labels any fallback this skill
supplies as a house rule rather than guidance.
The stale figure lived in three files, not one. All are updated, including the eval that would
otherwise grade against a rule the skill no longer states:
SKILL.mdcontext/gap-analysis.mdevals/evals.jsoneval 1New spoke.
context/hook-timing.mdcarries the budget-resolution ladder, the per-event costtable, the documented levers (narrow the
matcher, add anifrule,async: true, explicittimeout), and dated upstream-fact records per theupstream-driftconvention for the 600-secondcommand-hook
timeoutdefault and thePostToolUseno-block behavior. Putting the detail in aspoke keeps
SKILL.mdfrom growing against its 200-line soft target.Verification
Re-run after merging
origin/main(headf34310b7):The two remaining
check-skill.shwarnings (no Gotchas surface, 200-line soft target) predate thischange and are recorded in #4146 rather than folded in here.
Merge note
origin/mainhad advanced 57 commits, leaving this branch un-mergeable. Both conflicts were inclaude-configand are resolved inf34310b7:plugin.json: main rewrote the description and advanced to 0.44.0. Main's description is takenas-is and this branch's bump re-based on top as 0.44.1.
CHANGELOG.md: main shipped its own 0.40.41 (theresolve-convention-homeBOM fix), collidingwith the number this branch had used. Main's 0.40.41 is restored unchanged and this branch's
entry moved to 0.44.1.
Related
Refs #4146 (the design batch, deliberately separated so this correction does not wait on contested
work; it also tracks the pre-existing warnings above)
🤖 Generated with Claude Code
https://claude.ai/code/session_0114NEwLXgKoQfaPCEcKU8R3