Repository navigation
fix(vertex): in-loop context guard, consecutive tool-failure breaker, per-tool abort - #1136
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Single Commit Policy - COMPLIANTStatus: Policy requirements met • 1 commit • Valid format • Ready for merge 📊 View validation details📝 Commit Details
✅ Validation Results
🤖 Automated validation by NeuroLink Single Commit Enforcement |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (11)
📝 WalkthroughWalkthroughThis PR adds context-window guarding, abort-aware tool execution, consecutive-failure handling, recovery timeout budgeting, new stop-reason labeling, updated Claude context windows, and regression tests for Gemini and Anthropic native loops. ChangesContext Guard and Abort-Aware Tooling
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant googleVertex as googleVertex
participant createContextGuard as createContextGuard
participant Tool
participant Model as Provider Model
Caller->>googleVertex: start Gemini turn
googleVertex->>createContextGuard: initialize guard
loop tool loop
googleVertex->>Model: send prompt and tools
Model-->>googleVertex: usage and tool calls
googleVertex->>createContextGuard: noteUsage / noteAppendedChars
createContextGuard-->>googleVertex: shouldStop()?
alt context limit reached
googleVertex->>Model: tools-disabled synthesis
Model-->>googleVertex: final text or empty result
googleVertex-->>Caller: stopReason context-cap or completed
else continue
googleVertex->>Tool: execute with abort signal
Tool-->>googleVertex: result or failure text
googleVertex->>googleVertex: update failedTools breaker
end
end
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/providers/googleVertex.ts (1)
2239-2261: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGemini's
TOOL_NOT_FOUNDpath isn't wired into the newfailedToolsbreaker, unlike Anthropic's.This PR explicitly adds breaker coverage for
TOOL_NOT_FOUNDon the Anthropic loops (Line 4875-4884, Line 6242-6251) with the stated rationale: "a model grinding on a hallucinated/stale tool name must not burn the whole step budget on TOOL_NOT_FOUND round-trips." The equivalent Gemini branches (unchanged by this PR) still rely solely on thedo_not_retry: truehint in the payload with no hard breaker — so a Gemini model that ignores that instruction and keeps calling a nonexistent tool can still burn its entiremaxStepsbudget onTOOL_NOT_FOUNDround-trips, which is exactly the runaway-turn scenario this PR's title says it fixes for both Gemini and Claude.Given the pattern is already established in this file for thrown/error-shaped failures on the Gemini side, extending it to
TOOL_NOT_FOUNDis a small, low-risk addition for parity.Also applies to: 3383-3403
🤖 Prompt for 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. In `@src/lib/providers/googleVertex.ts` around lines 2239 - 2261, The Gemini TOOL_NOT_FOUND branch in googleVertex should also trip the new failedTools breaker, not just return do_not_retry in the payload. Update the error-handling path in the tool execution loop where call.name is missing to add the tool name to failedTools and short-circuit subsequent retries the same way the Anthropic TOOL_NOT_FOUND branches do. Keep the existing errorPayload, functionResponses, toolExecutions, and stepStorageResults behavior intact, and apply the same parity fix in the other Gemini TOOL_NOT_FOUND location referenced by the diff.
🧹 Nitpick comments (2)
src/lib/core/modules/ToolsManager.ts (1)
36-69: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider reusing the shared
raceWithAborthelper instead of duplicating it.
raceWithAbortSignalhere is functionally identical to the exportedraceWithAbortinsrc/lib/utils/async/withTimeout.ts(added in this same PR), differing only in the required-signaltyping and the abort error message. Importing the shared helper (or a thin wrapper) would avoid maintaining two copies of the ghost-swallow/listener logic.🤖 Prompt for 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. In `@src/lib/core/modules/ToolsManager.ts` around lines 36 - 69, The abort-racing logic in raceWithAbortSignal duplicates the shared raceWithAbort helper from async/withTimeout, so consolidate to avoid maintaining two copies of the same listener and rejection-swallowing behavior. Update ToolsManager to reuse the exported helper directly or add a very thin wrapper that only preserves the required AbortSignal typing and tool-specific abort error behavior, while keeping the shared implementation in one place. Use the unique symbols raceWithAbortSignal and raceWithAbort to locate the call site and helper import.test/continuous-test-suite-gemini-abort.ts (1)
1428-1475: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winError-shaped-result breaker test for Gemini is correct, but note there's no equivalent new test for the
TOOL_NOT_FOUNDcase (see the companion comment ongoogleVertex.tsLines 2239-2261/3383-3403 — Gemini'sTOOL_NOT_FOUNDbranch isn't wired into the breaker, so a test here would currently fail/timeout-loop rather than pass). Once that gap is closed, a matching test here would close the loop.🤖 Prompt for 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. In `@test/continuous-test-suite-gemini-abort.ts` around lines 1428 - 1475, The new breaker test covers error-shaped MCP results, but there is no equivalent case for TOOL_NOT_FOUND, so Gemini can still loop/timeout on missing-tool responses. Add a companion test in the same generate suite using runGenerate, makeProvider, and the tool-call mock setup to simulate a TOOL_NOT_FOUND response and assert it trips the breaker into a permanent failure instead of retrying indefinitely. Keep the test aligned with the existing failedTools breaker assertions so it matches the behavior once the googleVertex.ts TOOL_NOT_FOUND path is wired into the breaker.
🤖 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 `@src/lib/neurolink.ts`:
- Around line 6067-6078: The recovery timeout logic in neurolink should not let
MIN_RECOVERY_TURN_BUDGET_MS override a caller’s smaller turnTimeoutMs. In the
retry budget calculation around the retryTurnTimeoutMs assignment, ensure the
recovered timeout never exceeds the remaining caller-configured deadline when
options.turnTimeoutMs is finite and attemptStartTimeMs is set. Adjust the clamp
so the recovery budget respects the original turn timeout intent while still
handling negative or near-zero remaining time safely, using the existing
retryTurnTimeoutMs and MIN_RECOVERY_TURN_BUDGET_MS symbols to update the
calculation.
---
Outside diff comments:
In `@src/lib/providers/googleVertex.ts`:
- Around line 2239-2261: The Gemini TOOL_NOT_FOUND branch in googleVertex should
also trip the new failedTools breaker, not just return do_not_retry in the
payload. Update the error-handling path in the tool execution loop where
call.name is missing to add the tool name to failedTools and short-circuit
subsequent retries the same way the Anthropic TOOL_NOT_FOUND branches do. Keep
the existing errorPayload, functionResponses, toolExecutions, and
stepStorageResults behavior intact, and apply the same parity fix in the other
Gemini TOOL_NOT_FOUND location referenced by the diff.
---
Nitpick comments:
In `@src/lib/core/modules/ToolsManager.ts`:
- Around line 36-69: The abort-racing logic in raceWithAbortSignal duplicates
the shared raceWithAbort helper from async/withTimeout, so consolidate to avoid
maintaining two copies of the same listener and rejection-swallowing behavior.
Update ToolsManager to reuse the exported helper directly or add a very thin
wrapper that only preserves the required AbortSignal typing and tool-specific
abort error behavior, while keeping the shared implementation in one place. Use
the unique symbols raceWithAbortSignal and raceWithAbort to locate the call site
and helper import.
In `@test/continuous-test-suite-gemini-abort.ts`:
- Around line 1428-1475: The new breaker test covers error-shaped MCP results,
but there is no equivalent case for TOOL_NOT_FOUND, so Gemini can still
loop/timeout on missing-tool responses. Add a companion test in the same
generate suite using runGenerate, makeProvider, and the tool-call mock setup to
simulate a TOOL_NOT_FOUND response and assert it trips the breaker into a
permanent failure instead of retrying indefinitely. Keep the test aligned with
the existing failedTools breaker assertions so it matches the behavior once the
googleVertex.ts TOOL_NOT_FOUND path is wired into the breaker.
🪄 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: 6f606116-f7f7-4983-af6d-8fb3dfda70ca
📒 Files selected for processing (12)
src/lib/constants/contextWindows.tssrc/lib/core/constants.tssrc/lib/core/modules/ToolsManager.tssrc/lib/neurolink.tssrc/lib/providers/googleNativeGemini3.tssrc/lib/providers/googleVertex.tssrc/lib/types/generate.tssrc/lib/utils/async/index.tssrc/lib/utils/async/withTimeout.tssrc/lib/utils/mcpErrorText.tstest/continuous-test-suite-anthropic-cap.tstest/continuous-test-suite-gemini-abort.ts
ef65d47 to
3f26345
Compare
|
Addressed both CodeRabbit findings in the amended commit 3f26345 (single-commit policy):
Re-validated: |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
|
🎉 This PR is included in version 9.81.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Tara-ag
left a comment
Tara-ag
left a comment
There was a problem hiding this comment.
Review Summary
Reviewed the 12 changed files in this already-merged patch. No new blocking issues were identified beyond the points already raised and addressed in the existing review thread.
Existing concerns already handled
- Recovery timeout floor — CodeRabbit’s concern about
MIN_RECOVERY_TURN_BUDGET_MSoverriding a caller’s smallerturnTimeoutMswas fixed in3f263453by clamping the floor toMath.min(MIN_RECOVERY_TURN_BUDGET_MS, options.turnTimeoutMs). - Gemini
TOOL_NOT_FOUNDbreaker parity andraceWithAbortSignalduplication were raised by the prior automated review; they are noted as follow-up polish rather than merge blockers.
What was checked
- CLAUDE.md Critical Rules — no static provider imports outside the registry, no Gemini tools + JSON-schema conflict introduced,
formatProviderErroruntouched, public SDK API only gains the additive"context-cap"GenerateStopReasonmember (backward compatible). - Security — no hardcoded secrets, no unsafe eval/innerHTML, no injection vectors introduced.
- Correctness — context guard is fail-open until usage arrives; abort racing swallows abandoned settlements to avoid unhandled rejections; terminal
max_tokensclamping guards pre-4.5 Claude 400s on context-capped turns. - Tests — new coverage added in
continuous-test-suite-anthropic-cap.tsandcontinuous-test-suite-gemini-abort.tsfor guard trip, breaker behavior, and per-tool abort.
Approving as a post-merge review with no new blocking findings.
This branch was successfully deployed
3 participants
Summary
Fixes the runaway-turn incident class from the 2026-07-06 production RCA (claude-sonnet-5 turns accumulating 1,005,647 tokens over 78 tool steps and dying on Anthropic 400 "prompt is too long"; MCP-proxy-blocked tools retried for up to 200 steps; multi-tool steps overshooting
turnTimeoutMsby N × 300s). Releases as a patch via thefixcommit. All changes are inside the four native Vertex loops (Gemini + Claude-on-Vertex, generate + stream) plus their shared helpers — no public API breaks; one additiveGenerateStopReasonmember.In-loop context guard (new stopReason
context-cap)createContextGuard(googleNativeGemini3.ts) tracks the provider-reported prompt size of every step (input + cache read/writeon Anthropic,promptTokenCounton Gemini) plus projected growth from appended tool results.DEFAULT_CONTEXT_GUARD_RATIO) the loop stops calling tools and synthesizes a final answer from gathered results instead of stepping into a provider 400 that destroys the whole turn.stopReason: "completed"; only a failed synthesis surfaces the new"context-cap"with an honest message (never the step-cap text).max_tokensto the remaining window (pre-4.5 Claude models rejectinput + max_tokens > window).contextWindows: claude-sonnet-5 + Claude catch-all on vertex
claude-sonnet-5→ 1,000,000 (vertex + anthropic maps)."claude-"prefix catch-all (200K) on vertex so unknown Claude models no longer inherit the Gemini-shaped_default(1,048,576) — which sits ABOVE Anthropic's real 1M cap and let the overflow through the pre-dispatch check.Consecutive tool-failure breaker on the Anthropic loops
failedToolsbreaker (2 strikes →TOOL_PERMANENTLY_FAILEDshort-circuit) to both Anthropic loops.isError: true/{error}payloads viaextractToolFailureText) — the shape proxy-blocked tools return without throwing — andTOOL_NOT_FOUNDcalls.Abort correctness
toolTimeoutMspast the deadline).raceWithAbort(utils/async) races each tool execution against the turn's abort signal — live-verified: an 8sturnTimeoutMswith a 60s tool previously returned at ~68s, now returns at ~11.7s withstopReason: "time-limit". The abandoned call remains a bounded ghost (same tradeoff as the tool timeout); transport-level MCP cancellation is a tracked follow-up.abortSignal,toolCallId) through instead of dropping it.Overflow-recovery budget
turnTimeoutMs(floored atMIN_RECOVERY_TURN_BUDGET_MS= 30s) instead of a fresh full clock — onegenerate()can no longer stack ~2× the configured budget.Verification
test:anthropic-cap39/39 (6 new tests: guard-trip → synthesis, context-cap message, throw-breaker, isError-breaker, consecutive-reset, batch-abort),test:gemini-abort39/39 (2 new; 1 updated for the stricter per-tool abort semantics),test:mcp/test:mcp:bash/test:mcp:limitspass;tsc --strict, lint, full build + publint clean.stopReason: "context-cap", 1 step, honest message, no 400-destroyed turn — and the guard correctly observed the prompt via cache read/creation tokens; (2) an always-isErrortool was executed exactly 2× while the model requested it 3×, then the turn completed with a graceful summary; (3) the 8s-deadline/60s-tool run above.max_tokens, missing Gemini growth projection, non-prompt mid-tool abort — are all fixed in this commit.Summary by CodeRabbit
New Features
Bug Fixes