Skip to content

fix(acp): park session-limit failures until reset without burning retries - #5975

Draft
olympusbuildz wants to merge 2 commits into
block:mainfrom
olympusbuildz:fix/acp-session-limit-park-5918
Draft

olympusbuildz wants to merge 2 commits into
block:mainfrom
olympusbuildz:fix/acp-session-limit-park-5918

Conversation

@olympusbuildz

@olympusbuildz olympusbuildz commented Aug 15, 2026

Copy link
Copy Markdown

Problem

When a turn fails with a provider session limit error, buzz-acp retries on the ordinary exponential backoff budget (MAX_RETRIES = 10, ~25 minutes) and then dead-letters the batch. Subscription limits reset on a multi-hour wall clock, so the message is discarded before the agent could ever answer it — and it is never reprocessed when quota returns.

Observed Claude subscription text:

You've hit your session limit · resets 1:50pm (America/Buenos_Aires)

Root cause

handle_prompt_result already special-cases non-retryable auth errors (immediate dead-letter). Session-limit errors fall through to generic queue.requeue(), which burns the retry counter. The error string carries the reset instant; the harness ignored it.

Fix

  1. Classify session-limit / quota-window errors (is_session_limit_error).
  2. Parse a park delay from the message (resets in 2h, retry in 45m, wall-clock resets 1:50pm best-effort in host local TZ). Fallback 1h, cap 6h.
  3. New EventQueue::requeue_until parks the batch at retry_after without incrementing retry_counts.
  4. Wire the branch next to the auth path in handle_prompt_result.

Why it matters

Owner DMs and channel mentions are permanently lost during multi-hour subscription windows even though the agent process is healthy. Parking preserves the work until quota returns.

Test plan

. ./bin/activate-hermit
cargo test -p buzz-acp --lib is_session_limit
cargo test -p buzz-acp --lib parse_session_limit
cargo test -p buzz-acp --lib test_requeue_until
cargo test -p buzz-acp --lib is_auth_error

All of the above passed @ this head.

Full cargo test -p buzz-acp --lib: 783 passed; 2 failures are pre-existing on origin/main (config::tests::lazy_pool_defaults_off, idle_pool_sleep_defaults_disabled_and_accepts_cli_value) — same fail on clean main at 78cbffe when env defaults differ; not introduced by this diff.

cargo fmt -p buzz-acp clean; cargo clippy -p buzz-acp --lib -- -D warnings clean.

Risk / blast radius

  • Only PromptOutcome::Error messages matching high-precision limit phrases take the park path.
  • Transient non-limit errors still use ordinary requeue + dead-letter.
  • Wall-clock parse uses host local TZ (no new tzdb dep); relative delays and 1h fallback cover the common case. Worst case: extra parks until the window ends — still better than dead-letter at 25m.

Closest work

Fixes #5918

Peer-review harden

Addressed @themiguelamador review @ 37b5a0a35:

  • Named timezones in wall-clock reset text → 1h fallback (no host-local guess)
  • Relative delay scale clamps before Duration::from_secs_f64 (no inf panic)
  • requeue_until overflow log/docs match pop_back newest-at-back eviction

…ries

Provider session-limit errors (e.g. Claude "You've hit your session limit ·
resets 1:50pm …") self-heal after hours, but the ordinary EventQueue retry
budget exhausts in ~25 minutes and dead-letters the batch. Classify the
error, park via requeue_until with a parsed/fallback delay, and leave
retry_counts untouched so the message is still answered when quota returns.

Fixes block#5918

Signed-off-by: Olympusbuildz <Olympus.roots@outlook.com>
Co-authored-by: Olympusbuildz <Olympus.roots@outlook.com>
Signed-off-by: Olympusbuildz <Olympus.roots@outlook.com>

@themiguelamador themiguelamador left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found three issues in the submitted implementation:

  • The exact observed Claude message includes an explicit timezone (America/Buenos_Aires), but the parser ignored it and interpreted the wall clock in the host timezone. That can retry before the real reset and then roll the next attempt to tomorrow.
  • An oversized numeric delay from untrusted provider error text can overflow to infinity during unit conversion and make Duration::from_secs_f64 panic.
  • requeue_until removes from the back on overflow (the newest arrivals), while its warning said it dropped the oldest event.

I fixed all three in Complear/buzz commit f8aaed727 (review/pr-5975-fix): explicit named timezones now take the bounded one-hour fallback, duration conversion is clamped before constructing Duration, and the queue documentation/log accurately describe the eviction policy.

Verification: cargo test -p buzz-acp --lib (787 passed), focused parser/requeue/classifier tests, cargo clippy -p buzz-acp --lib -- -D warnings, and cargo fmt --all --check.

Address review on block#5975:
- Named timezones in wall-clock reset text (e.g. America/Buenos_Aires)
  fall back to the 1h park instead of host-local guessing.
- Oversized relative delays clamp before Duration construction (no inf panic).
- requeue_until overflow log/docs match pop_back newest-at-back eviction.

Signed-off-by: Olympusbuildz <Olympus.roots@outlook.com>
Co-authored-by: Olympusbuildz <Olympus.roots@outlook.com>
Signed-off-by: Olympusbuildz <Olympus.roots@outlook.com>
@olympusbuildz

Copy link
Copy Markdown
Author

Addressed review (new commit 37b5a0a35):

  1. Named TZresets 1:50pm (America/Buenos_Aires) no longer host-local guesses; falls back to 1h park.
  2. Overflow clamp — huge resets in Nh values clamp before Duration construction (no inf panic).
  3. Eviction logrequeue_until overflow message matches pop_back newest-at-back policy.

Mini focused: parse_session_limit (6) + requeue_until + is_session_limit green @ 37b5a0a35.

Thanks @themiguelamador.

@cristiansotogarciaxatech

Copy link
Copy Markdown

Field data from a different provider, because this fix has a gap that my logs make obvious.

I run a fleet of managed agents on Windows against several providers. I hit #5918 independently this morning, worked it back through the source, and landed on your branch. So the useful part first. This wants merging. Changes were requested on 2026-08-16, you addressed them the same week, and nothing has moved since 2026-08-19. That is a long time to sit on a bug that permanently destroys user messages.

Now the gap.

is_session_limit_error gates on prose. One of my providers returns this, verbatim:

WARN buzz_acp: agent_returned (application error — pipe intact) agent=0 outcome="error"
     configured_model=grok-4.6 pid=44940 error=Agent reported error (code -32003): Rate limited
WARN buzz_acp::queue: requeueing failed batch with backoff channel_id=… attempt=1 max=10 delay_secs=4.453
…
WARN buzz_acp::queue: requeueing failed batch with backoff channel_id=… attempt=10 max=10 delay_secs=253.016
ERROR buzz_acp::queue: dead-lettering batch after 10 retries — discarding 1 events channel_id=… attempt=11 events=1

Two words. No reset instant, no resets token, no you've hit your. Walk it through your classifier:

  • session limit, no
  • hit your limit, no
  • you've hit your and limit, no
  • rate limit and resets, half. rate limited does contain rate limit, but there is no resets, so the arm is false
  • usage limit and resets, no

It returns false. An actual rate limit falls straight through to the generic ladder and the message is destroyed 23 minutes later.

24 hours on one machine, 9 agents:

agent errors returned 283
parked by this PR's classifier 0
of those, -32003 Rate limited 138
dead-letters 15
events permanently destroyed 17
destroyed with -32003 Rate limited as the cause 6

Be careful with that 283, because I nearly overstated it myself. The other 137 are -32603 Internal error with no trailing text, and those are genuinely not quota errors, so falling through is the correct outcome for them. The real claim is the 138. Six messages destroyed in a single day by exactly the failure this PR exists to stop, and this PR does not stop them.

What I ran and what I did not. I did not compile your branch. is_session_limit_error is to_ascii_lowercase plus contains, so I replicated the predicate exactly and pushed my real error strings through it, with your own Claude subscription string as a positive control (parks) and an unrelated agent error as a negative control (does not park). That is a replication, not a compile, and I would rather say so than let you assume I built it.

The fix is small, because your delay side already covers the hard part. parse_session_limit_park_delay falls back to SESSION_LIMIT_FALLBACK_DELAY when nothing parses, which is precisely the no-reset-instant case. Only the gate needs widening.

My opinion, and it is the reason I bothered writing this. Stop matching English. -32003 is a code the agent already handed you and it means rate limited. It appears nowhere in this tree today, which is the whole bug in one line.

matches!(error, acp::AcpError::AgentError { code: -32003, .. })

Keep the phrase list, because Claude folds quota into -32603 and you need the prose for that. Add the code arm beside it. One extra branch, and it covers a provider your current tests cannot reach, including the one case where the error text gives you nothing to parse at all.

Happy to send this as a commit against your branch with tests instead of a comment. Your call.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

buzz-acp: session-limit failures are dead-lettered ~25 min into a multi-hour limit — the message is lost, never reprocessed when quota returns

3 participants