Skip to content

Conversation

@roomote
Copy link
Contributor

@roomote roomote bot commented Dec 30, 2025

Related GitHub Issue

Relates to: #10226

Description

This PR adds comprehensive test coverage for the command auto-approval functions in src/core/auto-approval/commands.ts which previously had no dedicated tests.

Key Findings from Investigation:

  1. git add -A with git add allowed DOES work correctly - The prefix matching logic is working as expected. The tests confirm this behavior.
  2. Multiline quoted strings do NOT work - This is the core issue [BUG] Multiline commands executed by an agent don't get auto approved #10226, and PR fix: handle multiline quoted strings in command auto-approval #10228 already has the fix (joinQuotedLines()).

Test Coverage Added:

  • containsDangerousSubstitution: 8 tests covering dangerous parameter expansions, indirect references, here-strings, zsh patterns
  • findLongestPrefixMatch: 10 tests covering prefix matching, case insensitivity, wildcards, edge cases
  • getSingleCommandDecision: 9 tests covering allowlist/denylist interactions, longest prefix match rule
  • isAutoApprovedSingleCommand: 6 tests covering single command approval logic
  • isAutoDeniedSingleCommand: 6 tests covering single command denial logic
  • getCommandDecision: 18 tests covering command chains, pipes, redirections, dangerous patterns
  • Real-world scenarios: 4 tests from issue [BUG] Multiline commands executed by an agent don't get auto approved #10226 (git add -A, git commit)

Note: One test for multiline git commit is skipped pending PR #10228 merge.

Test Procedure

Run the tests:

cd src && npx vitest run core/auto-approval/__tests__/commands.spec.ts

Expected: 57 tests pass, 1 skipped (multiline test pending #10228)

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 required.
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Documentation Updates

  • No documentation updates are required.

Additional Notes

This PR attempts to address Issue #10226 by:

  1. Adding test coverage that verifies the existing prefix matching logic works correctly for git add -A with git add allowed
  2. Documenting with a skipped test the multiline command issue that PR fix: handle multiline quoted strings in command auto-approval #10228 addresses

Feedback and guidance are welcome!

…ions

- Add 57 tests covering findLongestPrefixMatch, getSingleCommandDecision,
  isAutoApprovedSingleCommand, isAutoDeniedSingleCommand, getCommandDecision,
  and containsDangerousSubstitution functions
- Include real-world scenarios from issue #10226 (git add -A, git commit)
- Add test demonstrating git add -A correctly matches git add prefix
- Skip multiline git commit test pending PR #10228 merge

Related: #10226
@roomote
Copy link
Contributor Author

roomote bot commented Dec 30, 2025

Rooviewer Clock   See task on Roo Cloud

Review complete. No issues found.

The test file provides comprehensive coverage for the command auto-approval functions. All 57 tests pass with 1 properly documented as skipped (pending PR #10228).

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

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Dec 30, 2025
@roomote
Copy link
Contributor Author

roomote bot commented Dec 30, 2025

Closing this PR as requested by @Jerrill. The git add -A issue was a transient problem that is no longer reproducing, so the 57 tests verifying prefix matching are no longer necessary.

The core multiline quoted strings issue is addressed by PR #10228.

@roomote roomote bot closed this Dec 30, 2025
@github-project-automation github-project-automation bot moved this from Triage to Done in Roo Code Roadmap Dec 30, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Dec 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants