Mirror: Fix Kilo Quota OpenRouter error handling and retry flow (#5847)#29
Mirror: Fix Kilo Quota OpenRouter error handling and retry flow (#5847)#29jeremylongshore merged 4 commits intomainfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ✨ Finishing touches🧪 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 |
Summary of ChangesHello @jeremylongshore, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the robustness and user experience of the application by enhancing error handling for API providers and refining how slash commands are processed within AI tool responses. It also introduces dynamic updates for Model Context Protocol (MCP) server capabilities, ensuring the application stays synchronized with backend changes. Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Review Summary by QodoMirror: Fix Kilo Quota OpenRouter error handling and retry flow with enhanced MCP notifications
WalkthroughsDescription• **Fixed slash command processing in tool_result blocks**: Extended parseKiloSlashCommands to handle both string and array content formats in tool responses, addressing a regression where slash commands were only processed in text blocks • **Enhanced error handling for streaming operations**: Added robust error payload extraction with multiple fallback locations (metadata, responseBody, cause, nested errors) and improved status code resolution including AWS metadata support • **Improved MCP notification handling**: Added capability refresh handlers for resource, tool, and prompt list changes with automatic server capability updates when MCP servers notify of changes • **Expanded test coverage**: Added comprehensive test suites for NotificationService, slash command processing in tool responses, and error handler status extraction • **Added automated PR/issue triaging workflow**: Implemented GitHub Actions workflow for automatic labeling and categorization of issues and pull requests Diagramflowchart LR
A["Tool Result Blocks"] -->|"parseKiloSlashCommands"| B["Processed Slash Commands"]
C["Stream Errors"] -->|"handleProviderError"| D["Normalized Errors"]
E["MCP Notifications"] -->|"RefreshCapabilitiesCallback"| F["Updated Server Capabilities"]
G["Error Payloads"] -->|"extractErrorPayload"| H["Resolved Error Details"]
File Changes1. src/services/mcp/kilocode/__tests__/NotificationService.spec.ts
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several valuable improvements. The core changes focus on enhancing error handling for API providers, fixing an issue with slash command processing, and improving MCP notification handling. The error handling refactoring is particularly well-done, centralizing logic and making it more robust for various provider responses. The fix for slash commands in tool_result blocks is also a great addition, and the refactoring to a helper function improves code clarity. The changes to MCP notifications, especially silencing unhandled ones, will improve the user experience. I've included one suggestion to reduce some code duplication in the notification service. Overall, these are high-quality changes with good test coverage.
| // Handle resource list changes - refresh capabilities silently | ||
| client.setNotificationHandler(ResourceListChangedNotificationSchema, async () => { | ||
| console.log(`MCP ${name}: resources list changed, refreshing capabilities`) | ||
| try { | ||
| await onRefreshCapabilities?.(name) | ||
| } catch (error) { | ||
| console.error(`MCP ${name}: failed to refresh capabilities after resource list change:`, error) | ||
| } | ||
| }) | ||
|
|
||
| // Handle tool list changes - refresh capabilities silently | ||
| client.setNotificationHandler(ToolListChangedNotificationSchema, async () => { | ||
| console.log(`MCP ${name}: tools list changed, refreshing capabilities`) | ||
| try { | ||
| await onRefreshCapabilities?.(name) | ||
| } catch (error) { | ||
| console.error(`MCP ${name}: failed to refresh capabilities after tool list change:`, error) | ||
| } | ||
| }) | ||
|
|
||
| // Handle prompt list changes - refresh capabilities silently | ||
| client.setNotificationHandler(PromptListChangedNotificationSchema, async () => { | ||
| console.log(`MCP ${name}: prompts list changed, refreshing capabilities`) | ||
| try { | ||
| await onRefreshCapabilities?.(name) | ||
| } catch (error) { | ||
| console.error(`MCP ${name}: failed to refresh capabilities after prompt list change:`, error) | ||
| } | ||
| }) |
There was a problem hiding this comment.
The notification handlers for resource, tool, and prompt list changes are very similar. To reduce code duplication and improve maintainability, you could extract this logic into a helper function.
const createRefreshHandler = (listType: string) => async () => {
console.log(`MCP ${name}: ${listType} list changed, refreshing capabilities`)
try {
await onRefreshCapabilities?.(name)
} catch (error) {
console.error(`MCP ${name}: failed to refresh capabilities after ${listType} list change:`, error)
}
}
// Handle resource list changes - refresh capabilities silently
client.setNotificationHandler(ResourceListChangedNotificationSchema, createRefreshHandler("resources"))
// Handle tool list changes - refresh capabilities silently
client.setNotificationHandler(ToolListChangedNotificationSchema, createRefreshHandler("tools"))
// Handle prompt list changes - refresh capabilities silently
client.setNotificationHandler(PromptListChangedNotificationSchema, createRefreshHandler("prompts"))
|
Failed to generate code suggestions for PR |
Code Review by Qodo
1. Unmarked edits in processKiloUserContentMentions
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| // Helper function to process text through parseMentions and parseKiloSlashCommands | ||
| // Returns the processed text and whether a kilorules check is needed | ||
| const processTextContent = async ( | ||
| text: string, | ||
| localWorkflowToggles: ClineRulesToggles, | ||
| globalWorkflowToggles: ClineRulesToggles, | ||
| ): Promise<{ processedText: string; needsRulesFileCheck: boolean }> => { | ||
| const parsedText = await parseMentions( | ||
| text, | ||
| cwd, | ||
| urlContentFetcher, | ||
| fileContextTracker, | ||
| rooIgnoreController, | ||
| showRooIgnoredFiles, | ||
| includeDiagnosticMessages, | ||
| maxDiagnosticMessages, | ||
| maxReadFileLine, | ||
| ) | ||
|
|
||
| // when parsing slash commands, we still want to allow the user to provide their desired context | ||
| const { processedText, needsRulesFileCheck: needsCheck } = await parseKiloSlashCommands( | ||
| parsedText.text, | ||
| localWorkflowToggles, | ||
| globalWorkflowToggles, | ||
| ) | ||
|
|
||
| return { processedText, needsRulesFileCheck: needsCheck } | ||
| } |
There was a problem hiding this comment.
1. Unmarked edits in processkilousercontentmentions 📘 Rule violation ⛯ Reliability
Changes were made in upstream-shared src/ code without kilocode_change markers, increasing the risk of merge conflicts during upstream syncs. This makes it harder to identify Kilo-specific deltas when mirroring with Roo Code.
Agent Prompt
## Issue description
Edits in upstream-shared `src/` code were introduced without `kilocode_change` markers, which violates the upstream-sync annotation requirement and can increase merge conflicts.
## Issue Context
`src/core/mentions/processKiloUserContentMentions.ts` contains newly added helper logic and refactors that are Kilo-specific but not annotated.
## Fix Focus Areas
- src/core/mentions/processKiloUserContentMentions.ts[45-72]
- src/core/mentions/processKiloUserContentMentions.ts[92-150]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| { | ||
| "last_failure": "2026-02-14T20:42:41.363720889-06:00", | ||
| "failure_count": 7, | ||
| "backoff_until": "2026-02-14T21:12:41.36372132-06:00", | ||
| "needs_manual_sync": true, | ||
| "failure_reason": "git pull failed: exit status 1\nfatal: couldn't find remote ref review/PR-5667\n" | ||
| } No newline at end of file |
There was a problem hiding this comment.
2. Local .beads artifacts committed 🐞 Bug ⛨ Security
The PR adds multiple .beads/* runtime/state files (PID, sync backoff state, SQLite shm) that are machine-specific and not appropriate for source control, risking privacy leaks, repo bloat, and constant churn/merge conflicts.
Agent Prompt
### Issue description
The PR includes `.beads/*` runtime artifacts (PID, SQLite shm/WAL state, sync failure/backoff state). These are machine-specific, can leak local paths/PIDs, and will create ongoing repo churn and bloat.
### Issue Context
`.beads/daemon.lock` includes an absolute local path (`/home/...`) and PIDs, and other `.beads` files encode transient operational state.
### Fix Focus Areas
- .gitignore[1-80]
- .beads/sync-state.json[1-7]
- .beads/daemon.pid[1-1]
- .beads/beads.db-shm[1-5]
### Suggested approach
1. Remove `.beads/*` runtime files from git (delete or `git rm --cached` as appropriate).
2. Add `.beads/` (or at least `.beads/beads.db*`, `.beads/*.pid`, `.beads/*state*.json`, `.beads/*.lock`) to the root `.gitignore`.
3. If a specific export (e.g. `issues.jsonl`) is intentionally tracked, use negation rules (e.g. ignore `.beads/**` then `!.beads/issues.jsonl`).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Review: kilocode Kilo-Org#5847
Checklist
Findings1. Title is misleading -- this fixes Moonshot/AI SDK stream errors, not OpenRouter/Kilo Quota (severity: gray)The PR title says "Fix Kilo Quota | OpenRouter error handling" but the PR description and actual changes fix Moonshot stream error handling via the AI SDK path. The author notes this explicitly: "This PR focuses on the Moonshot/AI-SDK stream error path and shared error normalization hardening, not the older OpenRouter/Kilo quota branch." The title likely reflects the original branch intent before scope refinement. 2. Stream error escalation is the right fix (severity: gray)The core problem: AI SDK's if (part.type === "error") {
const streamError =
(part as { error?: unknown }).error ??
new Error(String((part as { message?: unknown }).message ?? "Unknown stream error"))
throw streamError
}This is correct -- error stream parts are terminal and should halt processing. 3. Error handler hardening is thorough (severity: gray)The
The 4. Try/catch wrapping preserves tool stream behavior (severity: gray)The entire stream processing loop and 5. The NO_OUTPUT_GENERATED_MESSAGE constant could be fragile (severity: gray)The error handler checks if the message includes the exact string "No output generated. Check the stream for errors." to decide whether to prefer nested provider messages. If the AI SDK changes this message, the fallback would stop working. However, the consequence of this check failing is only that users would see the generic message -- the same behavior as before this PR. So this is a progressive enhancement, not a regression risk. CI Status
Code SnippetsError payload extraction priority chain: const nestedMessage = responsePayload.message ?? nestedPayload.message
let msg = metadataRaw || nestedMessage || error.message || ""
// AI SDK can emit generic "No output generated" while nested payload has the real failure.
if (msg.includes(NO_OUTPUT_GENERATED_MESSAGE) && nestedMessage) {
msg = nestedMessage
}Before/after user experience (from Discord user @smetanokr): VerdictAPPROVE -- This is a well-targeted fix for a real user-reported issue. The stream error escalation correctly handles AI SDK error parts that were being silently dropped. The error handler hardening is thorough and preserves backwards compatibility. All CI checks pass. A Discord user confirmed the fix produces actionable error messages instead of generic ones. The misleading title is cosmetic and does not affect the code quality. |
Review Journal: kilocode Kilo-Org#5847
SummaryFixes silent error dropping in AI SDK stream processing that caused Moonshot quota errors to show as generic "No output generated" messages. Hardens the shared error handler to extract nested provider error payloads. Real user confirmed the fix via before/after screenshots. Approve. First ImpressionsTitle is misleading (mentions OpenRouter/Kilo Quota) but the actual changes fix Moonshot stream errors and harden the shared error handler. The PR description is honest about the scope shift. The author explicitly notes this is not the OpenRouter quota fix. What I Looked At
AnalysisThe root cause chain:
The fix addresses both layers:
The error handler additions are substantial (80 new lines of helper code) but well-structured:
Verification
Lessons Learned
Review methodology: AI PR Review Case Studies | Reviewed with GWI + Claude Code |
1 participant
Mirror of upstream PR #5847
Verdict: APPROVE | Tier 5
Review: kilocode-5847-review.md