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 fe59a10a2fe..65ad03dc88a 100644 --- a/src/kiro_crew/hooks.py +++ b/src/kiro_crew/hooks.py @@ -3463,10 +3463,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 e384e463fe0..636816cb82f 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, @@ -565,8 +565,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() @@ -593,7 +598,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)