diff --git a/cli.py b/cli.py index a9fedd7d07..dc95a45000 100644 --- a/cli.py +++ b/cli.py @@ -4089,28 +4089,30 @@ def _sync_cli_session_id_from_agent(cli) -> None: cli.session_id = cli.agent.session_id +# ``failure_reason`` values that say nothing about the task itself: the provider or the +# account is walled, so a Kanban worker signals "try later" instead of "I failed". +_QUOTA_WALL_REASONS = frozenset({"rate_limit", "upstream_rate_limit", "billing", "overloaded"}) + + def _single_query_exit_code(result) -> int: - """Map a one-shot turn result onto a process exit code. + """Map a one-shot turn result onto a process exit code, for both `-q` and `-Q`. - 0 success, 1 failure, and ``KANBAN_RATE_LIMIT_EXIT_CODE`` (EX_TEMPFAIL) when a - Kanban worker failed purely because the provider rate-limited or the account hit a - billing/quota wall. The dispatcher's reap classifier maps that sentinel to a - ``rate_limited`` exit and releases the task back to ``ready`` WITHOUT counting a - failure, so a multi-day quota window cannot trip the circuit breaker and - permanently block the card. - - Shared by both one-shot paths. It previously lived inline in the ``-Q`` path only, - which is how the ``-q`` path — the one the Kanban dispatcher actually spawns — - ended up with no exit contract at all. + 0 only when the turn completed; 130 when it was interrupted; 1 when it failed, stopped + partway (`partial`, `completed: False`) or never ran at all (credentials / agent init + failed, so ``result`` is not a dict). A Kanban worker (``HERMES_KANBAN_TASK`` set) that + failed purely on a provider rate-limit / billing wall exits ``KANBAN_RATE_LIMIT_EXIT_CODE`` + (EX_TEMPFAIL): the dispatcher books that run ``rate_limited`` and requeues the task + WITHOUT counting a failure, so a quota window cannot trip the circuit breaker. """ - if not (isinstance(result, dict) and result.get("failed")): + if not isinstance(result, dict): + return 1 + if result.get("interrupted"): + return 130 + if not (result.get("failed") or result.get("partial") or result.get("completed") is False): return 0 - if os.environ.get("HERMES_KANBAN_TASK") and result.get("failure_reason") in ("rate_limit", "billing"): - try: - from hermes_cli.kanban_db import KANBAN_RATE_LIMIT_EXIT_CODE - return KANBAN_RATE_LIMIT_EXIT_CODE - except Exception: - return 1 + if os.environ.get("HERMES_KANBAN_TASK") and result.get("failure_reason") in _QUOTA_WALL_REASONS: + from hermes_cli.kanban_db import KANBAN_RATE_LIMIT_EXIT_CODE + return KANBAN_RATE_LIMIT_EXIT_CODE return 1 @@ -4543,14 +4545,9 @@ def _run_single_query_mode(cli, query, image, quiet, oneshot, stream_json: bool cli._show_security_advisories() cli.chat(query, images=single_query_images or None) cli._print_exit_summary(clear_screen=False) - # A dispatcher-spawned Kanban worker must report its outcome in its exit code. - # This path fell through to an implicit 0 for every outcome, and the reaper - # reads rc=0 with the task still `running` as a protocol violation: a provider - # quota wall was re-dispatched straight back into the same wall until the - # violation budget auto-blocked the card. Plain `-q` runs by a person are - # unaffected: they still exit 0. - if os.environ.get("HERMES_KANBAN_TASK"): - sys.exit(_single_query_exit_code(getattr(cli, "_last_turn_result", None))) + # Same exit contract as `-Q`: scripts and the Kanban dispatcher read the outcome from + # the exit code. This path used to fall through to an implicit 0 for every outcome. + sys.exit(_single_query_exit_code(cli._last_turn_result)) finally: _finalize_single_query(cli) diff --git a/tests/hermes_cli/test_chat_q_exit_clear.py b/tests/hermes_cli/test_chat_q_exit_clear.py index 9382123d0a..0a78858cad 100644 --- a/tests/hermes_cli/test_chat_q_exit_clear.py +++ b/tests/hermes_cli/test_chat_q_exit_clear.py @@ -86,6 +86,7 @@ def test_single_query_main_skips_clear_on_exit_summary(monkeypatch): def chat(self, query, images=None): calls.append(("chat", query, images)) + self._last_turn_result = {"final_response": "done", "completed": True} return "done" def _print_exit_summary(self, clear_screen=True): @@ -101,8 +102,10 @@ def test_single_query_main_skips_clear_on_exit_summary(monkeypatch): lambda fake_cli: calls.append(("finalize", fake_cli.session_id)), ) - cli_mod.main(query="hello", quiet=False, toolsets="terminal") + with pytest.raises(SystemExit) as exc_info: # the one-shot path exits with the turn's outcome + cli_mod.main(query="hello", quiet=False, toolsets="terminal") + assert exc_info.value.code == 0 assert calls == [ ("claim", "cli", False), "query-label", diff --git a/tests/hermes_cli/test_single_query_exit_contract.py b/tests/hermes_cli/test_single_query_exit_contract.py index 0a223ec65c..22b4ac8788 100644 --- a/tests/hermes_cli/test_single_query_exit_contract.py +++ b/tests/hermes_cli/test_single_query_exit_contract.py @@ -1,10 +1,9 @@ -"""A dispatcher-spawned ``chat -q`` worker reports its outcome in its exit code. +"""One-shot ``chat -q`` runs report their outcome in the exit code, like ``-Q`` always did. -The Kanban dispatcher spawns workers as ``hermes ... chat -q `` (the -non-quiet one-shot path), which used to fall through to an implicit rc=0 for -every outcome. The reaper reads rc=0 with the task still ``running`` as a -protocol violation, so a provider quota wall re-dispatched the card straight -back into the same wall (#101800, #48000, #91177; salvage of #110917). +The non-quiet one-shot path used to fall through to an implicit rc=0 for every +outcome, so scripts could not tell a failed run from a good one and the Kanban +dispatcher (which spawns ``chat -q`` workers) booked a provider quota wall as a +protocol violation (#111770, #101800; salvage of #110917 / #97623). """ from __future__ import annotations @@ -39,20 +38,28 @@ def _run_non_quiet(monkeypatch, turn_result): _last_turn_result=turn_result, ) try: - cli._run_single_query_mode(stub, "work kanban task t_abc123", None, False, True) + cli._run_single_query_mode(stub, "do the thing", None, False, True) except SystemExit as exc: return exc.code return None -@pytest.mark.parametrize("reason", ["rate_limit", "billing"]) +@pytest.mark.parametrize("reason", ["rate_limit", "upstream_rate_limit", "billing", "overloaded"]) def test_dispatcher_spawned_worker_signals_a_quota_wall_not_a_protocol_violation(monkeypatch, reason): monkeypatch.setenv("HERMES_KANBAN_TASK", "t_abc123") code = _run_non_quiet(monkeypatch, {"failed": True, "failure_reason": reason}) assert code == KANBAN_RATE_LIMIT_EXIT_CODE -def test_a_human_one_shot_run_is_unaffected(monkeypatch): - """No HERMES_KANBAN_TASK: a person's ``-q`` run keeps exiting 0 even when the turn failed.""" - code = _run_non_quiet(monkeypatch, {"failed": True, "failure_reason": "rate_limit"}) - assert code is None +@pytest.mark.parametrize( + ("turn_result", "expected"), + [ + ({"final_response": "done", "completed": True}, 0), + ({"failed": True, "failure_reason": "rate_limit"}, 1), # a person's run: a wall is just a failure + ({"final_response": "half", "completed": False, "partial": True}, 1), + ({"completed": False, "interrupted": True}, 130), + (None, 1), # credentials / agent init failed before any turn ran + ], +) +def test_a_plain_one_shot_run_reports_its_outcome(monkeypatch, turn_result, expected): + assert _run_non_quiet(monkeypatch, turn_result) == expected diff --git a/tests/hermes_cli/test_single_query_session_finalize.py b/tests/hermes_cli/test_single_query_session_finalize.py index d754c6ae91..2c16487679 100644 --- a/tests/hermes_cli/test_single_query_session_finalize.py +++ b/tests/hermes_cli/test_single_query_session_finalize.py @@ -108,6 +108,7 @@ def test_human_single_query_main_finalizes_after_query(monkeypatch): def chat(self, query, images=None): calls.append(("chat", query, images)) + self._last_turn_result = {"final_response": "done", "completed": True} return "done" def _print_exit_summary(self, clear_screen=True): @@ -121,8 +122,11 @@ def test_human_single_query_main_finalizes_after_query(monkeypatch): lambda fake_cli: calls.append(("finalize", fake_cli.session_id)), ) - cli_mod.main(query="hello", quiet=False, toolsets="terminal") + # The non-quiet one-shot path exits with the turn's outcome (0 here), like ``-Q``. + with pytest.raises(SystemExit) as exc_info: + cli_mod.main(query="hello", quiet=False, toolsets="terminal") + assert exc_info.value.code == 0 assert calls == [ ("claim", "cli", False), "query-label", diff --git a/website/docs/reference/cli-commands.md b/website/docs/reference/cli-commands.md index 2274d59cfc..02bd0f9255 100644 --- a/website/docs/reference/cli-commands.md +++ b/website/docs/reference/cli-commands.md @@ -174,6 +174,19 @@ Once a conversation starts, its terminal record is always `result` — including `exit_code: 130` when it is interrupted with Ctrl-C. Treat that record as the completion signal; the process exit code matches its `exit_code`. +#### Exit codes for one-shot runs + +When chat answers and exits (`-Q`, `chat --oneshot`, or a query with non-TTY +stdio) the process exit code reports the turn's outcome, on both the quiet and +the non-quiet path: `0` the turn completed; `1` it failed, stopped partway +(`partial`), hit the iteration budget, or never ran (credentials / agent init +failed); `130` it was interrupted. A Kanban dispatcher-spawned worker +(`HERMES_KANBAN_TASK` set) whose turn failed only because the provider +rate-limited or overloaded it, or the account hit a billing/quota wall, exits +`75` (`EX_TEMPFAIL`) so the dispatcher requeues the task without counting a +failure. With `--format stream-json` the terminal `result` record carries the +same `exit_code`. + #### Delegation in finite chat runs When chat answers and exits (`-Q`, `chat --oneshot`, or a query with non-TTY diff --git a/website/docs/user-guide/features/kanban.md b/website/docs/user-guide/features/kanban.md index 30a156d616..7518d97de4 100644 --- a/website/docs/user-guide/features/kanban.md +++ b/website/docs/user-guide/features/kanban.md @@ -505,8 +505,8 @@ protocol. If the worker process exits with status 0 while the task is still `running`, the dispatcher treats that as a protocol violation and emits a `protocol_violation` event. A dispatcher-spawned worker whose turn failed therefore exits non-zero: `1` for an ordinary failure, and `75` -(`EX_TEMPFAIL`) when the provider rate-limited it or the account hit a -billing/quota wall — the dispatcher records that run as `rate_limited` and +(`EX_TEMPFAIL`) when the provider rate-limited or overloaded it, or the +account hit a billing/quota wall — the dispatcher records that run as `rate_limited` and requeues the task without counting a failure, so a quota window is never booked as a protocol violation.