Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
175 changes: 140 additions & 35 deletions apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
DEFAULT_FEATURE_THINKING,
} from "../../../shared/constants";
import type { AuthFailureInfo } from "../../../shared/types/terminal";
import { getGitHubConfig, githubFetch } from "./utils";
import { getGitHubConfig, githubFetch, normalizeRepoReference } from "./utils";
import { readSettingsFile } from "../../settings-utils";
import { getAugmentedEnv } from "../../env-utils";
import { getMemoryService, getDefaultDbPath } from "../../memory-service";
Expand All @@ -36,6 +36,102 @@
buildRunnerArgs,
} from "./utils/subprocess-runner";

/**
* GraphQL response type for PR list query
* Note: repository can be null if the repo doesn't exist or user lacks access
*/
interface GraphQLPRListResponse {
data: {
repository: {
pullRequests: {
pageInfo: {
hasNextPage: boolean;
endCursor: string | null;
};
nodes: Array<{
number: number;
title: string;
body: string | null;
state: string;
author: { login: string } | null;
headRefName: string;
baseRefName: string;
additions: number;
deletions: number;
changedFiles: number;
assignees: { nodes: Array<{ login: string }> };
createdAt: string;
updatedAt: string;
url: string;
}>;
};
} | null;
};
errors?: Array<{ message: string }>;
}

/**
* Make a GraphQL request to GitHub API
*/
async function githubGraphQL<T>(
token: string,
query: string,
variables: Record<string, unknown> = {}
): Promise<T> {
const response = await fetch("https://api.github.com/graphql", {
method: "POST",
headers: {
"Authorization": `Bearer ${token}`,
"Content-Type": "application/json",
"User-Agent": "Auto-Claude-UI",
},
Comment on lines +83 to +87

Check warning

Code scanning / CodeQL

File data in outbound network request Medium

Outbound network request depends on
file data
.
body: JSON.stringify({ query, variables }),

Check warning

Code scanning / CodeQL

File data in outbound network request Medium

Outbound network request depends on
file data
.
});

if (!response.ok) {
const errorBody = await response.text();
throw new Error(`GitHub GraphQL error: ${response.status} ${response.statusText} - ${errorBody}`);
}

const result = await response.json() as T & { errors?: Array<{ message: string }> };

// Check for GraphQL-level errors
if (result.errors && result.errors.length > 0) {
throw new Error(`GitHub GraphQL error: ${result.errors.map(e => e.message).join(", ")}`);
}

return result;
}

/**
* GraphQL query to fetch PRs with diff stats
*/
const LIST_PRS_QUERY = `
query($owner: String!, $repo: String!, $first: Int!, $after: String) {
repository(owner: $owner, name: $repo) {
pullRequests(states: OPEN, first: $first, after: $after, orderBy: {field: UPDATED_AT, direction: DESC}) {
pageInfo { hasNextPage endCursor }
nodes {
number
title
body
state
author { login }
headRefName
baseRefName
additions
deletions
changedFiles
assignees(first: 10) { nodes { login } }
createdAt
updatedAt
url
}
}
}
}
`;

/**
* Sanitize network data before writing to file
* Removes potentially dangerous characters and limits length
Expand Down Expand Up @@ -1231,11 +1327,11 @@
export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): void {
debugLog("Registering PR handlers");

// List open PRs with pagination support
// List open PRs - fetches up to 100 open PRs at once, client handles display pagination
ipcMain.handle(
IPC_CHANNELS.GITHUB_PR_LIST,
async (_, projectId: string, page: number = 1): Promise<PRData[]> => {
debugLog("listPRs handler called", { projectId, page });
async (_, projectId: string): Promise<PRData[]> => {
debugLog("listPRs handler called", { projectId });
const result = await withProjectOrNull(projectId, async (project) => {
const config = getGitHubConfig(project);
if (!config) {
Expand All @@ -1244,44 +1340,53 @@
}

try {
// Use pagination: per_page=100 (GitHub max), page=1,2,3...
const prs = (await githubFetch(
// Parse owner/repo from config
const normalizedRepo = normalizeRepoReference(config.repo);
const [owner, repo] = normalizedRepo.split("/");
if (!owner || !repo) {
debugLog("Invalid repo format", { repo: config.repo });
return [];
}

// Use GraphQL API to get PRs with diff stats (REST list endpoint doesn't include them)
// Fetches up to 100 open PRs (GitHub GraphQL max per request)
// Client-side pagination can be implemented if needed for repos with >100 open PRs
const response = await githubGraphQL<GraphQLPRListResponse>(
config.token,
`/repos/${config.repo}/pulls?state=open&per_page=100&page=${page}`
)) as Array<{
number: number;
title: string;
body?: string;
state: string;
user: { login: string };
head: { ref: string };
base: { ref: string };
additions: number;
deletions: number;
changed_files: number;
assignees?: Array<{ login: string }>;
created_at: string;
updated_at: string;
html_url: string;
}>;
LIST_PRS_QUERY,
{
owner,
repo,
first: 100, // GitHub GraphQL max is 100
after: null, // Start from beginning
}
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pagination broken: page parameter always ignored

High Severity

The page parameter accepted by the listPRs handler is completely ignored, causing all pagination requests to return the same first 100 PRs. The GraphQL after cursor is hardcoded to null regardless of the requested page, breaking "Load More" functionality. Repositories with more than 100 open PRs cannot display PRs beyond the first page, making them inaccessible through the UI.

Fix in Cursor Fix in Web


// Handle case where repository doesn't exist or user lacks access
if (!response.data.repository) {
debugLog("Repository not found or access denied", { owner, repo });
return [];
}

debugLog("Fetched PRs", { count: prs.length, page, samplePr: prs[0] });
const prs = response.data.repository.pullRequests.nodes;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Null repository causes crash without error handling

High Severity

Accessing response.data.repository.pullRequests.nodes without checking if repository is null causes a crash. GitHub's GraphQL API returns repository: null (without an error) when the repo doesn't exist, is private, or the token lacks access. This crashes the handler with "Cannot read property 'pullRequests' of null" instead of returning an empty array like the previous REST API implementation.

Fix in Cursor Fix in Web


debugLog("Fetched PRs via GraphQL", { count: prs.length, samplePr: prs[0] });
return prs.map((pr) => ({
number: pr.number,
title: pr.title,
body: pr.body ?? "",
state: pr.state,
author: { login: pr.user.login },
headRefName: pr.head.ref,
baseRefName: pr.base.ref,
additions: pr.additions ?? 0,
deletions: pr.deletions ?? 0,
changedFiles: pr.changed_files ?? 0,
assignees: pr.assignees?.map((a: { login: string }) => ({ login: a.login })) ?? [],
state: pr.state.toLowerCase(),
author: { login: pr.author?.login ?? "unknown" },
headRefName: pr.headRefName,
baseRefName: pr.baseRefName,
additions: pr.additions,
deletions: pr.deletions,
changedFiles: pr.changedFiles,
assignees: pr.assignees.nodes.map((a) => ({ login: a.login })),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Crash when assignee user account is deleted

Medium Severity

The assignees mapping crashes with "Cannot read property 'login' of null" when any assignee user account has been deleted or suspended. In GraphQL connections, the nodes array can contain null entries for deleted entities, but the code directly maps without filtering nulls: pr.assignees.nodes.map((a) => ({ login: a.login })). This breaks PR list loading whenever a PR has a deleted user as an assignee.

Fix in Cursor Fix in Web

files: [],
createdAt: pr.created_at,
updatedAt: pr.updated_at,
htmlUrl: pr.html_url,
createdAt: pr.createdAt,
updatedAt: pr.updatedAt,
htmlUrl: pr.url,
}));
} catch (error) {
debugLog("Failed to fetch PRs", {
Expand Down
9 changes: 5 additions & 4 deletions apps/frontend/src/preload/api/modules/github-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,8 @@ export interface GitHubAPI {
callback: (projectId: string, error: { error: string }) => void
) => IpcListenerCleanup;

// PR operations
listPRs: (projectId: string, page?: number) => Promise<PRData[]>;
// PR operations (fetches up to 100 open PRs at once - GitHub GraphQL limit)
listPRs: (projectId: string) => Promise<PRData[]>;
getPR: (projectId: string, prNumber: number) => Promise<PRData | null>;
runPRReview: (projectId: string, prNumber: number) => void;
cancelPRReview: (projectId: string, prNumber: number) => Promise<boolean>;
Expand Down Expand Up @@ -652,8 +652,9 @@ export const createGitHubAPI = (): GitHubAPI => ({
createIpcListener(IPC_CHANNELS.GITHUB_AUTOFIX_ANALYZE_PREVIEW_ERROR, callback),

// PR operations
listPRs: (projectId: string, page: number = 1): Promise<PRData[]> =>
invokeIpc(IPC_CHANNELS.GITHUB_PR_LIST, projectId, page),
// Fetches up to 100 open PRs at once (GitHub GraphQL limit)
listPRs: (projectId: string): Promise<PRData[]> =>
invokeIpc(IPC_CHANNELS.GITHUB_PR_LIST, projectId),

getPR: (projectId: string, prNumber: number): Promise<PRData | null> =>
invokeIpc(IPC_CHANNELS.GITHUB_PR_GET, projectId, prNumber),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps)
const {
prs,
isLoading,
isLoadingMore,
isLoadingPRDetails,
error,
selectedPRNumber,
Expand All @@ -79,7 +78,6 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps)
assignPR,
markReviewPosted,
refresh,
loadMore,
isConnected,
repoFullName,
getReviewStateForPR,
Expand Down Expand Up @@ -234,12 +232,10 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps)
prs={filteredPRs}
selectedPRNumber={selectedPRNumber}
isLoading={isLoading}
isLoadingMore={isLoadingMore}
hasMore={hasMore}
error={error}
getReviewStateForPR={getReviewStateForPR}
onSelectPR={selectPR}
onLoadMore={loadMore}
/>
</div>
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { useRef, useEffect, useCallback, useState } from 'react';
import { GitPullRequest, User, Clock, FileDiff, Loader2 } from 'lucide-react';
import { useState } from 'react';
import { GitPullRequest, User, Clock, FileDiff } from 'lucide-react';
import { ScrollArea } from '../../ui/scroll-area';
import { Badge } from '../../ui/badge';
import { cn } from '../../../lib/utils';
Expand Down Expand Up @@ -167,12 +167,10 @@ interface PRListProps {
prs: PRData[];
selectedPRNumber: number | null;
isLoading: boolean;
isLoadingMore: boolean;
hasMore: boolean;
hasMore: boolean; // True when 100 PRs returned (GitHub limit) - more may exist
error: string | null;
getReviewStateForPR: (prNumber: number) => PRReviewInfo | null;
onSelectPR: (prNumber: number) => void;
onLoadMore: () => void;
}

function formatDate(dateString: string): string {
Expand All @@ -199,42 +197,14 @@ export function PRList({
prs,
selectedPRNumber,
isLoading,
isLoadingMore,
hasMore,
error,
getReviewStateForPR,
onSelectPR,
onLoadMore
}: PRListProps) {
const { t } = useTranslation('common');
const loadMoreTriggerRef = useRef<HTMLDivElement>(null);
const [viewportElement, setViewportElement] = useState<HTMLDivElement | null>(null);

// Intersection Observer for infinite scroll
const handleIntersection = useCallback((entries: IntersectionObserverEntry[]) => {
const [entry] = entries;
if (entry.isIntersecting && hasMore && !isLoadingMore && !isLoading) {
onLoadMore();
}
}, [hasMore, isLoadingMore, isLoading, onLoadMore]);

useEffect(() => {
const trigger = loadMoreTriggerRef.current;
if (!trigger || !viewportElement) return;

const observer = new IntersectionObserver(handleIntersection, {
root: viewportElement,
rootMargin: '100px',
threshold: 0
});

observer.observe(trigger);

return () => {
observer.disconnect();
};
}, [handleIntersection, onLoadMore, viewportElement]);

if (isLoading && prs.length === 0) {
return (
<div className="flex-1 flex items-center justify-center">
Expand Down Expand Up @@ -338,23 +308,14 @@ export function PRList({
);
})}

{/* Load more trigger / Loading indicator */}
<div ref={loadMoreTriggerRef} className="py-4 flex justify-center">
{isLoadingMore ? (
<div className="flex items-center gap-2 text-muted-foreground">
<Loader2 className="h-4 w-4 animate-spin" />
<span className="text-sm">{t('prReview.loadingMore')}</span>
</div>
) : hasMore ? (
<span className="text-xs text-muted-foreground opacity-50">
{t('prReview.scrollForMore')}
</span>
) : prs.length > 0 ? (
{/* Status indicator */}
{prs.length > 0 && (
<div className="py-4 flex justify-center">
<span className="text-xs text-muted-foreground opacity-50">
{t('prReview.allPRsLoaded')}
{hasMore ? t('prReview.maxPRsShown') : t('prReview.allPRsLoaded')}
</span>
) : null}
</div>
</div>
)}
</div>
</ScrollArea>
);
Expand Down
Loading
Loading