diff --git a/.github/actions/post-comment/action.yml b/.github/actions/post-comment/action.yml new file mode 100644 index 0000000000..951fd17042 --- /dev/null +++ b/.github/actions/post-comment/action.yml @@ -0,0 +1,31 @@ +name: "Post comment" +description: "Create or update a sticky PR comment identified by a caller-provided marker." +inputs: + marker: + description: "Non-empty marker identifying the comment. Use a unique, stable HTML comment for each distinct comment." + required: true + body: + description: "Full comment markdown." + required: true + issue-id: + description: "The issue or pull request number to post/update the comment on. Defaults to the current pull request if not provided." + required: false + token: + description: "The GitHub token to authenticate with." + required: true +runs: + using: composite + steps: + - name: Post comment + shell: bash + env: + BODY: ${{ inputs.body }} + MARKER: ${{ inputs.marker }} + GH_TOKEN: ${{ inputs.token }} + ISSUE_ID: ${{ inputs.issue-id || github.event.pull_request.number }} + run: | + npx tsx ./pr-checks/post-comment.ts \ + --body "$BODY" \ + --marker "$MARKER" \ + --issue-id "$ISSUE_ID" \ + --repository "$GITHUB_REPOSITORY" diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 2ae0594407..1e8b15ea60 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -92,6 +92,8 @@ jobs: contents: read runs-on: ubuntu-latest timeout-minutes: 10 + outputs: + changelog-modified: ${{ steps.changelog-check.outputs.modified }} concurrency: cancel-in-progress: ${{ github.event_name == 'pull_request' || false }} @@ -181,6 +183,20 @@ jobs: path: ${{ runner.temp }}/repo-size/ if-no-files-found: error + - name: Check for CHANGELOG.md changes + id: changelog-check + if: steps.fetch-base.outcome == 'success' + env: + BASE_SHA: ${{ steps.fetch-base.outputs.merge_base }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + if ! git diff --exit-code --quiet "$BASE_SHA" "$HEAD_SHA" -- CHANGELOG.md; then + echo "CHANGELOG.md was modified in this PR." + echo "modified=true" >> "$GITHUB_OUTPUT" + else + echo "modified=false" >> "$GITHUB_OUTPUT" + fi + - name: "Backport: Check out base ref" id: checkout-base if: ${{ startsWith(github.head_ref, 'backport-') }} @@ -201,8 +217,8 @@ jobs: exit 1 fi - post-repo-size-comment: - name: Post repo size comment + post-pr-comments: + name: Post PR comments needs: other-checks # Keep write permissions isolated from the job that checks out and tests PR code. This job only # posts the candidate comment body produced by the read-only `pr-checks` job. @@ -222,6 +238,25 @@ jobs: group: check-repo-size-${{ github.event.pull_request.number }} steps: + - name: Checkout repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + # Check out the base ref, not the PR's head, since this job runs with + # `pull-requests: write` permissions and must not execute untrusted code from the PR. + ref: ${{ github.event.pull_request.base.sha }} + sparse-checkout: | + pr-checks + .github/actions/post-comment + + - name: Set up Node.js + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: 24 + cache: "npm" + + - name: Install dependencies + run: npm ci --workspace=pr-checks + - name: Download repo size comment uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: @@ -251,3 +286,41 @@ jobs: else echo "Skipping repo size comment because the delta is below the threshold and no sticky comment exists." fi + + - name: Post a warning about modifying CHANGELOG.md + # Release, backport, mergeback, and bundle-update PRs deliberately modify CHANGELOG.md, + # so skip comment management entirely for these automation flows. + if: >- + needs.other-checks.outputs.changelog-modified == 'true' && + !startsWith(github.event.pull_request.head.ref, 'update-v') && + !startsWith(github.event.pull_request.head.ref, 'backport-v') && + !startsWith(github.event.pull_request.head.ref, 'mergeback/') && + !startsWith(github.event.pull_request.head.ref, 'update-bundle/') + uses: ./.github/actions/post-comment + with: + marker: "" + body: | + ⚠️ `CHANGELOG.md` has been modified in this PR. Please do not modify `CHANGELOG.md` directly. + Instead, create a change-note file in `unreleased-change-notes/`. The `CHANGELOG.md` + file will be automatically generated from those change-notes. + See [`unreleased-change-notes/README.md`](https://github.com/github/codeql-action/blob/main/unreleased-change-notes/README.md) for more information. + token: ${{ secrets.GITHUB_TOKEN }} + + - name: Mark the CHANGELOG.md concern as resolved + if: >- + needs.other-checks.outputs.changelog-modified != 'true' && + !startsWith(github.event.pull_request.head.ref, 'update-v') && + !startsWith(github.event.pull_request.head.ref, 'backport-v') && + !startsWith(github.event.pull_request.head.ref, 'mergeback/') && + !startsWith(github.event.pull_request.head.ref, 'update-bundle/') + uses: ./.github/actions/post-comment + with: + marker: "" + body: | + ⚠️ `CHANGELOG.md` has been modified in this PR. Please do not modify `CHANGELOG.md` directly. + Instead, create a change-note file in `unreleased-change-notes/`. The `CHANGELOG.md` + file will be automatically generated from those change-notes. + See [`unreleased-change-notes/README.md`](https://github.com/github/codeql-action/blob/main/unreleased-change-notes/README.md) for more information. + + **Edit:** `CHANGELOG.md` is no longer modified in this PR. This concern is resolved. + token: ${{ secrets.GITHUB_TOKEN }} diff --git a/pr-checks/post-comment.test.ts b/pr-checks/post-comment.test.ts new file mode 100644 index 0000000000..7babddcff3 --- /dev/null +++ b/pr-checks/post-comment.test.ts @@ -0,0 +1,270 @@ +#!/usr/bin/env npx tsx + +/* +Tests for post-comment.ts. +*/ + +import * as assert from "node:assert/strict"; +import { describe, it } from "node:test"; + +import { type ApiClient } from "./api-client"; +import { + findExistingComment, + type Options, + parseOptions, + resolveToken, + setComment, +} from "./post-comment"; + +/** A baseline set of valid CLI flags, as an array, for use in parseOptions tests. */ +const validFlags = [ + "--body", + "hello", + "--marker", + "", + "--issue-id", + "42", + "--repository", + "github/codeql-action", +]; + +/** Returns a copy of `validFlags` with the value following `flag` replaced by `value`. */ +function withFlag(flag: string, value: string): string[] { + const flags = [...validFlags]; + const index = flags.indexOf(flag); + flags[index + 1] = value; + return flags; +} + +/** Returns a copy of `validFlags` with `flag` (and its value) removed entirely. */ +function withoutFlag(flag: string): string[] { + const flags = [...validFlags]; + const index = flags.indexOf(flag); + flags.splice(index, 2); + return flags; +} + +describe("parseOptions", async () => { + await it("parses valid flags", () => { + const options = parseOptions(validFlags); + assert.deepEqual(options, { + body: "hello", + marker: "", + issueId: 42, + repository: { owner: "github", repo: "codeql-action" }, + }); + }); + + await it("rejects a missing --marker", () => { + assert.throws( + () => parseOptions(withoutFlag("--marker")), + /Missing required flag --marker/, + ); + }); + + await it("rejects an empty --marker", () => { + assert.throws( + () => parseOptions(withFlag("--marker", "")), + /Missing required flag --marker/, + ); + }); + + await it("rejects a whitespace-only --marker", () => { + assert.throws( + () => parseOptions(withFlag("--marker", " \t\n")), + /Missing required flag --marker/, + ); + }); +}); + +describe("resolveToken", async () => { + await it("reads the token from GH_TOKEN", () => { + assert.equal( + resolveToken({ GH_TOKEN: "gh-token", GITHUB_TOKEN: "gha-token" }), + "gh-token", + ); + }); + + await it("falls back to GITHUB_TOKEN", () => { + assert.equal(resolveToken({ GITHUB_TOKEN: "gha-token" }), "gha-token"); + }); + + await it("throws when no token is set", () => { + assert.throws(() => resolveToken({}), /Missing authentication token/); + }); +}); + +/** A minimal fake of the subset of `ApiClient` used by post-comment.ts. */ +function fakeClient(existingComments: Array<{ id: number; body?: string }>) { + const comments = existingComments.map((comment) => ({ ...comment })); + let nextId = + comments.reduce((maxId, comment) => Math.max(maxId, comment.id), 0) + 1; + + const client = { + paginate: async () => comments, + rest: { + issues: { + listComments: () => {}, + createComment: async ({ body }: { body: string }) => { + comments.push({ id: nextId++, body }); + }, + updateComment: async ({ + comment_id, + body, + }: { + comment_id: number; + body: string; + }) => { + const comment = comments.find(({ id }) => id === comment_id); + assert.ok(comment, `Comment ${comment_id} does not exist.`); + comment.body = body; + }, + }, + }, + }; + + return { client: client as unknown as ApiClient, comments }; +} + +describe("findExistingComment", async () => { + await it("finds the id of the first comment containing the marker", async () => { + const { client } = fakeClient([ + { id: 1, body: "unrelated" }, + { id: 2, body: "\nhello" }, + { id: 3, body: "\nanother" }, + ]); + const id = await findExistingComment( + client, + { owner: "owner", repo: "repo" }, + 1, + "", + ); + assert.equal(id, 2); + }); + + await it("returns undefined when no comment matches", async () => { + const { client } = fakeClient([{ id: 1, body: "unrelated" }]); + const id = await findExistingComment( + client, + { owner: "owner", repo: "repo" }, + 1, + "", + ); + assert.equal(id, undefined); + }); +}); + +/** A baseline set of `Options`, for use in `setComment` tests. */ +const baseOptions: Options = { + body: "hello", + marker: "", + issueId: 42, + repository: { owner: "owner", repo: "repo" }, +}; + +function assertCommentContent( + comment: { body?: string }, + marker: string, + body: string, +): void { + assert.ok(comment.body?.startsWith(marker)); + assert.ok(comment.body?.endsWith(body)); +} + +describe("setComment", async () => { + await it("uses the supplied marker without affecting other comments", async () => { + const marker = ""; + const { client, comments } = fakeClient([ + { id: 1, body: `${baseOptions.marker}\nunrelated warning` }, + { id: 2, body: `${marker}\nold warning` }, + ]); + const options = parseOptions(withFlag("--marker", marker)); + + await setComment(client, options); + + // Only the comment with the supplied marker changes. + assert.equal(comments.length, 2); + assert.deepEqual(comments[0], { + id: 1, + body: `${baseOptions.marker}\nunrelated warning`, + }); + assert.equal(comments[1].id, 2); + assertCommentContent(comments[1], marker, options.body); + }); + + await it("creates a new comment with the marker prepended", async () => { + const { client, comments } = fakeClient([]); + // No comment has the marker before posting. + assert.equal(comments.length, 0); + assert.ok( + !comments.some(({ body }) => body?.startsWith(baseOptions.marker)), + ); + + await setComment(client, baseOptions); + + // Posting adds exactly one comment with the marker before the body. + assert.equal(comments.length, 1); + assertCommentContent(comments[0], baseOptions.marker, baseOptions.body); + }); + + await it("creates a new comment when no existing comment matches", async () => { + const originalComments = [ + { id: 7, body: "unrelated" }, + { id: 2, body: "another unrelated comment" }, + ]; + const { client, comments } = fakeClient(originalComments); + // Existing comments do not match the marker. + assert.ok( + !comments.some(({ body }) => body?.startsWith(baseOptions.marker)), + ); + + await setComment(client, baseOptions); + + // A new comment is added without changing unrelated comments. + assert.equal(comments.length, originalComments.length + 1); + assert.deepEqual( + comments.slice(0, originalComments.length), + originalComments, + ); + const markedComments = comments.filter(({ body }) => + body?.startsWith(baseOptions.marker), + ); + // Exactly one comment contains the expected marker and body. + assert.equal(markedComments.length, 1); + assertCommentContent( + markedComments[0], + baseOptions.marker, + baseOptions.body, + ); + // The new comment's ID does not collide with existing IDs. + assert.equal(new Set(comments.map(({ id }) => id)).size, comments.length); + }); + + await it("updates the existing comment when one is found", async () => { + const { client, comments } = fakeClient([ + { id: 7, body: `${baseOptions.marker}\nold` }, + ]); + + await setComment(client, baseOptions); + + // Updating preserves the comment's ID without creating a duplicate. + assert.equal(comments.length, 1); + assert.equal(comments[0].id, 7); + assertCommentContent(comments[0], baseOptions.marker, baseOptions.body); + }); + + await it("replaces a warning with the resolved state using the same marker", async () => { + const { client, comments } = fakeClient([ + { id: 9, body: `${baseOptions.marker}\nold` }, + ]); + const body = + "`CHANGELOG.md` is not modified in this PR. No changelog action is needed."; + + await setComment(client, { ...baseOptions, body }); + + // The resolved state replaces the warning on the same marked comment. + assert.equal(comments.length, 1); + assert.equal(comments[0].id, 9); + assertCommentContent(comments[0], baseOptions.marker, body); + }); +}); diff --git a/pr-checks/post-comment.ts b/pr-checks/post-comment.ts new file mode 100644 index 0000000000..3f8abe40bd --- /dev/null +++ b/pr-checks/post-comment.ts @@ -0,0 +1,141 @@ +#!/usr/bin/env npx tsx + +/* +Generic CLI for creating or updating a "sticky" PR comment identified by a hidden +HTML-comment marker. This centralizes the gh api logic previously duplicated across PR-check +workflow steps that each managed their own marked comment (e.g. a CHANGELOG.md modification +warning). + +The marker is passed explicitly via `--marker` and can be any non-empty string, though an +HTML comment (e.g. ``) is recommended so it doesn't render visibly. It is used +both to find a pre-existing comment on the pull request to update, and is automatically prepended to +`--body` when posting or updating a comment so that future invocations can find it again. +The `.github/actions/post-comment` action requires callers to supply this value via its +`marker` input. +*/ + +import assert from "node:assert/strict"; +import { parseArgs } from "node:util"; + +import { type ApiClient, getApiClient } from "./api-client"; + +/** Represents the command-line options. */ +export interface Options { + /** The full comment body. */ + body: string; + /** The hidden HTML-comment marker used to find a pre-existing comment. */ + marker: string; + /** The issue/pull request number to post/update the comment on. */ + issueId: number; + /** The repository to operate on. */ + repository: { owner: string; repo: string }; +} + +/** Parses CLI arguments supplied by the composite action into `Options`. */ +export function parseOptions(argv?: string[]): Options { + const { values } = parseArgs({ + args: argv, + options: { + body: { type: "string" }, + marker: { type: "string" }, + "issue-id": { type: "string" }, + repository: { type: "string" }, + }, + strict: true, + }); + + assert(values.marker?.trim(), "Missing required flag --marker."); + const [owner, repo] = values.repository!.split("/"); + + return { + body: values.body!, + marker: values.marker!, + issueId: Number(values["issue-id"]), + repository: { owner, repo }, + }; +} + +/** Resolves the GitHub API token to use, from `GH_TOKEN` or `GITHUB_TOKEN`. */ +export function resolveToken(env: NodeJS.ProcessEnv): string { + const token = env.GH_TOKEN?.trim() || env.GITHUB_TOKEN?.trim(); + if (!token) { + throw new Error( + "Missing authentication token. Set GH_TOKEN or GITHUB_TOKEN.", + ); + } + return token; +} + +/** + * Finds the first comment on the given issue/pull request whose body starts with `marker`, if any. + */ +export async function findExistingComment( + client: ApiClient, + repository: { owner: string; repo: string }, + issueNumber: number, + marker: string, +): Promise { + const comments = await client.paginate(client.rest.issues.listComments, { + ...repository, + issue_number: issueNumber, + per_page: 100, + }); + return comments.find((comment) => comment.body?.startsWith(marker))?.id; +} + +/** + * Creates or updates a marked comment using `client` to talk to the GitHub API. + */ +export async function setComment( + client: ApiClient, + options: Options, +): Promise { + const { issueId, marker, repository } = options; + const body = `${marker}\n${options.body}`; + + const existingCommentId = await findExistingComment( + client, + repository, + issueId, + marker, + ); + if (existingCommentId === undefined) { + console.info("No existing comment found; creating a new comment."); + await client.rest.issues.createComment({ + ...repository, + issue_number: issueId, + body, + }); + } else { + console.info(`Updating existing comment ${existingCommentId}.`); + await client.rest.issues.updateComment({ + ...repository, + comment_id: existingCommentId, + body, + }); + } +} + +async function main(): Promise { + const options = parseOptions(); + + const token = resolveToken(process.env); + const client = getApiClient(token); + + await setComment(client, options); + + return 0; +} + +async function run(): Promise { + try { + process.exit(await main()); + } catch (err) { + console.error(err instanceof Error ? err.message : String(err)); + process.exit(1); + } +} + +if (import.meta.main) { + void run(); +}