From c287a25a878f040c523b9cd8729085ec7f2a1f90 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 1 Jul 2026 12:15:05 -0700 Subject: [PATCH 1/2] fix(github): evict a rejected installation token on PR-mutating calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mergePullRequest, closePullRequest, createPullRequestReview, updatePullRequestBranch, createIssueComment, and getLastCloserLogin all called createInstallationToken directly instead of routing through withInstallationTokenRetry — the only caller of expireCachedInstallationToken. Only the check-run creator and the comment poster in comments.ts got evict-and-retry. An installation suspended (or an App private key rotated) while a cached token was still within its ~1h TTL meant every merge/close/review/update-branch call kept reusing the same now-invalid token and re-failing with 401 until the cache entry expired on its own — no self-heal, unlike the paths that already had the retry wiring. Route all six through withInstallationTokenRetry, mirroring the existing comments.ts pattern: on a 401, evict the cached token and retry once with a freshly-minted one. createPullRequestReviewComments and createInstallationToken's other caller are unaffected — not in scope here. --- src/github/pr-actions.ts | 166 ++++++++++++++-------------- test/unit/github-pr-actions.test.ts | 71 ++++++++++++ 2 files changed, 157 insertions(+), 80 deletions(-) diff --git a/src/github/pr-actions.ts b/src/github/pr-actions.ts index f465a85714..d205a45dc7 100644 --- a/src/github/pr-actions.ts +++ b/src/github/pr-actions.ts @@ -1,4 +1,4 @@ -import { createInstallationToken } from "./app"; +import { createInstallationToken, withInstallationTokenRetry } from "./app"; import { githubRateLimitAdmissionKeyForInstallation, makeInstallationOctokit } from "./client"; import type { AgentActionMode } from "../settings/agent-execution"; import type { AutoMergeMethod } from "../types"; @@ -29,16 +29,17 @@ export async function createPullRequestReview( body: string, ): Promise<{ id: number }> { const { owner, repo } = splitRepo(repoFullName); - const token = await createInstallationToken(env, installationId); - const octokit = makeInstallationOctokit(env, token, "live", githubRateLimitAdmissionKeyForInstallation(installationId)); - const response = await octokit.request("POST /repos/{owner}/{repo}/pulls/{pull_number}/reviews", { - owner, - repo, - pull_number: pullNumber, - event, - body, + return withInstallationTokenRetry(env, installationId, async (token) => { + const octokit = makeInstallationOctokit(env, token, "live", githubRateLimitAdmissionKeyForInstallation(installationId)); + const response = await octokit.request("POST /repos/{owner}/{repo}/pulls/{pull_number}/reviews", { + owner, + repo, + pull_number: pullNumber, + event, + body, + }); + return { id: (response.data as { id: number }).id }; }); - return { id: (response.data as { id: number }).id }; } /** Post a quiet, NON-BLOCKING review (`event: "COMMENT"`) carrying line-anchored inline comments — the @@ -79,17 +80,18 @@ export async function mergePullRequest( options: { mergeMethod: AutoMergeMethod; sha?: string | undefined }, ): Promise<{ merged: boolean; sha: string | null }> { const { owner, repo } = splitRepo(repoFullName); - const token = await createInstallationToken(env, installationId); - const octokit = makeInstallationOctokit(env, token, "live", githubRateLimitAdmissionKeyForInstallation(installationId)); - const response = await octokit.request("PUT /repos/{owner}/{repo}/pulls/{pull_number}/merge", { - owner, - repo, - pull_number: pullNumber, - merge_method: options.mergeMethod, - ...(options.sha ? { sha: options.sha } : {}), + return withInstallationTokenRetry(env, installationId, async (token) => { + const octokit = makeInstallationOctokit(env, token, "live", githubRateLimitAdmissionKeyForInstallation(installationId)); + const response = await octokit.request("PUT /repos/{owner}/{repo}/pulls/{pull_number}/merge", { + owner, + repo, + pull_number: pullNumber, + merge_method: options.mergeMethod, + ...(options.sha ? { sha: options.sha } : {}), + }); + const data = response.data as { merged?: boolean; sha?: string }; + return { merged: data.merged ?? true, sha: data.sha ?? null }; }); - const data = response.data as { merged?: boolean; sha?: string }; - return { merged: data.merged ?? true, sha: data.sha ?? null }; } /** Rebase a PR onto its base via GitHub's update-branch (merges the current base into the PR head). Keeps a @@ -105,42 +107,45 @@ export async function updatePullRequestBranch( expectedHeadSha?: string | undefined, ): Promise { const { owner, repo } = splitRepo(repoFullName); - const token = await createInstallationToken(env, installationId); - const octokit = makeInstallationOctokit(env, token, "live", githubRateLimitAdmissionKeyForInstallation(installationId)); - await octokit.request("PUT /repos/{owner}/{repo}/pulls/{pull_number}/update-branch", { - owner, - repo, - pull_number: pullNumber, - ...(expectedHeadSha ? { expected_head_sha: expectedHeadSha } : {}), + await withInstallationTokenRetry(env, installationId, async (token) => { + const octokit = makeInstallationOctokit(env, token, "live", githubRateLimitAdmissionKeyForInstallation(installationId)); + await octokit.request("PUT /repos/{owner}/{repo}/pulls/{pull_number}/update-branch", { + owner, + repo, + pull_number: pullNumber, + ...(expectedHeadSha ? { expected_head_sha: expectedHeadSha } : {}), + }); }); } /** Post a plain issue/PR comment (used for the templated close message before closing). */ export async function createIssueComment(env: Env, installationId: number, repoFullName: string, issueNumber: number, body: string): Promise<{ id: number }> { const { owner, repo } = splitRepo(repoFullName); - const token = await createInstallationToken(env, installationId); - const octokit = makeInstallationOctokit(env, token, "live", githubRateLimitAdmissionKeyForInstallation(installationId)); - const response = await octokit.request("POST /repos/{owner}/{repo}/issues/{issue_number}/comments", { - owner, - repo, - issue_number: issueNumber, - body, + return withInstallationTokenRetry(env, installationId, async (token) => { + const octokit = makeInstallationOctokit(env, token, "live", githubRateLimitAdmissionKeyForInstallation(installationId)); + const response = await octokit.request("POST /repos/{owner}/{repo}/issues/{issue_number}/comments", { + owner, + repo, + issue_number: issueNumber, + body, + }); + return { id: (response.data as { id: number }).id }; }); - return { id: (response.data as { id: number }).id }; } /** Close a pull request (sets state=closed) without merging. */ export async function closePullRequest(env: Env, installationId: number, repoFullName: string, pullNumber: number): Promise<{ state: string }> { const { owner, repo } = splitRepo(repoFullName); - const token = await createInstallationToken(env, installationId); - const octokit = makeInstallationOctokit(env, token, "live", githubRateLimitAdmissionKeyForInstallation(installationId)); - const response = await octokit.request("PATCH /repos/{owner}/{repo}/pulls/{pull_number}", { - owner, - repo, - pull_number: pullNumber, - state: "closed", + return withInstallationTokenRetry(env, installationId, async (token) => { + const octokit = makeInstallationOctokit(env, token, "live", githubRateLimitAdmissionKeyForInstallation(installationId)); + const response = await octokit.request("PATCH /repos/{owner}/{repo}/pulls/{pull_number}", { + owner, + repo, + pull_number: pullNumber, + state: "closed", + }); + return { state: (response.data as { state: string }).state }; }); - return { state: (response.data as { state: string }).state }; } /** The last-closer lookup result. `coveredAllPages` is false when the bounded newest-events window did NOT reach @@ -155,47 +160,48 @@ export type LastCloserResult = { login: string | null; coveredAllPages: boolean export async function getLastCloserLogin(env: Env, installationId: number, repoFullName: string, issueNumber: number): Promise { try { const { owner, repo } = splitRepo(repoFullName); - const token = await createInstallationToken(env, installationId); - const octokit = makeInstallationOctokit(env, token, "live", githubRateLimitAdmissionKeyForInstallation(installationId)); - const requestPage = (page: number) => - octokit.request("GET /repos/{owner}/{repo}/issues/{issue_number}/events", { owner, repo, issue_number: issueNumber, per_page: ISSUE_EVENTS_PAGE_SIZE, page }); - const firstResponse = await requestPage(1); - const firstEvents = firstResponse.data as Array<{ event?: string; actor?: { login?: string | null } | null }>; - const lastPage = issueEventsLastPage(firstResponse.headers.link); - if (lastPage === null) { - // No rel="last" in the Link header. A genuine single page has no rel="next" either — return page 1 directly. - // But GitHub can paginate WITHOUT emitting rel="last" (only rel="next"); then trusting page 1 alone would let - // a later maintainer/bot close hide behind the un-enumerated tail and the reopen guard would fail OPEN. So - // follow rel="next" forward, tracking the latest close across pages (events are oldest-first → a later page's - // close supersedes), bounded by the same page budget. coveredAllPages holds ONLY if we reached the tail within - // budget; otherwise report not-covered so the caller fails closed. (#audit-rel-last) - if (!issueEventsHasNextPage(firstResponse.headers.link)) { - return { login: latestCloserInPage(firstEvents) ?? null, coveredAllPages: true }; + return await withInstallationTokenRetry(env, installationId, async (token) => { + const octokit = makeInstallationOctokit(env, token, "live", githubRateLimitAdmissionKeyForInstallation(installationId)); + const requestPage = (page: number) => + octokit.request("GET /repos/{owner}/{repo}/issues/{issue_number}/events", { owner, repo, issue_number: issueNumber, per_page: ISSUE_EVENTS_PAGE_SIZE, page }); + const firstResponse = await requestPage(1); + const firstEvents = firstResponse.data as Array<{ event?: string; actor?: { login?: string | null } | null }>; + const lastPage = issueEventsLastPage(firstResponse.headers.link); + if (lastPage === null) { + // No rel="last" in the Link header. A genuine single page has no rel="next" either — return page 1 directly. + // But GitHub can paginate WITHOUT emitting rel="last" (only rel="next"); then trusting page 1 alone would let + // a later maintainer/bot close hide behind the un-enumerated tail and the reopen guard would fail OPEN. So + // follow rel="next" forward, tracking the latest close across pages (events are oldest-first → a later page's + // close supersedes), bounded by the same page budget. coveredAllPages holds ONLY if we reached the tail within + // budget; otherwise report not-covered so the caller fails closed. (#audit-rel-last) + if (!issueEventsHasNextPage(firstResponse.headers.link)) { + return { login: latestCloserInPage(firstEvents) ?? null, coveredAllPages: true }; + } + let latestCloser = latestCloserInPage(firstEvents); + let hasNext = true; + for (let page = 2; hasNext && page <= ISSUE_EVENTS_RECENT_PAGE_LIMIT + 1; page += 1) { + const response = await requestPage(page); + const closer = latestCloserInPage(response.data as Array<{ event?: string; actor?: { login?: string | null } | null }>); + if (closer !== undefined) latestCloser = closer; + hasNext = issueEventsHasNextPage(response.headers.link); + } + const coveredAllPages = !hasNext; + return { login: coveredAllPages ? (latestCloser ?? null) : null, coveredAllPages }; } - let latestCloser = latestCloserInPage(firstEvents); - let hasNext = true; - for (let page = 2; hasNext && page <= ISSUE_EVENTS_RECENT_PAGE_LIMIT + 1; page += 1) { + if (lastPage <= 1) return { login: latestCloserInPage(firstEvents) ?? null, coveredAllPages: true }; + + // GitHub returns issue-events oldest-first. Use the Link header to inspect the newest bounded window instead + // of the oldest prefix, so a long self-generated timeline cannot hide a later maintainer/bot close. + const firstPageToRead = Math.max(2, lastPage - ISSUE_EVENTS_RECENT_PAGE_LIMIT + 1); + // We inspected the entire timeline only when the window reached page 2 (page 1 is read separately above). + const coveredAllPages = firstPageToRead === 2; + for (let page = lastPage; page >= firstPageToRead; page -= 1) { const response = await requestPage(page); const closer = latestCloserInPage(response.data as Array<{ event?: string; actor?: { login?: string | null } | null }>); - if (closer !== undefined) latestCloser = closer; - hasNext = issueEventsHasNextPage(response.headers.link); + if (closer !== undefined) return { login: closer, coveredAllPages }; } - const coveredAllPages = !hasNext; - return { login: coveredAllPages ? (latestCloser ?? null) : null, coveredAllPages }; - } - if (lastPage <= 1) return { login: latestCloserInPage(firstEvents) ?? null, coveredAllPages: true }; - - // GitHub returns issue-events oldest-first. Use the Link header to inspect the newest bounded window instead - // of the oldest prefix, so a long self-generated timeline cannot hide a later maintainer/bot close. - const firstPageToRead = Math.max(2, lastPage - ISSUE_EVENTS_RECENT_PAGE_LIMIT + 1); - // We inspected the entire timeline only when the window reached page 2 (page 1 is read separately above). - const coveredAllPages = firstPageToRead === 2; - for (let page = lastPage; page >= firstPageToRead; page -= 1) { - const response = await requestPage(page); - const closer = latestCloserInPage(response.data as Array<{ event?: string; actor?: { login?: string | null } | null }>); - if (closer !== undefined) return { login: closer, coveredAllPages }; - } - return { login: coveredAllPages ? (latestCloserInPage(firstEvents) ?? null) : null, coveredAllPages }; + return { login: coveredAllPages ? (latestCloserInPage(firstEvents) ?? null) : null, coveredAllPages }; + }); } catch { // On error we cannot prove we read the whole timeline — report not-covered so the caller decides conservatively. return { login: null, coveredAllPages: false }; diff --git a/test/unit/github-pr-actions.test.ts b/test/unit/github-pr-actions.test.ts index b364913d77..b3f64f2160 100644 --- a/test/unit/github-pr-actions.test.ts +++ b/test/unit/github-pr-actions.test.ts @@ -1,6 +1,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { generateKeyPairSync } from "node:crypto"; import { closePullRequest, createIssueComment, createPullRequestReview, createPullRequestReviewComments, getLastCloserLogin, mergePullRequest, updatePullRequestBranch } from "../../src/github/pr-actions"; +import { clearInstallationTokenCacheForTest } from "../../src/github/app"; import { createTestEnv } from "../helpers/d1"; function envWithKey() { @@ -88,6 +89,33 @@ describe("GitHub PR action primitives (#778)", () => { expect(calls[0]?.url).toMatch(/\/repos\/owner\/repo\/pulls\/7$/); }); + it("evicts a rejected installation token and retries once with a freshly-minted token on a 401 (#2263)", async () => { + clearInstallationTokenCacheForTest(); + let tokenMints = 0; + let closeAttempts = 0; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + if (url.includes("/access_tokens")) { + tokenMints += 1; + return Response.json({ token: `token-${tokenMints}` }); + } + if (url.endsWith("/pulls/7") && (init?.method ?? "GET") === "PATCH") { + closeAttempts += 1; + // The FIRST attempt uses the (stale, cached) token and is rejected; the RETRY, with a freshly-minted + // token, succeeds — mirroring the existing check-run/comment poster behavior via withInstallationTokenRetry. + if (closeAttempts === 1) return Response.json({ message: "Bad credentials" }, { status: 401 }); + return Response.json({ state: "closed" }); + } + return new Response("unexpected", { status: 500 }); + }); + + const result = await closePullRequest(envWithKey(), 998877, "owner/repo", 7); + + expect(result).toEqual({ state: "closed" }); + expect(closeAttempts).toBe(2); // one rejected attempt + one retry + expect(tokenMints).toBe(2); // the rejected token was evicted, forcing a fresh mint for the retry + }); + it("posts a plain issue comment", async () => { const calls: Array<{ method: string; url: string; body: Record }> = []; vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { @@ -287,6 +315,49 @@ describe("GitHub PR action primitives (#778)", () => { await expect(updatePullRequestBranch(envWithKey(), 123, "owner/repo", 55)).resolves.toBeUndefined(); expect(requestBodies.some((b) => !b.includes("expected_head_sha"))).toBe(true); }); + + it("updates branch WITH an expected head sha (includes expected_head_sha — TRUE branch of the spread ternary)", async () => { + const requestBodies: string[] = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "t" }); + if (url.includes("/pulls/56/update-branch")) { + requestBodies.push(String(init?.body ?? "")); + return new Response("{}", { status: 201, headers: { "content-type": "application/json" } }); + } + return new Response("unexpected", { status: 500 }); + }); + await expect(updatePullRequestBranch(envWithKey(), 123, "owner/repo", 56, "expected-sha-1")).resolves.toBeUndefined(); + expect(requestBodies.some((b) => b.includes('"expected_head_sha":"expected-sha-1"'))).toBe(true); + }); + + it("returns the page-1 closer when rel=last explicitly reports a single page (lastPage<=1)", async () => { + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "t" }); + if (url.includes("/issues/27/events")) { + return Response.json([{ event: "closed", actor: { login: "solo-page-closer" } }], { + headers: { link: '; rel="last"' }, + }); + } + return new Response("unexpected", { status: 500 }); + }); + await expect(getLastCloserLogin(envWithKey(), 123, "owner/repo", 27)).resolves.toEqual({ login: "solo-page-closer", coveredAllPages: true }); + }); + + it("returns null when rel=last explicitly reports a single page with no close event (?? null right branch)", async () => { + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "t" }); + if (url.includes("/issues/28/events")) { + return Response.json([{ event: "labeled" }], { + headers: { link: '; rel="last"' }, + }); + } + return new Response("unexpected", { status: 500 }); + }); + await expect(getLastCloserLogin(envWithKey(), 123, "owner/repo", 28)).resolves.toEqual({ login: null, coveredAllPages: true }); + }); }); function generateRsaPrivateKeyPem(): string { From 4b0ec78e9c66fa811e28b46474f494538373435a Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 1 Jul 2026 17:44:20 -0700 Subject: [PATCH 2/2] fix(github): close the remaining stale-token gaps in pr-actions.ts createPullRequestReviewComments and dismissLatestBotApproval still called createInstallationToken directly, so a rejected cached token would keep dropping inline review comments and skip stale-bot-approval cleanup until the cache's own TTL lapsed -- the same failure mode the other six functions in this file were just fixed for. Route both through withInstallationTokenRetry, mirroring the existing pattern exactly. --- src/github/pr-actions.ts | 68 +++++++++++++++-------------- test/unit/github-pr-actions.test.ts | 51 ++++++++++++++++++++++ 2 files changed, 86 insertions(+), 33 deletions(-) diff --git a/src/github/pr-actions.ts b/src/github/pr-actions.ts index b49be086e9..97f51f0de4 100644 --- a/src/github/pr-actions.ts +++ b/src/github/pr-actions.ts @@ -1,4 +1,4 @@ -import { createInstallationToken, withInstallationTokenRetry } from "./app"; +import { withInstallationTokenRetry } from "./app"; import { githubRateLimitAdmissionKeyForInstallation, makeInstallationOctokit } from "./client"; import type { AgentActionMode } from "../settings/agent-execution"; import type { AutoMergeMethod } from "../types"; @@ -67,17 +67,18 @@ export async function createPullRequestReviewComments( mode: AgentActionMode, ): Promise<{ id: number }> { const { owner, repo } = splitRepo(repoFullName); - const token = await createInstallationToken(env, installationId); - const octokit = makeInstallationOctokit(env, token, mode, githubRateLimitAdmissionKeyForInstallation(installationId)); - const response = await octokit.request("POST /repos/{owner}/{repo}/pulls/{pull_number}/reviews", { - owner, - repo, - pull_number: pullNumber, - commit_id: commitId, - event: "COMMENT", - comments, + return withInstallationTokenRetry(env, installationId, async (token) => { + const octokit = makeInstallationOctokit(env, token, mode, githubRateLimitAdmissionKeyForInstallation(installationId)); + const response = await octokit.request("POST /repos/{owner}/{repo}/pulls/{pull_number}/reviews", { + owner, + repo, + pull_number: pullNumber, + commit_id: commitId, + event: "COMMENT", + comments, + }); + return { id: (response.data as { id: number }).id }; }); - return { id: (response.data as { id: number }).id }; } /** Merge a pull request with the configured method. Pass `sha` to make the merge fail (409) if the head moved @@ -113,30 +114,31 @@ export async function mergePullRequest( export async function dismissLatestBotApproval(env: Env, installationId: number, repoFullName: string, pullNumber: number, message: string): Promise<{ dismissed: boolean }> { try { const { owner, repo } = splitRepo(repoFullName); - const token = await createInstallationToken(env, installationId); - const octokit = makeInstallationOctokit(env, token, "live", githubRateLimitAdmissionKeyForInstallation(installationId)); - const botLogin = `${env.GITHUB_APP_SLUG}[bot]`; - // Reviews are returned oldest-first; the LAST matching entry across ALL pages is the bot's most recent - // APPROVE. Stopping at page 1 would find (or miss) the wrong review on a PR with >100 total reviews. - let latestApprovalId: number | undefined; - for (let page = 1; page <= REVIEW_PAGE_LIMIT; page += 1) { - const response = await octokit.request("GET /repos/{owner}/{repo}/pulls/{pull_number}/reviews", { owner, repo, pull_number: pullNumber, per_page: REVIEW_PAGE_SIZE, page }); - const batch = response.data as Array<{ id: number; state?: string; user?: { login?: string | null } | null }>; - for (const review of batch) { - if (review.user?.login === botLogin && review.state === "APPROVED") latestApprovalId = review.id; + return await withInstallationTokenRetry(env, installationId, async (token) => { + const octokit = makeInstallationOctokit(env, token, "live", githubRateLimitAdmissionKeyForInstallation(installationId)); + const botLogin = `${env.GITHUB_APP_SLUG}[bot]`; + // Reviews are returned oldest-first; the LAST matching entry across ALL pages is the bot's most recent + // APPROVE. Stopping at page 1 would find (or miss) the wrong review on a PR with >100 total reviews. + let latestApprovalId: number | undefined; + for (let page = 1; page <= REVIEW_PAGE_LIMIT; page += 1) { + const response = await octokit.request("GET /repos/{owner}/{repo}/pulls/{pull_number}/reviews", { owner, repo, pull_number: pullNumber, per_page: REVIEW_PAGE_SIZE, page }); + const batch = response.data as Array<{ id: number; state?: string; user?: { login?: string | null } | null }>; + for (const review of batch) { + if (review.user?.login === botLogin && review.state === "APPROVED") latestApprovalId = review.id; + } + if (batch.length < REVIEW_PAGE_SIZE) break; } - if (batch.length < REVIEW_PAGE_SIZE) break; - } - if (latestApprovalId === undefined) return { dismissed: false }; - await octokit.request("PUT /repos/{owner}/{repo}/pulls/{pull_number}/reviews/{review_id}/dismissals", { - owner, - repo, - pull_number: pullNumber, - review_id: latestApprovalId, - message, - event: "DISMISS", + if (latestApprovalId === undefined) return { dismissed: false }; + await octokit.request("PUT /repos/{owner}/{repo}/pulls/{pull_number}/reviews/{review_id}/dismissals", { + owner, + repo, + pull_number: pullNumber, + review_id: latestApprovalId, + message, + event: "DISMISS", + }); + return { dismissed: true }; }); - return { dismissed: true }; } catch { return { dismissed: false }; } diff --git a/test/unit/github-pr-actions.test.ts b/test/unit/github-pr-actions.test.ts index ecb90272d8..4b8213ea52 100644 --- a/test/unit/github-pr-actions.test.ts +++ b/test/unit/github-pr-actions.test.ts @@ -62,6 +62,30 @@ describe("GitHub PR action primitives (#778)", () => { expect(calls[0]).toMatchObject({ method: "POST", body: { event: "COMMENT", commit_id: "headsha1", comments } }); }); + it("REGRESSION (#confirmed-bug, review round 2): createPullRequestReviewComments evicts a rejected installation token and retries once with a freshly-minted token", async () => { + clearInstallationTokenCacheForTest(); + let tokenMints = 0; + let postAttempts = 0; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + if (url.includes("/access_tokens")) { + tokenMints += 1; + return Response.json({ token: `token-${tokenMints}` }); + } + if (url.endsWith("/pulls/7/reviews") && (init?.method ?? "GET") === "POST") { + postAttempts += 1; + if (postAttempts === 1) return Response.json({ message: "Bad credentials" }, { status: 401 }); + return Response.json({ id: 71 }); + } + return new Response("unexpected", { status: 500 }); + }); + const comments = [{ path: "src/a.ts", line: 2, side: "RIGHT" as const, body: "**Nit:** guard this." }]; + const result = await createPullRequestReviewComments(envWithKey(), 998877, "owner/repo", 7, "headsha1", comments, "live"); + expect(result).toEqual({ id: 71 }); + expect(postAttempts).toBe(2); + expect(tokenMints).toBe(2); + }); + it("merges a PR with the method and head-sha guard", async () => { const calls: Array<{ method: string; url: string; body: Record }> = []; vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { @@ -487,6 +511,33 @@ describe("GitHub PR action primitives (#778)", () => { await expect(dismissLatestBotApproval(envWithKey(), 123, "owner/repo", 9, "retract")).resolves.toEqual({ dismissed: false }); }); + it("REGRESSION (#confirmed-bug, review round 2): dismissLatestBotApproval evicts a rejected installation token and retries once with a freshly-minted token", async () => { + clearInstallationTokenCacheForTest(); + let tokenMints = 0; + let getAttempts = 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/11/reviews") && !url.includes("/dismissals") && method === "GET") { + getAttempts += 1; + if (getAttempts === 1) return Response.json({ message: "Bad credentials" }, { status: 401 }); + return Response.json([{ id: 5, state: "APPROVED", user: { login: "gittensory[bot]" } }]); + } + if (url.includes("/pulls/11/reviews/5/dismissals") && method === "PUT") { + return Response.json({ id: 5, state: "DISMISSED" }); + } + return new Response("unexpected", { status: 500 }); + }); + const result = await dismissLatestBotApproval(envWithKey(), 998877, "owner/repo", 11, "stale approval retracted"); + expect(result).toEqual({ dismissed: true }); + expect(getAttempts).toBe(2); + expect(tokenMints).toBe(2); + }); + it("updates branch without an expected head sha (omits expected_head_sha — FALSE branch of the spread ternary)", async () => { const requestBodies: string[] = []; vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {