From 111cb3ebffa26f8e317096c69231ea1e70c784a8 Mon Sep 17 00:00:00 2001 From: Test User Date: Wed, 15 Apr 2026 09:15:24 -0700 Subject: [PATCH 1/3] =?UTF-8?q?feat(pr-history):=20Report=20Glitch=20actio?= =?UTF-8?q?n=20and=20REQ=E2=86=92PR=20back-link=20(#573)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add "Report Glitch" button to each merged PR row in the PR History panel. Clicking it fetches the PR's changed files and opens the Capture Glitch modal pre-populated with scope, source, and a PR reference note. REQs created this way store the GitHub PR URL as source_issue, which now renders as a clickable link on the REQ detail page. Backend: - GitHubIntegration.get_pr_files() fetches changed files for a PR - GET /api/v2/pr/{pr_number}/files endpoint exposes PR changed files Frontend: - CaptureGlitchModal accepts optional prNumber/prTitle/prUrl/initialScope props for pre-population and includes source_issue in the payload - PRHistoryPanel rows include [Report Glitch] button with loading state - REQ detail page renders source_issue as a GitHub link when applicable - PRFilesResponse type and prApi.getFiles() API method added --- codeframe/git/github_integration.py | 16 ++++++ codeframe/ui/routers/pr_v2.py | 48 ++++++++++++++++ tests/ui/test_pr_history.py | 57 ++++++++++++++++++- tests/unit/test_github_integration.py | 47 +++++++++++++++ .../proof/CaptureGlitchModal.test.tsx | 56 ++++++++++++++++++ .../components/proof/ProofDetailPage.test.tsx | 39 +++++++++++++ .../components/review/PRHistoryPanel.test.tsx | 44 +++++++++++++- web-ui/src/app/proof/[req_id]/page.tsx | 18 +++++- .../components/proof/CaptureGlitchModal.tsx | 30 ++++++++-- .../src/components/review/PRHistoryPanel.tsx | 47 +++++++++++++++ web-ui/src/lib/api.ts | 8 +++ web-ui/src/types/index.ts | 5 ++ 12 files changed, 404 insertions(+), 11 deletions(-) diff --git a/codeframe/git/github_integration.py b/codeframe/git/github_integration.py index 8f619b6a..280cedc5 100644 --- a/codeframe/git/github_integration.py +++ b/codeframe/git/github_integration.py @@ -378,6 +378,22 @@ async def close_pull_request(self, pr_number: int) -> bool: logger.info(f"Closed PR #{pr_number}") return data.get("state") == "closed" + async def get_pr_files(self, pr_number: int) -> List[str]: + """Get the list of files changed in a pull request. + + Args: + pr_number: PR number + + Returns: + List of filenames changed in the PR + + Raises: + GitHubAPIError: If API error occurs + """ + endpoint = f"/repos/{self.owner}/{self.repo_name}/pulls/{pr_number}/files" + data = await self._make_request(method="GET", endpoint=endpoint) + return [f["filename"] for f in data] + async def get_pr_ci_checks( self, pr_number: int, diff --git a/codeframe/ui/routers/pr_v2.py b/codeframe/ui/routers/pr_v2.py index 4ee851b8..249c6b6c 100644 --- a/codeframe/ui/routers/pr_v2.py +++ b/codeframe/ui/routers/pr_v2.py @@ -122,6 +122,12 @@ class PRHistoryItem(BaseModel): proof_snapshot: Optional[ProofSnapshotOut] +class PRFilesResponse(BaseModel): + """Response for PR changed files.""" + + files: list[str] + + class PRHistoryResponse(BaseModel): """Response for PR history list.""" @@ -390,6 +396,48 @@ async def get_pr_history( await client.close() +@router.get("/{pr_number}/files", response_model=PRFilesResponse) +@rate_limit_standard() +async def get_pr_files( + request: Request, + pr_number: int, + workspace: Workspace = Depends(get_v2_workspace), +) -> PRFilesResponse: + """Get the list of files changed in a pull request. + + Args: + pr_number: PR number + workspace: v2 Workspace (for context) + + Returns: + List of changed filenames + """ + client = _get_github_client() + try: + files = await client.get_pr_files(pr_number) + return PRFilesResponse(files=files) + except GitHubAPIError as e: + if e.status_code == 404: + raise HTTPException( + status_code=404, + detail=api_error("PR not found", ErrorCodes.NOT_FOUND, f"No PR #{pr_number}"), + ) + raise HTTPException( + status_code=e.status_code, + detail=api_error("GitHub API error", ErrorCodes.EXECUTION_FAILED, e.message), + ) + except HTTPException: + raise + except Exception as e: + logger.error(f"Failed to get files for PR #{pr_number}: {e}", exc_info=True) + raise HTTPException( + status_code=500, + detail=api_error("Failed to get PR files", ErrorCodes.EXECUTION_FAILED, str(e)), + ) + finally: + await client.close() + + @router.get("/{pr_number}", response_model=PRResponse) @rate_limit_standard() async def get_pull_request( diff --git a/tests/ui/test_pr_history.py b/tests/ui/test_pr_history.py index 52fac8fe..c9789f84 100644 --- a/tests/ui/test_pr_history.py +++ b/tests/ui/test_pr_history.py @@ -1,7 +1,7 @@ -"""Tests for GET /api/v2/pr/history endpoint (pr_v2 router). +"""Tests for PR v2 router endpoints: history and files. -These tests verify the PR history endpoint by mocking GitHubIntegration -so no real GitHub API calls are made. +These tests verify the PR history and PR files endpoints by mocking +GitHubIntegration so no real GitHub API calls are made. """ import shutil @@ -254,3 +254,54 @@ def test_author_included(self, test_client): assert resp.status_code == 200 assert resp.json()["pull_requests"][0]["author"] == "bob" + + +class TestGetPrFiles: + """Tests for GET /api/v2/pr/{pr_number}/files.""" + + def test_returns_file_list(self, test_client): + """Endpoint returns the list of changed files for a PR.""" + mock_client = _make_mock_client() + mock_client.get_pr_files = AsyncMock(return_value=["src/app.py", "tests/test_app.py"]) + + with patch("codeframe.ui.routers.pr_v2._get_github_client", return_value=mock_client): + resp = test_client.get("/api/v2/pr/42/files?workspace_path=/tmp") + + assert resp.status_code == 200 + body = resp.json() + assert body["files"] == ["src/app.py", "tests/test_app.py"] + + def test_returns_empty_list(self, test_client): + """Endpoint returns empty list when PR has no file changes.""" + mock_client = _make_mock_client() + mock_client.get_pr_files = AsyncMock(return_value=[]) + + with patch("codeframe.ui.routers.pr_v2._get_github_client", return_value=mock_client): + resp = test_client.get("/api/v2/pr/1/files?workspace_path=/tmp") + + assert resp.status_code == 200 + assert resp.json()["files"] == [] + + def test_pr_not_found(self, test_client): + """Endpoint returns 404 when PR does not exist.""" + from codeframe.git.github_integration import GitHubAPIError + + mock_client = _make_mock_client() + mock_client.get_pr_files = AsyncMock( + side_effect=GitHubAPIError(404, "Not Found"), + ) + + with patch("codeframe.ui.routers.pr_v2._get_github_client", return_value=mock_client): + resp = test_client.get("/api/v2/pr/99999/files?workspace_path=/tmp") + + assert resp.status_code == 404 + + def test_client_close_called(self, test_client): + """Client.close() is always called.""" + mock_client = _make_mock_client() + mock_client.get_pr_files = AsyncMock(return_value=[]) + + with patch("codeframe.ui.routers.pr_v2._get_github_client", return_value=mock_client): + test_client.get("/api/v2/pr/1/files?workspace_path=/tmp") + + mock_client.close.assert_awaited_once() diff --git a/tests/unit/test_github_integration.py b/tests/unit/test_github_integration.py index 592ff48d..55cc6238 100644 --- a/tests/unit/test_github_integration.py +++ b/tests/unit/test_github_integration.py @@ -299,6 +299,53 @@ async def test_rate_limit_error(self, github): assert exc_info.value.status_code == 403 +class TestGetPrFiles: + """Tests for GitHubIntegration.get_pr_files.""" + + @pytest.fixture + def github(self): + return GitHubIntegration( + token="ghp_test_token_12345", + repo="owner/test-repo", + ) + + @pytest.mark.asyncio + async def test_returns_list_of_filenames(self, github): + """get_pr_files returns a list of filename strings.""" + mock_response = [ + {"filename": "src/app.py", "status": "modified"}, + {"filename": "tests/test_app.py", "status": "added"}, + ] + + with patch.object(github, "_make_request", new_callable=AsyncMock) as mock_request: + mock_request.return_value = mock_response + files = await github.get_pr_files(42) + + assert files == ["src/app.py", "tests/test_app.py"] + mock_request.assert_called_once() + call_kwargs = mock_request.call_args.kwargs + assert call_kwargs["method"] == "GET" + assert "/pulls/42/files" in call_kwargs["endpoint"] + + @pytest.mark.asyncio + async def test_returns_empty_list_for_no_files(self, github): + """get_pr_files returns empty list when PR has no file changes.""" + with patch.object(github, "_make_request", new_callable=AsyncMock) as mock_request: + mock_request.return_value = [] + files = await github.get_pr_files(1) + + assert files == [] + + @pytest.mark.asyncio + async def test_propagates_api_error(self, github): + """get_pr_files propagates GitHubAPIError.""" + with patch.object(github, "_make_request", new_callable=AsyncMock) as mock_request: + mock_request.side_effect = GitHubAPIError(404, "Not Found") + with pytest.raises(GitHubAPIError) as exc_info: + await github.get_pr_files(99999) + assert exc_info.value.status_code == 404 + + class TestGitHubAPIError: """Tests for GitHubAPIError exception.""" diff --git a/web-ui/src/__tests__/components/proof/CaptureGlitchModal.test.tsx b/web-ui/src/__tests__/components/proof/CaptureGlitchModal.test.tsx index f5b6b6c8..dc063818 100644 --- a/web-ui/src/__tests__/components/proof/CaptureGlitchModal.test.tsx +++ b/web-ui/src/__tests__/components/proof/CaptureGlitchModal.test.tsx @@ -234,4 +234,60 @@ describe('CaptureGlitchModal', () => { expect((screen.getByLabelText(/Description/i) as HTMLTextAreaElement).value).toBe(''); }); }); + + describe('pre-population from PR', () => { + const PR_PROPS = { + ...DEFAULT_PROPS, + prNumber: 42, + prTitle: 'Fix login timeout', + prUrl: 'https://github.com/owner/repo/pull/42', + initialScope: 'src/auth.py\nsrc/utils.py', + }; + + it('pre-fills description with PR reference when prNumber is provided', () => { + setup(PR_PROPS); + const textarea = screen.getByLabelText(/Description/i) as HTMLTextAreaElement; + expect(textarea.value).toBe('Reported from PR #42: Fix login timeout'); + }); + + it('pre-fills scope with initialScope', () => { + setup(PR_PROPS); + const textarea = screen.getByLabelText(/Scope/i) as HTMLTextAreaElement; + expect(textarea.value).toBe('src/auth.py\nsrc/utils.py'); + }); + + it('includes source_issue in submission payload', async () => { + mockCapture.mockResolvedValue(MOCK_REQ); + setup(PR_PROPS); + + // Fill required fields + fireEvent.click(screen.getByRole('checkbox', { name: 'unit' })); + fireEvent.click(screen.getByRole('button', { name: /Capture Glitch/i })); + + await waitFor(() => { + expect(mockCapture).toHaveBeenCalledWith( + WORKSPACE, + expect.objectContaining({ + source_issue: 'https://github.com/owner/repo/pull/42', + }) + ); + }); + }); + + it('does not include source_issue when prUrl is not provided', async () => { + mockCapture.mockResolvedValue(MOCK_REQ); + setup(); + + fireEvent.change(screen.getByLabelText(/Description/i), { + target: { value: 'Something broke' }, + }); + fireEvent.click(screen.getByRole('checkbox', { name: 'unit' })); + fireEvent.click(screen.getByRole('button', { name: /Capture Glitch/i })); + + await waitFor(() => { + const callArgs = mockCapture.mock.calls[0][1]; + expect(callArgs.source_issue).toBeUndefined(); + }); + }); + }); }); diff --git a/web-ui/src/__tests__/components/proof/ProofDetailPage.test.tsx b/web-ui/src/__tests__/components/proof/ProofDetailPage.test.tsx index 73c6abbb..e44155f1 100644 --- a/web-ui/src/__tests__/components/proof/ProofDetailPage.test.tsx +++ b/web-ui/src/__tests__/components/proof/ProofDetailPage.test.tsx @@ -282,3 +282,42 @@ describe('ProofDetailPage — combined filters', () => { expect(evidenceRows()).toHaveLength(1); }); }); + +describe('ProofDetailPage — source_issue rendering', () => { + beforeEach(() => jest.clearAllMocks()); + + function setupWithReq(reqOverrides: Partial) { + const req = { ...REQ, ...reqOverrides }; + mockGetWorkspace.mockReturnValue(WORKSPACE); + mockUseSWR.mockImplementation((key: unknown) => { + if (!key) { + return { data: undefined, error: undefined, isLoading: false, mutate: jest.fn() } as unknown as ReturnType; + } + const k = String(key); + if (k.includes('/evidence')) { + return { data: [], error: undefined, isLoading: false, mutate: jest.fn() } as unknown as ReturnType; + } + return { data: req, error: undefined, isLoading: false, mutate: jest.fn() } as unknown as ReturnType; + }); + render(); + } + + it('renders source_issue as a clickable link when it is a GitHub URL', () => { + setupWithReq({ source_issue: 'https://github.com/owner/repo/pull/42' }); + const link = screen.getByRole('link', { name: /github\.com/i }); + expect(link).toHaveAttribute('href', 'https://github.com/owner/repo/pull/42'); + expect(link).toHaveAttribute('target', '_blank'); + }); + + it('renders source_issue as plain text when it is not a URL', () => { + setupWithReq({ source_issue: 'JIRA-123' }); + expect(screen.getByText(/JIRA-123/)).toBeInTheDocument(); + expect(screen.queryByRole('link', { name: /JIRA-123/ })).not.toBeInTheDocument(); + }); + + it('does not render source_issue when it is null', () => { + setupWithReq({ source_issue: null }); + expect(screen.queryByText(/Source PR/)).not.toBeInTheDocument(); + expect(screen.queryByText(/Issue:/)).not.toBeInTheDocument(); + }); +}); diff --git a/web-ui/src/__tests__/components/review/PRHistoryPanel.test.tsx b/web-ui/src/__tests__/components/review/PRHistoryPanel.test.tsx index cc8f26b1..d125989a 100644 --- a/web-ui/src/__tests__/components/review/PRHistoryPanel.test.tsx +++ b/web-ui/src/__tests__/components/review/PRHistoryPanel.test.tsx @@ -1,16 +1,20 @@ import React from 'react'; -import { render, screen, fireEvent } from '@testing-library/react'; +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; import useSWR from 'swr'; import { PRHistoryPanel } from '@/components/review/PRHistoryPanel'; +import { prApi } from '@/lib/api'; import type { PRHistoryResponse } from '@/types'; // ── Mocks ───────────────────────────────────────────────────────────────── jest.mock('swr'); jest.mock('@/lib/api', () => ({ - prApi: { getHistory: jest.fn() }, + prApi: { getHistory: jest.fn(), getFiles: jest.fn() }, + proofApi: { capture: jest.fn() }, })); +const mockGetFiles = prApi.getFiles as jest.MockedFunction; + const mockUseSWR = useSWR as jest.MockedFunction; // ── Helpers ─────────────────────────────────────────────────────────────── @@ -233,4 +237,40 @@ describe('PRHistoryPanel', () => { expect(key).toBeNull(); }); }); + + describe('Report Glitch button', () => { + it('renders a Report Glitch button for each PR row', () => { + withData(SAMPLE_HISTORY); + render(); + + const buttons = screen.getAllByRole('button', { name: /Report Glitch/i }); + expect(buttons).toHaveLength(2); + }); + + it('fetches PR files when Report Glitch is clicked', async () => { + mockGetFiles.mockResolvedValue(['src/auth.py', 'tests/test_auth.py']); + withData(SAMPLE_HISTORY); + render(); + + const buttons = screen.getAllByRole('button', { name: /Report Glitch/i }); + fireEvent.click(buttons[0]); + + await waitFor(() => { + expect(mockGetFiles).toHaveBeenCalledWith(WORKSPACE, 10); + }); + }); + + it('opens the capture modal after files are fetched', async () => { + mockGetFiles.mockResolvedValue(['src/auth.py']); + withData(SAMPLE_HISTORY); + render(); + + const buttons = screen.getAllByRole('button', { name: /Report Glitch/i }); + fireEvent.click(buttons[0]); + + await waitFor(() => { + expect(screen.getByRole('heading', { name: 'Capture Glitch' })).toBeInTheDocument(); + }); + }); + }); }); diff --git a/web-ui/src/app/proof/[req_id]/page.tsx b/web-ui/src/app/proof/[req_id]/page.tsx index 0fd289f3..9bf27732 100644 --- a/web-ui/src/app/proof/[req_id]/page.tsx +++ b/web-ui/src/app/proof/[req_id]/page.tsx @@ -260,7 +260,23 @@ export default function ProofDetailPage() {
{req.created_at && Created {new Date(req.created_at).toLocaleDateString()}} {req.source && Source: {req.source}} - {req.source_issue && Issue: {req.source_issue}} + {req.source_issue && ( + req.source_issue.startsWith('https://github.com') ? ( + + Source PR:{' '} + + {req.source_issue} + + + ) : ( + Issue: {req.source_issue} + ) + )} {req.created_by && By: {req.created_by}} {req.waiver?.expires && Waiver expires: {req.waiver.expires}}
diff --git a/web-ui/src/components/proof/CaptureGlitchModal.tsx b/web-ui/src/components/proof/CaptureGlitchModal.tsx index 6e4989d7..7769499a 100644 --- a/web-ui/src/components/proof/CaptureGlitchModal.tsx +++ b/web-ui/src/components/proof/CaptureGlitchModal.tsx @@ -40,9 +40,26 @@ export interface CaptureGlitchModalProps { workspacePath: string; onClose: () => void; onSuccess: (req: ProofRequirement) => void; + /** Pre-populate from a PR: PR number */ + prNumber?: number; + /** Pre-populate from a PR: PR title */ + prTitle?: string; + /** Pre-populate from a PR: GitHub PR URL (stored as source_issue) */ + prUrl?: string; + /** Pre-populate scope with changed files (newline-joined) */ + initialScope?: string; } -export function CaptureGlitchModal({ open, workspacePath, onClose, onSuccess }: CaptureGlitchModalProps) { +export function CaptureGlitchModal({ + open, + workspacePath, + onClose, + onSuccess, + prNumber, + prTitle, + prUrl, + initialScope, +}: CaptureGlitchModalProps) { const [description, setDescription] = useState(''); const [source, setSource] = useState('production'); const [scopeText, setScopeText] = useState(''); @@ -51,18 +68,20 @@ export function CaptureGlitchModal({ open, workspacePath, onClose, onSuccess }: const [submitting, setSubmitting] = useState(false); const [error, setError] = useState(null); - // Reset all state when the modal opens + // Reset all state when the modal opens, pre-filling from PR props if provided useEffect(() => { if (open) { - setDescription(''); + setDescription( + prNumber ? `Reported from PR #${prNumber}: ${prTitle ?? ''}` : '' + ); setSource('production'); - setScopeText(''); + setScopeText(initialScope ?? ''); setSelectedGates(new Set()); setSeverity('high'); setSubmitting(false); setError(null); } - }, [open]); + }, [open, prNumber, prTitle, initialScope]); function toggleGate(gate: string) { setSelectedGates((prev) => { @@ -110,6 +129,7 @@ export function CaptureGlitchModal({ open, workspacePath, onClose, onSuccess }: severity, source, created_by: 'human', + ...(prUrl ? { source_issue: prUrl } : {}), }; try { diff --git a/web-ui/src/components/review/PRHistoryPanel.tsx b/web-ui/src/components/review/PRHistoryPanel.tsx index 05bb5609..a3a4046a 100644 --- a/web-ui/src/components/review/PRHistoryPanel.tsx +++ b/web-ui/src/components/review/PRHistoryPanel.tsx @@ -8,13 +8,17 @@ import { ArrowUpRight01Icon, CheckmarkCircle01Icon, Cancel01Icon, + Alert02Icon, } from '@hugeicons/react'; import { prApi } from '@/lib/api'; import { Badge } from '@/components/ui/badge'; +import { Button } from '@/components/ui/button'; import { Card } from '@/components/ui/card'; +import { CaptureGlitchModal } from '@/components/proof'; import type { PRHistoryResponse, PRHistoryItem, + ProofRequirement, ProofSnapshot, GateBreakdownItem, } from '@/types'; @@ -42,6 +46,11 @@ export interface PRHistoryPanelProps { export function PRHistoryPanel({ workspacePath }: PRHistoryPanelProps) { const [expandedPR, setExpandedPR] = useState(null); + const [loadingFiles, setLoadingFiles] = useState(null); + const [glitchTarget, setGlitchTarget] = useState<{ + pr: PRHistoryItem; + files: string[]; + } | null>(null); const swrKey = workspacePath ? `/api/v2/pr/history?workspace_path=${encodeURIComponent(workspacePath)}` @@ -56,6 +65,18 @@ export function PRHistoryPanel({ workspacePath }: PRHistoryPanelProps) { setExpandedPR((prev) => (prev === prNumber ? null : prNumber)); }; + const handleReportGlitch = async (pr: PRHistoryItem) => { + setLoadingFiles(pr.number); + try { + const files = await prApi.getFiles(workspacePath, pr.number); + setGlitchTarget({ pr, files }); + } catch { + setGlitchTarget({ pr, files: [] }); + } finally { + setLoadingFiles(null); + } + }; + return (

PR History

@@ -120,6 +141,20 @@ export function PRHistoryPanel({ workspacePath }: PRHistoryPanelProps) { )} + )} + {glitchTarget && ( + setGlitchTarget(null)} + onSuccess={() => setGlitchTarget(null)} + /> + )} ); } diff --git a/web-ui/src/lib/api.ts b/web-ui/src/lib/api.ts index 1a417ac8..ab1d0992 100644 --- a/web-ui/src/lib/api.ts +++ b/web-ui/src/lib/api.ts @@ -53,6 +53,7 @@ import type { ProofRunSummary, ProofRunDetail, PRHistoryResponse, + PRFilesResponse, Session, SessionState, SessionListResponse, @@ -725,6 +726,13 @@ export const prApi = { }); return response.data; }, + + getFiles: async (workspacePath: string, prNumber: number): Promise => { + const response = await api.get(`/api/v2/pr/${prNumber}/files`, { + params: { workspace_path: workspacePath }, + }); + return response.data.files; + }, }; // Sessions API methods diff --git a/web-ui/src/types/index.ts b/web-ui/src/types/index.ts index 9f20ebe0..c06c7ddb 100644 --- a/web-ui/src/types/index.ts +++ b/web-ui/src/types/index.ts @@ -383,6 +383,11 @@ export interface CaptureGlitchRequest { severity: ProofSeverity; source: 'production' | 'qa' | 'dogfooding' | 'monitoring' | 'user_report'; created_by: string; + source_issue?: string; +} + +export interface PRFilesResponse { + files: string[]; } // Proof run types (mirrors proof_v2.py RunProofResponse + RunStatusResponse) From 2d24529d1ba6dc70c19c69e3b1a0c4c6a0aeeb1e Mon Sep 17 00:00:00 2001 From: Test User Date: Wed, 15 Apr 2026 09:21:13 -0700 Subject: [PATCH 2/3] fix: address claude-review feedback on PR #586 - Add per_page=100 to get_pr_files() to avoid silent truncation on PRs with 30+ changed files (GitHub API default is 30) - Show fallback note in scope when file fetch fails so user knows to enter scope manually (was silently empty) - Simplify redundant open={!!glitchTarget} to open (always true inside the conditional render block) - Remove unused ProofRequirement import from PRHistoryPanel --- codeframe/git/github_integration.py | 2 +- tests/unit/test_github_integration.py | 1 + web-ui/src/components/review/PRHistoryPanel.tsx | 6 +++--- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/codeframe/git/github_integration.py b/codeframe/git/github_integration.py index 280cedc5..6c210542 100644 --- a/codeframe/git/github_integration.py +++ b/codeframe/git/github_integration.py @@ -390,7 +390,7 @@ async def get_pr_files(self, pr_number: int) -> List[str]: Raises: GitHubAPIError: If API error occurs """ - endpoint = f"/repos/{self.owner}/{self.repo_name}/pulls/{pr_number}/files" + endpoint = f"/repos/{self.owner}/{self.repo_name}/pulls/{pr_number}/files?per_page=100" data = await self._make_request(method="GET", endpoint=endpoint) return [f["filename"] for f in data] diff --git a/tests/unit/test_github_integration.py b/tests/unit/test_github_integration.py index 55cc6238..405fa927 100644 --- a/tests/unit/test_github_integration.py +++ b/tests/unit/test_github_integration.py @@ -326,6 +326,7 @@ async def test_returns_list_of_filenames(self, github): call_kwargs = mock_request.call_args.kwargs assert call_kwargs["method"] == "GET" assert "/pulls/42/files" in call_kwargs["endpoint"] + assert "per_page=100" in call_kwargs["endpoint"] @pytest.mark.asyncio async def test_returns_empty_list_for_no_files(self, github): diff --git a/web-ui/src/components/review/PRHistoryPanel.tsx b/web-ui/src/components/review/PRHistoryPanel.tsx index a3a4046a..41aefcf0 100644 --- a/web-ui/src/components/review/PRHistoryPanel.tsx +++ b/web-ui/src/components/review/PRHistoryPanel.tsx @@ -18,7 +18,6 @@ import { CaptureGlitchModal } from '@/components/proof'; import type { PRHistoryResponse, PRHistoryItem, - ProofRequirement, ProofSnapshot, GateBreakdownItem, } from '@/types'; @@ -71,7 +70,8 @@ export function PRHistoryPanel({ workspacePath }: PRHistoryPanelProps) { const files = await prApi.getFiles(workspacePath, pr.number); setGlitchTarget({ pr, files }); } catch { - setGlitchTarget({ pr, files: [] }); + // Open the modal with a note so the user knows scope couldn't be loaded + setGlitchTarget({ pr, files: ['# Could not load changed files — enter scope manually'] }); } finally { setLoadingFiles(null); } @@ -203,7 +203,7 @@ export function PRHistoryPanel({ workspacePath }: PRHistoryPanelProps) { )} {glitchTarget && ( Date: Wed, 15 Apr 2026 09:24:37 -0700 Subject: [PATCH 3/3] fix: address CodeRabbit feedback on PR #586 - Paginate get_pr_files() to handle PRs with 100+ changed files (loop with per_page=100 until all pages exhausted) - Add @pytest.mark.v2 to TestGetPrFiles so tests run in enforced v2 profile - Use URL() constructor for strict host validation on source_issue links (rejects lookalike hosts like github.com.evil.tld) - Disable all Report Glitch buttons during any file fetch to prevent race conditions from concurrent clicks --- codeframe/git/github_integration.py | 20 ++++++++++++++++--- tests/unit/test_github_integration.py | 1 + web-ui/src/app/proof/[req_id]/page.tsx | 13 ++++++++---- .../src/components/review/PRHistoryPanel.tsx | 2 +- 4 files changed, 28 insertions(+), 8 deletions(-) diff --git a/codeframe/git/github_integration.py b/codeframe/git/github_integration.py index 6c210542..4b85d35e 100644 --- a/codeframe/git/github_integration.py +++ b/codeframe/git/github_integration.py @@ -381,6 +381,8 @@ async def close_pull_request(self, pr_number: int) -> bool: async def get_pr_files(self, pr_number: int) -> List[str]: """Get the list of files changed in a pull request. + Paginates through all pages (100 per page) to ensure completeness. + Args: pr_number: PR number @@ -390,9 +392,21 @@ async def get_pr_files(self, pr_number: int) -> List[str]: Raises: GitHubAPIError: If API error occurs """ - endpoint = f"/repos/{self.owner}/{self.repo_name}/pulls/{pr_number}/files?per_page=100" - data = await self._make_request(method="GET", endpoint=endpoint) - return [f["filename"] for f in data] + files: List[str] = [] + page = 1 + while True: + endpoint = ( + f"/repos/{self.owner}/{self.repo_name}/pulls/{pr_number}/files" + f"?per_page=100&page={page}" + ) + data = await self._make_request(method="GET", endpoint=endpoint) + if not isinstance(data, list) or not data: + break + files.extend(f["filename"] for f in data) + if len(data) < 100: + break + page += 1 + return files async def get_pr_ci_checks( self, diff --git a/tests/unit/test_github_integration.py b/tests/unit/test_github_integration.py index 405fa927..7d9a802b 100644 --- a/tests/unit/test_github_integration.py +++ b/tests/unit/test_github_integration.py @@ -299,6 +299,7 @@ async def test_rate_limit_error(self, github): assert exc_info.value.status_code == 403 +@pytest.mark.v2 class TestGetPrFiles: """Tests for GitHubIntegration.get_pr_files.""" diff --git a/web-ui/src/app/proof/[req_id]/page.tsx b/web-ui/src/app/proof/[req_id]/page.tsx index 9bf27732..cbb52d76 100644 --- a/web-ui/src/app/proof/[req_id]/page.tsx +++ b/web-ui/src/app/proof/[req_id]/page.tsx @@ -260,8 +260,13 @@ export default function ProofDetailPage() { diff --git a/web-ui/src/components/review/PRHistoryPanel.tsx b/web-ui/src/components/review/PRHistoryPanel.tsx index 41aefcf0..1e1e4dc9 100644 --- a/web-ui/src/components/review/PRHistoryPanel.tsx +++ b/web-ui/src/components/review/PRHistoryPanel.tsx @@ -145,7 +145,7 @@ export function PRHistoryPanel({ workspacePath }: PRHistoryPanelProps) { variant="ghost" size="sm" className="shrink-0 gap-1.5 text-xs" - disabled={loadingFiles === pr.number} + disabled={loadingFiles !== null} onClick={() => handleReportGlitch(pr)} > {loadingFiles === pr.number ? (