Skip to content

Preserve provider retries during Claude background drain - #1623

Open
ymichael wants to merge 2 commits into
mainfrom
bb/investigate-provider-rate-limit-failure-thr_ck4myt6kaw
Open

Preserve provider retries during Claude background drain#1623
ymichael wants to merge 2 commits into
mainfrom
bb/investigate-provider-rate-limit-failure-thr_ck4myt6kaw

Conversation

@ymichael

@ymichael ymichael commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • prevent trailing Claude background and sidechain events from opening an unaccepted provider-only turn after a terminal failure
  • reconcile provider-retry lifecycle events against server-owned recovery evidence instead of blindly cancelling scheduled retries
  • recover the latest failed turn with accepted input while preserving newer-request and manual-stop supersession
  • bump the host daemon protocol version because Claude adapter event behavior changes across the daemon boundary

Root cause

The provider-retry plugin initially scheduled the accepted rate-limited failure. While Claude background output drained, the adapter opened a synthetic turn with no client request or accepted input. Its thread.active event removed the scheduled retry, and recovery then inspected the synthetic failed turn and returned input-not-accepted.

Validation

  • provider-retry plugin: 22/22 tests passed
  • Claude adapter: 153/153 tests passed
  • provider recovery and stop lifecycle: 25/25 tests passed
  • database: 384/384 tests passed
  • host daemon contract: 36/36 tests passed
  • affected Turbo typechecks passed
  • git diff --check passed

The broader server run passed 1666 tests and had one unrelated existing failure in third-party-marketplaces.test.ts for a missing SVG content type.

AGENT GENERATED: by GPT-5

@bb-slop-cop bb-slop-cop Bot 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.

🚨 SLOP COP 🚨 · review

I am the SlopCop. I am reviewing this pull request now.

I will check security, code quality, performance, architecture, duplication, and the relevant end-to-end path. I will post one final review with the results.

Comment thread packages/agent-runtime/src/claude-code/translate-message.ts Outdated
Comment thread plugins/provider-retry/server.ts Outdated

@bb-slop-cop bb-slop-cop Bot 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.

🚨 SLOP COP 🚨 · review

Plain English summary: Claude can hit a usage limit while a background agent still sends final events. Those events could hide the failed user request and cancel its scheduled retry. This change keeps recovery tied to the latest accepted user turn. It also suppresses most false turns from late Claude events.

I found two issues:

  1. P2 — Active and idle events inspect every thread. The retry plugin now calls the recovery SDK on every normal transition. This adds two loopback calls and several synchronous SQLite reads to most turns. The active and idle handlers should inspect only threads that already have a retry entry.

  2. P2 — Bridge errors bypass the new suppression rule. The rule covers SDK messages, but the bridge-error envelope uses another path. A failed result followed by a stream error can still create a false, unaccepted turn and duplicate error output. The common Claude error path should honor the same suppression state.

I found no security issue. The retry checks remain scoped to one thread and require the exact accepted request before reuse.

I also checked architecture and duplication. The database helper has no existing equivalent. Its location in @bb/db is correct. I found no stale names or duplicate recovery policy.

Validation passed for the changed paths:

  • Claude adapter: 152 tests.
  • Server recovery: 15 tests.
  • Provider retry plugin: 21 tests.
  • Host daemon contract: 50 tests.
  • Type checks: all four affected packages.
  • GitHub CI: all required checks pass.

I did not run a browser test. This path needs a real Claude subscription limit and has no browser-only route.

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.

1 participant