docs(hooks): document the post-#7422 PreToolUse fail-closed exit contract

[7487]

Problem / Motivation

#7422 (5fbce9db) changed the PreToolUse script-hook exit-code contract: any exit that is neither 0 nor 2 now blocks the tool, where it previously only warned. The PR updated docs/system-specs/modules/learn-cron-dashboard.md, but the user-facing reference (docs/reference/kiro-cli/hooks.md, "Exit 2 blocks / Other: warning") and the ScriptHook docstring in src/kiro_crew/hooks.py still describe the old rule — they now state the opposite of shipped behavior, and they are exactly what a user reads before writing a hooks.json hook.

Why it matters

A user who writes a preToolUse hook that exits 1 to mean "issues found, but proceed" (or whose hook can fail transiently) gets every matched tool call denied, with the docs telling them that cannot happen. There is no per-hook opt-out (ScriptHook carries only enabled), so the docs are the only place to learn this before hitting it.

What changed (motivation → approach → change)

Verified the shipped behavior first: chat_runner.py's inner _fire() blocks on any non-0/2 preToolUse exit with BLOCKED:<hook>:<detail> (detail = error → stderr → exited with code <n>), and warns only on non-gating events.

• docs/reference/kiro-cli/hooks.md — this tree is an upstream mirror we do not author (docs/reference/README.md: "Fix an error upstream first; a local edit silently diverges from the source it claims to mirror"), and the mirrored lines are still correct about upstream kiro-cli — it is Kiro Crew's own re-implementation that diverged. So instead of rewriting the mirrored text in place, the divergence is called out in clearly-marked Kiro Crew blockquote notes under "Hook output" and "PreToolUse", following the existing precedent in steering.md ("Viewing and editing in Kiro Crew") and mcp/oauth-token-storage.md. The note also says plainly there is no per-hook opt-out. Happy to inline-edit the mirrored lines instead if that is preferred.
• src/kiro_crew/hooks.py — the ScriptHook docstring now states the fail-closed PreToolUse rule and that it diverges from Kiro CLI's own semantics (it previously claimed "Aligned with Kiro CLI hook semantics").
• The issue's note that the opt-out switch "is tracked in PreToolUse hooks cannot deny on the subagent/task-runner paths (fire_tool_hooks is informational) #7547" did not check out — PreToolUse hooks cannot deny on the subagent/task-runner paths (fire_tool_hooks is informational) #7547 is about fire_tool_hooks being informational on the subagent/task-runner paths — so the doc note deliberately links no tracking issue.

Tests

The two tests added by #7422 built their hook results with bare MagicMocks, so a renamed production field (e.g. error) would still match a truthy child mock and pass. They now use spec=ScriptHookResult (picking up the issue's "minor test note while here"); an attribute read outside the real field set now raises. test/test_dashboard_approval.py passes 74/74; black --check and flake8 clean on the touched files.

Manual verification

N/A — docs + docstring text and a mock-tightening; unit coverage sufficient (the behavior being documented is pinned by the existing #7422 tests).

Fixes #7744

🤖 Generated with Claude Code

[7487]

Triaged the shard-3 red: it is upstream main drift, not this diff.

test_no_new_gate_side_log_line_reads_the_baseline_redactor fails because #7424 (7d05970c) rewrote the heartbeat-suppression logger.info in src/kiro_crew/slack/gateway.py to call redact_and_truncate(task_text, 80) inline — redact_and_truncate is in _BASELINE_REDACTORS, so that is a 7th counted site — and landed without a census bump, racing the ratchet added in #7278. Verified by running the test's own scanner (_gate_side_baseline_log_sites) against slack/gateway.py at 7d05970c~1 (6 sites, matches the census) vs current main (7 sites; the new one is the heartbeat line, now ~5300).

Already tracked upstream as #7756/#7757, with the fix in flight in #7762 (converts that line to the composed-context read). I am deliberately not bumping _BASELINE_LOG_SITE_CENSUS to 7 here: the moment #7762 converts the site back, a 7 entry would trip the companion looseness assertion ("census is now looser than the code") in the same test class, and the census answer ("which process and why") is not mine to give for a line this PR does not touch.

Will update this branch from main once #7762 lands and main is green again.

[7487]

The census fix landed on main via #7761 (74ffc28, 2026-09-02; #7762 was closed as its duplicate). Merged upstream main into this branch (f194ebf) to pick it up — locally test_security_posture.py -k "gate_side or census" now passes (3/3), and test_dashboard_approval.py is 81/81 post-merge. Diff vs main is unchanged: the same three files.

[github-actions]

Design Review (Fable 5, fork) — ✅ PASS

Design-level review of f194ebf427871202107878687cef5519287b8c3b via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Docs corrected to match verified shipped behavior (chat_runner.py:5311), divergence marked in the mirror per existing precedent, authoritative copy kept in the owned spec.

[DESIGN-REVIEWED] f194ebf

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed f194ebf427871202107878687cef5519287b8c3b via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] f194ebf

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of f194ebf427871202107878687cef5519287b8c3b via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All claims verified against the base tree. The docs did contradict shipped behavior (chat_runner.py:5311-5336 blocks on any non-0/2 preToolUse exit; the base doc and docstring both said "warning"), the mirror-note convention is real (docs/reference/README.md:3-11, precedent at steering.md:78), the link anchor exists (learn-cron-dashboard.md:1953), and no other doc site still states the old contract. The one counted gap: the test tightening fixes 2 of 4 bare ScriptHookResult-shaped mocks in the file.

First-Principles-Verdict: CONCERNS

Every doc item corrects a verified falsehood; the riding-along test fix pins 2 of 4 bare hook-result mocks in the same file and leaves the other 2 fragile.

What this change ships

Intent: make the hook docs a user reads before writing a preToolUse hook stop stating the opposite of the fail-closed contract #7422 shipped. This is a FIX (fixes #7744).

1. Mirrored hooks doc gains a marked "Kiro Crew divergence" note on non-0/2 preToolUse exits — justified (base doc contradicted chat_runner.py:5311-5336; blockquote form follows docs/reference/README.md:3-11)
2. PreToolUse section gains a one-line pointer to that note — justified (section-level readers miss the note above)
3. ScriptHook docstring drops the false "Aligned with Kiro CLI hook semantics" claim — justified (hooks.py:3466 stated the opposite of shipped behavior)
4. Two fix(hooks): fail closed when a PreToolUse hook delivers no verdict #7422 tests pin mocks with spec=ScriptHookResult — declared rider; point patch, 2 siblings unfixed

Watch

Item 4's root cause — a bare MagicMock hook result matches any renamed field via truthy child mocks — has 2 unfixed siblings in the same file (grepped MagicMock( hook-result constructions in test/test_dashboard_approval.py): the _blocking_hook_store helper at line 76 and blocked_result at line 511. Same one-line spec= fix applies; fixing 2 of 4 leaves the false-pass class alive in the file the PR touched.

[FIRST-PRINCIPLES-REVIEWED] f194ebf

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed f194ebf427871202107878687cef5519287b8c3b via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] f194ebf

[bolichen97]

The docs now match shipped behavior: chat_runner's elif r.exit_code not in (0, 2) branch appends BLOCKED:<hook>:<detail> for HOOK_EVENT_PRE_TOOL_USE and warns for every other event, which is exactly what the new divergence note and the ScriptHook docstring state, including the no-per-hook-opt-out point. Correctly annotates the upstream mirror with a marked Kiro Crew note instead of rewriting mirrored text that is still accurate about upstream kiro-cli, and the spec=ScriptHookResult tightening turns a renamed field into a test failure rather than a truthy child mock.

[7487]

Closing as superseded. The conflict is #8402 (505e7c8, merged 2026-09-04 17:57Z, nine minutes after the approval here), which fixed #7744 across the same three files: it rewrites the mirrored hooks.md lines inline, updates the ScriptHook docstring, and hardens the #7422 tests with real ScriptHookResult instances (covering the _blocking_hook_store helper too, which this PR left as a bare mock). Merging main here would leave only a duplicate divergence note on top of that, so there is nothing left for this branch to add. Thanks for the review.