Skip to content

fix: enforce mandatory :start_line: in apply_diff tool#12200

Draft
roomote-v0[bot] wants to merge 1 commit intomainfrom
fix/enforce-start-line-in-apply-diff
Draft

fix: enforce mandatory :start_line: in apply_diff tool#12200
roomote-v0[bot] wants to merge 1 commit intomainfrom
fix/enforce-start-line-in-apply-diff

Conversation

@roomote-v0
Copy link
Copy Markdown
Contributor

@roomote-v0 roomote-v0 Bot commented Apr 26, 2026

Related GitHub Issue

Closes: #12199

Description

This PR attempts to address Issue #12199 by enforcing the :start_line: directive as mandatory in the apply_diff tool, matching what the tool prompt already documents.

Changes:

  1. src/core/diff/strategies/multi-search-replace.ts:

    • Made :start_line: mandatory in the regex pattern (removed trailing ? from the group)
    • Added :start_line: presence tracking (foundStartLine) in validateMarkerSequencing
    • Updated capture group indices to reflect the new regex structure
    • Updated error messages to reflect the requirement
  2. src/core/tools/ApplyDiffTool.ts:

    • Added early validation that :start_line: is present before calling applyDiff
  3. src/core/task/Task.ts:

    • Added sayAndCreateMissingStartLineError helper method following the existing sayAndCreateMissingParamError pattern
  4. Test files:

    • Updated all existing tests to include :start_line: directive
    • Added a new test case verifying that diffs without :start_line: are rejected

Test Procedure

  • All 64 tests in multi-search-replace.spec.ts and multi-search-replace-trailing-newline.spec.ts pass
  • Lint and type-check pass across the full monorepo
  • New test added: "rejects diff without :start_line:" validates the new enforcement

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue.
  • Scope: Changes are focused on the linked issue.
  • Self-Review: Performed a thorough self-review.
  • Testing: Updated and added tests to cover changes.
  • Documentation Impact: No documentation changes needed (the tool prompt already documents :start_line: as required).
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Feedback and guidance are welcome.

Interactively review PR in Roo Code Cloud

- Make :start_line: mandatory in the regex pattern (was optional)
- Add :start_line: presence tracking in validateMarkerSequencing
- Add early validation in ApplyDiffTool before calling applyDiff
- Add sayAndCreateMissingStartLineError helper method on Task
- Update error messages to reflect :start_line: requirement
- Update all existing tests to include :start_line: directive
- Add new test for rejecting diffs without :start_line:

Closes #12199
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] apply_diff fails complaining about insufficient match when the match is 100% perfect

1 participant