fix(claude-config): defer hook blockability to the upstream per-event exit-2 table - #2041
Conversation
… exit-2 table The exit-2 bullet in audit-instructions' conflict-criteria.md split hook events into six "blockable" and five "non-blockable" names and read as exhaustive. The hooks page's "Exit code 2 behavior per event" table documents far more, and this repository's own hooks register five events in neither list — ConfigChange and PostToolBatch block on exit 2, while StopFailure, PermissionDenied, and InstructionsLoaded have their exit code ignored — so an auditor holding this file could not grade any of them. The bullet now names that table as the sole authority and restates none of its rows: resolve the handler's event, read its row, pair on the row's own `Can block?` cell, and take the paired content from what the row states is prevented rather than assuming a tool call or a prompt. An event with no row, or an unreachable table, is recorded as `blockability-unresolved` on the same terms as the file's existing text-unresolved rule instead of being inferred. The SubagentStop subagent-scoping rule and the PostToolUse/PreToolUse worked pair survive as examples, so the criterion keeps its detection function. The recheck trigger no longer fires on a row added upstream, and eval 16 tests the lookup procedure rather than the memorized split. Refs #1989 (row 244) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Claude finished @kyle-sexton's task in 41s —— View job Security review complete
Scope: all 4 files changed in this PR (per
Findings: none. This PR is a documentation/reference-content change to a Claude Code skill's audit criteria, plus a matching eval-fixture update, changelog entry, and semver bump. There is no executable code, no shell/script logic, no GitHub Actions workflow or Specifically checked and ruled out for this diff:
No further action needed from a security standpoint. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fac543af4b
ℹ️ 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 finished @kyle-sexton's task in 3m 56s —— View job Review complete
Scope. Reviewed the 4 changed files against Independently verified (I ran these myself):
Author-claimed, not independently re-run by me (sandboxed shell blocked script execution in this session — One finding posted inline ( Everything else checked out: the pointer-not-copy rewrite is a real improvement (the old text was a closed 11-event partition against a table that now has 31 rows), the new |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Claude has reviewed this PR 5 times. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
Also narrow the conflict-criteria recheck exemption: a change to the SubagentStop, PostToolUse, or PreToolUse rows now fires a recheck, since the worked examples cite those rows' content (review threads on lines 24-25). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HbtPzLRe1yBavNpsmv5Tum
|
Claude finished @kyle-sexton's task in 43s —— View job Security review complete
Scope: all 4 files changed in this PR relative to
This synchronize event corresponds to commit Findings: none. Checked and ruled out for this diff:
No further action needed from a security standpoint. (Note: the two non-security review threads already on this PR — the recheck-trigger scope on |
|
Claude finished @kyle-sexton's task in 4m 6s —— View job Review complete
Lane scope. This repo has a What I independently checked (ran these myself):
Not independently re-run (would require live-doc fetch or script execution I didn't repeat this pass): the PR body's Test Plan commands ( Everything else checked out: the pointer-not-copy rewrite is sound — deferring to the upstream table instead of restating a closed partition is the right fix for the actual defect (the old 11-event list vs. the 31-row table); the new One unrelated note, not a defect: |
The PostToolUse worked example read the hook as an exit-2 emitter; it is advisory by design — always exit 0, findings via additionalContext — because exit 2 there could block nothing. The citation now states that, and the exit-2 hypothetical is generic rather than pinned to this hook. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HbtPzLRe1yBavNpsmv5Tum
|
Claude finished @kyle-sexton's task in 47s —— View job Security review complete
Scope: all 4 files changed in this PR at HEAD
Findings: none. This diff is entirely static Markdown reference-doc prose, a JSON eval-fixture text update, a changelog entry, and a manifest version bump. There is no executable code, no GitHub Actions workflow file, no Specifically checked and ruled out:
No further action needed from a security standpoint. (The two prior non-security review threads on this PR — the recheck-trigger scope and the |
|
Claude finished @kyle-sexton's task in 2m 24s —— View job Review complete
Lane scope. Per What I independently verified myself (not author-claimed):
Not independently re-run (would require executing repo scripts I don't have approval to run in this session): Conclusion: both open review threads on this PR (the recheck-trigger overbreadth flagged by Codex and my prior pass, and the |
Composed both sides: main's prompting-guide alignment (#2018, seven-skill description, eval 2 I27->I28) and this PR's eval-16 row-lookup rewrite. CHANGELOG entry retitled [0.26.1] -> [0.27.1] above main's [0.27.0]. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HbtPzLRe1yBavNpsmv5Tum
|
Claude finished @kyle-sexton's task in 47s —— View job Security review complete
Scope: all 4 files changed in this PR at HEAD
Findings: none. This diff is entirely static Markdown reference-doc prose, a JSON eval-fixture text update, a changelog entry, and a manifest version bump. There is no executable code, no GitHub Actions workflow file, no Specifically checked and ruled out:
No further action needed from a security standpoint. (The prior non-security review threads on this PR — the recheck-trigger scope and the |
|
Claude finished @kyle-sexton's task in 1m 23s —— View job Review complete
Lane scope. Per What changed in this synchronize event. I ran
The extra commits visible in Conclusion: no new findings. This synchronize event is a merge-forward-and-re-version housekeeping push with no substantive content change; the two issues raised earlier in review (Codex's and my own recheck-trigger comment, and my |
|
Claude finished @kyle-sexton's task in 2m 16s —— View job Review complete
Lane scope. Per One finding posted inline, and it's a real blocker: What I independently verified myself (not author-claimed):
Not independently re-run (sandboxed shell in this session blocked script execution, same as prior review passes on this PR): Everything else checks out: the pointer-not-copy rewrite of the blockability criterion is a real improvement — deferring to the upstream 31-row table instead of a stale 11-event partition is the correct fix for the defect described in the PR body, both previously-open review threads (recheck-trigger scope, |
|
Claude finished @kyle-sexton's task in 50s —— View job Security review complete
Scope: all 4 files changed in this PR at HEAD
This synchronize event corresponds to a merge of Findings: none (security). As with every prior pass, this diff is entirely static Markdown/JSON reference content and a manifest version bump — no executable code, no GitHub Actions workflow file, no Non-security note (flagging since it affects merge-worthiness, not filing as a finding since it's out of this lane's scope): This isn't a security issue (no code execution/parsing risk beyond changelog tooling), but it's a real content defect from the No further action needed from a security standpoint. |
… commit The prior merge commit pushed a CHANGELOG.md still carrying conflict markers and the 0.27.1 heading while plugin.json claimed 0.27.2. The row-244 entry now heads the file as 0.27.2 above main's 0.27.1 listing-trim entry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HbtPzLRe1yBavNpsmv5Tum
|
Claude finished @kyle-sexton's task in 43s —— View job Security review complete
Scope: all 4 files changed in this PR at HEAD
Findings: none. This diff is entirely static Markdown reference-doc prose, a JSON eval-fixture text update, a changelog entry, and a manifest version bump. There is no executable code, no GitHub Actions workflow file, no Specifically checked and ruled out:
No further action needed from a security standpoint. (The prior non-security review threads on this PR — the recheck-trigger scope and the |
…fig-hook-event-partition # Conflicts: # plugins/claude-config/CHANGELOG.md
|
Claude finished @kyle-sexton's task in 51s —— View job Security review complete
Scope: all 4 files changed in this PR at HEAD
Findings: none (security). As with every prior pass on this PR, this diff is entirely static Markdown/JSON reference content and a manifest version bump. No executable code, no GitHub Actions workflow file, no
Non-security note (out of this lane's scope, flagged for visibility only): No further action needed from a security standpoint. |
…s 0.27.3 The previous merge commit again left conflict markers in CHANGELOG.md while plugin.json moved to 0.27.3 (main shipped its own 0.27.2 mid-flight). The row-244 entry now heads the file as 0.27.3; main's 0.27.2 entry is preserved byte-identically beneath it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HbtPzLRe1yBavNpsmv5Tum
|
Claude finished @kyle-sexton's task in 49s —— View job Security review complete
Scope: all 4 files changed in this PR at HEAD
Findings: none. This synchronize event resolves the changelog merge conflict flagged by a prior security-lane pass on this PR: I checked As with every prior pass, this diff is entirely static Markdown/JSON reference content and a manifest version bump — no executable code, no GitHub Actions workflow file, no
No further action needed from a security standpoint. (The two non-security review threads previously raised on this PR — the recheck-trigger scope and the |
Version collision on the claude-config plugin: main shipped 0.27.3 (#2041) while this branch also staged a bump. This branch takes 0.27.4, the next free number above main, and both changelog entries are kept in full — they fix unrelated defects. An earlier merge on this branch had dropped the manifest's `description` field and unindented its `version` line. The manifest is restored byte-for-byte from main with only the version line changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
Summary
plugins/claude-config/skills/audit-instructions/reference/conflict-criteria.mdadjudicated ahook-half conflict pair by splitting hook events into two named lists — "Blockable events"
(
PreToolUse,UserPromptSubmit,Stop,SubagentStop,PreCompact,UserPromptExpansion) and"Non-blockable events" (
PostToolUse,Notification,SubagentStart,SessionStart,SessionEnd) — phrased as an exhaustive partition. That phrasing is the defect: the event set isupstream-owned and grows.
Upstream evidence. The hooks page's
#### Exit code 2 behavior per eventtable(https://code.claude.com/docs/en/hooks#exit-code-2-behavior-per-event, fetched 2026-08-08 via the
raw-markdown channel) carries 31 rows, not 11. Events this repository's own hooks register that
appeared in neither list:
ConfigChangepolicy_settings)PostToolBatchStopFailurePermissionDeniedInstructionsLoadedUnder the old text an auditor could grade none of these. Two further rows also show why a copied
list is the wrong shape even when complete:
PostToolBatchprevents "the agentic loop before thenext model call" and
TaskCreated"rolls back the task creation" — neither is a tool call or aprompt, so "the act it blocks" needs the row's own cell to supply it.
The fix (pointer-not-copy). The bullet now names that table as the sole authority and restates
none of its rows in either direction. The procedure: resolve the handler's event, read its row, and
pair on the row's own
Can block?cell — taking the paired content from what the row states isprevented rather than assuming a tool call or prompt; pairing as nothing when the cell says no; and
recording an event with no row (or an unreachable table) as
blockability-unresolved, reusing thefile's existing
text-unresolved/liveness-unresolvedreport-rather-than-infer shape instead ofinventing a third vocabulary.
The criterion keeps its detection function. Both worked examples survive, now marked as examples:
PostToolUse(blocks nothing, the tool already ran, with theplugins/actionlint/hooks/actionlint-check.shpointer) and the
PreToolUsecounterfactual that eval 16 turns on. TheSubagentStopsubagent-scoping sentence is kept as its own standing rule rather than a list member.
Also updated: the file's recheck trigger, which fired on "a change to which events exit 2 can
actually block" — now scoped to the table's removal, renaming, or restructuring, since a row added
upstream can no longer stale anything here. Review follow-up: the exemption is narrowed so that a
change to the
SubagentStop,PostToolUse, orPreToolUserows — the three the worked examplescite — does fire a recheck.
Files changed
plugins/claude-config/skills/audit-instructions/reference/conflict-criteria.md— the partitionrewrite, the recheck trigger, and the file header (1.3.0 → 1.4.0, 2026-08-08)
plugins/claude-config/skills/audit-instructions/evals/evals.json— eval 16'sexpected_outputand second expectation now test the row lookup rather than the memorized split; the
PostToolUseverdict the eval exists to check is unchanged, ids are not renumbered
plugins/claude-config/.claude-plugin/plugin.json— 0.25.0 → 0.27.1 (re-versioned twice aftermerges of main: first to 0.26.1 when 0.26.0 landed, then to 0.27.1 when 0.27.0 landed via feat(plugins): align instruction surfaces with the current prompting guide and add the posture-audit lane #2018)
plugins/claude-config/CHANGELOG.md—## [0.27.1]entry, placed above main's## [0.27.0]Deviations from the row brief
SKILL.md. They do not. A grep forblockable|non-blocking|blocks nothing|UserPromptExpansion|SessionEnd|PostToolUseFailure|Notificationacross
audit-instructions/returns onlyconflict-criteria.mdandevals.jsonfor thepartition;
SKILL.md:157carries a generic "the act it blocks under its event andmatcher"(event-agnostic, still correct) and
SKILL.md:160/:164concern stdout andadditionalContextscoping, not blockability. No
SKILL.mdedit was manufactured to satisfy the row's wording.mainand would have regressed the changelog order — the exact failure classcheck-changelog-parity.sh --check-orderexists to catch. Used the reallocated 0.25.2, thenre-versioned to 0.26.1 (via a merge of
origin/main, not a rebase) after 0.26.0 landed on main,then to 0.27.1 via a second merge of
origin/mainafter feat(plugins): align instruction surfaces with the current prompting guide and add the posture-audit lane #2018 landed 0.27.0 (that merge alsocomposed feat(plugins): align instruction surfaces with the current prompting guide and add the posture-audit lane #2018's evals.json changes — eval 2's I27 → I28 — with this PR's eval-16 rewrite).
asks only that evals stay consistent, and
evals.jsonis contended by sibling PRs.Test plan
scripts/check-changelog-parity.sh --check— PASS ("Every versioned plugin has a CHANGELOG.md")scripts/check-changelog-parity.sh --check-bump origin/main— PASS (version entry present athead, absent at base; re-run at 0.27.1 after the second merge of main)
scripts/check-changelog-parity.sh --check-order— PASS (all 72 changelogs newest-first)scripts/check-changed-skills.sh origin/main— PASS, 0 errors, 2 warnings, both pre-existing anduntouched by this diff (
SKILL.md398 lines vs the 200-line soft target;reference/criteria.md:976fresh-eyes declaration)markdownlint-cli2over the changed markdown — 0 errorsevals.jsonvalidated againstplugins/skill-quality/reference/evals.schema.jsonviaajv-cli(draft 2020-12) — valid; 16 eval cases parse
conflict-criteria.mdcan now grade an event namednowhere in the file (
Elicitation,FileChanged,PostToolBatch) — look up its row, quote thecell, or mark it
blockability-unresolved.Related
No linked issue — this PR closes nothing. It fixes one row (row 244) of the multi-row umbrella issue
#1989, which stays open for its remaining rows, so a closing keyword would be wrong here.