Rovodev rule: type guard extraction, closure testability, and minor cleanups

[dyoshikawa]

Background

PR #1396 ("refactor: address review findings from Rovodev PR #1377") was reviewed and all critical/high/mid findings were addressed before merge. The remaining low and mid findings are maintainability improvements that can be tackled in follow-up work. This issue consolidates them for future reference.

Details

Mid severity

1. Repeated union narrowing pattern in RovodevRule — extract a type guard function

• File: src/features/rules/rovodev-rule.ts (lines 104-108, 177, 189, 253)
• The pattern "nonRoot" in paths && paths.nonRoot and "alternativeRoots" in paths appears in multiple places to narrow the RovodevRuleSettablePaths | ToolRuleSettablePathsGlobal union.
• A dedicated type guard (e.g. isProjectPaths(paths): paths is RovodevRuleSettablePaths) would reduce duplication and make the two-mode design explicit.

2. buildDeletionRulesFromPaths is a closure that cannot be unit-tested independently

• File: src/features/rules/rules-processor.ts (lines 937-963)
• The helper captures this.baseDir, this.global, factory, and resolveRelativeDirPath from outer scope.
• If it grows or is needed elsewhere, consider promoting it to a private method with explicit parameters for independent testability.

Low severity

3. isAllowedModularRulesRelativePath checks all path segments, not just the final basename

• File: src/features/rules/rovodev-rule.ts (lines 52-59)
• A directory named agents.md (e.g. agents.md/foo.md) would also be rejected. If this is intentional, the JSDoc and constant name (DISALLOWED_ROVODEV_MODULAR_RULE_BASENAMES) should clarify that it applies to all segments.

4. **/.rovodev/.rulesync/ gitignore entry uses general feature tag

• File: src/cli/commands/gitignore-entries.ts (lines 197-201)
• Sibling entries use specific feature tags (subagents, skills), but this one uses general. If intentional (implementation detail that should always be ignored), an inline comment would help.

5. fromRootFile parameter name overrideDirPath is slightly misleading

• File: src/features/rules/rovodev-rule.ts (lines 159-173)
• It falls back to paths.root.relativeDirPath when undefined, so "override" implies it always overrides something. Consider relativeDirPath or explicitDirPath.

6. Test coverage: add positive cases with similar-but-allowed names

• File: src/features/rules/rovodev-rule.test.ts
• Current tests verify that mixed-case reserved names are rejected. Adding tests for names containing the substring "agents" but not matching (e.g. agents-custom.md, my-agents.md) would confirm the check is not overly broad.

7. Granular .rovodev/ gitignore entries risk drift

• File: .gitignore (lines 307-313)
• New files under .rovodev/ will need corresponding gitignore entries. Already covered by feature-change-guidelines.md, but worth keeping in mind.

8. JSDoc markdown bold in buildDeletionRulesFromPaths — already fixed in latest commit

• Replaced **Root mode** / **Non-root mode** with plain text labels.

Solution / Next Steps

1. Type guard extraction (item 1): Create isProjectPaths type guard in rovodev-rule.ts and replace all manual narrowing checks.
2. Closure testability (item 2): Monitor growth of buildDeletionRulesFromPaths. If it gains complexity, extract to a private method with explicit parameters.
3. Documentation / comments (items 3, 4, 5): Add clarifying JSDoc/comments for the path-segment check scope, general feature tag choice, and parameter naming.
4. Test coverage (item 6): Add positive test cases for similar-but-allowed names.
5. Items 7-8 are informational / already resolved.