Mirror: Fix ZenMux model metadata and native tool message handling (#5831)#30
Mirror: Fix ZenMux model metadata and native tool message handling (#5831)#30jeremylongshore merged 5 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 focuses on enhancing the stability and functionality of the ZenMux provider and improving overall error handling within the system. Key updates include refining ZenMux model metadata to better support native tools and ensuring reliable tool-calling behavior. Additionally, the changes introduce more robust error processing for streaming operations across various AI providers, allowing for more accurate error reporting and retry mechanisms. A significant fix addresses the processing of slash commands within tool results, ensuring user commands are correctly interpreted in all contexts. Finally, the integration with the Model Context Protocol now dynamically updates available capabilities, leading to a more responsive and up-to-date user experience. 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 ZenMux model metadata and native tool message handling
WalkthroughsDescription• **ZenMux native tools reliability**: Fixed native tool metadata merging, message format handling, and context window detection to prevent "tool not used" loops • **Slash command processing in tool results**: Extended parseKiloSlashCommands to handle tool_result blocks, fixing a regression where slash commands were not processed in tool results • **ZenMux model cache validation**: Added self-healing logic to reject stale cache entries with invalid context window values from v5.7.0 • **Enhanced error handling**: Improved nested error payload extraction and status code resolution for better error propagation from providers • **MCP notification handlers**: Added support for ResourceListChanged, ToolListChanged, and PromptListChanged notifications with dynamic capability refresh • **Stream error handling**: Added error handling for AI SDK stream error parts to surface provider-specific errors • **Comprehensive test coverage**: Added test suites for ZenMux native tools, MCP notifications, error handling, model fetching, and slash command processing • **GitHub Actions auto-triage**: Added workflow for automatic issue and PR labeling Diagramflowchart LR
A["ZenMux Model Fetcher"] -->|"native tools defaults"| B["Model Info"]
B -->|"merge metadata"| C["ZenMux Stream"]
C -->|"OpenAI format messages"| D["Native Tool Support"]
E["Tool Results"] -->|"parse slash commands"| F["Content Processing"]
G["MCP Server"] -->|"list change notifications"| H["Notification Service"]
H -->|"refresh callback"| I["Capability Refresh"]
J["Provider Errors"] -->|"extract nested payload"| K["Error Handler"]
K -->|"normalized errors"| L["Stream Processing"]
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 significant improvements and fixes. It correctly handles ZenMux model metadata, particularly the contextWindow, and adds support for native tools. Error handling has been made more robust across providers, with better extraction of details from streaming and nested errors. A bug preventing slash command processing in tool_result blocks has been fixed through a nice refactoring. Additionally, the MCP client is now more dynamic, capable of refreshing its capabilities in response to server notifications. The changes are well-tested and improve the overall stability and functionality of the system.
| tools, | ||
| tool_choice: tools ? "auto" : undefined, | ||
| tool_choice: toolChoice ?? "auto", | ||
| parallel_tool_calls: enableParallelToolCalls ? true : false, |
There was a problem hiding this comment.
The expression enableParallelToolCalls ? true : false is redundant as enableParallelToolCalls is already a boolean. You can simplify this to just enableParallelToolCalls for better readability.
parallel_tool_calls: enableParallelToolCalls,
|
Failed to generate code suggestions for PR |
Code Review by Qodo
1. zenmux.ts changes lack markers
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| ): Promise<Record<string, ModelInfo>> { | ||
| const models: Record<string, ModelInfo> = {} | ||
| const baseURL = "https://zenmux.ai/api/v1" | ||
| const baseURL = options?.openRouterBaseUrl || "https://zenmux.ai/api/v1" |
There was a problem hiding this comment.
1. zenmux.ts changes lack markers 📘 Rule violation ⛯ Reliability
Changes were made to an upstream-shared file under src/ without being wrapped in kilocode_change markers, increasing the risk of upstream sync merge conflicts. Only a subset of the modifications in the file are annotated.
Agent Prompt
## Issue description
Updates to an upstream-shared file under `src/` were introduced without `kilocode_change` markers around the modified sections, which can increase merge conflicts during upstream sync.
## Issue Context
Rule requires Kilo-specific modifications in upstream-shared code to be annotated with `kilocode_change` markers.
## Fix Focus Areas
- src/api/providers/fetchers/zenmux.ts[15-17]
- src/api/providers/fetchers/zenmux.ts[37-37]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| KILO_AUTO_APPROVAL_ENABLED: "true" | ||
| KILO_AUTO_APPROVAL_EXECUTE_ENABLED: "true" | ||
| KILO_AUTO_APPROVAL_EXECUTE_ALLOWED: "gh issue edit,gh pr edit" | ||
| KILO_AUTO_APPROVAL_EXECUTE_DENIED: "gh issue close,gh issue delete,gh issue transfer,gh issue lock,gh issue unlock,gh pr close,gh pr merge,gh repo,gh auth,gh secret,gh variable,rm,sudo,curl,wget,bash,sh,python,node,npm,npx" | ||
| KILO_TELEMETRY: "false" | ||
| # Determine event type and extract data | ||
| EVENT_TYPE: ${{ github.event_name }} | ||
| ITEM_NUMBER: ${{ github.event_name == 'issues' && github.event.issue.number || github.event.pull_request.number }} | ||
| ITEM_TITLE: ${{ github.event_name == 'issues' && github.event.issue.title || github.event.pull_request.title }} | ||
| ITEM_BODY: ${{ github.event_name == 'issues' && github.event.issue.body || github.event.pull_request.body }} | ||
| run: | | ||
| # Sanitize body - remove shell metacharacters | ||
| SAFE_BODY=$(echo "$ITEM_BODY" | head -c 2000 | tr -d '`$(){}[]|;&<>\\' | tr '\n' ' ') | ||
|
|
||
| # Determine gh command based on event type | ||
| if [ "$EVENT_TYPE" = "issues" ]; then | ||
| GH_CMD="gh issue edit" | ||
| ITEM_TYPE="issue" | ||
| else | ||
| GH_CMD="gh pr edit" | ||
| ITEM_TYPE="pull request" | ||
| fi | ||
|
|
||
| kilocode --auto "Triage this GitHub ${ITEM_TYPE}: | ||
|
|
||
| Number: ${ITEM_NUMBER} | ||
| Title: ${ITEM_TITLE} | ||
| Body: ${SAFE_BODY} | ||
|
|
||
| ## Your Task | ||
| Add appropriate labels to this ${ITEM_TYPE}. | ||
|
|
||
| ## Command Format | ||
| Use ONLY: ${GH_CMD} ${ITEM_NUMBER} --add-label \"<label>\" | ||
|
|
There was a problem hiding this comment.
3. Llm can over-edit via gh 🐞 Bug ⛨ Security
The auto-triage workflow enables LLM-driven command execution and allowlists gh issue edit / `gh pr edit`, but the CLI approval logic uses prefix matching—so these allowlist entries permit arbitrary edits (title/body/etc.), not just label adds. Because issue/PR title/body is untrusted user input, prompt injection can cause unintended repository actions within the granted permissions.
Agent Prompt
### Issue description
The workflow enables auto-execution of LLM-produced shell commands and allowlists `gh issue edit` / `gh pr edit`. The CLI allowlist matcher is prefix-based, so any `gh issue edit ...` command (including changing title/body) will be auto-approved. This is vulnerable to prompt injection from untrusted issue/PR content.
### Issue Context
The workflow’s prompt instructs “labels only”, but enforcement is not technical—only prompt-based. The approval layer is prefix-based, not argument-constrained.
### Fix Focus Areas
- .github/workflows/auto-triage.yml[33-74]
- cli/src/services/approvalDecision.ts[28-59]
- cli/src/services/approvalDecision.ts[171-236]
### Expected fix
Choose one:
1) Disable execute auto-approval in this workflow and implement labeling via GitHub API directly in the workflow (deterministic, no shell execution).
2) Replace allowed commands with a single wrapper script (e.g. `./scripts/triage-add-label.sh`) that:
- accepts only (item_type, item_number, label)
- validates `label` is in the known label list
- runs `gh issue/pr edit ... --add-label ...`
Then allowlist ONLY that wrapper script.
3) Extend the command-approval mechanism to support strict patterns/structured output so that only `--add-label` operations can be auto-approved (argument-level validation, not prefix-level).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Review: kilocode Kilo-Org#5831
Checklist
Findings1. NATIVE_TOOL_DEFAULTS spread ordering in getModel (severity: gray)let info = { ...NATIVE_TOOL_DEFAULTS, ...(this.models[id] ?? zenmuxDefaultModelInfo) }Since 2. createZenMuxStream signature change is well-handled (severity: gray)The method signature changes from 3. Stale cache self-healing is ZenMux-specific (severity: gray)The 4. Tool protocol lock respected via metadata.toolProtocol (severity: gray)The PR passes CI Status
Code SnippetsCore fix -- using pre-transformed messages for stream creation: // Before: rebuilt messages from raw, losing transforms
const openAiMessages: OpenAI.Chat.ChatCompletionMessageParam[] = [
{ role: "system", content: systemPrompt },
...convertToOpenAiMessages(messages),
]
// After: uses already-transformed openAiMessages passed from caller
async createZenMuxStream(
client: OpenAI,
openAiMessages: OpenAI.Chat.ChatCompletionMessageParam[],
model: { id: string; info: ModelInfo },
...
)Stale cache self-healing: function hasInvalidZenmuxContextWindow(models: ModelRecord): boolean {
return Object.values(models).some((model) => (model.contextWindow ?? 0) <= 0)
}VerdictAPPROVE -- This is a well-structured fix for two related ZenMux reliability issues. The context window cache hardening prevents erroneous condensing loops from stale v5.7.0 cache entries. The message pipeline refactor preserves transforms (especially for DeepSeek R1) and correctly gates native tool parameters based on the task-locked protocol. All CI checks pass. The test coverage is targeted and regression-focused. Community user testing on Discord confirms the fix resolves the reported "tool not used" retry loops. |
Review Journal: kilocode Kilo-Org#5831
SummaryFixes two related ZenMux provider bugs: (1) stale model cache with contextWindow=0 caused infinite condensing loops, and (2) native tool calling was broken because message transforms were being rebuilt instead of passed through. Well-tested fix with Discord user validation. Approve. First ImpressionsTwo changesets signal this is a combined fix. The PR description is thorough with clear problem statement, implementation details, and test instructions. The author (Neonsy) is a repeat contributor with good pattern awareness. What I Looked At
AnalysisThe root cause analysis is sound:
The tool protocol gating is done correctly: const tools = isNativeProtocol ? metadata?.tools : undefined
const toolChoice = isNativeProtocol ? metadata?.tool_choice : undefined
const parallelToolCalls = isNativeProtocol ? (metadata?.parallelToolCalls ?? false) : falseThis ensures XML-mode tasks never send native tool parameters. Verification
Lessons Learned
Review methodology: AI PR Review Case Studies | Reviewed with GWI + Claude Code |
1 participant
Mirror of upstream PR #5831
Verdict: APPROVE | Tier 5
Review: kilocode-5831-review.md