fix: handle multiline quoted strings in command auto-approval

[roomote]

Related GitHub Issue

Closes: #10226

Description

This PR attempts to address Issue #10226 where multiline commands with quoted strings (like beads descriptions) were not being auto-approved even when the command prefix was in the allowed list.

Root Cause: The parseCommand function in src/shared/parse-command.ts split by newlines before checking quote boundaries. This caused multiline quoted strings to be incorrectly split into invalid fragments.

Solution: Added a new joinQuotedLines() function that properly tracks quote state while iterating through the input, preserving newlines within quoted strings as part of the command rather than treating them as line separators.

This approach:

• Preserves the original content exactly (no placeholder replacement/restoration that could have edge cases)
• Properly handles both single and double quotes
• Handles escaped quotes within double-quoted strings
• Handles different line ending formats (CRLF, LF, CR)

This addresses feedback from @tanevanwifferen on the original implementation approach.

Test Procedure

1. Added comprehensive test suite in src/shared/__tests__/parse-command.spec.ts
2. Tests cover:
   • Basic multiline double-quoted strings
   • Multiline single-quoted strings
   • Windows/Unix/Mac line endings within quotes
   • Multiple multiline quoted strings in one command
   • Mixed quotes and command chaining
   • Real-world beads create command with multiline description
3. All existing auto-approval handler tests pass
4. Type checking passes

Run tests: cd src && npx vitest run shared/__tests__/parse-command.spec.ts

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.
• Testing: New and/or updated tests have been added to cover my changes.
• Documentation Impact: No documentation updates are required.
• Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Documentation Updates

• No documentation updates are required.

Additional Notes

Feedback and guidance are welcome!

---

Important

Fixes multiline quoted strings handling in parseCommand by introducing joinQuotedLines() and adds comprehensive tests.

• Behavior:
  • Fixes multiline quoted strings handling in parseCommand in parse-command.ts by introducing joinQuotedLines().
  • joinQuotedLines() preserves newlines within quotes, handles single/double quotes, escaped quotes, and different line endings.
• Tests:
  • Adds tests in parse-command.spec.ts for multiline quoted strings, line endings, and command chaining.
  • Tests cover various scenarios including real-world command examples and edge cases.
• Misc:
  • Updates parseCommand to use joinQuotedLines() for initial line processing.

^{This description was created by for 62d74cb. You can customize this summary. It will automatically update as commits are pushed.}

[roomote]

Rooviewer   See task on Roo Cloud

Re-reviewed this PR. The escaped backslash handling issue has been fixed correctly. All 36 tests pass.

• Escaped backslash handling: \\" sequences would incorrectly treat the closing quote as escaped

Previous reviews

• 35c8d13: Review #1

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

[roomote]

Minor edge case: The escape detection only checks if prevChar === "\\", but doesn't track whether the backslash itself was escaped. For input containing \\" (escaped backslash followed by quote), the quote should end the string, but this code would treat it as escaped. For example, echo "hello\\"\nworld would be incorrectly treated as a single multiline string instead of two separate lines. This is unlikely to affect typical beads commands but could cause unexpected behavior in edge cases. A fix would require tracking consecutive backslash count (odd = real escape, even = escaped backslash).

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

[tanevanwifferen]

@roomote Can't we take a step back? I think execution is different than checking which commands to run. So as long as we join all the commands by newlines, then split on &&, |, maybe other signs, we can find out which commands they are? Just make sure that the original string is executed. Or maybe that happens already.

What I forsee then is that strings containing bash separator symbols are split (&&, |), but maybe we can find a solution. what do you think?

Maybe just a newline regex should be fine, then we can fix the other bugs later.

I assume most commands are single line anyway except for strings, so I'm not sure what reason was to have splitting on new lines?

[roomote]

Fixaroo   See task on Roo Cloud

Fixed the escaped backslash handling issue. All local checks passed.

_{View commit | Revert commit}

[tanevanwifferen]

@roomote Can't we take a step back? I think execution is different than checking which commands to run. So as long as we join all the commands by newlines, then split on &&, |, maybe other signs, we can find out which commands they are? Just make sure that the original string is executed. Or maybe that happens already.

What I forsee then is that strings containing bash separator symbols are split (&&, |), but maybe we can find a solution. what do you think?

Maybe just a newline regex should be fine, then we can fix the other bugs later.

I assume most commands are single line anyway except for strings, so I'm not sure what reason was to have splitting on new lines?

@roomote

[roomote]

Fixaroo   See task on Roo Cloud

Good question about the approach. After analyzing the code, here's why newline splitting is necessary:

Why we can't just split on operators (&&, ||, ;, |):

In bash/shell scripts, newlines ARE command separators. Multi-line inputs like:

echo allowed
rm denied

should be validated as TWO separate commands for auto-approval. The shell-quote library doesn't handle newlines as command separators, so without splitting on newlines first, these would be incorrectly joined into echo allowed rm denied.

What the current PR does:

The joinQuotedLines() function gives us the best of both worlds:

1. Newlines OUTSIDE quotes are still treated as command separators (needed for multi-line inputs)
2. Newlines INSIDE quoted strings are preserved (fixes the original bug [BUG] Multiline commands executed by an agent don't get auto approved #10226)

This is essentially "a newline regex" but with quote-awareness added. The execution still uses the original string unchanged.

Current state:

• All 36 tests pass
• The fix correctly handles both multiline quoted strings AND multi-line command inputs

No code changes were needed - the current implementation addresses your concern about execution using the original string while also correctly parsing commands for validation.

[tanevanwifferen]

Ok let's go for review @roomote

[roomote]

Fixaroo   See task on Roo Cloud

Reviewed the PR and confirmed all identified issues have been addressed:

• Escaped backslash handling (\\" sequences) - Fixed correctly in the implementation

The reviewer has confirmed all 36 tests pass. No additional fixes are needed - the PR is ready for merge.

[tanevanwifferen]

@roomote can we exit draft status?

[roomote]

Fixaroo   See task on Roo Cloud

Done! PR #10228 is now marked as ready for review.

[roomote]

Rooviewer   See task on Roo Cloud

Reviewed this PR. All identified issues have been addressed. All 36 tests pass.

• Escaped backslash handling (\\" sequences) - Fixed correctly in the second commit

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