Skip to content
162 changes: 130 additions & 32 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,101 @@
buildRunnerArgs,
} from "./utils/subprocess-runner";

/**
* GraphQL response type for PR list query
*/
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;
}>;
};
};
};
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 @@ -1244,44 +1339,47 @@
}

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)
// GraphQL pagination uses cursor-based "after", simulate page-based by fetching first N*page
// For simplicity, we fetch 100 items for page 1, and handle pagination client-side
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The comment states, "GraphQL pagination uses cursor-based 'after', simulate page-based by fetching first N*page," but the current implementation only fetches the first 100 items (first: 100, after: null). If the intention is to truly simulate page-based pagination beyond the first 100 items, the after cursor should be utilized to fetch subsequent pages. If only the first 100 PRs are intended to be fetched, the comment should be updated to reflect this more accurately.

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, // First page
}

This comment was marked as outdated.

);
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


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", { count: prs.length, page, samplePr: prs[0] });
debugLog("Fetched PRs via GraphQL", { count: prs.length, page, 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
Loading