From ae7b5c8eab65ac9f0288bbc524b04cd90906e7e9 Mon Sep 17 00:00:00 2001 From: Christopher <210261288+Christopher-Schulze@users.noreply.github.com> Date: Sat, 15 Aug 2026 03:47:34 +0200 Subject: [PATCH] fix(batch_runner): propagate fatal and validation errors as non-zero exit codes Python Fire serializes the return value of a wrapped function but does not use that value as the process exit code. Error paths in main() that used or therefore caused the process to exit 0, swallowing fatal errors and argument-validation failures. Raise SystemExit(1) on every error path so batch_runner returns a non-zero exit code when it cannot run. Success paths (e.g. --list_distributions) are left unchanged. Closes NousResearch/hermes-agent#86524. --- batch_runner.py | 28 ++++++------ tests/test_batch_runner_exit_code.py | 64 ++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 14 deletions(-) create mode 100644 tests/test_batch_runner_exit_code.py diff --git a/batch_runner.py b/batch_runner.py index f2f00e0de1..af6dd75c1e 100644 --- a/batch_runner.py +++ b/batch_runner.py @@ -1246,21 +1246,21 @@ def main( # Validate required arguments if not dataset_file: print("āŒ Error: --dataset_file is required") - return - + raise SystemExit(1) + if not batch_size or batch_size < 1: print("āŒ Error: --batch_size must be a positive integer") - return - + raise SystemExit(1) + if not run_name: print("āŒ Error: --run_name is required") - return - + raise SystemExit(1) + # Parse provider preferences (comma-separated strings to lists) providers_allowed_list = [p.strip() for p in providers_allowed.split(",")] if providers_allowed else None providers_ignored_list = [p.strip() for p in providers_ignored.split(",")] if providers_ignored else None providers_order_list = [p.strip() for p in providers_order.split(",")] if providers_order else None - + # Build reasoning_config from CLI flags # --reasoning_disabled takes priority, then --reasoning_effort, then default (medium) reasoning_config = None @@ -1273,10 +1273,10 @@ def main( valid_efforts = ["none", "minimal", "low", "medium", "high", "xhigh", "max", "ultra"] if reasoning_effort not in valid_efforts: print(f"āŒ Error: --reasoning_effort must be one of: {', '.join(valid_efforts)}") - return + raise SystemExit(1) reasoning_config = {"enabled": True, "effort": reasoning_effort} print(f"🧠 Reasoning effort: {reasoning_effort}") - + # Load prefill messages from JSON file if provided prefill_messages = None if prefill_messages_file: @@ -1285,12 +1285,12 @@ def main( prefill_messages = json.load(f) if not isinstance(prefill_messages, list): print("āŒ Error: prefill_messages_file must contain a JSON array of messages") - return + raise SystemExit(1) print(f"šŸ’¬ Loaded {len(prefill_messages)} prefill messages from {prefill_messages_file}") except Exception as e: print(f"āŒ Error loading prefill messages: {e}") - return - + raise SystemExit(1) + # Initialize and run batch runner try: runner = BatchRunner( @@ -1317,12 +1317,12 @@ def main( ) runner.run(resume=resume) - + except Exception as e: print(f"\nāŒ Fatal error: {e}") if verbose: traceback.print_exc() - return 1 + raise SystemExit(1) if __name__ == "__main__": diff --git a/tests/test_batch_runner_exit_code.py b/tests/test_batch_runner_exit_code.py new file mode 100644 index 0000000000..7329c4622c --- /dev/null +++ b/tests/test_batch_runner_exit_code.py @@ -0,0 +1,64 @@ +"""Regression tests for batch_runner process exit codes. + +Python Fire serializes the return value of the wrapped function but does not +use it as the process exit code. Before the fix, all of ``main``'s error paths +returned ``0`` because a bare ``return`` or ``return 1`` was treated as the +function result, not a non-zero exit status. +""" + +import subprocess +import sys +from pathlib import Path + +import pytest + + +REPO_ROOT = Path(__file__).parent.parent +PYTHON = sys.executable + + +def _run(*args): + return subprocess.run( + [PYTHON, "batch_runner.py", *args], + cwd=REPO_ROOT, + text=True, + capture_output=True, + ) + + +def test_missing_dataset_file_exits_nonzero(): + result = _run("--run_name=test") + assert result.returncode == 1 + assert "--dataset_file is required" in result.stdout + + +def test_invalid_batch_size_exits_nonzero(): + result = _run("--dataset_file=/tmp/data.jsonl", "--batch_size=-1", "--run_name=test") + assert result.returncode == 1 + assert "--batch_size must be a positive integer" in result.stdout + + +def test_invalid_reasoning_effort_exits_nonzero(): + result = _run( + "--dataset_file=/tmp/data.jsonl", + "--batch_size=1", + "--run_name=test", + "--reasoning_effort=invalid", + ) + assert result.returncode == 1 + assert "--reasoning_effort must be one of" in result.stdout + + +def test_invalid_prefill_messages_file_exits_nonzero(tmp_path): + bad_prefill = tmp_path / "prefill.json" + bad_prefill.write_text("not json", encoding="utf-8") + result = _run( + "--dataset_file=/tmp/data.jsonl", + "--batch_size=1", + "--run_name=test", + f"--prefill_messages_file={bad_prefill}", + ) + assert result.returncode == 1 + assert "Error loading prefill messages" in result.stdout + +