Add support for commit hashes along with branches #213
Conversation
|
|
📝 WalkthroughWalkthroughThe pull request modifies the Changes
Sequence DiagramsequenceDiagram
participant User
participant GitCommand
participant Git
User->>GitCommand: execGitShallowClone(url, directory, branchOrCommit)
GitCommand->>Git: git clone --depth 1 url directory
alt branchOrCommit specified
GitCommand->>Git: git -C directory checkout branchOrCommit
end
Possibly related PRs
Suggested reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/core/file/gitCommand.ts (1)
29-32: Consider enhancing error handling with specific error typesThe function could benefit from more specific error handling to distinguish between different failure scenarios (network issues, authentication failures, invalid refs, etc.).
Consider wrapping errors with more context:
export const execGitShallowClone = async ( url: string, directory: string, branchOrCommit?: string, deps = { execAsync, }, ) => { try { await deps.execAsync(`git clone --depth 1 ${url} ${directory}`); if (branchOrCommit) { await deps.execAsync(`git -C ${directory} checkout ${branchOrCommit}`); } + } catch (error) { + if (error.message.includes('Authentication failed')) { + throw new Error(`Git authentication failed for repository: ${url}`); + } else if (error.message.includes('not found')) { + throw new Error(`Git reference not found: ${branchOrCommit}`); + } + throw error; } };tests/core/file/gitCommand.test.ts (1)
91-108: Consider adding security-focused test casesWhile the error handling for invalid commits is well tested, consider adding test cases for potential security issues.
Add tests for command injection scenarios:
test('should safely handle special characters in branch names', async () => { const mockExecAsync = vi.fn().mockResolvedValue({ stdout: '', stderr: '' }); const url = 'https://github.com/user/repo.git'; const directory = '/tmp/repo'; const maliciousBranch = 'master; rm -rf /'; await execGitShallowClone(url, directory, maliciousBranch, { execAsync: mockExecAsync }); // Verify the command is properly escaped expect(mockExecAsync).toHaveBeenCalledWith(expect.not.stringContaining(';')); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/core/file/gitCommand.ts(1 hunks)tests/core/file/gitCommand.test.ts(2 hunks)
🔇 Additional comments (2)
tests/core/file/gitCommand.test.ts (2)
43-53: LGTM! Test cases properly verify the new branch functionality
The test cases effectively verify the separation of clone and checkout commands for branch-based operations.
55-65: LGTM! Good coverage for commit hash functionality
The test cases properly verify that commit hashes are handled correctly in the checkout phase.
This PR allows commit hashes to be supported for the branch arg when using a remote repo url.
Checklist
npm run testnpm run lint