test: add global mode e2e tests for commands, skills, subagents, mcp

[dyoshikawa-claw]

Summary

PR #1268 で skills の global mode 修正と e2e テストが追加されたのに合わせて、他の features (commands, subagents, mcp) にも global mode の e2e テストを追加しました。

• useGlobalTestDirectories() を使ったテストパターンを各 feature に適用
• 各ターゲットで home directory への出力を検証
• non-root が無視されるテストも追加

Test Results

✅ skills - 全テスト成功 (PR #1268 で修正済み)
✅ commands - claudecode, opencode は成功 / ❌ copilot は失敗
✅ subagents - claudecode は成功 / ❌ copilot, opencode は失敗
❌ mcp - 全ターゲット失敗

失敗するテストは global mode の実装が必要な箇所を示しています。別 PR で対応予定です。

Related

• fix: skills not written to global directory in global mode #1268 (skills global mode fix)

[github-actions]

PR #1271 Review - Overall Verdict: ✅ MERGEABLE

No critical or high severity blockers found. Issues are medium severity or lower.

---

Code Review Findings

1. [MEDIUM] Inconsistent test coverage across targets - Global mode tests have varying target coverage:

   • Commands: tests claudecode, cursor, opencode (missing geminicli)
   • Skills: tests claudecode, cursor, opencode
   • Subagents: tests claudecode, cursor, opencode
   • MCP: tests claudecode, geminicli, opencode (missing cursor, codexcli)
   • Recommendation: Ensure consistent target coverage or document exclusions

2. [LOW] Minor target rename mixed with test additions - Changes from copilot to cursor in lines 13-14 of e2e-commands.spec.ts and e2e-subagents.spec.ts are mixed with new tests

   • Recommendation: Consider separating renames into dedicated commits

3. [LOW] Permissive string assertions - Tests use toContain() for simple string matching (e.g., line 132 in e2e-mcp.spec.ts)

   • Recommendation: Consider parsing JSON/TOML or using snapshot testing for more robust assertions

4. [LOW] MCP non-root test uses legacy path - Line 172 in e2e-mcp.spec.ts writes to .rulesync/.mcp.json

   • Recommendation: Add comment explaining legacy path testing

5. [LOW] Missing error case tests - No tests for edge cases like multiple root: true files, missing root: true, or unwritable home directories

6. [INFO] Good testing practices - Follows project guidelines, uses useGlobalTestDirectories() correctly, proper cleanup, consistent structure

---

Security Review Findings

7. [LOW] Missing HOME_DIR validation (pre-existing issue, not introduced by PR) - The HOME_DIR environment variable in getHomeDirectory() is used without validation, bypassing validateBaseDir() checks

   • Location: src/utils/file.ts:261-264, src/config/config-resolver.ts:164-166
   • Risk: Low for CLI tool but could be problematic in CI/CD pipelines
   • Recommendation: Add path validation to reject relative paths and path traversal

8. [INFO] No path traversal vulnerabilities - Test directories use controlled locations, join() with hardcoded components, and crypto.randomBytes() for unique names

9. [INFO] No injection vulnerabilities - All test content is hardcoded, no external data processing

10. [INFO] Safe command execution - Uses execFileAsync() without shell, validates RULESYNC_CMD environment variable

11. [INFO] Proper test isolation - Unique temp directories, cleanup in afterEach, serial execution, scoped environment variables

---

Recommendation: Approve with minor improvements to address inconsistent target coverage (finding #1) and consider adding HOME_DIR validation (finding #7) in a follow-up PR.

github run