Skip to content

Conversation

@mcowger
Copy link
Contributor

@mcowger mcowger commented Dec 2, 2025

Handle case where JSON content was parsed into an object by the tool call parser. If the arguments to the tool call are valid JSON (e.g. the model attempts to create a .json file) the earlier call to JSON.parse() will have deeply parsed the returned contents. If that has happened, convert back to string.

Fixes: #8972

Description

[ What changed? Feel free to be brief. ]

AI Code Review

  • Team members only: AI review runs automatically when PR is opened or marked ready for review
  • Team members can also trigger a review by commenting @continue-review

Checklist

  • I've read the contributing guide
  • The relevant docs, if any, have been updated or created
  • The relevant tests, if any, have been updated or created

Screen recording or screenshot

NA

Tests

Added 6 tests to core/tools/parseArgs.vitest.ts for new check.


Summary by cubic

Fixes failures when create_new_file receives JSON content parsed into an object by ensuring contents is always a string. Prevents crashes when creating .json files. Fixes #8972.

  • Bug Fixes
    • In getStringArg, JSON.stringify object/array values to safely handle parsed JSON.
    • Throw a clear error for non-string values after conversion.
    • Added tests covering contents object/array handling and edge cases.

Written for commit e04c21f. Summary will update automatically on new commits.

Handle case where JSON content was parsed into an object by the tool call parser.  If the arguments to the tool call are valid JSON (e.g. the model attempts to create a .json file) the earlier call to JSON.parse() will have deeply parsed the returned contents. If that has happened, convert back to string.

Fixes: continuedev#8972
@mcowger mcowger requested a review from a team as a code owner December 2, 2025 20:16
@mcowger mcowger requested review from sestinj and removed request for a team December 2, 2025 20:16
@continue
Copy link
Contributor

continue bot commented Dec 2, 2025

Keep this PR in a mergeable state →

Learn more

All Green is an AI agent that automatically:

✅ Addresses code review comments

✅ Fixes failing CI checks

✅ Resolves merge conflicts

@continue-development-app
Copy link

Keep this PR in a mergeable state →

Learn more

All Green is an AI agent that automatically:

✅ Addresses code review comments

✅ Fixes failing CI checks

✅ Resolves merge conflicts

1 similar comment
@continue-development-app
Copy link

Keep this PR in a mergeable state →

Learn more

All Green is an AI agent that automatically:

✅ Addresses code review comments

✅ Fixes failing CI checks

✅ Resolves merge conflicts

@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Dec 2, 2025
@github-actions
Copy link

github-actions bot commented Dec 2, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

Copy link
Collaborator

@RomneyDa RomneyDa left a comment

Choose a reason for hiding this comment

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

@mcowger could we add this special handling in createFileImpl instead of here?

OR, I think you could just remove special handling for contents and always make sure it's a string within getStringArg!

@github-project-automation github-project-automation bot moved this from Todo to In Progress in Issues and PRs Dec 2, 2025
@continue
Copy link
Contributor

continue bot commented Dec 2, 2025

Reviewed the PR changes. This is an internal bug fix in the tool argument parser that handles JSON content correctly when creating JSON files. The changes don't require documentation updates since:

  1. This fixes internal parsing logic, not user-facing behavior
  2. The existing documentation correctly describes the create_new_file tool functionality
  3. The fix is transparent to users - it restores expected behavior rather than changing the tool's API or capabilities

The fix ensures that when models create .json files with JSON content, the parser correctly handles the case where the content gets parsed into an object and converts it back to a string.

@mcowger
Copy link
Contributor Author

mcowger commented Dec 2, 2025

I have read the CLA Document and I hereby sign the CLA

@mcowger
Copy link
Contributor Author

mcowger commented Dec 2, 2025

@mcowger could we add this special handling in createFileImpl instead of here?

OR, I think you could just remove special handling for contents and always make sure it's a string within getStringArg!

Good call. Removed the special case handling.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 2 files

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 2 files

Copy link
Collaborator

@RomneyDa RomneyDa left a comment

Choose a reason for hiding this comment

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

@mcowger this looks good to me. I haven't been able to replicate the failing case locally - it seems like claude and openai models mostly escape JSON strings correctly. Could you share which model you find this error often happens with?

EDIT I see M2 in the issue but wasn't sure which model that was. Model + provider combo would be helpful

@mcowger
Copy link
Contributor Author

mcowger commented Dec 2, 2025

I can replicate on Synthetic.new. It doesn't happen for me on OpenRouter either.

Happy to provide a key privately for you to test so you dont have to give them money.

@RomneyDa
Copy link
Collaborator

RomneyDa commented Dec 2, 2025

@mcowger which model are you working with?
EDIT this was a silly question, I see M2 on openrouter now
https://openrouter.ai/minimax/minimax-m2

@mcowger
Copy link
Contributor Author

mcowger commented Dec 2, 2025

@mcowger which model are you working with?

Minimax M2 (note, I cannot replicate this on OpenRouter). I cannot replicate this via GLM4.6 on the same provider, so it appears to be an interaction between the inference and the model and continue.

@mcowger
Copy link
Contributor Author

mcowger commented Dec 2, 2025

I captured traces of the differences in the output:

https://gist.github.com/mcowger/3aea3a2314d9f5540e7bbf68d9cafcd7

GLM outputs double escaped strings, while M2 does not.

That should be enough to replicate with?

@RomneyDa
Copy link
Collaborator

RomneyDa commented Dec 2, 2025

This seems like could be a valid issue for other models as well, and if the tool implementation is looking for a string arg specifically, like the create new file tool, this fix seems fine.

I guess you would still see issues with MCP tools when they expect string JSON and receive JSON object. The solution might be to detect "type": "string" in the tool definition and use getStringArg to replace at the time of calling the MCP tool. Do you use MCP tools at all? I could open a separate issue for that

@RomneyDa
Copy link
Collaborator

RomneyDa commented Dec 2, 2025

Got it, thanks for the traces. It's no clear why the model would escape differently between providers (as far as I understand this would only happen because of something on the provider side) but I could see this happening in other situations as well and this fix won't hurt.

@mcowger
Copy link
Contributor Author

mcowger commented Dec 2, 2025

Agree its a provider side issue, and I've filed a bug with them.

I dont use MCPs much, but I agree it could impact them as well.

Copy link
Collaborator

@RomneyDa RomneyDa left a comment

Choose a reason for hiding this comment

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

This looks good for now, can address MCP on a separate issue if it comes up

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Dec 3, 2025
@RomneyDa RomneyDa merged commit c6fc60b into continuedev:main Dec 3, 2025
57 of 58 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Issues and PRs Dec 3, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Dec 3, 2025
@sestinj
Copy link
Contributor

sestinj commented Dec 4, 2025

🎉 This PR is included in version 1.35.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@sestinj
Copy link
Contributor

sestinj commented Dec 4, 2025

🎉 This PR is included in version 1.7.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

lgtm This PR has been approved by a maintainer released size:M This PR changes 30-99 lines, ignoring generated files.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

create_new_file fails on pure JSON contents

3 participants