fix(cli): hooks test prints the decision for failing hooks
_print_run_result returned early on error or timeout, before the parsed decision block, so a fail-closed hook that correctly blocks rendered identically to a fail-open hook that lets the call through. run_once always sets result["parsed"] (agent/shell_hooks.py::_evaluate_result), so the value exists for error/timeout too — only the display omitted it (#115968). Convert the early returns to an if/elif/else chain that falls through to the shared parsed tail. Display-only; no change to the hook contract or evaluation.
This commit is contained in:
@@ -210,15 +210,17 @@ def _cmd_test(args) -> None:
|
||||
def _print_run_result(result: Dict[str, Any]) -> None:
|
||||
if result.get("error"):
|
||||
print(f" ✗ error: {result['error']}")
|
||||
return
|
||||
if result.get("timed_out"):
|
||||
elif result.get("timed_out"):
|
||||
print(f" ✗ timed out after {result['elapsed_seconds']}s")
|
||||
return
|
||||
print(f" exit={result.get('returncode')} elapsed={result.get('elapsed_seconds', 0)}s")
|
||||
for stream in ("stdout", "stderr"):
|
||||
text = (result.get(stream) or "").strip()
|
||||
if text:
|
||||
print(f" {stream}: {_truncate(text, 400)}")
|
||||
else:
|
||||
print(f" exit={result.get('returncode')} elapsed={result.get('elapsed_seconds', 0)}s")
|
||||
for stream in ("stdout", "stderr"):
|
||||
text = (result.get(stream) or "").strip()
|
||||
if text:
|
||||
print(f" {stream}: {_truncate(text, 400)}")
|
||||
# run_once always sets ``parsed`` (agent/shell_hooks.py::_evaluate_result), so it is available even
|
||||
# when the hook errored or timed out. A failing hook's decision is the one thing `hooks test` exists
|
||||
# to show — failed-open and failed-closed must not render identically (#115968).
|
||||
parsed = result.get("parsed")
|
||||
if parsed:
|
||||
print(f" parsed (Hermes wire shape): {json.dumps(parsed)}")
|
||||
|
||||
@@ -203,3 +203,51 @@ class TestHooksDoctor:
|
||||
)
|
||||
assert "not allowlisted" in out.lower()
|
||||
assert "skipped JSON smoke test" in out
|
||||
|
||||
|
||||
def test_print_run_result_shows_decision_for_error_and_timeout():
|
||||
"""A failing hook's decision must be printed, not hidden by early returns (#115968).
|
||||
|
||||
run_once always sets ``parsed`` (agent/shell_hooks.py::_evaluate_result), so `hooks test` must show
|
||||
it even when the hook errored or timed out. Before this fix the ``return`` on error/timeout skipped
|
||||
the parsed tail, so a fail-closed blocker rendered identically to a fail-open pass-through.
|
||||
"""
|
||||
parsed = {"action": "block", "message": "hook failed closed: command not found"}
|
||||
|
||||
buf = io.StringIO()
|
||||
with redirect_stdout(buf):
|
||||
hooks_cli._print_run_result(
|
||||
{"error": "command not found", "parsed": parsed}
|
||||
)
|
||||
out = buf.getvalue()
|
||||
assert "✗ error: command not found" in out
|
||||
assert '"action": "block"' in out
|
||||
assert '"message": "hook failed closed: command not found"' in out
|
||||
|
||||
buf = io.StringIO()
|
||||
with redirect_stdout(buf):
|
||||
hooks_cli._print_run_result(
|
||||
{"timed_out": True, "elapsed_seconds": 2.33, "parsed": parsed}
|
||||
)
|
||||
out = buf.getvalue()
|
||||
assert "✗ timed out after 2.33s" in out
|
||||
assert '"action": "block"' in out
|
||||
|
||||
|
||||
def test_print_run_result_happy_path_unchanged():
|
||||
"""The normal success path still prints exit, streams, and the decision."""
|
||||
buf = io.StringIO()
|
||||
with redirect_stdout(buf):
|
||||
hooks_cli._print_run_result(
|
||||
{
|
||||
"returncode": 0,
|
||||
"elapsed_seconds": 0.1,
|
||||
"stdout": "ok",
|
||||
"stderr": "",
|
||||
"parsed": {"action": "allow"},
|
||||
}
|
||||
)
|
||||
out = buf.getvalue()
|
||||
assert "exit=0" in out
|
||||
assert "stdout: ok" in out
|
||||
assert '"action": "allow"' in out
|
||||
|
||||
Reference in New Issue
Block a user