diff --git a/CHANGELOG.md b/CHANGELOG.md index f50ad012..cc419b8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed + +- **`watch-pr` crashes on repos with non-main default branch** — `fetchFileChanges` hardcoded `origin/main` for git diff, causing failures on repos using `master`, `develop`, or other base branches. Now uses the PR's actual base branch from GitHub metadata. + ## [0.19.0] - 2026-03-04 ### Breaking Changes diff --git a/packages/cli/src/services/github-fetcher.ts b/packages/cli/src/services/github-fetcher.ts index 9b02b0da..15ce1f24 100644 --- a/packages/cli/src/services/github-fetcher.ts +++ b/packages/cli/src/services/github-fetcher.ts @@ -272,14 +272,17 @@ export class GitHubFetcher { * Uses git diff --numstat to get file change statistics. * * @param _prNumber - PR number (unused - we use git diff from current branch) + * @param baseBranch - Base branch name (e.g., 'main', 'master', 'develop') * @returns File change context */ - async fetchFileChanges(_prNumber: number): Promise { + async fetchFileChanges(_prNumber: number, baseBranch: string): Promise { + const baseRef = `origin/${baseBranch}`; + // Get diff stats using getDiffStats from @vibe-validate/git (architectural compliance) - const diffOutput = getDiffStats('origin/main', 'HEAD'); + const diffOutput = getDiffStats(baseRef, 'HEAD'); // Get commit count using getCommitCount from @vibe-validate/git (architectural compliance) - const commitCount = getCommitCount('origin/main', 'HEAD'); + const commitCount = getCommitCount(baseRef, 'HEAD'); // Parse diff output const lines = diffOutput.trim().split('\n').filter((line) => line.length > 0); diff --git a/packages/cli/src/services/watch-pr-orchestrator.ts b/packages/cli/src/services/watch-pr-orchestrator.ts index 66fbd4c2..af440459 100644 --- a/packages/cli/src/services/watch-pr-orchestrator.ts +++ b/packages/cli/src/services/watch-pr-orchestrator.ts @@ -157,8 +157,8 @@ export class WatchPROrchestrator { // Fetch history summary const historySummary = await this.historyBuilder.buildSummary(prMetadata.branch); - // Fetch file changes - const changes = await this.fetcher.fetchFileChanges(prNumber); + // Fetch file changes (use PR's base branch, not hardcoded 'main') + const changes = await this.fetcher.fetchFileChanges(prNumber, prMetadata.base_branch); // Determine overall status let status: PRStatus = 'passed'; @@ -351,8 +351,8 @@ export class WatchPROrchestrator { jobs.map(job => this.buildCheckFromJob(job, runId, runDetails.workflow)) ); - // Fetch file changes - const changes = await this.fetcher.fetchFileChanges(prNumber); + // Fetch file changes (use PR's base branch, not hardcoded 'main') + const changes = await this.fetcher.fetchFileChanges(prNumber, prMetadata.base_branch); // Determine status const status = this.determineOverallStatus(actionChecks); diff --git a/packages/cli/test/services/github-fetcher.test.ts b/packages/cli/test/services/github-fetcher.test.ts index 1843ad27..7d20d984 100644 --- a/packages/cli/test/services/github-fetcher.test.ts +++ b/packages/cli/test/services/github-fetcher.test.ts @@ -499,7 +499,7 @@ describe('GitHubFetcher', () => { vi.spyOn(gitPackage, 'getDiffStats').mockReturnValue(mockDiffOutput); vi.spyOn(gitPackage, 'getCommitCount').mockReturnValue(mockCommitCount); - const changes = await fetcher.fetchFileChanges(123); + const changes = await fetcher.fetchFileChanges(123, 'main'); expect(changes).toEqual({ files_changed: 4, @@ -523,7 +523,7 @@ describe('GitHubFetcher', () => { vi.spyOn(gitPackage, 'getDiffStats').mockReturnValue(mockDiffOutput); vi.spyOn(gitPackage, 'getCommitCount').mockReturnValue(mockCommitCount); - const changes = await fetcher.fetchFileChanges(123); + const changes = await fetcher.fetchFileChanges(123, 'main'); expect(changes.top_files).toHaveLength(10); expect(changes.files_changed).toBe(20); @@ -539,7 +539,7 @@ describe('GitHubFetcher', () => { vi.spyOn(gitPackage, 'getDiffStats').mockReturnValue(mockDiffOutput); vi.spyOn(gitPackage, 'getCommitCount').mockReturnValue(mockCommitCount); - const changes = await fetcher.fetchFileChanges(123); + const changes = await fetcher.fetchFileChanges(123, 'main'); expect(changes.top_files?.[0].new_file).toBe(true); }); @@ -552,7 +552,7 @@ describe('GitHubFetcher', () => { vi.spyOn(gitPackage, 'getDiffStats').mockReturnValue(mockDiffOutput); vi.spyOn(gitPackage, 'getCommitCount').mockReturnValue(mockCommitCount); - const changes = await fetcher.fetchFileChanges(123); + const changes = await fetcher.fetchFileChanges(123, 'main'); expect(changes).toEqual({ files_changed: 0, @@ -569,7 +569,20 @@ describe('GitHubFetcher', () => { throw new Error('git: not a git repository'); }); - await expect(fetcher.fetchFileChanges(123)).rejects.toThrow(); + await expect(fetcher.fetchFileChanges(123, 'main')).rejects.toThrow(); + }); + + it('should use the provided base branch instead of hardcoded main', async () => { + const mockDiffOutput = '1\t0\tfile.ts'; + const mockCommitCount = '1'; + + const diffSpy = vi.spyOn(gitPackage, 'getDiffStats').mockReturnValue(mockDiffOutput); + const countSpy = vi.spyOn(gitPackage, 'getCommitCount').mockReturnValue(mockCommitCount); + + await fetcher.fetchFileChanges(123, 'master'); + + expect(diffSpy).toHaveBeenCalledWith('origin/master', 'HEAD'); + expect(countSpy).toHaveBeenCalledWith('origin/master', 'HEAD'); }); }); diff --git a/packages/cli/test/services/watch-pr-orchestrator.test.ts b/packages/cli/test/services/watch-pr-orchestrator.test.ts index ecf4e474..13bb7b3b 100644 --- a/packages/cli/test/services/watch-pr-orchestrator.test.ts +++ b/packages/cli/test/services/watch-pr-orchestrator.test.ts @@ -592,7 +592,7 @@ describe('WatchPROrchestrator', () => { // Verify PR context is still fetched expect(fetchPRSpy).toHaveBeenCalledWith(mockPRNumber); - expect(fetchFileChangesSpy).toHaveBeenCalledWith(mockPRNumber); + expect(fetchFileChangesSpy).toHaveBeenCalledWith(mockPRNumber, expect.any(String)); }); });