Skip to content

Conversation

@Ruakij
Copy link
Contributor

@Ruakij Ruakij commented Jun 5, 2025

Related GitHub Issue

Closes: #4113

Description

Fix tool writeToFile with missing or empty path or missing content.
Checks earlier right after waiting for path & content parameters to be populated, so when block is non-partial, they immediately check instead of later.
This is required as alot of code after requires both to be valid.

Additional unit tests are added to cover these scenarios.

Future work should probably separate partial from non-partial-block logic and modularize the big function for reduced code-complexity.

Test Procedure

  • Run Tests
  • Try tool-calling writeToFile with missing or empty path/content parameters (see screenshot)

Type of Change

  • 🐛 Bug Fix: Non-breaking change that fixes an issue.
  • New Feature
  • 💥 Breaking Change
  • ♻️ Refactor
  • 💅 Style
  • 📚 Documentation
  • ⚙️ Build/CI
  • 🧹 Chore

Pre-Submission Checklist

  • Issue Linked
  • Scope
  • Self-Review
  • 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
  • Documentation Impact
  • Changeset
  • Contribution Guidelines

Screenshots / Videos

image

Documentation Updates

  • No documentation updates are required.

Additional Notes

N/A

Get in Touch

Discord: ruakij


Important

Moves parameter validation earlier in writeToFileTool to handle missing or empty path and content immediately, with new tests added.

  • Behavior:
    • Moves relPath and newContent checks earlier in writeToFileTool to immediately handle missing or empty parameters.
    • Ensures writeToFileTool returns early if path or content is missing or empty, preventing further processing.
  • Testing:
    • Adds unit tests in writeToFileTool.test.ts to verify behavior when path or content is missing or empty.
  • Misc:
    • Suggests future refactoring to separate partial and non-partial logic for reduced complexity.

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

…ool earlier and run after they exist or block is non-partial

Add tests covering the issue.
@Ruakij Ruakij requested review from cte and mrubens as code owners June 5, 2025 10:40
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. bug Something isn't working labels Jun 5, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Jun 5, 2025
Copy link
Member

@daniel-lxs daniel-lxs left a comment

Choose a reason for hiding this comment

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

@Ruakij
Thank you for the fix!

LGTM

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 7, 2025
@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Needs Review] in Roo Code Roadmap Jun 7, 2025
@mrubens mrubens merged commit 8f58737 into RooCodeInc:main Jun 12, 2025
22 checks passed
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Jun 12, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jun 12, 2025
cte pushed a commit that referenced this pull request Jun 24, 2025
#4378)

Fix #4113: Move relPath & newContent checks in writeToFileTool earlier and run after they exist or block is non-partial
Add tests covering the issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer PR - Needs Review size:M This PR changes 30-99 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Bug: writeToFileTool silently fails on missing or empty path/content parameters

4 participants