fix: Add missing directory creation and fix regex patterns in roo int…

[johnlindquist]

…egration tests

• Add mkdirSync calls to create .roo/rules directories before writing test files
• Import mkdirSync from 'fs' module
• Fix regex patterns to use [\s\S]* instead of .* to match across newlines in frontmatter
• Fixes ENOENT errors and regex matching failures in roo.test.ts

Summary by CodeRabbit

• Tests
  • Improved test setup to automatically create required directories before writing files.
  • Made content-matching assertions more flexible to accommodate formatting differences.
  • Enhanced reliability of roundtrip and export scenarios by ensuring inputs are prepared consistently.
  • Reduced flakiness by handling missing directory structures within test cases.
  • Overall, tests are more robust and tolerant to variations in directory layout and content formatting.

[coderabbitai]

Caution

Review failed

The pull request is closed.

Walkthrough

Updates a single integration test to ensure directories are created before file writes, imports mkdirSync, and loosens regex assertions to match variable frontmatter/content spacing. Adds recursive directory creation for rulesDir and nested dirs, and ensures inputRulesDir exists for roundtrip test setup.

Changes

Cohort / File(s)	Summary
Integration tests — Roo test/integration/roo.test.ts	Import mkdirSync; create rulesDir and nested dirs with recursive mkdir; ensure inputRulesDir exists before writes; relax assertions by replacing fixed-newline frontmatter checks with [\\s\\S]* patterns; minor setup adjustments across tests.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• Add support for Roo Code and JetBrains Junie #16 — Modifies the same integration test file; appears to be the earlier Roo test addition that this PR adjusts for setup and assertion robustness.

Poem

I burrow through tests, neat and keen,
Carving dirs where none have been.
Regex paws, soft yet wide,
Catch all spaces that hide inside.
Thump-thump—roundtrip’s set just right,
A happy hare in greys of night. 🐇✨

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch claude/issue-18-20251005-1714

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4c11c5e and 164b27a.

📒 Files selected for processing (1)

• test/integration/roo.test.ts (6 hunks)

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[claude]

PR Review: Fix roo integration test issues

Summary

This PR fixes two critical issues in the Roo integration tests:

1. Missing directory creation: Adds mkdirSync calls to create .roo/rules directories before writing test files
2. Regex pattern fixes: Updates regex patterns to use [\s\S]* instead of .* for multiline matching in frontmatter

Code Quality ✅

Positive aspects:

• Minimal, focused changes that address specific test failures
• Proper use of { recursive: true } option for mkdirSync to handle nested directory creation
• Consistent with existing patterns in the codebase (similar directory creation appears in other test files and exporters)

Regex improvements:
The regex changes from .* to [\s\S]* are correct:

• .* matches any character except newlines
• [\s\S]* matches any character including newlines, which is necessary for YAML frontmatter that spans multiple lines

Potential Issues ⚠️

1. Redundant directory creation (minor):

   • Line 24-25: Creates both rulesDir and nestedDir with recursive: true
   • The second call (mkdirSync(nestedDir, { recursive: true })) already creates parent directories, so the first call is technically redundant
   • However, this is more explicit and readable, so it's acceptable

2. Test isolation concern:

   • The tests create temporary directories in beforeEach and clean them up in afterEach
   • The new mkdirSync calls are properly scoped within individual tests
   • ✅ No issues here

Best Practices ✅

Good practices observed:

• Using { recursive: true } prevents errors if parent directories don't exist
• Changes are consistent across all affected test cases
• The fix addresses the root cause (missing directories) rather than masking symptoms

Performance Considerations ✅

• The mkdirSync calls are synchronous, which is appropriate for test setup code
• Minimal performance impact (only affects test execution, not production code)
• No unnecessary filesystem operations

Security Concerns ✅

• No security issues identified
• Uses tmpdir() for test directories, which is secure
• Proper cleanup with rmSync in afterEach

Test Coverage ✅

Coverage improvements:

• The tests now properly validate the full import/export cycle
• Tests cover:
  • Public and private rules
  • Nested directory structures
  • Frontmatter preservation
  • Roundtrip import/export

However, consider adding:

• A test case that verifies the behavior when .roo/rules already exists
• Error handling tests for scenarios where directory creation might fail (though less critical for integration tests)

Specific Code Review

Line 2: Import addition

```typescript
-import { mkdtempSync, rmSync, writeFileSync, readFileSync, readdirSync } from 'fs'
+import { mkdtempSync, rmSync, writeFileSync, readFileSync, readdirSync, mkdirSync } from 'fs'
```
✅ Correct - adds the necessary import

Lines 24-25: Directory creation

```typescript

• mkdirSync(rulesDir, { recursive: true })
• mkdirSync(nestedDir, { recursive: true })
  ```
  ✅ Correct - ensures directories exist before writeFileSync calls

Line 147: Another directory creation

```typescript

• mkdirSync(inputRulesDir, { recursive: true })
  ```
  ✅ Correct - fixes the same issue in the roundtrip test

Lines 126, 178, 204: Regex pattern fixes

```typescript
-expect(publicContent).toMatch(/^---\n.*alwaysApply: true\n.*description: Exported public rule\n---\n\n# Exported Public Content/)
+expect(publicContent).toMatch(/^---[\s\S]*alwaysApply: true[\s\S]description: Exported public rule[\s\S]---\n\n# Exported Public Content/)
```
✅ Correct - properly matches multiline YAML frontmatter

Minor suggestion: These regexes could be more robust by being less strict about the exact order of YAML fields (YAML doesn't guarantee order). Consider:
```typescript
expect(publicContent).toMatch(/^---[\s\S]*---\n\n# Exported Public Content/)
expect(publicContent).toContain('alwaysApply: true')
expect(publicContent).toContain('description: Exported public rule')
```
This would make tests more resilient to YAML serialization order changes.

Comparison with Source Code

Looking at /home/runner/work/dotagent/dotagent/src/exporters.ts:472-493, the exportToRoo function already uses mkdirSync(rulesDir, { recursive: true }) (line 474), so this PR aligns the tests with the production code's assumptions.

Recommendations

1. Merge this PR - it fixes legitimate bugs in the test suite
2. Consider a follow-up PR to make regex assertions less order-dependent (low priority)
3. Verify the tests pass after merging to ensure the fixes are complete

Conclusion

Verdict: LGTM (Looks Good To Me) ✅

This is a solid bug fix that:

• Addresses the root cause of test failures
• Uses appropriate APIs and patterns
• Follows existing code conventions
• Has no security or performance concerns
• Improves test reliability

The changes are minimal, focused, and low-risk.

[github-actions]

🎉 This PR is included in version 2.8.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀