diff --git a/.github/actions/post-comment/action.yml b/.github/actions/post-comment/action.yml new file mode 100644 index 0000000000..f2233bc97e --- /dev/null +++ b/.github/actions/post-comment/action.yml @@ -0,0 +1,46 @@ +name: "Post comment" +description: "Create, update, or delete a sticky PR comment identified by an automatically-derived HTML-comment marker. If this action is used more than once within the same job, give each step an explicit 'id:' so the derived marker stays unique." +inputs: + body: + description: "Full comment markdown." + required: true + action-condition: + description: "The business condition ('true' or 'false') that selects which action to perform." + required: true + action-if-true: + description: "Action to perform when action-condition is 'true'. One of: none, insert, upsert, delete." + required: true + action-if-false: + description: "Action to perform when action-condition is 'false'. One of: none, insert, upsert, delete." + required: true + issue-id: + description: "The issue or pull request number to post/update/delete 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 }} + # Derived from the workflow name, job id, and step id/sequence-number so that repeated + # runs of the same step find the same comment (for upsert/delete), while distinct steps + # (even within the same job) get distinct markers. + MARKER: + GH_TOKEN: ${{ inputs.token }} + ISSUE_ID: ${{ inputs.issue-id || github.event.pull_request.number }} + ACTION_CONDITION: ${{ inputs.action-condition }} + ACTION_IF_TRUE: ${{ inputs.action-if-true }} + ACTION_IF_FALSE: ${{ inputs.action-if-false }} + run: | + npx tsx ./pr-checks/post-comment.ts \ + --body "$BODY" \ + --marker "$MARKER" \ + --action-condition "$ACTION_CONDITION" \ + --action-if-true "$ACTION_IF_TRUE" \ + --action-if-false "$ACTION_IF_FALSE" \ + --issue-id "$ISSUE_ID" \ + --repository "$GITHUB_REPOSITORY" diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 2ae0594407..1fbab56124 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-changed: ${{ steps.changelog-check.outputs.changed }} 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 "changed=true" >> "$GITHUB_OUTPUT" + else + echo "changed=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,24 @@ 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 + id: post-changelog-warning + # Release, backport, mergeback, and bundle-update PRs deliberately modify CHANGELOG.md, + # so skip comment management entirely for these automation flows. + if: >- + !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: + 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. + action-condition: ${{ needs.other-checks.outputs.changelog-changed }} + action-if-true: upsert + action-if-false: delete + 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..db6d455393 --- /dev/null +++ b/pr-checks/post-comment.test.ts @@ -0,0 +1,299 @@ +#!/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, + performAction, + resolveToken, +} from "./post-comment"; + +/** A baseline set of valid CLI flags, as an array, for use in parseOptions tests. */ +const validFlags = [ + "--body", + "hello", + "--marker", + "", + "--action-condition", + "true", + "--action-if-true", + "insert", + "--action-if-false", + "delete", + "--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: "", + actionCondition: true, + actionIfTrue: "insert", + actionIfFalse: "delete", + issueId: 42, + repository: { owner: "github", repo: "codeql-action" }, + }); + }); + + await it("parses action-condition=false", () => { + const options = parseOptions(withFlag("--action-condition", "false")); + assert.equal(options.actionCondition, false); + }); + + await it("rejects a missing --body", () => { + assert.throws( + () => parseOptions(withoutFlag("--body")), + /Missing required flag --body/, + ); + }); + + 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 an invalid --action-condition value", () => { + assert.throws( + () => parseOptions(withFlag("--action-condition", "yes")), + /Invalid value 'yes' for --action-condition/, + ); + }); + + await it("rejects an invalid --action-if-true value", () => { + assert.throws( + () => parseOptions(withFlag("--action-if-true", "bogus")), + /Invalid value 'bogus' for --action-if-true/, + ); + }); + + await it("rejects an invalid --action-if-false value", () => { + assert.throws( + () => parseOptions(withFlag("--action-if-false", "bogus")), + /Invalid value 'bogus' for --action-if-false/, + ); + }); + + await it("rejects a missing --issue-id", () => { + assert.throws( + () => parseOptions(withoutFlag("--issue-id")), + /Missing required flag --issue-id/, + ); + }); + + await it("rejects a non-numeric --issue-id", () => { + assert.throws( + () => parseOptions(withFlag("--issue-id", "not-a-number")), + /Invalid value 'not-a-number' for --issue-id/, + ); + }); + + await it("rejects a missing --repository", () => { + assert.throws( + () => parseOptions(withoutFlag("--repository")), + /Missing required flag --repository/, + ); + }); + + await it("rejects a --repository that isn't in the form 'owner/repo'", () => { + assert.throws( + () => parseOptions(withFlag("--repository", "not-a-slug")), + /Invalid value 'not-a-slug' for --repository/, + ); + }); +}); + +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 calls: { + createComment: unknown[]; + updateComment: unknown[]; + deleteComment: unknown[]; + } = { createComment: [], updateComment: [], deleteComment: [] }; + + const client = { + paginate: async () => existingComments, + rest: { + issues: { + listComments: () => {}, + createComment: async (params: unknown) => { + calls.createComment.push(params); + }, + updateComment: async (params: unknown) => { + calls.updateComment.push(params); + }, + deleteComment: async (params: unknown) => { + calls.deleteComment.push(params); + }, + }, + }, + }; + + return { client: client as unknown as ApiClient, calls }; +} + +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 `performAction` tests. */ +const baseOptions: Options = { + body: "hello", + marker: "", + actionCondition: true, + actionIfTrue: "insert", + actionIfFalse: "delete", + issueId: 42, + repository: { owner: "owner", repo: "repo" }, +}; + +describe("performAction", async () => { + await it("does nothing for 'none'", async () => { + const { client, calls } = fakeClient([]); + await performAction(client, "none", baseOptions); + assert.deepEqual(calls, { + createComment: [], + updateComment: [], + deleteComment: [], + }); + }); + + await it("creates a new comment for 'insert', with the marker prepended", async () => { + const { client, calls } = fakeClient([]); + await performAction(client, "insert", baseOptions); + assert.deepEqual(calls.createComment, [ + { + owner: "owner", + repo: "repo", + issue_number: 42, + body: "\nhello", + }, + ]); + assert.deepEqual(calls.updateComment, []); + assert.deepEqual(calls.deleteComment, []); + }); + + await it("creates a new comment for 'upsert' when no existing comment is found", async () => { + const { client, calls } = fakeClient([{ id: 1, body: "unrelated" }]); + await performAction(client, "upsert", baseOptions); + assert.deepEqual(calls.createComment, [ + { + owner: "owner", + repo: "repo", + issue_number: 42, + body: "\nhello", + }, + ]); + assert.deepEqual(calls.updateComment, []); + }); + + await it("updates the existing comment for 'upsert' when one is found", async () => { + const { client, calls } = fakeClient([ + { id: 7, body: "\nold" }, + ]); + await performAction(client, "upsert", baseOptions); + assert.deepEqual(calls.updateComment, [ + { + owner: "owner", + repo: "repo", + comment_id: 7, + body: "\nhello", + }, + ]); + assert.deepEqual(calls.createComment, []); + }); + + await it("does nothing for 'delete' when no existing comment is found", async () => { + const { client, calls } = fakeClient([{ id: 1, body: "unrelated" }]); + await performAction(client, "delete", baseOptions); + assert.deepEqual(calls.deleteComment, []); + }); + + await it("deletes the existing comment for 'delete' when one is found", async () => { + const { client, calls } = fakeClient([ + { id: 9, body: "\nold" }, + ]); + await performAction(client, "delete", baseOptions); + assert.deepEqual(calls.deleteComment, [ + { owner: "owner", repo: "repo", comment_id: 9 }, + ]); + }); +}); diff --git a/pr-checks/post-comment.ts b/pr-checks/post-comment.ts new file mode 100644 index 0000000000..6c1a1e5e54 --- /dev/null +++ b/pr-checks/post-comment.ts @@ -0,0 +1,267 @@ +#!/usr/bin/env npx tsx + +/* +Generic CLI for creating, updating, or deleting 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 so that +`upsert`/`delete` actions know which comment to operate on, and is automatically prepended to +`--body` when posting or updating a comment so that future invocations can find it again. Callers +typically derive this value automatically (see `.github/actions/post-comment`) rather than +hand-authoring it. +*/ + +import { parseArgs } from "node:util"; + +import { type ApiClient, getApiClient } from "./api-client"; + +/** The set of valid actions that can be performed against a (possibly pre-existing) marked comment. */ +export const ACTIONS = ["none", "insert", "upsert", "delete"] as const; +/** An action to perform against a (possibly pre-existing) marked comment. */ +export type Action = (typeof ACTIONS)[number]; + +/** 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 condition that selects which action to perform. */ + actionCondition: boolean; + /** The action to perform when `actionCondition` is `true`. */ + actionIfTrue: Action; + /** The action to perform when `actionCondition` is `false`. */ + actionIfFalse: Action; + /** The issue/pull request number to post/update/delete the comment on. */ + issueId: number; + /** The repository to operate on. */ + repository: { owner: string; repo: string }; +} + +/** + * Validates and parses a required CLI flag's raw string `value`, delegating the actual + * value-specific validation/conversion to `parse`. + */ +function parseFlag( + flag: string, + value: string | undefined, + parse: (value: string) => T, +): T { + if (value === undefined) { + throw new Error(`Missing required flag --${flag}.`); + } + try { + return parse(value); + } catch (err) { + const reason = err instanceof Error ? err.message : String(err); + throw new Error(`Invalid value '${value}' for --${flag}. ${reason}`); + } +} + +/** Parses and validates `value` as an `Action`. */ +function parseAction(flag: string, value: string | undefined): Action { + return parseFlag(flag, value, (raw) => { + if (!(ACTIONS as readonly string[]).includes(raw)) { + throw new Error(`Must be one of: ${ACTIONS.join(", ")}.`); + } + return raw as Action; + }); +} + +/** Parses and validates `value` as a boolean. */ +function parseBoolean(flag: string, value: string | undefined): boolean { + return parseFlag(flag, value, (raw) => { + if (raw !== "true" && raw !== "false") { + throw new Error("Must be 'true' or 'false'."); + } + return raw === "true"; + }); +} + +/** Parses and validates `value` as an issue/pull request number. */ +function parseIssueId(value: string | undefined): number { + return parseFlag("issue-id", value, (raw) => { + const parsed = Number(raw); + if (!Number.isInteger(parsed) || parsed <= 0) { + throw new Error("Must be a positive integer."); + } + return parsed; + }); +} + +/** Parses and validates `value` as an `owner/repo` repository reference. */ +function parseRepository(value: string | undefined): { + owner: string; + repo: string; +} { + return parseFlag("repository", value, (raw) => { + const [owner, repo, ...rest] = raw.split("/"); + if (!owner || !repo || rest.length > 0) { + throw new Error("Must be in the form 'owner/repo'."); + } + return { owner, repo }; + }); +} + +/** Parses the command-line arguments into `Options`. */ +export function parseOptions(argv?: string[]): Options { + const { values } = parseArgs({ + args: argv, + options: { + body: { type: "string" }, + marker: { type: "string" }, + "action-condition": { type: "string" }, + "action-if-true": { type: "string" }, + "action-if-false": { type: "string" }, + "issue-id": { type: "string" }, + repository: { type: "string" }, + }, + strict: true, + }); + + if (values.body === undefined) { + throw new Error("Missing required flag --body."); + } + if (values.marker === undefined || values.marker.trim() === "") { + throw new Error("Missing required flag --marker."); + } + + return { + body: values.body, + marker: values.marker, + actionCondition: parseBoolean( + "action-condition", + values["action-condition"], + ), + actionIfTrue: parseAction("action-if-true", values["action-if-true"]), + actionIfFalse: parseAction("action-if-false", values["action-if-false"]), + issueId: parseIssueId(values["issue-id"]), + repository: parseRepository(values.repository), + }; +} + +/** 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; +} + +/** + * Performs the resolved `action` against a (possibly pre-existing) marked comment, using + * `client` to talk to the GitHub API. This is the core dispatch logic of this script, extracted + * from `main` so it can be unit-tested with an injected (fake) `client`. + */ +export async function performAction( + client: ApiClient, + action: Action, + options: Options, +): Promise { + const { issueId, marker, repository } = options; + const body = `${marker}\n${options.body}`; + + let existingCommentId: number | undefined; + if (action === "upsert" || action === "delete") { + existingCommentId = await findExistingComment( + client, + repository, + issueId, + marker, + ); + } + + switch (action) { + case "none": + console.info("No action needed."); + break; + case "insert": + console.info("Creating a new comment."); + await client.rest.issues.createComment({ + ...repository, + issue_number: issueId, + body, + }); + break; + case "upsert": + 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, + }); + } + break; + case "delete": + if (existingCommentId === undefined) { + console.info("No existing comment found to delete; skipping."); + } else { + console.info(`Deleting existing comment ${existingCommentId}.`); + await client.rest.issues.deleteComment({ + ...repository, + comment_id: existingCommentId, + }); + } + break; + } +} + +async function main(): Promise { + const options = parseOptions(); + const action = options.actionCondition + ? options.actionIfTrue + : options.actionIfFalse; + console.info(`Resolved action: ${action}`); + + const token = resolveToken(process.env); + const client = getApiClient(token); + + await performAction(client, action, 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(); +}