Skip to content

fix(kanban): preserve quota exits in non-quiet workers - #98969

Closed
Jamesrobertsonldn wants to merge 1 commit into
NousResearch:mainfrom
Jamesrobertsonldn:codex/fix-kanban-rate-limit-exit
Closed

Jamesrobertsonldn wants to merge 1 commit into
NousResearch:mainfrom
Jamesrobertsonldn:codex/fix-kanban-rate-limit-exit

Conversation

@Jamesrobertsonldn

@Jamesrobertsonldn Jamesrobertsonldn commented Aug 31, 2026 •

Copy link
Copy Markdown

Summary

  • classify structured rate_limit and billing failures for every Kanban one-shot worker
  • capture that classification before fallible post-turn rendering
  • preserve the existing human-readable response, exit summary, and single-query finalization
  • return the dispatcher's existing EX_TEMPFAIL code 75 from the outer CLI boundary
  • fail closed with code 1 if the Kanban constant cannot be imported

Root cause

The fully quiet -Q path already translated a structured provider quota failure into code 75, but the ordinary non-quiet one-shot path printed the same terminal failure and returned code 0. The dispatcher therefore recorded a protocol violation and incremented the card failure counter.

This change reuses the structured conversation result and the existing dispatcher sentinel. It deliberately does not parse logs, make all workers quiet, or raise SystemExit from reusable HermesCLI.chat().

Related prior work inspected: #85083 and #97623. This patch preserves normal non-quiet observability and keeps process-exit policy at main().

Validation

Canonical wrapper:

  • scripts/run_tests.sh tests/cli/test_kanban_rate_limit_exit.py tests/cli/test_cli_interrupt_ack_race.py tests/hermes_cli/test_kanban_db.py tests/cli/test_single_query_session_finalize.py -q — 50 passed, 0 failed, 1 Windows-only skip
  • scripts/run_tests.sh tests/cli/test_chat_q_exit_clear.py tests/cli/test_exit_summary_resume_hint.py tests/hermes_cli/test_signal_handler_kanban_worker.py -q — 11 passed, 0 failed

@Jamesrobertsonldn
Jamesrobertsonldn force-pushed the codex/fix-kanban-rate-limit-exit branch from 86dd3e5 to 50dcd4a Compare August 31, 2026 00:54
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard comp/cron Cron scheduler and job management P3 Low — cosmetic, nice to have sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation labels Aug 31, 2026
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

PR 98969 — fix(kanban): preserve quota exits in non-quiet workers

Summary: cli.py extracts _kanban_worker_rate_limit_exit_code(result) -> Optional[int] (checks HERMES_KANBAN_TASK + result.failed + failure_reason in (rate_limit, billing)), stores self._kanban_worker_exit_code in HermesCLI.chat after agent thread join (before _prompt_start_time freeze and any output flush), and consumes it after _print_exit_summary in the one-shot path. Deduplicates the inline exit-code logic in run_agent/_finalize_single_query that previously was recomputed after output and could be lost if post-turn rendering failed. Adds tests/cli/test_kanban_rate_limit_exit.py and test_cli_interrupt_ack_race.py guard.

Strengths:

  • Correctly preserves the dispatcher's temporary-failure sentinel (KANBAN_RATE_LIMIT_EXIT_CODE) even if _print_exit_summary / flush raises, preventing quota cards from being counted as task failures and tripping the circuit breaker.
  • cli.py:1487-1505 _kanban_worker_rate_limit_exit_code is pure and unit-testable; the try: from hermes_cli.kanban_db import KANBAN_RATE_LIMIT_EXIT_CODE except: return 1 fallback is safe for import-circularity.
  • Two paths (quiet single-query vs human-readable) now share the same helper, eliminating drift.

Non-blocking observations:

  • cli.py:1719 self._kanban_worker_exit_code = None reset per chat() turn is correct for multi-turn sessions; verify it is also cleared on early returns (credential refresh failure) — None is already the default, so okay.
  • cli.py:22133-22135 if _kanban_exit_code is not None: sys.exit(_kanban_exit_code) in the outer one-shot boundary runs after cli._print_exit_summary(clear_screen=False); ensure sys.exit still runs even if _print_exit_summary raised (the finally: _finalize_single_query is after, but the sys.exit is inside the try, so a _print_exit_summary exception would skip it). In practice _kanban_worker_exit_code would be lost on that exception path — consider moving the sys.exit to finally or wrapping _print_exit_summary. Low probability, non-blocking.
  • No change to HERMES_KANBAN_TASK env handling for non-kanban runs (correctly stays None → 0/1 contract).

Verdict: Correct exit-code preservation fix with good test guard. Minor finally placement nit non-blocking.

@teknium1

Copy link
Copy Markdown
Collaborator

Landed on main: #112500 (1b2a75f) gave the non-quiet chat -q Kanban worker the 0/1/75 exit contract, and #112545 (23863cc) made the contract unconditional (a person's chat -q now exits 1 on failure / partial / never-ran, 130 interrupted) and widened the 75 sentinel to upstream_rate_limit and overloaded. Same mechanism as this PR: a shared exit mapping in cli.py plus the raw turn result recorded at settle.

Credit: this cluster reached thirteen PRs before one landed and my sweep missed this one on the first pass (different vocabulary). Precedence on the mechanism goes to the earliest — noted on the merged PR. Closing as implemented on main; thank you for the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard comp/cron Cron scheduler and job management P3 Low — cosmetic, nice to have sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants