diff --git a/cli.py b/cli.py index 4ee46971ee..a9fedd7d07 100644 --- a/cli.py +++ b/cli.py @@ -4544,11 +4544,11 @@ def _run_single_query_mode(cli, query, image, quiet, oneshot, stream_json: bool 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 reap - # classifier reads rc=0 with the task still `running` as a protocol violation, - # which blocks the card on the FIRST occurrence. A provider quota wall - # therefore killed the card permanently, along with everything queued behind - # it. Interactive and plain `-q` runs are unaffected: they still exit 0. + # 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))) finally: diff --git a/tests/hermes_cli/test_single_query_exit_contract.py b/tests/hermes_cli/test_single_query_exit_contract.py index 6e044f30f4..0a223ec65c 100644 --- a/tests/hermes_cli/test_single_query_exit_contract.py +++ b/tests/hermes_cli/test_single_query_exit_contract.py @@ -1,19 +1,14 @@ -"""One-shot runs map their outcome onto an exit code, on BOTH one-shot paths. +"""A dispatcher-spawned ``chat -q`` worker reports its outcome in its exit code. -The Kanban dispatcher spawns workers as ``hermes ... chat -q `` — the -non-quiet single-query path. That path had no exit contract at all: it ran the -turn and fell through to an implicit 0 whatever happened. The dispatcher's reap -classifier reads rc=0 with the task still ``running`` as a protocol violation -and blocks the card on the first occurrence, so one provider quota wall killed -the card permanently along with every card queued behind it. - -The EX_TEMPFAIL sentinel that exists to prevent exactly that was wired into the -``-Q`` path only. These tests pin the contract on both. +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). """ from __future__ import annotations -import os from types import SimpleNamespace import pytest @@ -22,56 +17,19 @@ import cli from hermes_cli.kanban_db import KANBAN_RATE_LIMIT_EXIT_CODE -# -------------------------------------------------------------------------- -# the shared mapping -# -------------------------------------------------------------------------- - @pytest.fixture(autouse=True) def _no_inherited_kanban_env(monkeypatch): monkeypatch.delenv("HERMES_KANBAN_TASK", raising=False) monkeypatch.delenv("HERMES_KANBAN_GOAL_MODE", raising=False) -@pytest.mark.parametrize("result", [ - {"final_response": "ok"}, - {"failed": False}, - None, - "not a dict", -]) -def test_a_run_that_did_not_fail_exits_zero(result): - assert cli._single_query_exit_code(result) == 0 - - -def test_a_plain_failure_exits_one(): - assert cli._single_query_exit_code({"failed": True, "failure_reason": "tool_error"}) == 1 - - -@pytest.mark.parametrize("reason", ["rate_limit", "billing"]) -def test_a_kanban_worker_on_a_quota_wall_exits_with_the_sentinel(monkeypatch, reason): - monkeypatch.setenv("HERMES_KANBAN_TASK", "t_abc123") - result = {"failed": True, "failure_reason": reason} - assert cli._single_query_exit_code(result) == KANBAN_RATE_LIMIT_EXIT_CODE - - -@pytest.mark.parametrize("reason", ["rate_limit", "billing"]) -def test_the_sentinel_is_for_kanban_workers_only(reason): - """A human's one-shot run that hit a quota wall is an ordinary failure.""" - assert "HERMES_KANBAN_TASK" not in os.environ - assert cli._single_query_exit_code({"failed": True, "failure_reason": reason}) == 1 - - -def test_a_kanban_worker_failing_for_another_reason_still_exits_one(monkeypatch): - monkeypatch.setenv("HERMES_KANBAN_TASK", "t_abc123") - assert cli._single_query_exit_code({"failed": True, "failure_reason": "tool_error"}) == 1 - - -# -------------------------------------------------------------------------- -# the path the dispatcher actually spawns: `chat -q`, non-quiet -# -------------------------------------------------------------------------- - -def _fake_cli(turn_result): - """A CLI stub exercising only what the non-quiet one-shot tail touches.""" - return SimpleNamespace( +def _run_non_quiet(monkeypatch, turn_result): + """Drive ``_run_single_query_mode`` down the non-quiet tail; return the exit code (None = fell through).""" + monkeypatch.setattr(cli, "_should_seed_interactive", lambda *a, **k: False) + monkeypatch.setattr(cli, "_collect_query_images", lambda q, i: (q, [])) + monkeypatch.setattr(cli, "_collect_kanban_task_images", lambda imgs: []) + monkeypatch.setattr(cli, "_finalize_single_query", lambda c: None) + stub = SimpleNamespace( _single_query_mode=False, _claim_active_session=lambda *a, **k: True, console=SimpleNamespace(print=lambda *a, **k: None), @@ -80,75 +38,21 @@ def _fake_cli(turn_result): _print_exit_summary=lambda **k: None, _last_turn_result=turn_result, ) - - -def _run_non_quiet(monkeypatch, turn_result): - monkeypatch.setattr(cli, "_should_seed_interactive", lambda *a, **k: False) - monkeypatch.setattr(cli, "_collect_query_images", lambda q, i: (q, [])) - monkeypatch.setattr(cli, "_collect_kanban_task_images", lambda imgs: []) - monkeypatch.setattr(cli, "_finalize_single_query", lambda c: None) - stub = _fake_cli(turn_result) try: cli._run_single_query_mode(stub, "work kanban task t_abc123", None, False, True) except SystemExit as exc: return exc.code - return None # fell through without exiting + return None @pytest.mark.parametrize("reason", ["rate_limit", "billing"]) def test_dispatcher_spawned_worker_signals_a_quota_wall_not_a_protocol_violation(monkeypatch, reason): - """The regression. rc=0 here is read as a protocol violation and blocks the card.""" 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_dispatcher_spawned_worker_reports_an_ordinary_failure_as_nonzero(monkeypatch): - monkeypatch.setenv("HERMES_KANBAN_TASK", "t_abc123") - code = _run_non_quiet(monkeypatch, {"failed": True, "failure_reason": "tool_error"}) - assert code == 1 - - -def test_dispatcher_spawned_worker_that_succeeded_exits_zero(monkeypatch): - monkeypatch.setenv("HERMES_KANBAN_TASK", "t_abc123") - code = _run_non_quiet(monkeypatch, {"final_response": "done"}) - assert code == 0 - - def test_a_human_one_shot_run_is_unaffected(monkeypatch): - """No HERMES_KANBAN_TASK: the path must not start exiting non-zero on people.""" + """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 - - -# -------------------------------------------------------------------------- -# the plumbing that makes the outcome reachable from that path -# -------------------------------------------------------------------------- - -def test_settling_a_turn_records_the_raw_result(): - """chat() returns a rendered string; the exit mapping needs the result dict.""" - from hermes_cli.cli_chat_turn_mixin import CLIChatTurnMixin - - stub = SimpleNamespace( - _prompt_start_time=None, - _prompt_duration=0.0, - _flush_stream=lambda: None, - conversation_history=[], - agent=None, - ) - result = {"failed": True, "failure_reason": "rate_limit"} - turn = SimpleNamespace( - result=result, use_streaming_tts=False, text_queue=None, tts_thread=None, - ) - - CLIChatTurnMixin._chat_settle_turn(stub, turn) - - assert stub._last_turn_result is result - - -def test_the_recorded_result_defaults_to_none(): - """A CLI that never completed a turn must map to 0, not raise.""" - from hermes_cli.cli_chat_turn_mixin import CLIChatTurnMixin - - assert CLIChatTurnMixin._last_turn_result is None - assert cli._single_query_exit_code(CLIChatTurnMixin._last_turn_result) == 0