Skip to content

fix(server): preserve buffered Claude usage-limit failures - #19543

Open
omar7550 wants to merge 3 commits into
pingdotgg:mainfrom
omar7550:fix/claude-buffered-usage-limit-result
Open

omar7550 wants to merge 3 commits into
pingdotgg:mainfrom
omar7550:fix/claude-buffered-usage-limit-result

Conversation

@omar7550

@omar7550 omar7550 commented Oct 14, 2026 •

Copy link
Copy Markdown

A buffered Claude quota failure could be replaced by a later empty task-notification success, leaving the run completed without the error needed for usage-limit recovery. Replay now selects the latest meaningful result, with a last-result fallback when the buffer is entirely empty notifications.

The newer-turn guard still reads the actual buffer tail. This prevents an older quota failure from finishing a newer turn that has already started.

Related to #16815; preserves the successor-turn behavior from #15816. Original fix by @omar7550 in this PR. The takeover integrates current main and folds the combined failure/successor regression into the existing parameterized fixture.

Validation: four focused replay cases pass, including quota preservation, all-empty settlement, and successor turns with and without a preceding failure. The quota case fails with main's selection; the combined case fails when selection and successor detection share the filtered result. Server typecheck, formatting, and targeted lint pass apart from an existing unused-layer warning.

The wider wake group has an unchanged foreground-subagent timeout on both baseline and fixed versions; this is not claimed as a passing full adapter suite. No live subscription limit was triggered.

Independent Opus and Sol reviews found no blockers in the complete integrated diff.

Prepared with Claude Opus 5.5, Claude Fable 5.1, and GPT-6.1 Sol via T3 Code, coordinated with Codex.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Oct 14, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 9810a69

Macroscope's review found this PR approvable — This is a narrowly scoped Claude background-continuation bug fix that preserves quota failures without changing schemas, defaults, deployment behavior, or static-analysis settings. The production logic is localized and accompanied by regression coverage for both the failure-preservation and all-empty fallback paths.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 75c14e95-b1eb-4810-b5c6-5063711133de
📥 Commits

Reviewing files that changed from the base of the PR and between 9810a69 and 7d85373.

📒 Files selected for processing: 2
  • apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts
  • apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

📝 Walkthrough

Claude continuation replay now skips qualifying empty, zero-turn task-notification results when selecting the result used to settle a turn. Tests cover wake ordering, quota failures, and empty result strings.

Changes: Claude continuation result selection

Layer / File(s) Summary
Filter lifecycle results during continuation replay
apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts, apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts
Replay selects the last result that does not match the completed, successful, empty, zero-turn task-notification shape. If every result matches, replay falls back to the final result. Tests cover empty lifecycle results before wake attachment, including after a quota failure, and use an empty result string in a continuation test.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 7d853

The continuation keeps an earlier usage-limit failure from being displaced by an empty notification. No merge-blocking issue was identified.

### Architecture Summary

Architecture risk: 🔵 Low · up to 7d853

The change affects 1 system.

Changed systems: apps/server

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — apps/server (service) was modified; 2 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts: Adds an empty, zero-turn task-notification result after wakeTurnInit in the rejected-wake scenario, checking that this lifecycle frame does not replace the root turn’s usage-limit result.
  • observed — Modified behavior in apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts: Changes the zero-turn continuation result fixture from descriptive text to an empty string.
  • observed — Modified behavior in apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts: Replaces the single wake-ordering test with cases for an empty lifecycle result alone and one preceded by a quota failure. Both cases verify that the continuation remains unsettled while Claude is thinking, then completes with the actual wake output.
  • observed — Modified behavior in apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts: The empty-turn-before-next-turn check still uses the raw last result. Replay now selects the last result that is not an empty task-notification success (success, no error, zero turns, empty result, completed status), falling back to the raw last result if none qualify.
Pre-merge checks: Passed 4
Check name Status Explanation
Linked Issues check Passed The only directly linked issue, #17862, is closed. It supplies historical context only. No active directly linked issue supplies coding requirements for this pull request.
Out of Scope Changes check Passed The whole-PR summary shows changes only in ClaudeAdapterV2.ts and ClaudeAdapterV2.test.ts. The code preserves the raw buffer tail for successor-turn detection and excludes only empty task-notification…
Title check Passed The title clearly describes the main change: preserving buffered Claude usage-limit failures. It is concise and uses the required conventional commit format.
Description check Passed The description explains the problem, implementation, scope, related issues, focused verification, known limitations, and tooling used. It does not reproduce the template headings or provide an explic…
Full details: Out of Scope Changes check

Explanation

The whole-PR summary shows changes only in ClaudeAdapterV2.ts and ClaudeAdapterV2.test.ts. The code preserves the raw buffer tail for successor-turn detection and excludes only empty task-notification successes from replay selection, with an all-empty fallback. The tests cover quota preservation, empty settlement, and successor turns. No unrelated settings, schema, client, or recovery-policy changes are shown.

Full details: Description check

Explanation

The description explains the problem, implementation, scope, related issues, focused verification, known limitations, and tooling used. It does not reproduce the template headings or provide an explicit maintainer approval comment, but it is otherwise sufficiently complete.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@juliusmarminge

Copy link
Copy Markdown
Member

Taking over this one. I’ll reduce the tests to the useful coverage, update the implementation against current main, and verify it before publishing the changes.

This branch has not been deployed

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

Labels

size:S 10-29 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants