fix: address remaining review findings from PR #1039

[saitota]

Accidentally closed PR #1209 by deleting the fork. Recreating with the same diff.

Summary

• Replace hardcoded .rulesync/skills/.curated/ path in gitignore.ts with RULESYNC_CURATED_SKILLS_RELATIVE_DIR_PATH constant
• Add logger.error() with source name for GitHubClientError before logGitHubAuthHints(), deduplicate error logging
• Add GitLab URL/provider prefix handling to normalizeSourceKey using DRY loop over known git hosts
• Change GitLab unsupported error from throw to logger.warn() + early return for graceful skipping

Closes #1042

References

• fix: address PR #1032 review findings for install command #1039
• feat: add rulesync install command for declarative skill sources #1032

Test plan

• pnpm check
• pnpm test

[dyoshikawa-claw]

/opencode review

[github-actions]

Code Review Summary for PR #1218

Based on the code review and security review, here are the integrated findings:

Code Quality Findings

1. ✅ Path constant usage - Correctly replaces hardcoded path with RULESYNC_CURATED_SKILLS_RELATIVE_DIR_PATH constant, following project guidelines
2. ✅ Error logging improvement - Deduplicates error logging, ensuring users always see what failed regardless of error type
3. ✅ DRY refactoring - Provider prefix loop is more maintainable than individual if statements
4. ✅ Graceful GitLab handling - Early return with warning is better than throwing, allowing processing of other sources
5. ✅ Test coverage - New tests cover all GitLab URL prefix variants
6. ⚠️ Test alignment - Existing test in sources.test.ts should verify logger.warn instead of "log error"
7. ⚠️ URL prefix ordering inconsistency - GitLab prefixes aren't ordered the same as GitHub (www variants before bare domain)
8. ⚠️ Consider extracting constants - Prefix lists could be extracted to named constants for future maintainability

Security Findings

9. ✅ No critical vulnerabilities - All changes are safe with no security regressions
10. ⚠️ Potential duplicate logging - GitHubClientError may be logged twice (line 125 + inside logGitHubAuthHints())
11. ✅ Safe URL normalization - No injection risk, correct case-insensitive handling
12. ✅ No sensitive information exposure - Error messages don't contain tokens or credentials
13. ✅ No command injection risks - No shell command execution with user input

Overall Assessment

✅ APPROVED - The PR correctly addresses the review findings from PR #1039. Minor suggestions (ordering consistency, extracting constants, duplicate logging) are optional improvements and don't block merging.

github run