diff --git a/.github/workflows/issue_fixed_comment.yml b/.github/workflows/issue_fixed_comment.yml index 92993d319a7..b98a86c4dfa 100644 --- a/.github/workflows/issue_fixed_comment.yml +++ b/.github/workflows/issue_fixed_comment.yml @@ -6,8 +6,12 @@ on: workflow_dispatch: inputs: issue_number: - description: "Closed issue number to comment on manually." - required: true + description: "Closed issue number to comment on and close the superseded pull requests of. Ignored by a sweep." + required: false + sweep: + description: "Close every open pull request whose linked issues were all fixed on the default branch. Reads every open pull request, so run it at most once an hour." + type: boolean + default: false pull_request: paths: - .github/workflows/issue_fixed_comment.yml @@ -39,16 +43,17 @@ jobs: with: bun-version: "1.4.0" - - name: Test the closer lookup, the release placement and the comment + - name: Test the closer lookup, the release placement, the comment and the superseded pull request close run: bun test scripts/comment-fixed-issue.test.ts comment-fixed-issue: if: github.event_name != 'pull_request' && github.repository == 'BerriAI/litellm' runs-on: ubuntu-latest - timeout-minutes: 5 + timeout-minutes: 15 permissions: contents: read issues: write + pull-requests: write steps: - name: Checkout scripts uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0 @@ -59,13 +64,16 @@ jobs: - name: Setup Bun uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 with: - # Exact version, never latest: the next step holds an issues: write token + # Exact version, never latest: the next step holds issues: write and pull-requests: write tokens bun-version: "1.4.0" - - name: Name the release that carries the fix + - name: Name the release that carries the fix and close the pull requests it supersedes + shell: bash run: bun run scripts/comment-fixed-issue.ts | tee -a "${GITHUB_STEP_SUMMARY}" env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} ISSUE_NUMBER: ${{ github.event.issue.number || github.event.inputs.issue_number }} + SWEEP: ${{ github.event.inputs.sweep }} DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} DRY_RUN: ${{ vars.ISSUE_FIXED_COMMENT_ENABLED != 'true' }} + CLOSE_PRS_DRY_RUN: ${{ vars.ISSUE_FIXED_CLOSE_PRS_ENABLED != 'true' }} diff --git a/scripts/comment-fixed-issue.test.ts b/scripts/comment-fixed-issue.test.ts index f9cd41d96d8..fef5c0f4007 100644 --- a/scripts/comment-fixed-issue.test.ts +++ b/scripts/comment-fixed-issue.test.ts @@ -3,62 +3,140 @@ import { describe, expect, test } from "bun:test"; import type { Comment, GitHubApi } from "./auto-close-duplicates"; import { FIXED_MARKER, + OPEN_PULL_REQUESTS_QUERY, + SUPERSEDED_MARKER, + closeVerdict, closerOf, - commentFixedIssue, + describeIssue, + describeSweep, + fixOf, fixedBody, + handleFixedIssue, nextMinor, parseVersion, placement, readConfig, releaseCandidate, + supersededBody, + sweep, type ClosedIssue, type FixedConfig, + type IssueClosure, + type LinkedIssue, + type LinkedPullRequest, + type PullRequestsPage, } from "./comment-fixed-issue"; const MERGE_COMMIT = "68c4c82ac977b48b2b81ee8d633d5771307c6162"; +const ISSUE = 41750; const mergedPr = { __typename: "PullRequest" as const, number: 41767, merged: true, baseRefName: "main", + repository: { nameWithOwner: "BerriAI/litellm" }, mergeCommit: { oid: MERGE_COMMIT }, }; -type Closer = ClosedIssue["timelineItems"]["nodes"][number]["closer"]; +const commitCloser = { __typename: "Commit" as const, oid: MERGE_COMMIT, repository: { nameWithOwner: "BerriAI/litellm" } }; -const closedBy = (closer: Closer, state: ClosedIssue["state"] = "CLOSED"): ClosedIssue => ({ +type Closer = IssueClosure["timelineItems"]["nodes"][number]["closer"]; + +const closure = (closer: Closer, state: IssueClosure["state"] = "CLOSED"): IssueClosure => ({ state, timelineItems: { nodes: [{ closer }] }, }); +const page = (pages: readonly (readonly LinkedPullRequest[])[], index: number): PullRequestsPage => ({ + pageInfo: { hasNextPage: index + 1 < pages.length, endCursor: String(index + 1) }, + nodes: pages[index] ?? [], +}); + +const cursorIndex = (after: string | null): number => (after === null ? 0 : Number(after)); + +const closedBy = (closer: Closer, state?: IssueClosure["state"], linked: readonly LinkedPullRequest[] = []): ClosedIssue => ({ + ...closure(closer, state), + closedByPullRequestsReferences: page([linked], 0), +}); + +const reopenedAt = (createdAt: string): LinkedPullRequest["reopens"] => ({ nodes: [{ createdAt }] }); + +const linkedIssue = (number: number, closer: Closer = mergedPr, state?: IssueClosure["state"]): LinkedIssue => ({ + number, + repository: { nameWithOwner: "BerriAI/litellm" }, + ...closure(closer, state), +}); + +const links = (...issues: readonly LinkedIssue[]): LinkedPullRequest["closingIssuesReferences"] => ({ totalCount: issues.length, nodes: issues }); + +const openPr = (number: number, overrides: Partial = {}): LinkedPullRequest => ({ + number, + state: "OPEN", + baseRefName: "main", + repository: { nameWithOwner: "BerriAI/litellm" }, + closingIssuesReferences: links(linkedIssue(ISSUE)), + reopens: { nodes: [] }, + ...overrides, +}); + const pyproject = (version: string): string => `[project]\nname = "litellm"\nversion = "${version}"\n\n[tool.commitizen]\nversion = "${version}"\n`; -const config: FixedConfig = { repo: "BerriAI/litellm", issueNumber: 41750, defaultBranch: "main", dryRun: false }; +const config: FixedConfig = { repo: "BerriAI/litellm", defaultBranch: "main", commentDryRun: false, closeDryRun: false }; + +const prFix = (issue = ISSUE, number = 41767) => ({ issue, source: { kind: "pull_request" as const, number, oid: MERGE_COMMIT } }); +const commitFix = (issue = ISSUE) => ({ issue, source: { kind: "commit" as const, oid: MERGE_COMMIT } }); +const oneFixBody = supersededBody([prFix()], "main"); + +const noPause = async (): Promise => {}; + +const supersededComment: Comment = { + id: 2, + body: `${SUPERSEDED_MARKER}\n#41750 was fixed by #41767 on main, so this pull request is closed. Reopen it if something was missed.`, + created_at: "2026-09-18T00:00:00Z", + user: { type: "Bot", login: "github-actions[bot]" }, +}; interface World { readonly issue?: ClosedIssue | null; readonly comments?: readonly Comment[]; + readonly pullRequestComments?: Readonly>; + readonly openPullRequests?: readonly (readonly LinkedPullRequest[])[]; + readonly linkedPullRequests?: readonly (readonly LinkedPullRequest[])[]; readonly version?: string; // Which existing rc.1 tags contain the merge commit; a tag absent from the map does not exist readonly tags?: Readonly>; + readonly reachable?: Readonly>; } function fakeApi(world: World = {}): { readonly api: GitHubApi; readonly writes: string[] } { const writes: string[] = []; const tags = world.tags ?? {}; + const reachable = world.reachable ?? { main: [MERGE_COMMIT] }; + const pages = world.openPullRequests ?? []; const api: GitHubApi = { request: async (method: string, path: string, body?: object): Promise => { if (method === "POST" && path === "/graphql") { - return { data: { repository: { issue: world.issue === undefined ? closedBy(mergedPr) : world.issue } } } as T; + const { query, variables } = body as { query: string; variables: { after: string | null } }; + if (query === OPEN_PULL_REQUESTS_QUERY) { + return { data: { repository: { pullRequests: page(pages, cursorIndex(variables.after)) } } } as T; + } + const issue = world.issue === undefined ? closedBy(mergedPr) : world.issue; + if (issue === null || world.linkedPullRequests === undefined) { + return { data: { repository: { issue } } } as T; + } + const closedByPullRequestsReferences = page(world.linkedPullRequests, cursorIndex(variables.after)); + return { data: { repository: { issue: { ...issue, closedByPullRequestsReferences } } } } as T; } if (method !== "GET") { writes.push(`${method} ${path} ${JSON.stringify(body)}`); return {} as T; } - if (path.startsWith("/repos/BerriAI/litellm/issues/41750/comments")) { - return (world.comments ?? []) as T; + const comments = /^\/repos\/BerriAI\/litellm\/issues\/(\d+)\/comments/.exec(path); + if (comments !== null) { + const number = Number(comments[1]); + return ((number === ISSUE ? world.comments : world.pullRequestComments?.[number]) ?? []) as T; } if (path === `/repos/BerriAI/litellm/contents/pyproject.toml?ref=${MERGE_COMMIT}`) { return { content: btoa(pyproject(world.version ?? "1.103.0")).replace(/(.{60})/g, "$1\n") } as T; @@ -68,6 +146,10 @@ function fakeApi(world: World = {}): { readonly api: GitHubApi; readonly writes: return (matching[1] in tags ? [{ ref: `refs/tags/${matching[1]}` }] : []) as T; } const compare = /^\/repos\/BerriAI\/litellm\/compare\/(.+)\.\.\.(.+)$/.exec(path); + const branch = compare === null ? undefined : reachable[compare[1] ?? ""]; + if (compare !== null && branch !== undefined) { + return { status: branch.includes(compare[2] ?? "") ? "behind" : "diverged" } as T; + } if (compare !== null && compare[2] === MERGE_COMMIT) { return { status: tags[compare[1]] ? "behind" : "ahead" } as T; } @@ -79,23 +161,28 @@ function fakeApi(world: World = {}): { readonly api: GitHubApi; readonly writes: describe("closerOf", () => { test("a pull request merged into the default branch is the fix", () => { - expect(closerOf(closedBy(mergedPr), "main")).toEqual({ kind: "pull_request", number: 41767, mergeCommit: MERGE_COMMIT }); + expect(closerOf(closedBy(mergedPr), "BerriAI/litellm", "main")).toEqual({ kind: "pull_request", number: 41767, mergeCommit: MERGE_COMMIT }); }); test("an issue closed by hand, by a commit, or by an unmerged pull request gets no comment", () => { - expect(closerOf(closedBy(null), "main")).toEqual({ kind: "skip", reason: "closed by hand, not by a pull request" }); - expect(closerOf(closedBy({ __typename: "Commit", oid: MERGE_COMMIT }), "main").kind).toBe("skip"); - expect(closerOf(closedBy({ ...mergedPr, merged: false }), "main").kind).toBe("skip"); - expect(closerOf(closedBy({ ...mergedPr, mergeCommit: null }), "main").kind).toBe("skip"); + expect(closerOf(closedBy(null), "BerriAI/litellm", "main")).toEqual({ kind: "skip", reason: "closed by hand, not by a pull request" }); + expect(closerOf(closedBy(commitCloser), "BerriAI/litellm", "main").kind).toBe("skip"); + expect(closerOf(closedBy({ ...mergedPr, merged: false }), "BerriAI/litellm", "main").kind).toBe("skip"); + expect(closerOf(closedBy({ ...mergedPr, mergeCommit: null }), "BerriAI/litellm", "main").kind).toBe("skip"); }); test("a pull request merged into a release branch is not a fix on main", () => { - const verdict = closerOf(closedBy({ ...mergedPr, baseRefName: "release/1.102.0rc2" }), "main"); + const verdict = closerOf(closedBy({ ...mergedPr, baseRefName: "release/1.102.0rc2" }), "BerriAI/litellm", "main"); expect(verdict).toEqual({ kind: "skip", reason: "#41767 merged into release/1.102.0rc2, not main" }); }); test("an issue reopened after the close event is left alone", () => { - expect(closerOf(closedBy(mergedPr, "OPEN"), "main")).toEqual({ kind: "skip", reason: "the issue is open again" }); + expect(closerOf(closedBy(mergedPr, "OPEN"), "BerriAI/litellm", "main")).toEqual({ kind: "skip", reason: "the issue is open again" }); + }); + + test("a pull request merged in a fork closes the issue on GitHub but is no fix here", () => { + const forkPr = { ...mergedPr, number: 9, repository: { nameWithOwner: "someone/litellm" } }; + expect(closerOf(closedBy(forkPr), "BerriAI/litellm", "main")).toEqual({ kind: "skip", reason: "closed by someone/litellm#9, a pull request in another repository" }); }); }); @@ -173,44 +260,327 @@ describe("fixedBody", () => { }); }); -describe("commentFixedIssue", () => { +describe("fixOf", () => { + test("a merged pull request or a commit is a fix, whatever branch it was merged into", () => { + expect(fixOf(closure(mergedPr), "BerriAI/litellm")).toEqual({ kind: "pull_request", number: 41767, oid: MERGE_COMMIT }); + expect(fixOf(closure({ ...mergedPr, baseRefName: "litellm_internal_staging" }), "BerriAI/litellm")).toEqual({ kind: "pull_request", number: 41767, oid: MERGE_COMMIT }); + expect(fixOf(closure(commitCloser), "BerriAI/litellm")).toEqual({ kind: "commit", oid: MERGE_COMMIT }); + }); + + test("a hand close, a fork's pull request, an unmerged pull request, or a reopened issue is no fix", () => { + expect(fixOf(closure(null), "BerriAI/litellm")).toEqual({ kind: "skip", reason: "was closed by hand" }); + expect(fixOf(closure({ ...mergedPr, repository: { nameWithOwner: "someone/litellm" } }), "BerriAI/litellm")).toEqual({ kind: "skip", reason: "was closed from someone/litellm" }); + expect(fixOf(closure({ ...commitCloser, repository: { nameWithOwner: "someone/litellm" } }), "BerriAI/litellm")).toEqual({ kind: "skip", reason: "was closed from someone/litellm" }); + expect(fixOf(closure({ ...mergedPr, merged: false }), "BerriAI/litellm")).toEqual({ kind: "skip", reason: "was closed by #41767, which is not merged" }); + expect(fixOf(closure({ ...mergedPr, mergeCommit: null }), "BerriAI/litellm").kind).toBe("skip"); + expect(fixOf(closure(mergedPr, "OPEN"), "BerriAI/litellm")).toEqual({ kind: "skip", reason: "is open again" }); + }); +}); + +describe("closeVerdict", () => { + test("an open pull request whose only linked issue was fixed by a merged pull request is a candidate", () => { + expect(closeVerdict(openPr(41760), config)).toEqual({ kind: "candidate", fixes: [prFix()] }); + }); + + test("every linked issue has to be fixed, and each fix is named", () => { + const both = openPr(41760, { closingIssuesReferences: links(linkedIssue(ISSUE), linkedIssue(41751, commitCloser)) }); + expect(closeVerdict(both, config)).toEqual({ kind: "candidate", fixes: [prFix(), commitFix(41751)] }); + }); + + test("a pull request that still links an open issue keeps its work", () => { + const stillOpen = openPr(41760, { closingIssuesReferences: links(linkedIssue(ISSUE), linkedIssue(41751, null, "OPEN")) }); + expect(closeVerdict(stillOpen, config)).toEqual({ kind: "skip", reason: "still linked to open #41751" }); + }); + + test("a linked issue closed by hand or by an unmerged pull request is not a fix that supersedes the pull request", () => { + expect(closeVerdict(openPr(41760, { closingIssuesReferences: links(linkedIssue(ISSUE, null)) }), config)).toEqual({ + kind: "skip", + reason: `#${ISSUE} was closed by hand`, + }); + const unmerged = openPr(41760, { closingIssuesReferences: links(linkedIssue(ISSUE, { ...mergedPr, merged: false })) }); + expect(closeVerdict(unmerged, config)).toEqual({ kind: "skip", reason: `#${ISSUE} was closed by #41767, which is not merged` }); + }); + + test("a pull request linking an issue in another repository, or more issues than the query reads, is left alone", () => { + const foreign = { ...linkedIssue(41751), repository: { nameWithOwner: "mlflow/mlflow" } }; + expect(closeVerdict(openPr(41760, { closingIssuesReferences: links(linkedIssue(ISSUE), foreign) }), config)).toEqual({ + kind: "skip", + reason: "links mlflow/mlflow#41751", + }); + const truncated = openPr(41760, { closingIssuesReferences: { totalCount: 11, nodes: [linkedIssue(ISSUE)] } }); + expect(closeVerdict(truncated, config)).toEqual({ kind: "skip", reason: "links 11 issues, more than the 10 this workflow reads" }); + }); + + test("a pull request against a release line is a backport and stays open, one against a retired development branch does not", () => { + for (const base of ["release/v1.102.0-rc.2", "stable/v1.83.14", "v_1_83_3_stable_patch"]) { + expect(closeVerdict(openPr(41760, { baseRefName: base }), config)).toEqual({ kind: "skip", reason: `targets the release line ${base}` }); + } + expect(closeVerdict(openPr(41760, { baseRefName: "litellm_internal_staging" }), config).kind).toBe("candidate"); + }); + + test("a pull request in a fork, one that is not open, or one linking no issue is left alone", () => { + expect(closeVerdict(openPr(1, { repository: { nameWithOwner: "someone/litellm" } }), config)).toEqual({ kind: "skip", reason: "lives in someone/litellm" }); + expect(closeVerdict(openPr(41767, { state: "MERGED" }), config)).toEqual({ kind: "skip", reason: "is merged" }); + expect(closeVerdict(openPr(41760, { closingIssuesReferences: links() }), config)).toEqual({ kind: "skip", reason: "links no issue" }); + }); +}); + +describe("supersededBody", () => { + test("names each issue with the pull request or commit that fixed it, invites a reopen, and carries the marker the rerun looks for", () => { + expect(oneFixBody.startsWith(SUPERSEDED_MARKER)).toBe(true); + expect(oneFixBody).toContain("#41750 was fixed by #41767 on main"); + expect(oneFixBody).toContain("Reopen it"); + expect(supersededBody([prFix(), commitFix(41751)], "main")).toContain("#41750 was fixed by #41767 and #41751 by commit 68c4c82ac9 on main"); + }); + + test("stays within the 25-word comment rule for one and two fixes", () => { + for (const fixes of [[prFix()], [commitFix()], [prFix(), commitFix(41751)]]) { + const words = supersededBody(fixes, "main").replace(SUPERSEDED_MARKER, "").trim().split(/\s+/); + expect(words.length).toBeGreaterThanOrEqual(15); + expect(words.length).toBeLessThanOrEqual(25); + } + }); +}); + +describe("handleFixedIssue", () => { test("a real run posts one comment naming the pull request and the release", async () => { const { api, writes } = fakeApi(); - const verdict = await commentFixedIssue(api, config); - expect(verdict).toMatchObject({ kind: "commented", pullRequest: 41767, tag: "v1.103.0-rc.1" }); + const { comment, pullRequests } = await handleFixedIssue(api, config, ISSUE, noPause); + expect(comment).toMatchObject({ kind: "commented", pullRequest: 41767, tag: "v1.103.0-rc.1" }); + expect(pullRequests).toEqual([]); expect(writes).toHaveLength(1); expect(writes[0]).toContain("POST /repos/BerriAI/litellm/issues/41750/comments"); expect(writes[0]).toContain("Fixed by #41767. This ships in v1.103.0-rc.1 and up"); }); - test("a dry run renders the comment and writes nothing", async () => { - const { api, writes } = fakeApi(); - const verdict = await commentFixedIssue(api, { ...config, dryRun: true }); - expect(verdict.kind).toBe("commented"); - expect(writes).toEqual([]); + test("every other open pull request linked to the fixed issue is commented on and then closed, a pause before every write", async () => { + const { api, writes } = fakeApi({ issue: closedBy(mergedPr, "CLOSED", [openPr(41760), openPr(41761)]) }); + let pauses = 0; + const { pullRequests } = await handleFixedIssue(api, config, ISSUE, async () => { + pauses += 1; + }); + expect(pullRequests.map((pullRequest) => pullRequest.kind)).toEqual(["closed", "closed"]); + expect(writes).toEqual([ + expect.stringContaining("POST /repos/BerriAI/litellm/issues/41750/comments"), + `POST /repos/BerriAI/litellm/issues/41760/comments ${JSON.stringify({ body: oneFixBody })}`, + 'PATCH /repos/BerriAI/litellm/pulls/41760 {"state":"closed"}', + expect.stringContaining("POST /repos/BerriAI/litellm/issues/41761/comments"), + 'PATCH /repos/BerriAI/litellm/pulls/41761 {"state":"closed"}', + ]); + expect(pauses).toBe(4); }); - test("an issue that already carries the comment is not commented twice", async () => { + test("the fixed-in comment and the closing are gated separately: a close dry run still posts the comment and closes nothing", async () => { + const { api, writes } = fakeApi({ issue: closedBy(mergedPr, "CLOSED", [openPr(41760)]) }); + let pauses = 0; + const outcome = await handleFixedIssue(api, { ...config, closeDryRun: true }, ISSUE, async () => { + pauses += 1; + }); + expect(outcome.comment.kind).toBe("commented"); + expect(outcome.pullRequests).toEqual([{ kind: "closed", number: 41760, body: oneFixBody }]); + expect(writes).toHaveLength(1); + expect(writes[0]).toContain("/issues/41750/comments"); + expect(pauses).toBe(0); + }); + + test("a comment dry run still closes the pull requests for real", async () => { + const { api, writes } = fakeApi({ issue: closedBy(mergedPr, "CLOSED", [openPr(41760)]) }); + const outcome = await handleFixedIssue(api, { ...config, commentDryRun: true }, ISSUE, noPause); + expect(outcome.comment.kind).toBe("commented"); + expect(writes).toEqual([expect.stringContaining("/issues/41760/comments"), 'PATCH /repos/BerriAI/litellm/pulls/41760 {"state":"closed"}']); + }); + + test("an issue that already carries the fixed-in comment is not commented twice, and its linked pull requests still get closed", async () => { const existing: Comment = { id: 1, body: `${FIXED_MARKER}\nFixed by #41767. This ships in v1.103.0-rc.1 and up.`, created_at: "2026-09-18T00:00:00Z", user: { type: "Bot", login: "github-actions[bot]" }, }; - const { api, writes } = fakeApi({ comments: [existing] }); - expect(await commentFixedIssue(api, config)).toEqual({ kind: "skip", reason: "already carries a fixed-in comment" }); + const { api, writes } = fakeApi({ comments: [existing], issue: closedBy(mergedPr, "CLOSED", [openPr(41760)]) }); + const outcome = await handleFixedIssue(api, config, ISSUE, noPause); + expect(outcome.comment).toEqual({ kind: "skip", reason: "already carries a fixed-in comment" }); + expect(writes).toEqual([expect.stringContaining("/issues/41760/comments"), 'PATCH /repos/BerriAI/litellm/pulls/41760 {"state":"closed"}']); + }); + + test("a fix merged into the retired development branch or pushed as a commit counts once it is on the default branch", async () => { + const stagingPr = { ...mergedPr, baseRefName: "litellm_internal_staging" }; + const staging = openPr(41760, { baseRefName: "litellm_internal_staging", closingIssuesReferences: links(linkedIssue(ISSUE, stagingPr)) }); + const byCommit = openPr(41761, { closingIssuesReferences: links(linkedIssue(41751, commitCloser)) }); + const { api, writes } = fakeApi({ issue: closedBy(mergedPr, "CLOSED", [staging, byCommit]) }); + const { pullRequests } = await handleFixedIssue(api, config, ISSUE, noPause); + expect(pullRequests).toEqual([ + { kind: "closed", number: 41760, body: oneFixBody }, + { kind: "closed", number: 41761, body: supersededBody([commitFix(41751)], "main") }, + ]); + expect(writes).toHaveLength(5); + expect(writes[3]).toContain("#41751 was fixed by commit 68c4c82ac9 on main"); + }); + + test("a fix whose commit never reached the default branch supersedes nothing", async () => { + const { api, writes } = fakeApi({ issue: closedBy(mergedPr, "CLOSED", [openPr(41760)]), reachable: { main: [] } }); + const { pullRequests } = await handleFixedIssue(api, config, ISSUE, noPause); + expect(pullRequests).toEqual([{ kind: "skip", number: 41760, reason: "#41750 was fixed by #41767, which is not on main" }]); + expect(writes).toHaveLength(1); + expect(writes[0]).toContain("/issues/41750/comments"); + }); + + test("the default branch comes from the config for the containment check and the comment alike", async () => { + const stagingConfig = { ...config, defaultBranch: "litellm_internal_staging" }; + const closer = { ...mergedPr, baseRefName: "litellm_internal_staging" }; + const { api, writes } = fakeApi({ + issue: closedBy(closer, "CLOSED", [openPr(41760, { closingIssuesReferences: links(linkedIssue(ISSUE, closer)) })]), + reachable: { litellm_internal_staging: [MERGE_COMMIT] }, + }); + const { pullRequests } = await handleFixedIssue(api, stagingConfig, ISSUE, noPause); + expect(pullRequests).toEqual([{ kind: "closed", number: 41760, body: supersededBody([prFix()], "litellm_internal_staging") }]); + expect(writes[1]).toContain("on litellm_internal_staging, so this pull request is closed"); + }); + + test("the closer sits in the linked list as merged and gets neither a line nor a write", async () => { + const { api, writes } = fakeApi({ issue: closedBy(mergedPr, "CLOSED", [openPr(41767, { state: "MERGED" }), openPr(41760)]) }); + const { pullRequests } = await handleFixedIssue(api, config, ISSUE, noPause); + expect(pullRequests).toEqual([{ kind: "closed", number: 41760, body: oneFixBody }]); + expect(writes.map((write) => write.split(" ")[1])).toEqual([ + "/repos/BerriAI/litellm/issues/41750/comments", + "/repos/BerriAI/litellm/issues/41760/comments", + "/repos/BerriAI/litellm/pulls/41760", + ]); + }); + + test("a pull request this workflow closed once and its author reopened stays open", async () => { + const reopened = openPr(41760, { reopens: reopenedAt("2026-09-19T00:00:00Z") }); + const { api, writes } = fakeApi({ + issue: closedBy(mergedPr, "CLOSED", [reopened, openPr(41761)]), + pullRequestComments: { 41760: [supersededComment] }, + }); + const { pullRequests } = await handleFixedIssue(api, config, ISSUE, noPause); + expect(pullRequests[0]).toEqual({ kind: "skip", number: 41760, reason: "was closed by this workflow once and reopened" }); + expect(pullRequests[1]?.kind).toBe("closed"); + expect(writes.filter((write) => write.includes("41760"))).toEqual([]); + }); + + test("a pull request whose comment landed but whose close failed is closed on the next run without a second comment", async () => { + const reopenedBeforeTheComment = openPr(41761, { reopens: reopenedAt("2026-09-17T00:00:00Z") }); + const { api, writes } = fakeApi({ + issue: closedBy(mergedPr, "CLOSED", [openPr(41760), reopenedBeforeTheComment]), + pullRequestComments: { 41760: [supersededComment], 41761: [supersededComment] }, + }); + const { pullRequests } = await handleFixedIssue(api, config, ISSUE, noPause); + expect(pullRequests).toEqual([ + { kind: "closed", number: 41760, body: supersededComment.body }, + { kind: "closed", number: 41761, body: supersededComment.body }, + ]); + expect(writes.filter((write) => write.includes("/4176"))).toEqual([ + 'PATCH /repos/BerriAI/litellm/pulls/41760 {"state":"closed"}', + 'PATCH /repos/BerriAI/litellm/pulls/41761 {"state":"closed"}', + ]); + }); + + test("a superseded marker pasted by anyone but the workflow neither keeps a pull request open nor replaces its comment", async () => { + const forged: Comment = { ...supersededComment, id: 3, user: { type: "User", login: "someone" } }; + const { api, writes } = fakeApi({ + issue: closedBy(mergedPr, "CLOSED", [openPr(41760, { reopens: reopenedAt("2026-09-19T00:00:00Z") })]), + pullRequestComments: { 41760: [forged] }, + }); + const { pullRequests } = await handleFixedIssue(api, config, ISSUE, noPause); + expect(pullRequests).toEqual([{ kind: "closed", number: 41760, body: oneFixBody }]); + expect(writes.filter((write) => write.includes("/41760"))).toEqual([ + `POST /repos/BerriAI/litellm/issues/41760/comments ${JSON.stringify({ body: oneFixBody })}`, + 'PATCH /repos/BerriAI/litellm/pulls/41760 {"state":"closed"}', + ]); + }); + + test("every page of linked pull requests is read, not just the first", async () => { + const { api } = fakeApi({ linkedPullRequests: [[openPr(41760)], [openPr(41761)], [openPr(41762)]] }); + const { pullRequests } = await handleFixedIssue(api, config, ISSUE, noPause); + expect(pullRequests).toEqual([ + { kind: "closed", number: 41760, body: oneFixBody }, + { kind: "closed", number: 41761, body: oneFixBody }, + { kind: "closed", number: 41762, body: oneFixBody }, + ]); + }); + + test("a linked pull request from a fork or one still tied to another open issue is reported, not closed", async () => { + const fork = openPr(1, { repository: { nameWithOwner: "someone/litellm" } }); + const busy = openPr(41762, { closingIssuesReferences: links(linkedIssue(ISSUE), linkedIssue(41751, null, "OPEN")) }); + const { api, writes } = fakeApi({ issue: closedBy(mergedPr, "CLOSED", [fork, busy]) }); + const { pullRequests } = await handleFixedIssue(api, config, ISSUE, noPause); + expect(pullRequests).toEqual([ + { kind: "skip", number: 1, reason: "lives in someone/litellm" }, + { kind: "skip", number: 41762, reason: "still linked to open #41751" }, + ]); + expect(writes).toHaveLength(1); + }); + + test("a hand-closed issue gets no comment and leaves its linked pull requests open with the reason on each", async () => { + const byHand = openPr(41760, { closingIssuesReferences: links(linkedIssue(ISSUE, null)) }); + const { api, writes } = fakeApi({ issue: closedBy(null, "CLOSED", [byHand]) }); + const outcome = await handleFixedIssue(api, config, ISSUE, noPause); + expect(outcome.comment).toEqual({ kind: "skip", reason: "closed by hand, not by a pull request" }); + expect(outcome.pullRequests).toEqual([{ kind: "skip", number: 41760, reason: `#${ISSUE} was closed by hand` }]); expect(writes).toEqual([]); }); - test("a hand-closed issue never reaches the release lookup or the API writes", async () => { - const { api, writes } = fakeApi({ issue: closedBy(null) }); - expect((await commentFixedIssue(api, config)).kind).toBe("skip"); + test("an issue closed by a commit on the default branch gets no comment but still closes its linked pull requests", async () => { + const byCommit = openPr(41760, { closingIssuesReferences: links(linkedIssue(ISSUE, commitCloser)) }); + const { api, writes } = fakeApi({ issue: closedBy(commitCloser, "CLOSED", [byCommit]) }); + const { comment, pullRequests } = await handleFixedIssue(api, config, ISSUE, noPause); + expect(comment).toEqual({ kind: "skip", reason: "closed by commit 68c4c82ac9, not by a pull request" }); + expect(pullRequests).toEqual([{ kind: "closed", number: 41760, body: supersededBody([commitFix()], "main") }]); + expect(writes).toEqual([expect.stringContaining("/issues/41760/comments"), 'PATCH /repos/BerriAI/litellm/pulls/41760 {"state":"closed"}']); + expect(writes[0]).toContain("#41750 was fixed by commit 68c4c82ac9 on main"); + }); + + test("an issue closed from the retired development branch gets no comment but still closes its linked pull requests once the fix is on the default branch", async () => { + const stagingPr = { ...mergedPr, baseRefName: "litellm_internal_staging" }; + const staging = openPr(41760, { closingIssuesReferences: links(linkedIssue(ISSUE, stagingPr)) }); + const { api, writes } = fakeApi({ issue: closedBy(stagingPr, "CLOSED", [staging]) }); + const { comment, pullRequests } = await handleFixedIssue(api, config, ISSUE, noPause); + expect(comment).toEqual({ kind: "skip", reason: "#41767 merged into litellm_internal_staging, not main" }); + expect(pullRequests).toEqual([{ kind: "closed", number: 41760, body: oneFixBody }]); + expect(writes.map((write) => write.split(" ")[1])).toEqual(["/repos/BerriAI/litellm/issues/41760/comments", "/repos/BerriAI/litellm/pulls/41760"]); + }); + + test("an issue that is open again gets no comment and its linked pull requests are neither read nor touched", async () => { + const { api, writes } = fakeApi({ issue: closedBy(mergedPr, "OPEN", [openPr(41760)]) }); + const outcome = await handleFixedIssue(api, config, ISSUE, noPause); + expect(outcome).toEqual({ comment: { kind: "skip", reason: "the issue is open again" }, pullRequests: [] }); expect(writes).toEqual([]); }); test("a number that is not an issue in the repository is a skip", async () => { const { api, writes } = fakeApi({ issue: null }); - expect(await commentFixedIssue(api, config)).toEqual({ kind: "skip", reason: "not an issue in this repository" }); + const outcome = await handleFixedIssue(api, config, ISSUE, noPause); + expect(outcome.comment).toEqual({ kind: "skip", reason: "not an issue in this repository" }); + expect(writes).toEqual([]); + }); +}); + +describe("sweep", () => { + test("walks every page of open pull requests and closes the ones whose linked issues were all fixed", async () => { + const unlinked = openPr(41700, { closingIssuesReferences: links() }); + const busy = openPr(41701, { closingIssuesReferences: links(linkedIssue(41751, null, "OPEN")) }); + const { api, writes } = fakeApi({ openPullRequests: [[unlinked, openPr(41760)], [busy, openPr(41761)]] }); + const outcome = await sweep(api, { ...config, commentDryRun: true }, noPause); + expect(outcome.considered).toBe(4); + expect(outcome.pullRequests).toEqual([ + { kind: "closed", number: 41760, body: oneFixBody }, + { kind: "skip", number: 41701, reason: "still linked to open #41751" }, + { kind: "closed", number: 41761, body: oneFixBody }, + ]); + expect(writes).toEqual([ + expect.stringContaining("POST /repos/BerriAI/litellm/issues/41760/comments"), + 'PATCH /repos/BerriAI/litellm/pulls/41760 {"state":"closed"}', + expect.stringContaining("POST /repos/BerriAI/litellm/issues/41761/comments"), + 'PATCH /repos/BerriAI/litellm/pulls/41761 {"state":"closed"}', + ]); + }); + + test("a sweep dry run lists what it would close and writes nothing", async () => { + const { api, writes } = fakeApi({ openPullRequests: [[openPr(41760)]] }); + const outcome = await sweep(api, { ...config, closeDryRun: true }, noPause); + expect(outcome.pullRequests.map((pullRequest) => pullRequest.kind)).toEqual(["closed"]); expect(writes).toEqual([]); }); }); @@ -218,10 +588,24 @@ describe("commentFixedIssue", () => { describe("readConfig", () => { const env = { GITHUB_TOKEN: "t", GITHUB_REPOSITORY: "BerriAI/litellm", ISSUE_NUMBER: "41750", DEFAULT_BRANCH: "main" }; - test("reads the four inputs and treats anything but the literal true as a real run", () => { - expect(readConfig(env)).toEqual({ token: "t", repo: "BerriAI/litellm", issueNumber: 41750, defaultBranch: "main", dryRun: false }); - expect(readConfig({ ...env, DRY_RUN: "true" }).dryRun).toBe(true); - expect(readConfig({ ...env, DRY_RUN: "false" }).dryRun).toBe(false); + test("reads the inputs and treats anything but the literal true as a real run for each gate", () => { + expect(readConfig(env)).toEqual({ + token: "t", + repo: "BerriAI/litellm", + defaultBranch: "main", + commentDryRun: false, + closeDryRun: false, + run: { kind: "issue", number: 41750 }, + }); + expect(readConfig({ ...env, DRY_RUN: "true" })).toMatchObject({ commentDryRun: true, closeDryRun: false }); + expect(readConfig({ ...env, CLOSE_PRS_DRY_RUN: "true" })).toMatchObject({ commentDryRun: false, closeDryRun: true }); + expect(readConfig({ ...env, DRY_RUN: "false", CLOSE_PRS_DRY_RUN: "false" })).toMatchObject({ commentDryRun: false, closeDryRun: false }); + }); + + test("a sweep needs no issue number and anything but the literal true is an issue run", () => { + expect(readConfig({ ...env, ISSUE_NUMBER: undefined, SWEEP: "true" }).run).toEqual({ kind: "sweep" }); + expect(readConfig({ ...env, SWEEP: "false" }).run).toEqual({ kind: "issue", number: 41750 }); + expect(() => readConfig({ ...env, ISSUE_NUMBER: "", SWEEP: "false" })).toThrow("dispatch with an issue_number or with sweep ticked"); }); test("refuses a missing token, repo, branch or a bad issue number", () => { @@ -232,3 +616,31 @@ describe("readConfig", () => { expect(() => readConfig({ ...env, ISSUE_NUMBER: "abc" })).toThrow("ISSUE_NUMBER"); }); }); + +describe("step summary", () => { + const closed = { kind: "closed" as const, number: 41760, body: oneFixBody }; + const left = { kind: "skip" as const, number: 41762, reason: "still linked to open #41751" }; + const commented = { kind: "commented" as const, pullRequest: 41767, tag: "v1.103.0-rc.1", body: fixedBody(41767, { tag: "v1.103.0-rc.1", shipped: false }) }; + + test("an issue run names the comment, each close, and each pull request left open", () => { + const summary = describeIssue(config, ISSUE, { comment: commented, pullRequests: [closed, left] }); + expect(summary).toContain("#41750: commented, fixed by #41767 in v1.103.0-rc.1"); + expect(summary).toContain("#41760: closed with: #41750 was fixed by #41767 on main"); + expect(summary).toContain("#41762: left open, still linked to open #41751"); + expect(summary).not.toContain("DRY RUN"); + }); + + test("a close dry run names the repo variable that turns closing on", () => { + const summary = describeIssue({ ...config, closeDryRun: true }, ISSUE, { comment: commented, pullRequests: [closed] }); + expect(summary).toContain("ISSUE_FIXED_CLOSE_PRS_ENABLED"); + expect(summary).toContain("#41760: DRY RUN, would close with: #41750 was fixed by #41767 on main"); + }); + + test("a sweep summary counts what it saw and lists only the closes", () => { + const summary = describeSweep(config, { considered: 3522, pullRequests: [closed, left] }); + expect(summary).toContain("Swept 3522 open pull requests, 2 linked to an issue, 1 closed"); + expect(summary).toContain("#41760: closed with:"); + expect(summary).not.toContain("#41762"); + expect(describeSweep({ ...config, closeDryRun: true }, { considered: 3522, pullRequests: [closed] })).toContain("1 would be closed, DRY RUN, set the ISSUE_FIXED_CLOSE_PRS_ENABLED"); + }); +}); diff --git a/scripts/comment-fixed-issue.ts b/scripts/comment-fixed-issue.ts index 480b5e90249..450d530d90c 100644 --- a/scripts/comment-fixed-issue.ts +++ b/scripts/comment-fixed-issue.ts @@ -6,35 +6,66 @@ declare const process: { readonly env: Readonly base.startsWith("release/") || base.includes("stable"); + +const CLOSURE_FRAGMENT = `fragment Closure on Issue { + state + timelineItems(last: 1, itemTypes: [CLOSED_EVENT]) { + nodes { + ... on ClosedEvent { + closer { + __typename + ... on PullRequest { number merged baseRefName repository { nameWithOwner } mergeCommit { oid } } + ... on Commit { oid repository { nameWithOwner } } } } } } }`; +const LINKED_PULL_REQUEST_FRAGMENT = `fragment Linked on PullRequest { + number + state + baseRefName + repository { nameWithOwner } + closingIssuesReferences(first: ${MAX_LINKED_ISSUES}) { totalCount nodes { number repository { nameWithOwner } ...Closure } } + reopens: timelineItems(last: 1, itemTypes: [REOPENED_EVENT]) { nodes { ... on ReopenedEvent { createdAt } } } +}`; + +export const CLOSER_QUERY = `query($owner: String!, $name: String!, $number: Int!, $after: String) { + repository(owner: $owner, name: $name) { + issue(number: $number) { + ...Closure + closedByPullRequestsReferences(first: ${MAX_LINKED_PULL_REQUESTS}, after: $after) { + pageInfo { hasNextPage endCursor } + nodes { ...Linked } + } + } + } +} +${CLOSURE_FRAGMENT} +${LINKED_PULL_REQUEST_FRAGMENT}`; + +export const OPEN_PULL_REQUESTS_QUERY = `query($owner: String!, $name: String!, $after: String) { + repository(owner: $owner, name: $name) { + pullRequests(states: OPEN, first: ${SWEEP_PAGE_SIZE}, after: $after) { + pageInfo { hasNextPage endCursor } + nodes { ...Linked } + } + } +} +${CLOSURE_FRAGMENT} +${LINKED_PULL_REQUEST_FRAGMENT}`; + const skip = (reason: string): { readonly kind: "skip"; readonly reason: string } => ({ kind: "skip", reason }); -export function closerOf(issue: ClosedIssue, defaultBranch: string): Closer { +export function closerOf(issue: IssueClosure, repo: string, defaultBranch: string): Closer { if (issue.state !== "CLOSED") { - return skip("the issue is open again"); + return skip(OPEN_AGAIN); } const closer = issue.timelineItems.nodes[0]?.closer ?? null; if (closer === null) { @@ -94,6 +192,9 @@ export function closerOf(issue: ClosedIssue, defaultBranch: string): Closer { if (closer.__typename === "Commit") { return skip(`closed by commit ${closer.oid.slice(0, 10)}, not by a pull request`); } + if (closer.repository.nameWithOwner !== repo) { + return skip(`closed by ${closer.repository.nameWithOwner}#${closer.number}, a pull request in another repository`); + } if (!closer.merged || closer.mergeCommit === null) { return skip(`closed by #${closer.number}, which is not merged`); } @@ -121,8 +222,8 @@ async function tagExists(api: GitHubApi, repo: string, tag: string): Promise ref.ref === `refs/tags/${tag}`); } -async function tagContains(api: GitHubApi, repo: string, tag: string, sha: string): Promise { - const comparison = await api.request("GET", `/repos/${repo}/compare/${tag}...${sha}`); +async function refContains(api: GitHubApi, repo: string, ref: string, sha: string): Promise { + const comparison = await api.request("GET", `/repos/${repo}/compare/${ref}...${sha}`); return comparison.status === "behind" || comparison.status === "identical"; } @@ -139,7 +240,7 @@ async function firstReleaseWith( if (!(await tagExists(api, repo, tag))) { return { kind: "release", tag, shipped: false }; } - if (await tagContains(api, repo, tag, sha)) { + if (await refContains(api, repo, tag, sha)) { return { kind: "release", tag, shipped: true }; } if (bumpsLeft === 0) { @@ -164,21 +265,13 @@ export function fixedBody(pullRequest: number, release: { readonly tag: string; return `${FIXED_MARKER}\nFixed by #${pullRequest}. ${availability}`; } -export async function commentFixedIssue(api: GitHubApi, config: FixedConfig): Promise { - const [owner, name] = config.repo.split("/"); - const response = await api.request("POST", "/graphql", { - query: CLOSER_QUERY, - variables: { owner, name, number: config.issueNumber }, - }); - const issue = response.data?.repository?.issue ?? null; - if (issue === null) { - return skip("not an issue in this repository"); - } - const closer = closerOf(issue, config.defaultBranch); - if (closer.kind === "skip") { - return closer; - } - const issuePath = `/repos/${config.repo}/issues/${config.issueNumber}`; +export async function commentFixedIssue( + api: GitHubApi, + config: FixedConfig, + issueNumber: number, + closer: { readonly number: number; readonly mergeCommit: string }, +): Promise { + const issuePath = `/repos/${config.repo}/issues/${issueNumber}`; const comments = await listAll(api, `${issuePath}/comments`); if (comments.some((comment) => comment.body.includes(FIXED_MARKER))) { return skip("already carries a fixed-in comment"); @@ -188,37 +281,259 @@ export async function commentFixedIssue(api: GitHubApi, config: FixedConfig): Pr return release; } const body = fixedBody(closer.number, release); - if (!config.dryRun) { + if (!config.commentDryRun) { await api.request("POST", `${issuePath}/comments`, { body }); } return { kind: "commented", pullRequest: closer.number, tag: release.tag, body }; } -export function readConfig(env: Readonly>): FixedConfig & { readonly token: string } { +export function fixOf(issue: IssueClosure, repo: string): FixVerdict { + if (issue.state !== "CLOSED") { + return skip("is open again"); + } + const closer = issue.timelineItems.nodes[0]?.closer ?? null; + if (closer === null) { + return skip("was closed by hand"); + } + if (closer.repository.nameWithOwner !== repo) { + return skip(`was closed from ${closer.repository.nameWithOwner}`); + } + if (closer.__typename === "Commit") { + return { kind: "commit", oid: closer.oid }; + } + if (!closer.merged || closer.mergeCommit === null) { + return skip(`was closed by #${closer.number}, which is not merged`); + } + return { kind: "pull_request", number: closer.number, oid: closer.mergeCommit.oid }; +} + +export function closeVerdict(pullRequest: LinkedPullRequest, config: FixedConfig): CloseVerdict { + if (pullRequest.repository.nameWithOwner !== config.repo) { + return skip(`lives in ${pullRequest.repository.nameWithOwner}`); + } + if (pullRequest.state !== "OPEN") { + return skip(`is ${pullRequest.state.toLowerCase()}`); + } + if (isReleaseLine(pullRequest.baseRefName)) { + return skip(`targets the release line ${pullRequest.baseRefName}`); + } + const { totalCount, nodes: linked } = pullRequest.closingIssuesReferences; + if (linked.length === 0) { + return skip("links no issue"); + } + if (totalCount > linked.length) { + return skip(`links ${totalCount} issues, more than the ${MAX_LINKED_ISSUES} this workflow reads`); + } + const foreign = linked.find((issue) => issue.repository.nameWithOwner !== config.repo); + if (foreign !== undefined) { + return skip(`links ${foreign.repository.nameWithOwner}#${foreign.number}`); + } + const stillOpen = linked.find((issue) => issue.state === "OPEN"); + if (stillOpen !== undefined) { + return skip(`still linked to open #${stillOpen.number}`); + } + const verdicts = linked.map((issue) => ({ issue: issue.number, fix: fixOf(issue, config.repo) })); + for (const { issue, fix } of verdicts) { + if (fix.kind === "skip") { + return skip(`#${issue} ${fix.reason}`); + } + } + return { + kind: "candidate", + fixes: verdicts.flatMap(({ issue, fix }) => (fix.kind === "skip" ? [] : [{ issue, source: fix }])), + }; +} + +function describeSource(source: FixSource): string { + return source.kind === "pull_request" ? `#${source.number}` : `commit ${source.oid.slice(0, 10)}`; +} + +async function fixOffDefaultBranch(api: GitHubApi, config: FixedConfig, fixes: readonly Fix[]): Promise { + const onBranch = await Promise.all(fixes.map((fix) => refContains(api, config.repo, config.defaultBranch, fix.source.oid))); + return fixes.find((_, index) => !onBranch[index]); +} + +export function supersededBody(fixes: readonly Fix[], defaultBranch: string): string { + const pairs = fixes + .map((fix, index) => `#${fix.issue} ${index === 0 ? "was fixed by" : "by"} ${describeSource(fix.source)}`) + .join(" and "); + return `${SUPERSEDED_MARKER}\n${pairs} on ${defaultBranch}, so this pull request is closed. Reopen it if something was missed.`; +} + +function reopenedAfter(pullRequest: LinkedPullRequest, comment: Comment): boolean { + const reopen = pullRequest.reopens.nodes[0]; + return reopen !== undefined && Date.parse(reopen.createdAt) > Date.parse(comment.created_at); +} + +async function closePullRequest( + api: GitHubApi, + config: FixedConfig, + pullRequest: LinkedPullRequest, + pause: () => Promise, +): Promise { + const verdict = closeVerdict(pullRequest, config); + if (verdict.kind === "skip") { + return { kind: "skip", number: pullRequest.number, reason: verdict.reason }; + } + const offBranch = await fixOffDefaultBranch(api, config, verdict.fixes); + if (offBranch !== undefined) { + const fix = describeSource(offBranch.source); + return { kind: "skip", number: pullRequest.number, reason: `#${offBranch.issue} was fixed by ${fix}, which is not on ${config.defaultBranch}` }; + } + const issuePath = `/repos/${config.repo}/issues/${pullRequest.number}`; + const comments = await listAll(api, `${issuePath}/comments`); + const marker = comments.find((comment) => comment.user.login === WORKFLOW_LOGIN && comment.body.includes(SUPERSEDED_MARKER)); + if (marker !== undefined && reopenedAfter(pullRequest, marker)) { + return { kind: "skip", number: pullRequest.number, reason: "was closed by this workflow once and reopened" }; + } + const body = marker?.body ?? supersededBody(verdict.fixes, config.defaultBranch); + if (config.closeDryRun) { + return { kind: "closed", number: pullRequest.number, body }; + } + if (marker === undefined) { + await pause(); + await api.request("POST", `${issuePath}/comments`, { body }); + } + await pause(); + await api.request("PATCH", `/repos/${config.repo}/pulls/${pullRequest.number}`, { state: "closed" }); + return { kind: "closed", number: pullRequest.number, body }; +} + +export function closePullRequests( + api: GitHubApi, + config: FixedConfig, + candidates: readonly LinkedPullRequest[], + pause: () => Promise, +): Promise { + return candidates.reduce>( + async (previous, candidate) => [...(await previous), await closePullRequest(api, config, candidate, pause)], + Promise.resolve([]), + ); +} + +type NextPage = (after: string | null) => Promise; + +async function collectPages(page: PullRequestsPage, nextPage: NextPage): Promise { + if (!page.pageInfo.hasNextPage) { + return page.nodes; + } + return [...page.nodes, ...(await collectPages(await nextPage(page.pageInfo.endCursor), nextPage))]; +} + +async function closedIssue(api: GitHubApi, config: FixedConfig, issueNumber: number, after: string | null): Promise { + const [owner, name] = config.repo.split("/"); + const response = await api.request("POST", "/graphql", { + query: CLOSER_QUERY, + variables: { owner, name, number: issueNumber, after }, + }); + return response.data?.repository?.issue ?? null; +} + +export async function handleFixedIssue( + api: GitHubApi, + config: FixedConfig, + issueNumber: number, + pause: () => Promise, +): Promise { + const issue = await closedIssue(api, config, issueNumber, null); + if (issue === null) { + return { comment: skip("not an issue in this repository"), pullRequests: [] }; + } + if (issue.state !== "CLOSED") { + return { comment: skip(OPEN_AGAIN), pullRequests: [] }; + } + const closer = closerOf(issue, config.repo, config.defaultBranch); + const comment = closer.kind === "skip" ? closer : await commentFixedIssue(api, config, issueNumber, closer); + const nextPage: NextPage = async (after) => { + const more = await closedIssue(api, config, issueNumber, after); + if (more === null) { + throw new Error(`#${issueNumber} came back without data while reading its linked pull requests after cursor ${after}`); + } + return more.closedByPullRequestsReferences; + }; + const linked = await collectPages(issue.closedByPullRequestsReferences, nextPage); + const open = linked.filter((pullRequest) => pullRequest.state === "OPEN"); + const pullRequests = await closePullRequests(api, config, open, pause); + return { comment, pullRequests }; +} + +async function openPullRequests(api: GitHubApi, config: FixedConfig): Promise { + const [owner, name] = config.repo.split("/"); + const nextPage: NextPage = async (after) => { + const response = await api.request("POST", "/graphql", { + query: OPEN_PULL_REQUESTS_QUERY, + variables: { owner, name, after }, + }); + const page = response.data?.repository?.pullRequests; + if (page === undefined) { + throw new Error(`open pull requests after cursor ${after} came back without data: ${JSON.stringify(response)}`); + } + return page; + }; + return collectPages(await nextPage(null), nextPage); +} + +export async function sweep(api: GitHubApi, config: FixedConfig, pause: () => Promise): Promise { + const open = await openPullRequests(api, config); + const linked = open.filter((pullRequest) => pullRequest.closingIssuesReferences.nodes.length > 0); + return { considered: open.length, pullRequests: await closePullRequests(api, config, linked, pause) }; +} + +export function readConfig( + env: Readonly>, +): FixedConfig & { readonly token: string; readonly run: Run } { const token = env.GITHUB_TOKEN; const repo = env.GITHUB_REPOSITORY; const defaultBranch = env.DEFAULT_BRANCH; if (!token || !repo || !/^[\w.-]+\/[\w.-]+$/.test(repo) || !defaultBranch) { throw new Error("GITHUB_TOKEN, GITHUB_REPOSITORY (owner/repo) and DEFAULT_BRANCH are required"); } + const config = { token, repo, defaultBranch, commentDryRun: env.DRY_RUN === "true", closeDryRun: env.CLOSE_PRS_DRY_RUN === "true" }; + if (env.SWEEP === "true") { + return { ...config, run: { kind: "sweep" } }; + } const issueNumber = Number(env.ISSUE_NUMBER); if (!Number.isInteger(issueNumber) || issueNumber <= 0) { - throw new Error(`ISSUE_NUMBER must be a positive integer, got "${env.ISSUE_NUMBER}"`); + throw new Error(`ISSUE_NUMBER must be a positive integer, got "${env.ISSUE_NUMBER}": dispatch with an issue_number or with sweep ticked`); } - return { token, repo, issueNumber, defaultBranch, dryRun: env.DRY_RUN === "true" }; + return { ...config, run: { kind: "issue", number: issueNumber } }; } -function describe(config: FixedConfig, verdict: FixedVerdict): string { - if (verdict.kind === "skip") { - return `#${config.issueNumber}: skipped, ${verdict.reason}`; +const CLOSE_DRY_RUN_HINT = "set the ISSUE_FIXED_CLOSE_PRS_ENABLED repo variable to true to close pull requests"; + +function describeClose(config: FixedConfig, outcome: CloseOutcome): string { + if (outcome.kind === "skip") { + return `#${outcome.number}: left open, ${outcome.reason}`; } - if (config.dryRun) { - return `#${config.issueNumber}: DRY RUN, set the ISSUE_FIXED_COMMENT_ENABLED repo variable to true to post this:\n\n${verdict.body}`; - } - return `#${config.issueNumber}: commented, fixed by #${verdict.pullRequest} in ${verdict.tag}`; + const text = outcome.body.replace(`${SUPERSEDED_MARKER}\n`, ""); + return config.closeDryRun ? `#${outcome.number}: DRY RUN, would close with: ${text}` : `#${outcome.number}: closed with: ${text}`; +} + +export function describeIssue(config: FixedConfig, issueNumber: number, outcome: IssueOutcome): string { + const comment = + outcome.comment.kind === "skip" + ? `#${issueNumber}: skipped, ${outcome.comment.reason}` + : config.commentDryRun + ? `#${issueNumber}: DRY RUN, set the ISSUE_FIXED_COMMENT_ENABLED repo variable to true to post this:\n\n${outcome.comment.body}` + : `#${issueNumber}: commented, fixed by #${outcome.comment.pullRequest} in ${outcome.comment.tag}`; + const hint = config.closeDryRun && outcome.pullRequests.some((pullRequest) => pullRequest.kind === "closed") ? [`Closing is a DRY RUN, ${CLOSE_DRY_RUN_HINT}`] : []; + return [comment, ...hint, ...outcome.pullRequests.map((pullRequest) => describeClose(config, pullRequest))].join("\n"); +} + +export function describeSweep(config: FixedConfig, outcome: SweepOutcome): string { + const closed = outcome.pullRequests.filter((pullRequest) => pullRequest.kind === "closed"); + const verb = config.closeDryRun ? `would be closed, DRY RUN, ${CLOSE_DRY_RUN_HINT}` : "closed"; + const header = `Swept ${outcome.considered} open pull requests, ${outcome.pullRequests.length} linked to an issue, ${closed.length} ${verb}`; + return [header, ...closed.map((pullRequest) => describeClose(config, pullRequest))].join("\n"); } if (import.meta.main) { - const { token, ...config } = readConfig(process.env); - console.log(describe(config, await commentFixedIssue(githubApi(token), config))); + const { token, run, ...config } = readConfig(process.env); + const api = githubApi(token); + const pause = (): Promise => new Promise((resolve) => setTimeout(resolve, CLOSE_PAUSE_MS)); + console.log( + run.kind === "sweep" + ? describeSweep(config, await sweep(api, config, pause)) + : describeIssue(config, run.number, await handleFixedIssue(api, config, run.number, pause)), + ); }