Skip to content

fix: suppress quota banner after successful stream#104

Merged
Nomadcxx merged 2 commits into
mainfrom
fix/quota-banner-after-success
Jun 17, 2026
Merged

fix: suppress quota banner after successful stream#104
Nomadcxx merged 2 commits into
mainfrom
fix/quota-banner-after-success

Conversation

@Nomadcxx

Copy link
Copy Markdown
Owner

Small fix for the false quota banner we saw with Composer 2.5 Fast.

What changed:

  • track whether streaming has already produced useful assistant output or a successful result
  • if cursor-agent exits non-zero with quota text after that, log it as diagnostic instead of appending a second assistant error chunk
  • keep the existing hard-error path when no successful streamed output was seen
  • add an integration regression where the mock streams an answer, writes quota text to stderr, and exits 1

Test plan:

  • npm test -- tests/integration/opencode-loop.integration.test.ts
  • npm test -- tests/unit/errors.test.ts tests/unit/streaming/openai-sse.test.ts
  • npm run verify:issue-92
  • npm test

@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a664c517-54cd-4d0a-aac6-2955a57f40a0

📥 Commits

Reviewing files that changed from the base of the PR and between d2cee4e and 6a1cc76.

📒 Files selected for processing (1)
  • tests/integration/opencode-loop.integration.test.ts

📝 Walkthrough

Walkthrough

The plugin introduces a sawSuccessfulStreamOutput boolean in both Bun and Node streaming paths, set when successful result events or assistant/thinking SSE chunks are observed. On non-zero cursor-agent exit, a new shouldTreatCursorAgentFailureAsDiagnostic predicate (quota type + saw output) gates whether eviction and error-chunk emission are skipped. A matching integration test and mock scenario validate this silent-passthrough behavior.

Changes

Quota-exit diagnostic passthrough

Layer / File(s) Summary
Helper predicates for quota diagnostic detection
src/plugin.ts
Adds isSuccessfulResultEvent and shouldTreatCursorAgentFailureAsDiagnostic local helpers; the latter checks parseAgentError(errSource).type === "quota" and requires sawSuccessfulStreamOutput to be true.
Bun streaming: sawSuccessfulStreamOutput tracking
src/plugin.ts
Declares the flag at Bun streaming scope and sets it on successful result events in the main loop, on assistant/thinking SSE chunks during live streaming, and again during flush/remainder processing.
Bun non-zero exit: conditional diagnostic skip
src/plugin.ts
Replaces the unconditional eviction + error-chunk path with a conditional branch: quota failures after successful output log a warning and return early; all other failures follow the existing maybeEvictResumeChatId + error-chunk path.
Node streaming: sawSuccessfulStreamOutput tracking
src/plugin.ts
Mirrors the Bun tracking pattern for the Node path: declares the flag and sets it in the Node streaming loop and SSE conversion path.
Node non-zero exit: conditional diagnostic skip
src/plugin.ts
Applies the same quota diagnostic skip to the Node non-zero exit handler.
Mock scenario and integration test
tests/integration/opencode-loop.integration.test.ts
Adds the assistant-text-quota-exit mock branch (stderr "Cursor usage limit", exit 1) and an integration test asserting no quota/error text in assembled output and a [DONE]-terminated SSE stream.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • Nomadcxx/opencode-cursor#99: Introduces the session-resume and pool plumbing in src/plugin.ts that this PR's non-zero exit handling and resume-chat-id eviction logic directly builds on.
  • Nomadcxx/opencode-cursor#100: Hardens which cursor-agent failures should evict the cached resume chat id, the same code path this PR further modifies by gating eviction on the quota diagnostic predicate.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: suppressing quota banners after successful streaming, which matches the core fix implemented in the changeset.
Description check ✅ Passed The description is directly related to the changeset, explaining the quota banner fix, the tracking mechanism, conditional error handling, and the regression test added.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/integration/opencode-loop.integration.test.ts`:
- Around line 220-223: In the mock for the "assistant-text-quota-exit" scenario,
replace the immediate process.exit(1) call with setting process.exitCode to 1
and then returning. This allows any buffered stderr output from the
process.stderr.write call to be properly flushed before the process terminates,
preventing nondeterministic test failures caused by incomplete stream writes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ad9d717e-4ef7-4977-bc0f-cfd41b7e256b

📥 Commits

Reviewing files that changed from the base of the PR and between c7b08c1 and d2cee4e.

📒 Files selected for processing (2)
  • src/plugin.ts
  • tests/integration/opencode-loop.integration.test.ts

Comment thread tests/integration/opencode-loop.integration.test.ts
Use process.exitCode instead of process.exit so buffered mock output
is flushed before the child process ends.

Co-authored-by: Cursor <cursoragent@cursor.com>
@Nomadcxx Nomadcxx merged commit fce9e62 into main Jun 17, 2026
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