From 41a3b9d1ee2b607c91202e867aafb6ad8b79c69c Mon Sep 17 00:00:00 2001 From: Stefano Fiorini Date: Thu, 5 Mar 2026 22:59:23 -0600 Subject: [PATCH] fix(reviewer-runtime): cover timeout and exit semantics --- skills/reviewer-runtime/run-review.sh | 6 +++-- skills/reviewer-runtime/tests/smoke-test.sh | 30 +++++++++++++++++++-- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/skills/reviewer-runtime/run-review.sh b/skills/reviewer-runtime/run-review.sh index ef302e7..55eb202 100755 --- a/skills/reviewer-runtime/run-review.sh +++ b/skills/reviewer-runtime/run-review.sh @@ -5,6 +5,8 @@ DEFAULT_POLL_SECONDS=10 DEFAULT_SOFT_TIMEOUT_SECONDS=600 DEFAULT_STALL_WARNING_SECONDS=300 DEFAULT_HARD_TIMEOUT_SECONDS=1800 +EXIT_COMPLETED_EMPTY_OUTPUT=80 +EXIT_NEEDS_OPERATOR_DECISION=81 COMMAND_FILE="" STDOUT_FILE="" @@ -271,7 +273,7 @@ main() { append_status error needs-operator-decision "hard timeout reached; terminating reviewer child for operator intervention" kill_child_process_group trap - EXIT - exit 12 + exit "$EXIT_NEEDS_OPERATOR_DECISION" fi done @@ -293,7 +295,7 @@ main() { fi append_status error completed-empty-output "reviewer exited successfully with empty stdout" - exit 10 + exit "$EXIT_COMPLETED_EMPTY_OUTPUT" fi append_status error failed "reviewer exited with code $child_exit_code" diff --git a/skills/reviewer-runtime/tests/smoke-test.sh b/skills/reviewer-runtime/tests/smoke-test.sh index 1f09014..bfb86a7 100644 --- a/skills/reviewer-runtime/tests/smoke-test.sh +++ b/skills/reviewer-runtime/tests/smoke-test.sh @@ -12,7 +12,7 @@ fail() { assert_file_contains() { local file=$1 local pattern=$2 - if ! rg -q --fixed-strings "$pattern" "$file"; then + if ! grep -qF "$pattern" "$file"; then echo "Expected pattern not found: $pattern" >&2 echo "--- $file ---" >&2 sed -n '1,200p' "$file" >&2 || true @@ -142,7 +142,7 @@ exit 7 local exit_code=$? fi - assert_nonzero_exit "$exit_code" + assert_exit_code "$exit_code" 7 assert_file_contains "$stderr_file" "boom" assert_file_contains "$status_file" "state=failed" } @@ -218,6 +218,31 @@ sleep 30 fi } +test_hard_timeout_escalation() { + local dir=$1 + local command_file=$dir/hard-timeout.sh + local stdout_file=$dir/hard-timeout.stdout + local stderr_file=$dir/hard-timeout.stderr + local status_file=$dir/hard-timeout.status + + make_command "$command_file" ' +sleep 30 +' + + if run_helper "$command_file" "$stdout_file" "$stderr_file" "$status_file" \ + --poll-seconds 1 \ + --soft-timeout-seconds 2 \ + --stall-warning-seconds 2 \ + --hard-timeout-seconds 4; then + local exit_code=0 + else + local exit_code=$? + fi + + assert_exit_code "$exit_code" 81 + assert_file_contains "$status_file" "state=needs-operator-decision" +} + main() { [[ -x "$HELPER_PATH" ]] || fail "helper is not executable: $HELPER_PATH" @@ -230,6 +255,7 @@ main() { test_nonzero_failure "$tmp_dir" test_empty_output_is_terminal "$tmp_dir" test_signal_cleanup "$tmp_dir" + test_hard_timeout_escalation "$tmp_dir" echo "PASS: reviewer runtime smoke tests" }