From 54cac0d679530f8089df4145d9d6882cdb0ec3c7 Mon Sep 17 00:00:00 2001 From: 7487 <1042653432@qq.com> Date: Wed, 2 Sep 2026 06:39:39 +0800 Subject: [PATCH] docs(hooks): document the post-#7422 PreToolUse fail-closed exit contract #7422 made a preToolUse script hook that exits with anything other than 0 or 2 block the tool (undelivered verdict resolves to deny), but the user-facing hooks reference and the ScriptHook docstring still described the old warn-only rule. - docs/reference/kiro-cli/hooks.md is an upstream mirror we do not author (docs/reference/README.md), so the mirrored text stays intact and the divergence is called out in clearly-marked Kiro Crew notes, following the existing precedent in steering.md and mcp/oauth-token-storage.md. The note also says plainly that there is no per-hook opt-out, so nobody writes a warn-style exit-1 hook expecting the old contract. - The ScriptHook docstring now states the fail-closed PreToolUse rule and that it diverges from Kiro CLI's own semantics. - The two #7422 tests now build their hook results with spec=ScriptHookResult, so a renamed production field fails the test instead of matching a truthy child mock. Fixes #7744 Co-Authored-By: Claude Fable 5 --- docs/reference/kiro-cli/hooks.md | 15 +++++++++++++++ src/kiro_crew/hooks.py | 7 +++++-- test/test_dashboard_approval.py | 18 +++++++++++++++--- 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/docs/reference/kiro-cli/hooks.md b/docs/reference/kiro-cli/hooks.md index 4db61af97d1..d8bb89b6d5a 100644 --- a/docs/reference/kiro-cli/hooks.md +++ b/docs/reference/kiro-cli/hooks.md @@ -23,6 +23,18 @@ Hooks receive JSON via STDIN: - **Exit 2**: (PreToolUse only) block tool execution, STDERR returned to LLM - **Other**: failed, STDERR shown as warning +> **Kiro Crew divergence** (script hooks in `hooks.json`, managed from the +> dashboard's Hooks page): for a `preToolUse` hook, any exit that is neither 0 +> nor 2 — including a timeout, a crash, or an unexecutable command — **blocks +> the tool** instead of warning. Exit 0 is a delivered allow and exit 2 a +> delivered deny; every other outcome is an undelivered verdict, and the gate +> fails closed (`BLOCKED::`). There is no per-hook opt-out, so a +> `preToolUse` hook must reserve nonzero exits for "block" — a hook that exits +> 1 to mean "issues found, but proceed" will deny the tool. Non-gating events +> (`postToolUse`, `userPromptSubmit`, `stop`) keep the warn-only behavior +> above. See [learn-cron-dashboard](../../system-specs/modules/learn-cron-dashboard.md), +> § Tool-Refusal Recovery. + ## Tool matching Use `matcher` field. Supports canonical names and aliases: @@ -45,6 +57,9 @@ Runs when user submits prompt. Receives `prompt` field. Exit 0 → STDOUT added ### PreToolUse Runs before tool execution. Can block (exit 2). Receives `tool_name`, `tool_input`. +> **Kiro Crew:** any non-0/2 exit also blocks — see the divergence note under +> [Hook output](#hook-output). + ### PostToolUse Runs after tool execution. Receives `tool_name`, `tool_input`, `tool_response`. diff --git a/src/kiro_crew/hooks.py b/src/kiro_crew/hooks.py index 4605598d277..8cd259340bb 100644 --- a/src/kiro_crew/hooks.py +++ b/src/kiro_crew/hooks.py @@ -3384,10 +3384,13 @@ def _hook_subprocess_env(hook: "ScriptHook", context: str) -> dict[str, str]: class ScriptHook: """Executable hook that runs a shell command on a trigger event. - Aligned with Kiro CLI hook semantics: + Exit-code contract (consumed by ``chat_runner``'s inner ``_fire()``): - Exit 0: success (stdout → context for AgentSpawn/UserPromptSubmit) - Exit 2: block tool (PreToolUse only, stderr → LLM) - - Other: warning (stderr shown to user) + - Other: PreToolUse BLOCKS (fail closed — 0 and 2 are the only delivered + verdicts, so a timeout, crash, or any other nonzero exit resolves to + deny); every other event warns only (stderr shown to user). This + diverges from Kiro CLI's own hook semantics, which warn on non-0/2. """ id: str = "" diff --git a/test/test_dashboard_approval.py b/test/test_dashboard_approval.py index 90b242fadf0..1197a4f708a 100644 --- a/test/test_dashboard_approval.py +++ b/test/test_dashboard_approval.py @@ -22,7 +22,7 @@ parse_cls_meta, ) from kiro_crew.history import ConversationLog -from kiro_crew.hooks import HOOK_EVENT_PRE_TOOL_USE, ToolHookResult +from kiro_crew.hooks import HOOK_EVENT_PRE_TOOL_USE, ScriptHookResult, ToolHookResult from kiro_crew.providers.base import ( EVENT_COMPLETE, EVENT_PERMISSION_REQUEST, @@ -563,8 +563,13 @@ async def test_auto_approve_blocked_when_pretooluse_hook_delivers_no_verdict( """ cb = _context_builder(ToolHookResult.auto_approve()) hook_store = _make_hook_store() + # spec= pins the mock to real ScriptHookResult fields, so a renamed + # production field (e.g. `error`) fails here instead of silently + # matching a truthy child mock. hook_store.fire = AsyncMock( - return_value=[MagicMock(hook_name="policy-gate", **result_kwargs)] + return_value=[ + MagicMock(spec=ScriptHookResult, hook_name="policy-gate", **result_kwargs) + ] ) state, client = _make_state(tmp_path, context_builder=cb, hook_store=hook_store) slot = _make_slot() @@ -591,7 +596,14 @@ async def test_auto_approve_allowed_when_pretooluse_hook_exits_zero(self, tmp_pa hook_store = _make_hook_store() hook_store.fire = AsyncMock( return_value=[ - MagicMock(exit_code=0, stdout="", stderr="", error="", hook_name="policy-gate") + MagicMock( + spec=ScriptHookResult, + exit_code=0, + stdout="", + stderr="", + error="", + hook_name="policy-gate", + ) ] ) state, client = _make_state(tmp_path, context_builder=cb, hook_store=hook_store)