diff --git a/src/integrations/project-tracker-adapter.ts b/src/integrations/project-tracker-adapter.ts index e928e5a85f..80e76ec0f2 100644 --- a/src/integrations/project-tracker-adapter.ts +++ b/src/integrations/project-tracker-adapter.ts @@ -1,4 +1,4 @@ -import { createInstallationToken } from "../github/app"; +import { createInstallationToken, withInstallationTokenRetry } from "../github/app"; import { githubRateLimitAdmissionKeyForInstallation, makeInstallationOctokit } from "../github/client"; import type { AgentActionMode } from "../settings/agent-execution"; import { createIssueComment } from "../github/pr-actions"; @@ -78,22 +78,24 @@ export class GitHubMilestonesAdapter implements ProjectTrackerAdapter { async listOpenMilestones(ctx: ProjectTrackerContext): Promise { const { owner, repo } = parseRepoFullName(ctx.repoFullName); - const token = await createInstallationToken(ctx.env, ctx.installationId); - const octokit = makeInstallationOctokit(ctx.env, token, "live", githubRateLimitAdmissionKeyForInstallation(ctx.installationId)); - const milestones: GitHubMilestone[] = []; - for (let page = 1; page <= GITHUB_LIST_PAGE_LIMIT; page += 1) { - const response = await octokit.request("GET /repos/{owner}/{repo}/milestones", { - owner, - repo, - state: "open", - per_page: 100, - page, - }); - const batch = response.data as GitHubMilestone[]; - milestones.push(...batch); - if (batch.length < 100) break; - } - return milestones.map((milestone) => ({ id: String(milestone.number), title: milestone.title })); + // #9316: same withInstallationTokenRetry self-heal every src/github write helper uses (#6191). + return withInstallationTokenRetry(ctx.env, ctx.installationId, async (token) => { + const octokit = makeInstallationOctokit(ctx.env, token, "live", githubRateLimitAdmissionKeyForInstallation(ctx.installationId)); + const milestones: GitHubMilestone[] = []; + for (let page = 1; page <= GITHUB_LIST_PAGE_LIMIT; page += 1) { + const response = await octokit.request("GET /repos/{owner}/{repo}/milestones", { + owner, + repo, + state: "open", + per_page: 100, + page, + }); + const batch = response.data as GitHubMilestone[]; + milestones.push(...batch); + if (batch.length < 100) break; + } + return milestones.map((milestone) => ({ id: String(milestone.number), title: milestone.title })); + }); } // Inert here -- see GitHubProjectsAdapter. @@ -105,15 +107,17 @@ export class GitHubMilestonesAdapter implements ProjectTrackerAdapter { const milestoneNumber = parsePositiveIntegerId(milestoneId); if (milestoneNumber === null) return { attached: false }; const { owner, repo } = parseRepoFullName(ctx.repoFullName); - const token = await createInstallationToken(ctx.env, ctx.installationId); - const octokit = makeInstallationOctokit(ctx.env, token, mode, githubRateLimitAdmissionKeyForInstallation(ctx.installationId)); - await octokit.request("PATCH /repos/{owner}/{repo}/issues/{issue_number}", { - owner, - repo, - issue_number: pullNumber, - milestone: milestoneNumber, + // #9316: same withInstallationTokenRetry self-heal every src/github write helper uses (#6191). + return withInstallationTokenRetry(ctx.env, ctx.installationId, async (token) => { + const octokit = makeInstallationOctokit(ctx.env, token, mode, githubRateLimitAdmissionKeyForInstallation(ctx.installationId)); + await octokit.request("PATCH /repos/{owner}/{repo}/issues/{issue_number}", { + owner, + repo, + issue_number: pullNumber, + milestone: milestoneNumber, + }); + return { attached: true }; }); - return { attached: true }; } } @@ -199,13 +203,14 @@ export async function resolveProjectV2Fields(ctx: ProjectTrackerContext, project export class GitHubProjectsAdapter implements ProjectTrackerAdapter { async listOpenProjects(ctx: ProjectTrackerContext): Promise { const { owner } = parseRepoFullName(ctx.repoFullName); - const token = await createInstallationToken(ctx.env, ctx.installationId); - const octokit = makeInstallationOctokit(ctx.env, token, "live", githubRateLimitAdmissionKeyForInstallation(ctx.installationId)); - const projects: ProjectV2Node[] = []; - let after: string | null = null; - for (let page = 1; page <= GITHUB_LIST_PAGE_LIMIT; page += 1) { - const response: ListOpenProjectsGraphQlResponse = await octokit.graphql( - `query($login: String!, $after: String) { + // #9316: same withInstallationTokenRetry self-heal every src/github write helper uses (#6191). + return withInstallationTokenRetry(ctx.env, ctx.installationId, async (token) => { + const octokit = makeInstallationOctokit(ctx.env, token, "live", githubRateLimitAdmissionKeyForInstallation(ctx.installationId)); + const projects: ProjectV2Node[] = []; + let after: string | null = null; + for (let page = 1; page <= GITHUB_LIST_PAGE_LIMIT; page += 1) { + const response: ListOpenProjectsGraphQlResponse = await octokit.graphql( + `query($login: String!, $after: String) { repositoryOwner(login: $login) { __typename ... on Organization { @@ -216,15 +221,16 @@ export class GitHubProjectsAdapter implements ProjectTrackerAdapter { } } }`, - { login: owner, after }, - ); - const projectsV2 = response.repositoryOwner?.projectsV2; - if (!projectsV2) break; // owner is a User (or has zero projects) -- see the module-level comment above. - projects.push(...projectsV2.nodes.filter((project) => !project.closed && project.public)); - if (!projectsV2.pageInfo.hasNextPage) break; - after = projectsV2.pageInfo.endCursor; - } - return projects.map((project) => ({ id: project.id, title: project.title })); + { login: owner, after }, + ); + const projectsV2 = response.repositoryOwner?.projectsV2; + if (!projectsV2) break; // owner is a User (or has zero projects) -- see the module-level comment above. + projects.push(...projectsV2.nodes.filter((project) => !project.closed && project.public)); + if (!projectsV2.pageInfo.hasNextPage) break; + after = projectsV2.pageInfo.endCursor; + } + return projects.map((project) => ({ id: project.id, title: project.title })); + }); } // Inert here -- see GitHubMilestonesAdapter. @@ -235,19 +241,21 @@ export class GitHubProjectsAdapter implements ProjectTrackerAdapter { async attachToProject(ctx: ProjectTrackerContext, pullNumber: number, projectId: string, mode: AgentActionMode = "live"): Promise { if (typeof projectId !== "string" || projectId.trim().length === 0) return { attached: false }; const { owner, repo } = parseRepoFullName(ctx.repoFullName); - const token = await createInstallationToken(ctx.env, ctx.installationId); - const octokit = makeInstallationOctokit(ctx.env, token, mode, githubRateLimitAdmissionKeyForInstallation(ctx.installationId)); - const pr = await octokit.request("GET /repos/{owner}/{repo}/pulls/{pull_number}", { owner, repo, pull_number: pullNumber }); - const contentId = (pr.data as PullRequestNodeIdResponse).node_id; - const response = await octokit.graphql( - `mutation($projectId: ID!, $contentId: ID!) { + // #9316: same withInstallationTokenRetry self-heal every src/github write helper uses (#6191). + return withInstallationTokenRetry(ctx.env, ctx.installationId, async (token) => { + const octokit = makeInstallationOctokit(ctx.env, token, mode, githubRateLimitAdmissionKeyForInstallation(ctx.installationId)); + const pr = await octokit.request("GET /repos/{owner}/{repo}/pulls/{pull_number}", { owner, repo, pull_number: pullNumber }); + const contentId = (pr.data as PullRequestNodeIdResponse).node_id; + const response = await octokit.graphql( + `mutation($projectId: ID!, $contentId: ID!) { addProjectV2ItemById(input: {projectId: $projectId, contentId: $contentId}) { item { id } } }`, - { projectId, contentId }, - ); - return { attached: response.addProjectV2ItemById?.item != null }; + { projectId, contentId }, + ); + return { attached: response.addProjectV2ItemById?.item != null }; + }); } // Inert here -- see GitHubMilestonesAdapter. diff --git a/test/unit/project-tracker-adapter.test.ts b/test/unit/project-tracker-adapter.test.ts index 4b2e681e61..679844cec3 100644 --- a/test/unit/project-tracker-adapter.test.ts +++ b/test/unit/project-tracker-adapter.test.ts @@ -12,6 +12,7 @@ import { resolveProjectV2Fields, type ProjectTrackerRef, } from "../../src/integrations/project-tracker-adapter"; +import { clearInstallationTokenCacheForTest } from "../../src/github/app"; import { createTestEnv } from "../helpers/d1"; function generateRsaPrivateKeyPem(): string { @@ -78,6 +79,57 @@ describe("GitHubMilestonesAdapter (#3183)", () => { } }); + it("listOpenMilestones: evicts a stale installation token and retries once on 401 (#9316)", async () => { + clearInstallationTokenCacheForTest(); + let tokenMints = 0; + let milestoneAttempts = 0; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) { + tokenMints += 1; + return Response.json({ token: `token-${tokenMints}` }); + } + if (url.includes("/milestones")) { + milestoneAttempts += 1; + if (milestoneAttempts === 1) return Response.json({ message: "Bad credentials" }, { status: 401 }); + return Response.json([{ number: 14, title: "Self-host reliability roadmap" }]); + } + return new Response("unexpected", { status: 500 }); + }); + const adapter = new GitHubMilestonesAdapter(); + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }); + await expect(adapter.listOpenMilestones({ env, installationId: 998877, repoFullName: "JSONbored/gittensory" })).resolves.toEqual([ + { id: "14", title: "Self-host reliability roadmap" }, + ]); + expect(milestoneAttempts).toBe(2); + expect(tokenMints).toBe(2); + }); + + it("attachToMilestone: evicts a stale installation token and retries once on 401 (#9316)", async () => { + clearInstallationTokenCacheForTest(); + let tokenMints = 0; + let patchAttempts = 0; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url.includes("/access_tokens")) { + tokenMints += 1; + return Response.json({ token: `token-${tokenMints}` }); + } + if (url.includes("/issues/4") && method === "PATCH") { + patchAttempts += 1; + if (patchAttempts === 1) return Response.json({ message: "Bad credentials" }, { status: 401 }); + return Response.json({ number: 4, milestone: { number: 14 } }); + } + return new Response("unexpected", { status: 500 }); + }); + const adapter = new GitHubMilestonesAdapter(); + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }); + await expect(adapter.attachToMilestone({ env, installationId: 998877, repoFullName: "JSONbored/gittensory" }, 4, "14")).resolves.toEqual({ attached: true }); + expect(patchAttempts).toBe(2); + expect(tokenMints).toBe(2); + }); + it("listOpenMilestones fetches and maps open milestones from the REST API", async () => { vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { const url = input.toString(); @@ -338,6 +390,72 @@ describe("GitHubProjectsAdapter (#3184)", () => { await expect(adapter.listOpenProjects({ env, installationId: 123, repoFullName: padded })).rejects.toThrow(/Invalid repository full name/); } }); + + it("listOpenProjects: evicts a stale installation token and retries once on 401 (#9316)", async () => { + clearInstallationTokenCacheForTest(); + let tokenMints = 0; + let graphqlAttempts = 0; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) { + tokenMints += 1; + return Response.json({ token: `token-${tokenMints}` }); + } + if (url.endsWith("/graphql")) { + graphqlAttempts += 1; + if (graphqlAttempts === 1) return Response.json({ message: "Bad credentials" }, { status: 401 }); + return Response.json({ + data: { + repositoryOwner: { + __typename: "Organization", + projectsV2: { + nodes: [{ id: "PVT_1", title: "Self-host reliability", closed: false, public: true }], + pageInfo: { hasNextPage: false, endCursor: null }, + }, + }, + }, + }); + } + return new Response("unexpected", { status: 500 }); + }); + const adapter = new GitHubProjectsAdapter(); + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }); + await expect(adapter.listOpenProjects({ env, installationId: 998877, repoFullName: "some-org/gittensory" })).resolves.toEqual([ + { id: "PVT_1", title: "Self-host reliability" }, + ]); + expect(graphqlAttempts).toBe(2); + expect(tokenMints).toBe(2); + }); + + it("attachToProject: evicts a stale installation token and retries once on 401 (#9316)", async () => { + clearInstallationTokenCacheForTest(); + let tokenMints = 0; + let pullGetAttempts = 0; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url.includes("/access_tokens")) { + tokenMints += 1; + return Response.json({ token: `token-${tokenMints}` }); + } + if (url.includes("/pulls/4") && method === "GET") { + pullGetAttempts += 1; + if (pullGetAttempts === 1) return Response.json({ message: "Bad credentials" }, { status: 401 }); + return Response.json({ number: 4, node_id: "PR_kwABC" }); + } + if (url.endsWith("/graphql")) { + return Response.json({ data: { addProjectV2ItemById: { item: { id: "PVTI_1" } } } }); + } + return new Response("unexpected", { status: 500 }); + }); + const adapter = new GitHubProjectsAdapter(); + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }); + await expect(adapter.attachToProject({ env, installationId: 998877, repoFullName: "some-org/gittensory" }, 4, "PVT_1")).resolves.toEqual({ + attached: true, + }); + expect(pullGetAttempts).toBe(2); + expect(tokenMints).toBe(2); + }); }); describe("resolveProjectV2Fields (#3184)", () => {