Skip to content

Conversation

@daniel-lxs
Copy link
Member

@daniel-lxs daniel-lxs commented Nov 18, 2025

Excludes XML tool call examples from the MODES section when native tool protocol is enabled to avoid confusing the model with XML syntax when it should use native tool calling.


Important

Excludes XML tool examples from MODES section when native protocol is enabled by checking tool call IDs.

  • Behavior:
    • Excludes XML tool examples from MODES section when native protocol is enabled in modes.ts.
    • Determines protocol by checking for tool call ID in presentAssistantMessage.ts.
  • Functions:
    • Updates getModesSection() in modes.ts to accept skipXmlExamples parameter.
    • Modifies generatePrompt() in system.ts to pass isNativeProtocol to getModesSection().
  • Misc:
    • Adjusts logic in presentAssistantMessage() in presentAssistantMessage.ts to use tool call ID for protocol determination.

This description was created by Ellipsis for b360ba2. You can customize this summary. It will automatically update as commits are pushed.

@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. bug Something isn't working labels Nov 18, 2025
@roomote
Copy link
Contributor

roomote bot commented Nov 18, 2025

Rooviewer Clock   See task on Roo Cloud

Review complete. No issues found.

The changes successfully address the race condition by using tool call ID presence as a stable indicator of the protocol used, and appropriately exclude XML examples from the MODES section when native protocol is enabled.

Previous reviews

Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues.

@daniel-lxs daniel-lxs force-pushed the fix/native-tool-protocol-race-condition branch from b360ba2 to 4bd65f9 Compare November 18, 2025 20:00
@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Nov 18, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Review] in Roo Code Roadmap Nov 18, 2025
@hannesrudolph hannesrudolph added PR - Needs Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Nov 18, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Nov 18, 2025
@mrubens mrubens merged commit b0c254c into main Nov 18, 2025
19 checks passed
@mrubens mrubens deleted the fix/native-tool-protocol-race-condition branch November 18, 2025 20:48
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Nov 18, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Nov 18, 2025
mini2s added a commit to zgsm-ai/costrict that referenced this pull request Nov 19, 2025
* fix: prevent MCP server restart when toggling tool permissions (RooCodeInc#8633)

* fix: prevent MCP server restart when toggling tool permissions

Add isProgrammaticUpdate flag to distinguish between programmatic config
updates and user-initiated file changes. Skip file watcher processing
during programmatic updates to prevent unnecessary server restarts.

* fix(mcp): prevent server reconnection when toggling disabled state

Fixed bug where MCP servers would reconnect instead of staying disabled when toggled off. The issue was that toggleServerDisabled() used stale in-memory config instead of reading the fresh config from disk after writing the disabled flag.

Changes:
Added readServerConfigFromFile() helper to read and validate server config from disk
Updated disable path to read fresh config before calling connectToServer()
Updated enable path to read fresh config before calling connectToServer()
This ensures the disabled: true flag is properly read, causing connectToServer() to create a disabled placeholder connection instead of actually connecting the server.

+ refactor(mcp): use safeWriteJson for atomic config writes

Replace JSON.stringify + fs.writeFile with safeWriteJson in McpHub.ts
to prevent data corruption through atomic writes with file locking.

* fix(mcp): prevent race condition in isProgrammaticUpdate flag

Replace multiple independent reset timers with a single timer that gets
cleared and rescheduled on each programmatic config update. This prevents
the flag from being reset prematurely when multiple rapid updates occur,
which could cause unwanted server restarts during the file watcher's
debounce period.

+ fix(mcp): ensure isProgrammaticUpdate flag cleanup with try-finally

Wrap safeWriteJson() calls in try-finally blocks to guarantee the
isProgrammaticUpdate flag is always reset, even if the write operation
fails. This prevents the flag from being stuck at true indefinitely,
which would cause subsequent user-initiated config changes to be
silently ignored.

* docs(readme): update readme images and image compression

* docs: replace inline base64 images with image file references

* Merge remote-tracking branch 'upstream/main' into roo-to-main

* feat(terminal): refactor execa command execution for shell handling

* Feat: Add Minimax Provider (fixes RooCodeInc#8818) (RooCodeInc#8820)

Co-authored-by: xiaose <[email protected]>
Co-authored-by: Matt Rubens <[email protected]>

* Release v3.29.4 (