fix(cli): one-shot chat -q exits non-zero on failure; 75 covers upstream 429 and overload
The non-quiet one-shot path exited 0 unless a Kanban worker was running, so scripts could not tell a failed `hermes chat -q` from a good one and an incomplete turn (partial, iteration budget) still read as success (#111770). Both one-shot paths now share one contract: 0 completed, 1 failed / partial / incomplete / never ran, 130 interrupted. The Kanban EX_TEMPFAIL sentinel also fires for `upstream_rate_limit` (aggregator's upstream 429) and `overloaded` (503/529): neither says anything about the task, so the dispatcher should requeue without a failure tick rather than count it toward the breaker.
This commit is contained in:
@@ -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)
|
||||
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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 <prompt>`` (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
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user