feat: opt-in Claude 1M beta header with safe 200k fallback#514
feat: opt-in Claude 1M beta header with safe 200k fallback#514ndycode wants to merge 13 commits intoNoeFabris:devfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds two config fields ( Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ 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. Comment |
Add experimental Claude long-context beta header support for Claude 4.6 models behind config flags, with automatic one-time fallback to stable 200k behavior when provider rejects the beta header. Includes schema/docs/model metadata updates and request-level tests covering header injection, dedupe, retry-disable behavior, and rejection detection. Refs NoeFabris#506 Co-authored-by: Codex <noreply@openai.com>
ndycode
force-pushed
the
fix/issue-506-claude-1m-beta-fallback
branch
from
b2e0b50 to
fe568b7
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/plugin/config/schema.test.ts (1)
72-86: Add an assertion forminLengthonclaude_long_context_beta_header.The schema enforces non-empty header values; locking that in tests will prevent silent regression.
✅ Proposed test hardening
- const schema = JSON.parse(readFileSync(schemaPath, "utf8")) as { - properties?: Record<string, { type?: string; default?: unknown; description?: string }>; - }; + const schema = JSON.parse(readFileSync(schemaPath, "utf8")) as { + properties?: Record<string, { type?: string; default?: unknown; description?: string; minLength?: number }>; + }; @@ expect(claudeLongContextBetaHeader).toMatchObject({ type: "string", default: "context-1m-2025-08-07", + minLength: 1, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugin/config/schema.test.ts` around lines 72 - 86, The test for claude_long_context_beta_header misses asserting the schema's non-empty constraint; update the "documents claude_long_context_beta_header in the JSON schema" test to assert that schema.properties?.claude_long_context_beta_header has a minLength of 1 (or a numeric minLength > 0), e.g. check the existence and numeric value of minLength on the claude_long_context_beta_header object so the non-empty header requirement is locked into the test.src/plugin.ts (1)
2379-2388: Consider redactingreasonPreviewbefore debug logging.At Line 2379, the preview is taken from raw provider body text; those payloads can occasionally echo user/tool content. A lightweight scrub before logging would reduce accidental sensitive-data exposure.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugin.ts` around lines 2379 - 2388, The preview derived from errorBodyText (reasonPreview) may contain echoed user/tool content and should be redacted before logging; update the code around reasonPreview, pushDebug, and log.debug to run a lightweight scrub function (e.g., redactSensitive or scrubText) on the extracted string — trimming, replacing likely secrets (emails, tokens, long hex sequences, credit-card patterns) with placeholders and/or masking everything after a short safe length — and then log the redactedPreview (not the raw reasonPreview) and continue to include prepared.claudeLongContextBetaHeader and prepared.effectiveModel as before so logs no longer contain unredacted provider payloads.src/plugin/request.test.ts (1)
95-125: Add one positive 403 case for rejection detection.At Line 116, the suite verifies non-4xx behavior, but there is no positive 403 scenario. Since the detector intentionally supports both 400 and 403, adding a 403-positive test would lock that contract in.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugin/request.test.ts` around lines 95 - 125, Add a positive 403 test for the isUnsupportedClaudeLongContextBetaError detector: inside the existing "isUnsupportedClaudeLongContextBetaError" describe block add an it case that constructs a JSON body with error.message containing "unsupported anthropic-beta header context-1m-2025-08-07" (same pattern as the 400 positive case) calls isUnsupportedClaudeLongContextBetaError(403, body) and asserts the result is true so the test suite verifies both 400 and 403 are detected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/plugin/config/schema.ts`:
- Line 245: The schema for claude_long_context_beta_header currently uses
z.string().min(1) which allows whitespace-only values; update the validator for
claude_long_context_beta_header to trim input before length check by using
z.string().trim().min(1).default("context-1m-2025-08-07") so whitespace-only
headers are rejected prior to being sent to the provider.
---
Nitpick comments:
In `@src/plugin.ts`:
- Around line 2379-2388: The preview derived from errorBodyText (reasonPreview)
may contain echoed user/tool content and should be redacted before logging;
update the code around reasonPreview, pushDebug, and log.debug to run a
lightweight scrub function (e.g., redactSensitive or scrubText) on the extracted
string — trimming, replacing likely secrets (emails, tokens, long hex sequences,
credit-card patterns) with placeholders and/or masking everything after a short
safe length — and then log the redactedPreview (not the raw reasonPreview) and
continue to include prepared.claudeLongContextBetaHeader and
prepared.effectiveModel as before so logs no longer contain unredacted provider
payloads.
In `@src/plugin/config/schema.test.ts`:
- Around line 72-86: The test for claude_long_context_beta_header misses
asserting the schema's non-empty constraint; update the "documents
claude_long_context_beta_header in the JSON schema" test to assert that
schema.properties?.claude_long_context_beta_header has a minLength of 1 (or a
numeric minLength > 0), e.g. check the existence and numeric value of minLength
on the claude_long_context_beta_header object so the non-empty header
requirement is locked into the test.
In `@src/plugin/request.test.ts`:
- Around line 95-125: Add a positive 403 test for the
isUnsupportedClaudeLongContextBetaError detector: inside the existing
"isUnsupportedClaudeLongContextBetaError" describe block add an it case that
constructs a JSON body with error.message containing "unsupported anthropic-beta
header context-1m-2025-08-07" (same pattern as the 400 positive case) calls
isUnsupportedClaudeLongContextBetaError(403, body) and asserts the result is
true so the test suite verifies both 400 and 403 are detected.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (11)
README.mdassets/antigravity.schema.jsondocs/CONFIGURATION.mddocs/MODEL-VARIANTS.mdscript/build-schema.tssrc/plugin.tssrc/plugin/config/models.tssrc/plugin/config/schema.test.tssrc/plugin/config/schema.tssrc/plugin/request.test.tssrc/plugin/request.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Greptile Review
🧰 Additional context used
🧬 Code graph analysis (3)
src/plugin/config/schema.test.ts (1)
src/plugin/config/schema.ts (1)
DEFAULT_CONFIG(469-524)
src/plugin/request.test.ts (1)
src/plugin/request.ts (2)
isUnsupportedClaudeLongContextBetaError(760-788)prepareAntigravityRequest(810-1654)
src/plugin.ts (1)
src/plugin/request.ts (1)
isUnsupportedClaudeLongContextBetaError(760-788)
🔇 Additional comments (14)
assets/antigravity.schema.json (1)
123-133: Looks good: new long-context config fields are well-scoped and backward-compatible.The defaults, types, and descriptions for both new keys are clear and align with the fallback behavior described in the PR.
README.md (1)
123-124: Docs update is clear and consistent.The 200k-base labeling and explicit opt-in/fallback note reduce ambiguity for users selecting Claude models.
Also applies to: 143-143, 194-194, 199-199
script/build-schema.ts (1)
44-47: Nice schema-generation sync.Adding these keys to
optionDescriptionskeeps generated schema docs aligned with runtime config surface.src/plugin/config/models.ts (1)
71-71: Model label clarification is good.The added “200k base” suffix improves UX without affecting behavior.
Also applies to: 76-76
src/plugin/config/schema.test.ts (1)
50-70: Good test coverage for the new config defaults and schema docs.This block cleanly validates the new boolean flag contract.
docs/MODEL-VARIANTS.md (1)
107-109: Clear and helpful clarification.The 200k-default note plus beta fallback behavior is documented in the right place for variant users.
Also applies to: 115-115
docs/CONFIGURATION.md (1)
38-39: Strong configuration docs update.The option table, behavior section, and defaults table are aligned and make the opt-in fallback path easy to understand.
Also applies to: 56-72, 191-191
src/plugin/config/schema.ts (1)
483-484: Default config wiring for new fields looks correct.
DEFAULT_CONFIGis aligned with the new opt-in behavior and default header token.src/plugin/request.test.ts (1)
692-789: Strong coverage for long-context header behavior.This block cleanly validates apply/non-apply conditions, header deduplication, and retry-disable behavior.
src/plugin.ts (2)
123-140: Toast dedupe helper is well-bounded and practical.The per-session gate plus capped set size is a solid anti-spam guard for fallback warnings.
2359-2393: The 4xx beta-rejection fallback flow is implemented cleanly.The guard on
claudeLongContextBetaApplied, one-time retry disable, toast throttling, and same-endpoint retry (i -= 1) all align with a safe fallback path.src/plugin/request.ts (3)
724-746: Header append and long-context state plumbing look solid.The shared
appendAnthropicBetaHeaderpath avoids duplicate token insertion, and the applied/header return metadata cleanly supports outer retry fallback decisions.Also applies to: 1596-1606, 1612-1612, 1649-1650
760-788: Unsupported-beta detector logic is pragmatic for fallback triggering.The status gate plus keyword checks are a reasonable balance for safely switching to the stable path when provider beta-header support is absent.
748-751: No change needed—the exact match for Sonnet is intentional and correct.The model resolver only emits tier variants (
-low,-medium,-high) for Claude Opus-thinking models, not for Sonnet. Sonnet 4.6 resolves to the exact base nameclaude-sonnet-4-6without suffix variants. The asymmetric matching—exact equality for Sonnet vs. prefix for Opus—correctly reflects this design difference.Likely an incorrect or invalid review comment.
Code Review SummaryStatus: No New Issues Found | Recommendation: Merge OverviewThis PR has undergone multiple iterations with 11 fix commits addressing review feedback. The current diff represents the final state after these corrections.
Previous Review Comments (Already Addressed)The existing 27 inline comments were from earlier review cycles. The following have been addressed in subsequent commits:
Other Observations (Not in Diff)Remaining architectural concerns from prior reviews (not in current diff, already documented in inline comments):
Files Reviewed (15 files)
|
Greptile SummaryThis PR introduces an opt-in experimental long-context header for Claude 4.6 models, with automatic one-time fallback to the stable 200k path on provider rejection. It also adds a log scrubbing utility for sanitising sensitive fields in rejection previews, and extends test scripts with per-model and per-turn timeout overrides. Key changes:
Confidence Score: 4/5
Sequence DiagramsequenceDiagram
participant Client
participant Plugin as plugin.ts
participant Prepare as prepareAntigravityRequest
participant Provider as Antigravity Provider
Client->>Plugin: fetch request
Plugin->>Prepare: prepare with claudeLongContextBetaEnabled=true
alt eligible Claude 4.6 model AND not disabled
Prepare->>Prepare: appendAnthropicBetaHeader context-1m-2025-08-07
Prepare-->>Plugin: claudeLongContextBetaApplied=true
else disabled or ineligible
Prepare-->>Plugin: claudeLongContextBetaApplied=false
end
Plugin->>Provider: fetch prepared request
alt Provider accepts
Provider-->>Plugin: 200 OK
Plugin-->>Client: response
else Provider rejects beta header 400/403/422
Provider-->>Plugin: error response
Plugin->>Plugin: isUnsupportedClaudeLongContextBetaError?
alt YES - beta rejection
Plugin->>Plugin: disableForRetry=true
Plugin->>Plugin: refund hybrid token if consumed
opt first rejection this session
Plugin->>Client: warning toast fallback to 200k
end
Plugin->>Plugin: log rejection via scrubTextForLog
Plugin->>Prepare: retry with disableForRetry=true
Prepare-->>Plugin: claudeLongContextBetaApplied=false
Plugin->>Provider: fetch without beta header
Provider-->>Plugin: 200 OK stable 200k path
Plugin-->>Client: response
else NO - unrelated error
Plugin->>Plugin: normal error handling
end
end
Last reviewed commit: a2045e8 |
- trim beta header config to reject whitespace-only values - assert minLength in schema JSON test - add positive 403 detection test for beta rejection - scrub provider error preview before debug logging Co-authored-by: Codex <noreply@openai.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/plugin/logging-utils.ts`:
- Around line 85-90: The current redaction chain assigned to the scrubbed
variable misses credentials when the value is prefixed with a scheme like
"Bearer"; update the authorization/api key/token/password regex used in the
.replace on scrubbed to also accept and redact an optional auth scheme and its
token (e.g., match patterns like "authorization\s*[:=]\s*Bearer\s+[^\s,;]+" or
generalize to capture an optional word scheme before the secret), so that both
"authorization: Bearer abc123" and "authorization: abc123" result in the secret
being replaced with "[redacted]" while preserving the header label; modify the
replace call that references the group for
(?:authorization|api[_-]?key|token|secret|password) to include an optional
scheme capture and redact the token portion.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (6)
src/plugin.tssrc/plugin/config/schema.test.tssrc/plugin/config/schema.tssrc/plugin/logging-utils.test.tssrc/plugin/logging-utils.tssrc/plugin/request.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/plugin/config/schema.ts
- src/plugin/config/schema.test.ts
📜 Review details
🧰 Additional context used
🧬 Code graph analysis (3)
src/plugin/logging-utils.test.ts (1)
src/plugin/logging-utils.ts (1)
scrubTextForLog(79-93)
src/plugin/request.test.ts (1)
src/plugin/request.ts (2)
isUnsupportedClaudeLongContextBetaError(760-788)prepareAntigravityRequest(810-1654)
src/plugin.ts (2)
src/plugin/request.ts (1)
isUnsupportedClaudeLongContextBetaError(760-788)src/plugin/logging-utils.ts (1)
scrubTextForLog(79-93)
🔇 Additional comments (6)
src/plugin/logging-utils.test.ts (1)
72-88: Good targeted coverage for the new scrub utility.The tests validate both redaction markers and post-scrub truncation/normalization paths clearly.
src/plugin/request.test.ts (2)
95-135: Detection tests for unsupported long-context beta errors are well-scoped.Nice mix of true/false cases across status classes and message variants.
702-799: Claude beta-header request-path tests cover the critical branches.The added cases (apply, skip, dedupe, retry-disable) are exactly the right integration checks here.
src/plugin.ts (3)
125-142: One-time fallback toast dedupe is implemented cleanly.The set-based guard plus bounded eviction keeps behavior predictable and memory-controlled.
2049-2052: Request option wiring for long-context beta controls looks correct.These fields are passed through in the right place to support initial apply + retry downgrade behavior.
2361-2397: The beta-rejection downgrade path is robust and safely retried once.Good use of single-shot disablement, per-session toast throttling, and scrubbed reason logging before retry.
Co-authored-by: Codex <noreply@openai.com>
|
@ndycode have you tested it? |
Improve long-context beta rejection detection so Claude retries reliably on varied provider 4xx payloads while avoiding false positives for interleaved-thinking. Add targeted regression coverage for generic anthropic-beta rejection handling.\n\nCo-authored-by: Codex <noreply@openai.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/plugin/request.ts (1)
760-813: Narrow detection to parsed error message before falling back to full body scanAt Line 773, matching against the full raw body can over-match unrelated fields. Prefer parsing
error.messagefirst and scanning that text, then fallback to whole-body scan only if parsing fails.♻️ Suggested refinement
- const lower = bodyText.toLowerCase(); + let sourceText = bodyText; + try { + const parsed = JSON.parse(bodyText) as { error?: { message?: string } }; + if (typeof parsed?.error?.message === "string" && parsed.error.message.trim()) { + sourceText = parsed.error.message; + } + } catch { + // keep raw body fallback + } + const lower = sourceText.toLowerCase();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugin/request.ts` around lines 760 - 813, In isUnsupportedClaudeLongContextBetaError, avoid scanning the entire raw bodyText first; instead try to parse bodyText as JSON and extract a specific error message field (e.g., error.message, message, or error?.message) into a parsedMessage string, lower-case it and run the existing keyword checks (mentionsExpectedHeader, mentionsLongContextToken, mentionsAnthropicBeta, mentionsInterleavedThinking, mentionsBeta, mentionsUnsupported, mentionsInvalidArgument, mentionsHeaderValueIssue and hasRejectionSignal) against parsedMessage; only if JSON parsing fails or parsedMessage is empty should you fall back to scanning the full bodyText lower-cased as the current implementation does. Ensure you reference the normalizedExpectedHeader logic and keep the same boolean conditions and return paths in isUnsupportedClaudeLongContextBetaError.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/plugin.ts`:
- Line 2019: The variable disableClaudeLongContextBetaForRetry is being
reinitialized inside the per-account loop which allows later accounts to resend
the rejected beta header and triggers extra 4xx retries; move the declaration
and initialization of disableClaudeLongContextBetaForRetry out of the
account-rotation loop so its state persists for the entire request, ensure the
retry/fallback logic that checks disableClaudeLongContextBetaForRetry (the code
path interacting with the Claude beta header and the retry attempt) reads this
shared flag, and only reset it at request start (not per-account) so once the
beta is disabled for retry the subsequent accounts follow the fallback path.
---
Nitpick comments:
In `@src/plugin/request.ts`:
- Around line 760-813: In isUnsupportedClaudeLongContextBetaError, avoid
scanning the entire raw bodyText first; instead try to parse bodyText as JSON
and extract a specific error message field (e.g., error.message, message, or
error?.message) into a parsedMessage string, lower-case it and run the existing
keyword checks (mentionsExpectedHeader, mentionsLongContextToken,
mentionsAnthropicBeta, mentionsInterleavedThinking, mentionsBeta,
mentionsUnsupported, mentionsInvalidArgument, mentionsHeaderValueIssue and
hasRejectionSignal) against parsedMessage; only if JSON parsing fails or
parsedMessage is empty should you fall back to scanning the full bodyText
lower-cased as the current implementation does. Ensure you reference the
normalizedExpectedHeader logic and keep the same boolean conditions and return
paths in isUnsupportedClaudeLongContextBetaError.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (3)
src/plugin.tssrc/plugin/request.test.tssrc/plugin/request.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Greptile Review
🧰 Additional context used
🧬 Code graph analysis (2)
src/plugin/request.test.ts (1)
src/plugin/request.ts (2)
isUnsupportedClaudeLongContextBetaError(760-812)prepareAntigravityRequest(834-1678)
src/plugin.ts (2)
src/plugin/request.ts (1)
isUnsupportedClaudeLongContextBetaError(760-812)src/plugin/logging-utils.ts (1)
scrubTextForLog(79-96)
🔇 Additional comments (4)
src/plugin/request.ts (1)
724-746: Clean header composition and gated beta injection pathThe
anthropic-betamerge logic and the Claude 4.6 eligibility/apply flow are implemented cleanly. The returnedclaudeLongContextBetaApplied+ header metadata also makes downstream fallback handling straightforward.Also applies to: 748-751, 1620-1630, 1636-1637, 1673-1674
src/plugin/request.test.ts (1)
95-159: Good regression coverage for beta rejection + fallback header behaviorThis test set meaningfully protects the new behavior matrix (detection, injection, dedupe, and retry-disable semantics).
Also applies to: 726-823
src/plugin.ts (2)
125-142: Toast throttling helper is solidBounded session tracking with eviction is a good fit to prevent repeated fallback toasts.
2387-2399: Nice log hygiene for rejection reasonUsing
scrubTextForLogwith a max preview cap is a strong privacy/safety improvement for error telemetry.
- persist long-context beta disable flag across account rotation for one request - align retry gating and detector support for 400/403/422 rejections - tighten detection to avoid false positives from non-rejection beta mentions - harden token scrubbing for padded base64-like values and add regression coverage Co-authored-by: Codex <noreply@openai.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/plugin/logging-utils.test.ts (1)
72-98: Add a regression test for quoted JSON key redaction.Great coverage on unquoted formats; please add one case for JSON-style secrets (
"authorization":"Bearer ..."/"token":"...") to lock in the redaction guarantee.✅ Suggested test addition
+ it("scrubs quoted JSON-style secret fields", () => { + const raw = '{"authorization":"Bearer abc123","token":"abc123","email":"user@example.com"}' + const scrubbed = scrubTextForLog(raw, 500) + + expect(scrubbed).toContain('"authorization":"[redacted]"') + expect(scrubbed).toContain('"token":"[redacted]"') + expect(scrubbed).toContain('"email":"[redacted-email]"') + expect(scrubbed).not.toContain("abc123") + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugin/logging-utils.test.ts` around lines 72 - 98, Add a regression unit test to ensure scrubTextForLog correctly redacts JSON-style quoted keys: create a test that passes a JSON-like string containing quoted keys such as "authorization":"Bearer abc123" and "token":"abc123" into scrubTextForLog (use same max length params as other tests), then assert the output contains "[redacted]" or "[redacted-token]/[redacted-email]" for those fields and does not contain the raw token or "Bearer" or the unredacted quoted values; reference scrubTextForLog in the new test to mirror the style of existing tests (e.g., the "scrubs sensitive values from error previews" test) so the regression is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/plugin.ts`:
- Around line 135-138: The eviction guard currently uses a truthy check on the
first key which incorrectly skips empty-string keys; change the condition that
tests the result of
CLAUDE_LONG_CONTEXT_FALLBACK_TOAST_SESSIONS.values().next().value (stored in
variable first) to explicitly check for undefined (e.g., first !== undefined) so
empty-string keys are treated as valid and will be deleted when present.
In `@src/plugin/logging-utils.ts`:
- Around line 87-90: The current .replace call in logging-utils.ts misses
JSON-quoted keys/values; update the regex used in that replace to also accept an
optional surrounding double quotes on the key and to match values wrapped in
quotes (and still handle optional "Bearer"/"Basic" prefixes and unquoted forms)
so patterns like "authorization":"Bearer abc123" and "token":"abc123" are
redacted; locate the .replace(...) invocation in logging-utils.ts and modify its
pattern to allow quoted keys and quoted value tokens while preserving the same
replacement behavior (i.e., capture the key prefix and substitute the value with
"[redacted]").
In `@src/plugin/request.ts`:
- Around line 801-811: The fallback condition can falsely enable long-context
logic; update the final return in the same conditional block to also require
that mentionsInterleavedThinking is false. Specifically, when computing the
final boolean using mentionsAnthropicBeta, hasRejectionSignal and
lower.includes("context"), add a check for !mentionsInterleavedThinking (in the
same place where normalizedExpectedHeader.startsWith("context-1m") is checked)
so the fallback won't trigger for interleaved-thinking-related rejections.
---
Nitpick comments:
In `@src/plugin/logging-utils.test.ts`:
- Around line 72-98: Add a regression unit test to ensure scrubTextForLog
correctly redacts JSON-style quoted keys: create a test that passes a JSON-like
string containing quoted keys such as "authorization":"Bearer abc123" and
"token":"abc123" into scrubTextForLog (use same max length params as other
tests), then assert the output contains "[redacted]" or
"[redacted-token]/[redacted-email]" for those fields and does not contain the
raw token or "Bearer" or the unredacted quoted values; reference scrubTextForLog
in the new test to mirror the style of existing tests (e.g., the "scrubs
sensitive values from error previews" test) so the regression is covered.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (5)
src/plugin.tssrc/plugin/logging-utils.test.tssrc/plugin/logging-utils.tssrc/plugin/request.test.tssrc/plugin/request.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Greptile Review
🧰 Additional context used
🧬 Code graph analysis (4)
src/plugin.ts (2)
src/plugin/request.ts (1)
isUnsupportedClaudeLongContextBetaError(760-811)src/plugin/logging-utils.ts (1)
scrubTextForLog(79-96)
src/plugin/request.ts (1)
scripts/check-quota.mjs (1)
lower(95-95)
src/plugin/logging-utils.test.ts (1)
src/plugin/logging-utils.ts (1)
scrubTextForLog(79-96)
src/plugin/request.test.ts (1)
src/plugin/request.ts (2)
isUnsupportedClaudeLongContextBetaError(760-811)prepareAntigravityRequest(833-1677)
🔇 Additional comments (4)
src/plugin/request.test.ts (1)
95-179: Strong test coverage for beta rejection detection and fallback header behavior.These cases map well to the expected 400/403/422 handling and retry-path header toggling/deduplication.
Also applies to: 746-842
src/plugin.ts (1)
1564-1565: Fallback wiring is well-implemented.Persisting the retry-disable flag at request scope, propagating it to request preparation, and retrying once on the same endpoint after beta rejection is a clean and safe degradation path.
Also applies to: 2049-2052, 2361-2408
src/plugin/request.ts (2)
724-746: Nice header merge/dedup helper.This helper is clean and prevents duplicate beta tokens while preserving existing values.
821-826: Good option plumbing and applied-header telemetry.The new option surface, gating, and returned
claudeLongContextBetaApplied/claudeLongContextBetaHeadermetadata are consistently wired end-to-end.Also applies to: 855-856, 913-915, 1619-1629, 1672-1673
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
- harden fallback-toast eviction check for empty-string keys - tighten Claude long-context rejection matching to avoid generic context false positives - allow versioned Claude Sonnet 4.6 models for beta header injection - redact quoted JSON credential keys and fix credit-card separator regex - add regression coverage for new detection and scrubbing paths Co-authored-by: Codex <noreply@openai.com>
- narrow condition-3 fallback signals to avoid generic INVALID_ARGUMENT false positives - add regression test for expectedHeader call-site behavior on generic context wording - document intentional request-scoped disable flag across account rotation Co-authored-by: Codex <noreply@openai.com>
- refund hybrid token on Claude beta-fallback retry to avoid double consumption - narrow card redaction pattern to grouped card formats only - add regression coverage for plain long numeric identifiers Co-authored-by: Codex <noreply@openai.com>
- add per-model timeout overrides and richer failure diagnostics for model e2e - add global/per-test/per-turn timeout overrides for regression e2e - switch model suite from retired gemini-3-pro-preview to gemini-3.1-pro-preview Co-authored-by: Codex <noreply@openai.com>
- mark google/gemini-2.5-pro as optional in model e2e matrix - keep optional model failures visible without failing the suite - increase prompt-too-long-recovery default timeout to 180000ms Co-authored-by: Codex <noreply@openai.com>
Refined Claude long-context beta rejection detection to avoid quota/rate-limit false positives, removed redundant invalid_argument handling, and removed dead fallback logic. Aligned Sonnet 4.6 non-thinking handling for versioned model names and strip client thinkingConfig for that model family. Adjusted token scrubbing to preserve key names while still redacting long token values, and extended schema/test typing coverage. Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Prioritize exact timeout model overrides before suffix aliases and tighten long-context beta rejection detection to require explicit context-1m mention. Added regression coverage for unrelated foo-beta rejection messages. Co-authored-by: Codex <noreply@openai.com>
| "[redacted-card]", | ||
| ) | ||
| .replace(/(?<==)[A-Za-z0-9+/_-][A-Za-z0-9+/_=-]{39,}(?![A-Za-z0-9+/_=-])/g, "[redacted-token]") | ||
| .replace(/(?<![A-Za-z0-9+/_-])[A-Za-z0-9+/_-]{40,}(?![A-Za-z0-9+/_=-])/g, "[redacted-token]") |
There was a problem hiding this comment.
Standalone token regex does not match tokens ending with = padding
The negative lookahead (?![A-Za-z0-9+/_=-]) includes =, which means that when a 40+ character token is followed immediately by = padding characters (e.g., Base64 values with == at the end), the match is rejected entirely — the regex does not match at all, and no redaction occurs.
The credential-field regex (line 88) covers this correctly for recognised keywords like authorization and token. The gap is for custom or unknown header names whose values appear in raw error body text without a recognised key prefix.
Consider updating the lookahead to explicitly allow trailing padding:
.replace(/(?<![A-Za-z0-9+/_-])[A-Za-z0-9+/_-]{40,}[=]*(?![A-Za-z0-9+/_-])/g, "[redacted-token]")This preserves the key-isolation intent while explicitly consuming any trailing = padding characters that are part of the token value.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin/logging-utils.ts
Line: 97
Comment:
**Standalone token regex does not match tokens ending with `=` padding**
The negative lookahead `(?![A-Za-z0-9+/_=-])` includes `=`, which means that when a 40+ character token is followed immediately by `=` padding characters (e.g., Base64 values with `==` at the end), the match is rejected entirely — the regex does not match at all, and no redaction occurs.
The credential-field regex (line 88) covers this correctly for recognised keywords like `authorization` and `token`. The gap is for custom or unknown header names whose values appear in raw error body text without a recognised key prefix.
Consider updating the lookahead to explicitly allow trailing padding:
```typescript
.replace(/(?<![A-Za-z0-9+/_-])[A-Za-z0-9+/_-]{40,}[=]*(?![A-Za-z0-9+/_-])/g, "[redacted-token]")
```
This preserves the key-isolation intent while explicitly consuming any trailing `=` padding characters that are part of the token value.
How can I resolve this? If you propose a fix, please make it concise.
2 participants
Summary
Deep Audit Notes
Behavior Matrix
Validation
pm run typecheck
pm test
pm run build
All passed locally in this branch.
Refs #506
Related duplicate context: #512