Skip to content

Conversation

@roomote
Copy link
Contributor

@roomote roomote bot commented Nov 11, 2025

This PR attempts to address Issue #9172. Feedback and guidance are welcome.

Problem

When using the kimi-k2-thinking model via OpenRouter with "show thinking prompts" enabled, XML tags were being displayed in the reasoning/thinking output, making it difficult to read.

Solution

  • Added a stripXmlTags method to the OpenRouterHandler class that removes XML tags while preserving the content between them
  • Applied XML tag stripping specifically for the moonshot/kimi-k2-thinking model in the reasoning output stream
  • Other models continue to display reasoning content unchanged

Testing

  • Added unit tests to verify XML tags are stripped for the kimi-k2-thinking model
  • Added unit tests to verify XML tags are preserved for other models
  • All existing tests continue to pass

Changes

  • Modified src/api/providers/openrouter.ts to add XML tag stripping logic
  • Added tests in src/api/providers/__tests__/openrouter.spec.ts

Fixes #9172


Important

Adds XML tag stripping to moonshot/kimi-k2-thinking model reasoning output in OpenRouterHandler.

  • Behavior:
    • Adds stripXmlTags method to OpenRouterHandler to remove XML tags from reasoning output for moonshot/kimi-k2-thinking model.
    • Applies XML tag stripping only to moonshot/kimi-k2-thinking model; other models remain unchanged.
  • Testing:
    • Adds unit tests in openrouter.spec.ts to verify XML tags are stripped for moonshot/kimi-k2-thinking and preserved for other models.
    • Ensures all existing tests pass.
  • Changes:
    • Modifies openrouter.ts to include XML tag stripping logic in createMessage() function.

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

- Added stripXmlTags method to remove XML tags from reasoning content
- Applied XML tag stripping specifically for moonshot/kimi-k2-thinking model
- Added tests to verify XML tags are stripped for kimi model but preserved for others
- Fixes #9172 where XML tags were displayed in thinking prompts
@roomote roomote bot requested review from cte, jr and mrubens as code owners November 11, 2025 18:58
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Nov 11, 2025
@roomote
Copy link
Contributor Author

roomote bot commented Nov 11, 2025

Rooviewer Clock   See task on Roo Cloud

Review completed. Found 1 issue that should be addressed:

  • Overly aggressive XML tag removal pattern could strip legitimate content like mathematical comparisons or generic type syntax

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

*/
private stripXmlTags(text: string): string {
// Remove XML tags but preserve the content between them
return text.replace(/<[^>]*>/g, "")

Check failure

Code scanning / CodeQL

Incomplete multi-character sanitization High

This string may still contain
<script
, which may cause an HTML element injection vulnerability.

Copilot Autofix

AI 5 days ago

To effectively fix this issue, we must ensure the removal of all potentially dangerous HTML/XML tags—even if the data may contain multiple consecutive or malformed tags. The most robust approach is to use a vetted HTML sanitizer (like sanitize-html) to prevent corner-case vulnerabilities. However, if adding a dependency is undesirable due to scope, we should at least repeatedly apply the sanitization logic until no changes occur, eliminating all instances of tags in line with the presented best practices (looping until stable).

The change applies to the function stripXmlTags in src/api/providers/openrouter.ts.
Edit lines 105–108 so that the regexp-based tag removal is performed in a loop until the string is stable. If possible, use a built-in JavaScript method—do not attempt to write your own custom sanitizer.

No additional imports are needed for this looped regex (unless the project wants to use sanitize-html, but the fixes will show both). We'll show the preferred loop-based fix, matching the project's approach and keeping dependencies stable.


Suggested changeset 1
src/api/providers/openrouter.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/api/providers/openrouter.ts b/src/api/providers/openrouter.ts
--- a/src/api/providers/openrouter.ts
+++ b/src/api/providers/openrouter.ts
@@ -104,7 +104,12 @@
 	 */
 	private stripXmlTags(text: string): string {
 		// Remove XML tags but preserve the content between them
-		return text.replace(/<[^>]*>/g, "")
+		let previous;
+		do {
+			previous = text;
+			text = text.replace(/<[^>]*>/g, "");
+		} while (text !== previous);
+		return text;
 	}
 
 	override async *createMessage(
EOF
@@ -104,7 +104,12 @@
*/
private stripXmlTags(text: string): string {
// Remove XML tags but preserve the content between them
return text.replace(/<[^>]*>/g, "")
let previous;
do {
previous = text;
text = text.replace(/<[^>]*>/g, "");
} while (text !== previous);
return text;
}

override async *createMessage(
Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
*/
private stripXmlTags(text: string): string {
// Remove XML tags but preserve the content between them
return text.replace(/<[^>]*>/g, "")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regex /<[^>]*>/g will strip all angle bracket pairs, which could unintentionally remove legitimate content in reasoning output. For example, mathematical comparisons like a < b, generic type syntax like Array<string>, or arrow operators like -> could be partially or fully removed if they appear in reasoning content. Consider using a more specific pattern that targets only complete XML tags (e.g., /< *\/?[a-zA-Z][^>]*>/g) or document which specific tags need to be removed and target those explicitly.

Fix it with Roo Code or mention @roomote and request a fix.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Nov 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

[BUG] Thinking shows xml (kimi-k2-thinking)

3 participants