From a20698f802585b7bef5d3abe6ebc32935e5214d2 Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Fri, 18 Sep 2026 17:37:02 -0700 Subject: [PATCH] ci(issues): comment which release carries the fix when a pull request closes an issue --- .github/workflows/issue_fixed_comment.yml | 71 +++++++ scripts/comment-fixed-issue.test.ts | 234 ++++++++++++++++++++++ scripts/comment-fixed-issue.ts | 224 +++++++++++++++++++++ 3 files changed, 529 insertions(+) create mode 100644 .github/workflows/issue_fixed_comment.yml create mode 100644 scripts/comment-fixed-issue.test.ts create mode 100644 scripts/comment-fixed-issue.ts diff --git a/.github/workflows/issue_fixed_comment.yml b/.github/workflows/issue_fixed_comment.yml new file mode 100644 index 00000000000..92993d319a7 --- /dev/null +++ b/.github/workflows/issue_fixed_comment.yml @@ -0,0 +1,71 @@ +name: Issue fixed comment + +on: + issues: + types: [closed] + workflow_dispatch: + inputs: + issue_number: + description: "Closed issue number to comment on manually." + required: true + pull_request: + paths: + - .github/workflows/issue_fixed_comment.yml + - scripts/comment-fixed-issue.ts + - scripts/comment-fixed-issue.test.ts + - scripts/auto-close-duplicates.ts + +permissions: {} + +concurrency: + group: issue-fixed-comment-${{ github.event.issue.number || github.event.inputs.issue_number || github.run_id }} + cancel-in-progress: false + +jobs: + comment-fixed-issue-tests: + if: github.event_name == 'pull_request' + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + steps: + - name: Checkout repository + uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0 + with: + persist-credentials: false + + - name: Setup Bun + uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 + with: + bun-version: "1.4.0" + + - name: Test the closer lookup, the release placement and the comment + 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 + permissions: + contents: read + issues: write + steps: + - name: Checkout scripts + uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0 + with: + sparse-checkout: scripts + persist-credentials: false + + - 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 + bun-version: "1.4.0" + + - name: Name the release that carries the fix + 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 }} + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} + DRY_RUN: ${{ vars.ISSUE_FIXED_COMMENT_ENABLED != 'true' }} diff --git a/scripts/comment-fixed-issue.test.ts b/scripts/comment-fixed-issue.test.ts new file mode 100644 index 00000000000..f9cd41d96d8 --- /dev/null +++ b/scripts/comment-fixed-issue.test.ts @@ -0,0 +1,234 @@ +import { describe, expect, test } from "bun:test"; + +import type { Comment, GitHubApi } from "./auto-close-duplicates"; +import { + FIXED_MARKER, + closerOf, + commentFixedIssue, + fixedBody, + nextMinor, + parseVersion, + placement, + readConfig, + releaseCandidate, + type ClosedIssue, + type FixedConfig, +} from "./comment-fixed-issue"; + +const MERGE_COMMIT = "68c4c82ac977b48b2b81ee8d633d5771307c6162"; + +const mergedPr = { + __typename: "PullRequest" as const, + number: 41767, + merged: true, + baseRefName: "main", + mergeCommit: { oid: MERGE_COMMIT }, +}; + +type Closer = ClosedIssue["timelineItems"]["nodes"][number]["closer"]; + +const closedBy = (closer: Closer, state: ClosedIssue["state"] = "CLOSED"): ClosedIssue => ({ + state, + timelineItems: { nodes: [{ closer }] }, +}); + +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 }; + +interface World { + readonly issue?: ClosedIssue | null; + readonly comments?: readonly Comment[]; + readonly version?: string; + // Which existing rc.1 tags contain the merge commit; a tag absent from the map does not exist + readonly tags?: Readonly>; +} + +function fakeApi(world: World = {}): { readonly api: GitHubApi; readonly writes: string[] } { + const writes: string[] = []; + const tags = world.tags ?? {}; + 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; + } + 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; + } + 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; + } + const matching = /^\/repos\/BerriAI\/litellm\/git\/matching-refs\/tags\/(.+)$/.exec(path); + if (matching !== null) { + return (matching[1] in tags ? [{ ref: `refs/tags/${matching[1]}` }] : []) as T; + } + const compare = /^\/repos\/BerriAI\/litellm\/compare\/(.+)\.\.\.(.+)$/.exec(path); + if (compare !== null && compare[2] === MERGE_COMMIT) { + return { status: tags[compare[1]] ? "behind" : "ahead" } as T; + } + throw new Error(`unexpected ${method} ${path}`); + }, + }; + return { api, 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 }); + }); + + 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"); + }); + + 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"); + 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" }); + }); +}); + +describe("version helpers", () => { + test("parseVersion reads the project version and ignores everything else", () => { + expect(parseVersion(pyproject("1.103.0"))).toBe("1.103.0"); + expect(parseVersion('[project]\nversion = "1.103.0rc1"\n')).toBeUndefined(); + expect(parseVersion("[project]\nname = 'litellm'\n")).toBeUndefined(); + }); + + test("the first rc of a version is the release that carries a fix merged under it", () => { + expect(releaseCandidate("1.103.0")).toBe("v1.103.0-rc.1"); + }); + + test("nextMinor bumps the minor and resets the patch", () => { + expect(nextMinor("1.103.0")).toBe("1.104.0"); + expect(nextMinor("1.99.4")).toBe("1.100.0"); + }); +}); + +describe("placement", () => { + test("no rc yet: the fix ships in the rc.1 of the version at the merge commit", async () => { + const { api } = fakeApi({ version: "1.103.0" }); + expect(await placement(api, "BerriAI/litellm", MERGE_COMMIT)).toEqual({ kind: "release", tag: "v1.103.0-rc.1", shipped: false }); + }); + + test("rc.1 already cut with the commit in it: the fix is out", async () => { + const { api } = fakeApi({ version: "1.102.0", tags: { "v1.102.0-rc.1": true } }); + expect(await placement(api, "BerriAI/litellm", MERGE_COMMIT)).toEqual({ kind: "release", tag: "v1.102.0-rc.1", shipped: true }); + }); + + test("rc.1 cut before the merge while main still said that version: the fix waits for the next minor", async () => { + const { api } = fakeApi({ version: "1.102.0", tags: { "v1.102.0-rc.1": false } }); + expect(await placement(api, "BerriAI/litellm", MERGE_COMMIT)).toEqual({ kind: "release", tag: "v1.103.0-rc.1", shipped: false }); + }); + + test("keeps walking minors while each rc.1 exists without the commit, then gives up", async () => { + const twoTaken = fakeApi({ version: "1.102.0", tags: { "v1.102.0-rc.1": false, "v1.103.0-rc.1": false } }); + expect(await placement(twoTaken.api, "BerriAI/litellm", MERGE_COMMIT)).toEqual({ kind: "release", tag: "v1.104.0-rc.1", shipped: false }); + + const allTaken = fakeApi({ + version: "1.102.0", + tags: { "v1.102.0-rc.1": false, "v1.103.0-rc.1": false, "v1.104.0-rc.1": false, "v1.105.0-rc.1": false }, + }); + expect((await placement(allTaken.api, "BerriAI/litellm", MERGE_COMMIT)).kind).toBe("skip"); + }); + + test("a pyproject without a version line is a skip, not a comment", async () => { + const { api } = fakeApi({ version: "not-a-version" }); + expect((await placement(api, "BerriAI/litellm", MERGE_COMMIT)).kind).toBe("skip"); + }); +}); + +describe("fixedBody", () => { + test("names the pull request and the first release, and carries the marker the rerun looks for", () => { + const body = fixedBody(41767, { tag: "v1.103.0-rc.1", shipped: false }); + expect(body.startsWith(FIXED_MARKER)).toBe(true); + expect(body).toContain("Fixed by #41767."); + expect(body).toContain("ships in v1.103.0-rc.1 and up"); + expect(body).toContain("dev pre-release"); + }); + + test("a release that is already out says so instead of promising one", () => { + const body = fixedBody(41767, { tag: "v1.102.0-rc.1", shipped: true }); + expect(body).toContain("is in v1.102.0-rc.1 and up"); + expect(body).not.toContain("ships in"); + }); + + test("stays within the 25-word comment rule either way", () => { + for (const shipped of [true, false]) { + const words = fixedBody(41767, { tag: "v1.103.0-rc.1", shipped }).replace(FIXED_MARKER, "").trim().split(/\s+/); + expect(words.length).toBeGreaterThanOrEqual(15); + expect(words.length).toBeLessThanOrEqual(25); + } + }); +}); + +describe("commentFixedIssue", () => { + 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" }); + 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("an issue that already carries the comment is not commented twice", 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" }); + 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"); + 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" }); + expect(writes).toEqual([]); + }); +}); + +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("refuses a missing token, repo, branch or a bad issue number", () => { + expect(() => readConfig({ ...env, GITHUB_TOKEN: undefined })).toThrow("GITHUB_TOKEN"); + expect(() => readConfig({ ...env, GITHUB_REPOSITORY: "litellm" })).toThrow("owner/repo"); + expect(() => readConfig({ ...env, DEFAULT_BRANCH: "" })).toThrow("DEFAULT_BRANCH"); + expect(() => readConfig({ ...env, ISSUE_NUMBER: "0" })).toThrow("ISSUE_NUMBER"); + expect(() => readConfig({ ...env, ISSUE_NUMBER: "abc" })).toThrow("ISSUE_NUMBER"); + }); +}); diff --git a/scripts/comment-fixed-issue.ts b/scripts/comment-fixed-issue.ts new file mode 100644 index 00000000000..480b5e90249 --- /dev/null +++ b/scripts/comment-fixed-issue.ts @@ -0,0 +1,224 @@ +#!/usr/bin/env bun + +import { githubApi, listAll, type Comment, type GitHubApi } from "./auto-close-duplicates"; + +declare const process: { readonly env: Readonly> }; + +export interface FixedConfig { + readonly repo: string; + readonly issueNumber: number; + readonly defaultBranch: string; + readonly dryRun: boolean; +} + +interface PullRequestCloser { + readonly __typename: "PullRequest"; + readonly number: number; + readonly merged: boolean; + readonly baseRefName: string; + readonly mergeCommit: { readonly oid: string } | null; +} + +interface CommitCloser { + readonly __typename: "Commit"; + readonly oid: string; +} + +export interface ClosedIssue { + readonly state: "OPEN" | "CLOSED"; + readonly timelineItems: { + readonly nodes: readonly { readonly closer: PullRequestCloser | CommitCloser | null }[]; + }; +} + +interface TimelineResponse { + readonly data?: { readonly repository?: { readonly issue: ClosedIssue | null } }; +} + +interface MatchingRef { + readonly ref: string; +} + +interface Comparison { + readonly status: "ahead" | "behind" | "identical" | "diverged"; +} + +interface FileContent { + readonly content: string; +} + +export type Closer = + | { readonly kind: "pull_request"; readonly number: number; readonly mergeCommit: string } + | { readonly kind: "skip"; readonly reason: string }; + +export type Placement = + | { readonly kind: "release"; readonly tag: string; readonly shipped: boolean } + | { readonly kind: "skip"; readonly reason: string }; + +export type FixedVerdict = + | { readonly kind: "commented"; readonly pullRequest: number; readonly tag: string; readonly body: string } + | { readonly kind: "skip"; readonly reason: string }; + +export const FIXED_MARKER = ""; +const MAX_MINOR_BUMPS = 3; + +export const CLOSER_QUERY = `query($owner: String!, $name: String!, $number: Int!) { + repository(owner: $owner, name: $name) { + issue(number: $number) { + state + timelineItems(last: 1, itemTypes: [CLOSED_EVENT]) { + nodes { + ... on ClosedEvent { + closer { + __typename + ... on PullRequest { number merged baseRefName mergeCommit { oid } } + ... on Commit { oid } + } + } + } + } + } + } +}`; + +const skip = (reason: string): { readonly kind: "skip"; readonly reason: string } => ({ kind: "skip", reason }); + +export function closerOf(issue: ClosedIssue, defaultBranch: string): Closer { + if (issue.state !== "CLOSED") { + return skip("the issue is open again"); + } + const closer = issue.timelineItems.nodes[0]?.closer ?? null; + if (closer === null) { + return skip("closed by hand, not by a pull request"); + } + if (closer.__typename === "Commit") { + return skip(`closed by commit ${closer.oid.slice(0, 10)}, not by a pull request`); + } + if (!closer.merged || closer.mergeCommit === null) { + return skip(`closed by #${closer.number}, which is not merged`); + } + if (closer.baseRefName !== defaultBranch) { + return skip(`#${closer.number} merged into ${closer.baseRefName}, not ${defaultBranch}`); + } + return { kind: "pull_request", number: closer.number, mergeCommit: closer.mergeCommit.oid }; +} + +export function parseVersion(pyproject: string): string | undefined { + return /^version = "(\d+\.\d+\.\d+)"$/m.exec(pyproject)?.[1]; +} + +export function releaseCandidate(version: string): string { + return `v${version}-rc.1`; +} + +export function nextMinor(version: string): string { + const [major, minor] = version.split(".").map(Number); + return `${major}.${minor + 1}.0`; +} + +async function tagExists(api: GitHubApi, repo: string, tag: string): Promise { + const refs = await api.request("GET", `/repos/${repo}/git/matching-refs/tags/${tag}`); + return refs.some((ref) => 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}`); + return comparison.status === "behind" || comparison.status === "identical"; +} + +// The first rc of a version is cut straight from main, so a fix merged while pyproject says X.Y.Z ships in +// vX.Y.Z-rc.1 unless that rc was already cut without it, in which case it waits for the next minor's rc.1 +async function firstReleaseWith( + api: GitHubApi, + repo: string, + sha: string, + version: string, + bumpsLeft: number, +): Promise { + const tag = releaseCandidate(version); + if (!(await tagExists(api, repo, tag))) { + return { kind: "release", tag, shipped: false }; + } + if (await tagContains(api, repo, tag, sha)) { + return { kind: "release", tag, shipped: true }; + } + if (bumpsLeft === 0) { + return skip(`${tag} exists without ${sha.slice(0, 10)} and the next ${MAX_MINOR_BUMPS} rc.1 tags are taken too`); + } + return firstReleaseWith(api, repo, sha, nextMinor(version), bumpsLeft - 1); +} + +export async function placement(api: GitHubApi, repo: string, mergeCommit: string): Promise { + const file = await api.request("GET", `/repos/${repo}/contents/pyproject.toml?ref=${mergeCommit}`); + const version = parseVersion(atob(file.content.replace(/\n/g, ""))); + if (version === undefined) { + return skip(`pyproject.toml at ${mergeCommit.slice(0, 10)} has no version line`); + } + return firstReleaseWith(api, repo, mergeCommit, version, MAX_MINOR_BUMPS); +} + +export function fixedBody(pullRequest: number, release: { readonly tag: string; readonly shipped: boolean }): string { + const availability = release.shipped + ? `This is in ${release.tag} and up, so upgrading to that release or any newer one picks it up.` + : `This ships in ${release.tag} and up, and the next dev pre-release cut from main will carry it too.`; + 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}`; + const comments = await listAll(api, `${issuePath}/comments`); + if (comments.some((comment) => comment.body.includes(FIXED_MARKER))) { + return skip("already carries a fixed-in comment"); + } + const release = await placement(api, config.repo, closer.mergeCommit); + if (release.kind === "skip") { + return release; + } + const body = fixedBody(closer.number, release); + if (!config.dryRun) { + 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 } { + 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 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}"`); + } + return { token, repo, issueNumber, defaultBranch, dryRun: env.DRY_RUN === "true" }; +} + +function describe(config: FixedConfig, verdict: FixedVerdict): string { + if (verdict.kind === "skip") { + return `#${config.issueNumber}: skipped, ${verdict.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}`; +} + +if (import.meta.main) { + const { token, ...config } = readConfig(process.env); + console.log(describe(config, await commentFixedIssue(githubApi(token), config))); +}