Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
28 changes: 22 additions & 6 deletions src/github/backfill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3276,6 +3276,19 @@ export function isStatusRollupGraphQlEnabled(env: { GITHUB_STATUS_ROLLUP_GRAPHQL
return /^(1|true|yes|on)$/i.test(env.GITHUB_STATUS_ROLLUP_GRAPHQL ?? "");
}

// Mirrors parseRepoFullName in labels.ts / assignees.ts (#8311): backfill keeps its own local copy rather than
// importing a shared one. Returns null for malformed input so each GraphQL read helper preserves its existing
// fail-soft return contract (null / undefined / []).
function parseBackfillRepoFullName(repoFullName: string): { owner: string; name: string } | null {
const parts = repoFullName.split("/");
const owner = parts[0];
const name = parts[1];
if (parts.length !== 2 || !owner || !name || /\s/.test(repoFullName)) {
return null;
}
return { owner, name };
}

/**
* GraphQL equivalent of {@link fetchLiveCiAggregate}: ONE bounded query returns the head commit's statusCheckRollup
* (check-runs AND classic statuses, unified) plus its check-suites — replacing the paginated /check-runs + /status
Expand All @@ -3295,8 +3308,9 @@ export async function fetchLiveCiAggregateViaGraphQl(
advisoryCheckRuns?: ReadonlyArray<{ name: string; appSlug: string }> | null,
): Promise<LiveCiAggregate | null> {
if (!headSha || !token) return null;
const [owner, name] = repoFullName.split("/");
if (!owner || !name) return null;
const parsed = parseBackfillRepoFullName(repoFullName);
if (!parsed) return null;
const { owner, name } = parsed;
const query = `query LoopOverLiveCiRollup { repository(owner: ${JSON.stringify(owner)}, name: ${JSON.stringify(name)}) { object(oid: ${JSON.stringify(headSha)}) { ... on Commit { statusCheckRollup { contexts(first: 100) { nodes { __typename ... on CheckRun { name conclusion status startedAt detailsUrl title summary checkSuite { databaseId app { slug } } } ... on StatusContext { context state description targetUrl } } pageInfo { hasNextPage } } } checkSuites(first: 100) { nodes { status app { slug } } pageInfo { hasNextPage } } } } } }`;
const result = await githubGraphQl<{
data?: {
Expand Down Expand Up @@ -4058,8 +4072,9 @@ export async function fetchLivePullRequestReviewDecision(
admissionKey?: GitHubRateLimitAdmissionKey,
): Promise<string | undefined> {
if (!token) return undefined;
const [owner, name] = repoFullName.split("/");
if (!owner || !name) return undefined;
const parsed = parseBackfillRepoFullName(repoFullName);
if (!parsed) return undefined;
const { owner, name } = parsed;
const query = `query { repository(owner: ${JSON.stringify(owner)}, name: ${JSON.stringify(name)}) { pullRequest(number: ${prNumber}) { reviewDecision } } }`;
const result = await githubGraphQl<{ data?: { repository?: { pullRequest?: { reviewDecision?: string | null } | null } | null }; errors?: unknown[] }>(
env,
Expand Down Expand Up @@ -4130,8 +4145,9 @@ export async function fetchLiveReviewThreadBlockers(
admissionKey?: GitHubRateLimitAdmissionKey,
): Promise<ReviewThreadBlocker[]> {
if (!token) return [];
const [owner, name] = repoFullName.split("/");
if (!owner || !name) return [];
const parsed = parseBackfillRepoFullName(repoFullName);
if (!parsed) return [];
const { owner, name } = parsed;
const threads: Array<GitHubReviewThreadNode | null> = [];
let cursor: string | null = null;
const seenCursors = new Set<string>();
Expand Down
16 changes: 16 additions & 0 deletions test/unit/backfill-2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1660,6 +1660,18 @@ describe("GitHub backfill", () => {
// (`liveReviewDecision ?? pr.reviewDecision`), so a PR approved at backfill time and later flipped to
// CHANGES_REQUESTED still read as APPROVED -> approvalsSatisfied -> merge. The sentinel is a real VALUE
// precisely so it survives that `??` and the stale stored decision can never be substituted.
describe("fetchLivePullRequestReviewDecision — repoFullName guard (#9317)", () => {
it("returns undefined for extra-segment and whitespace-padded slugs before any GraphQL call", async () => {
const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" });
const fetchSpy = vi.fn(async () => Response.json({ data: { repository: { pullRequest: { reviewDecision: "APPROVED" } } } }));
vi.stubGlobal("fetch", fetchSpy);
for (const repoFullName of ["owner/repo/extra", "owner/ repo", " owner/repo"]) {
expect(await fetchLivePullRequestReviewDecision(env, repoFullName, 7, "public-token")).toBeUndefined();
}
expect(fetchSpy).not.toHaveBeenCalled();
});
});

describe("fetchLivePullRequestReviewDecision — partial-response guard (#9052)", () => {
it("returns the unreadable sentinel on a 200-with-errors response, so the stale stored decision cannot win the ?? fallback", async () => {
const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" });
Expand Down Expand Up @@ -2528,6 +2540,10 @@ describe("GitHub backfill", () => {
expect(fetchSpy).not.toHaveBeenCalled();
await expect(fetchLiveReviewThreadBlockers(env, "malformed", 1, "public-token")).resolves.toEqual([]);
expect(fetchSpy).not.toHaveBeenCalled();
for (const repoFullName of ["owner/repo/extra", "owner/ repo", " owner/repo"]) {
await expect(fetchLiveReviewThreadBlockers(env, repoFullName, 1, "public-token")).resolves.toEqual([]);
}
expect(fetchSpy).not.toHaveBeenCalled();
await expect(fetchLiveReviewThreadBlockers(env, "JSONbored/gittensory", 1, "public-token")).resolves.toEqual([]);
expect(fetchSpy).toHaveBeenCalledTimes(1);
});
Expand Down
10 changes: 10 additions & 0 deletions test/unit/graphql-status-rollup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,16 @@ describe("fetchLiveCiAggregateViaGraphQl — verdicts", () => {
expect(await fetchLiveCiAggregateViaGraphQl(env, "no-slash", SHA, TOKEN)).toBeNull();
});

// #9317: segment-count + whitespace guard, matching pr-actions.ts/assignees.ts/labels.ts (#8311).
it("returns null for extra-segment and whitespace-padded repo slugs before any GraphQL call (#9317)", async () => {
const fetchSpy = vi.fn(async () => Response.json(graphqlBody({ runs: [{ name: "build", conclusion: "SUCCESS", status: "COMPLETED" }] })));
vi.stubGlobal("fetch", fetchSpy);
for (const repoFullName of ["owner/repo/extra", "owner/ repo", " owner/repo"]) {
expect(await fetchLiveCiAggregateViaGraphQl(env, repoFullName, SHA, TOKEN)).toBeNull();
}
expect(fetchSpy).not.toHaveBeenCalled();
});

it("returns null on a GraphQL error or an unexpected/absent commit (→ REST fallback)", async () => {
stubGraphql(graphqlBody(), { status: 500 });
expect(await fetchLiveCiAggregateViaGraphQl(env, REPO, SHA, TOKEN)).toBeNull();
Expand Down
Loading