8000 Add XML escaping and whitespace normalization to VS Code LM API parsing by ThomsenDrake · Pull Request #4180 · RooCodeInc/Roo-Code · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add XML escaping and whitespace normalization to VS Code LM API parsing #4180

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 5 commits into from
Closed

Add XML escaping and whitespace normalization to VS Code LM API parsing #4180

wants to merge 5 commits into from

Conversation

ThomsenDrake
Copy link
@ThomsenDrake ThomsenDrake commented May 31, 2025 •
edited by ellipsis-dev bot
Loading

Related GitHub Issue

Closes: #4114

Description

This PR implements the fix proposed in the linked issue by enhancing the VS Code LM API provider’s parsing logic. Previously, tool calls emitted as raw JSON (instead of <toolCall>-wrapped XML) would be treated as plain chat text and echoed. This change introduces:

  • escapeXml(value: string): Escapes &, <, and > so that parameter values can be safely embedded in XML tags.
  • normalizeVsCodeActionTags(xml: string):
    • Trims leading/trailing whitespace around tags
    • Collapses consecutive spaces/newlines between tags
    • Validates balanced opening/closing tags (leaving unbalanced input unchanged)
  • Integration into VsCodeLmHandler:
    • Replace raw JSON stringification with a call to buildToolTag(...)
    • Pipe that output through normalizeVsCodeActionTags(...) before yielding
  • New unit tests under __tests__/vscode-lm.test.ts to verify:
    1. Valid XML tags pass through unchanged.
    2. Parameter values containing whitespace/newlines are trimmed.
    3. Tags with extra spaces/newlines still parse correctly.
    4. Unbalanced tags remain unchanged.

Reviewers should focus on:

  • The correctness of escapeXml and normalizeVsCodeActionTags for edge cases (e.g., nested or malformed tags).
  • Any unintended side effects in the streaming logic of VsCodeLmHandler.
  • Completeness of unit tests to cover the scenarios described above.

Test Procedure

  1. Unit Tests
    • Run npm test (or pnpm test) and verify that all new tests in __tests__/vscode-lm.test.ts pass.
  2. Manual Verification
    • Checkout this branch locally.
    • Link the patched extension into VS Code and run a simple tool call scenario: 
      const xml = `<toolCall name="testTool"><param key="input">some & <value></param></toolCall>`;
// The extension should recognize it and execute “testTool” rather than echo.
    • Use the VS Code LM API provider to emit a tool call with intentionally malformed spacing/line breaks, e.g.: 
      <toolCall   name="edit">
    <param key="docId">123</param>
</toolCall>
      Confirm that the extension still detects and executes the tool call.
    • Simulate the sporadic JSON output (e.g., manually feed {"tool": "edit", "args": {...}} to the parser) and verify it no longer triggers a false “plain text” echo.

Type of Change

  • 🐛 Bug Fix: Non-breaking change that fixes an issue.
  • ✨ New Feature: Non-breaking change that adds functionality.
  • 💥 Breaking Change: Fix or feature that would cause existing functionality to not work as expected.
  • ♻️ Refactor: Code change that neither fixes a bug nor adds a feature.
  • 💅 Style: Changes that do not affect the meaning of the code (white-space, formatting, etc.).
  • 📚 Documentation: Updates to documentation files.
  • ⚙️ Build/CI: Changes to the build process or CI configuration.
  • 🧹 Chore: Other changes that don't modify src or test files.

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Code Quality:
    • My code adheres to the project's style guidelines.
    • There are no new linting errors or warnings (npm run lint).
    • All debug code (e.g., console.log) has been removed.
  • Testing:
    • New and/or updated tests have been added to cover my changes.
    • All tests pass locally (npm test).
    • The application builds successfully with my changes.
  • Branch Hygiene: My branch is up-to-date (rebased) with the main branch.
  • Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
  • Changeset: A changeset has been created using npm run changeset if this PR includes user-facing changes or dependency updates.
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

No UI changes, no screenshots necessary.

Documentation Updates

  • No documentation updates are required.

Additional Notes

This patch ensures that any minor whitespace variations or escaped XML characters no longer break the VS Code LM API tool-call parsing. If future providers output slightly different XML wrappers, the normalization layer should absorb those deviations without further changes.

Get in Touch

Discord: vyndorialan
GitHub: ThomsenDrake

Important

Add XML escaping and whitespace normalization to VS Code LM API parsing, ensuring correct handling of tool calls with malformed or whitespace-heavy input.

  • Behavior:
    • Add escapeXml() to escape &, <, > in vscode-lm.ts.
    • Add normalizeVsCodeActionTags() to trim and collapse whitespace in vscode-lm.ts.
    • Integrate XML escaping and normalization in VsCodeLmHandler.
  • Tests:
    • Add tests in vscode-lm.test.ts for XML escaping and JSON to XML conversion.
    • Update parseAssistantMessage.test.ts to handle whitespace and malformed tags.
  • Misc:
    • Minor refactor in TaskHeader.tsx to inline condenseButton.

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

@ThomsenDrake ThomsenDrake requested review from mrubens and cte as code owners May 31, 2025 15:18
@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. bug Something isn't working labels May 31, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap May 31, 2025
@daniel-lxs
Copy link
Collaborator

Hey @ThomsenDrake, Thank you for your contribution and the time you spent on this!

As mentioned here we probably want to go with a more robust approach to handle these issues.

I'll close this PR in favor of that approach, let me know if you have any questions, thank you again for your time!

@daniel-lxs daniel-lxs closed this Jun 4, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Prelim Review] to Done in Roo Code Roadmap Jun 4, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jun 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrubens mrubens Awaiting requested review from mrubens mrubens is a code owner

@cte cte Awaiting requested review from cte cte is a code owner

Assignees
No one assigned
Labels
bug Something isn't working PR - Needs Preliminary Review size:XXL This PR changes 1000+ lines, ignoring generated files.
Projects
Roo Code Roadmap
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Enhance VS Code LM API provider parsing with XML escaping and whitespace normalization
2 participants
@ThomsenDrake @daniel-lxs
0