diff --git a/.github/actions/post-comment/action.yml b/.github/actions/post-comment/action.yml new file mode 100644 index 0000000000..2bf84cf289 --- /dev/null +++ b/.github/actions/post-comment/action.yml @@ -0,0 +1,41 @@ +name: "Post comment" +description: "Create or update a sticky PR comment identified by a caller-provided marker." +inputs: + marker: + description: > + Non-empty plain-text identifier for the comment. Use a unique, stable value + for each distinct comment; it is automatically wrapped in a hidden HTML comment. + required: true + body: + description: "Full comment markdown." + required: true + update-only: + description: Do nothing if no comment with the specified marker exists. + required: false + default: "false" + 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 }} + UPDATE_ONLY: ${{ (inputs.update-only == 'true' && '--update-only') || '' }} + 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" \ + $UPDATE_ONLY \ + --issue-id "$ISSUE_ID" \ + --repository "$GITHUB_REPOSITORY" diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 5d2b1576f7..b72c457742 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -95,6 +95,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 }} @@ -183,6 +185,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-') }} @@ -203,8 +219,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. @@ -224,6 +240,22 @@ jobs: group: check-repo-size-${{ github.event.pull_request.number }} steps: + - name: Checkout repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + # Check out main, not the PR's head, since this job runs with + # `pull-requests: write` permissions and must not execute untrusted code from the PR. + ref: main + + - name: Set up Node.js + uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.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@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2 with: @@ -253,3 +285,42 @@ 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: "post-comment:CHANGELOG.md" + 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 == 'false' && + !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: "post-comment:CHANGELOG.md" + update-only: true + 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/api-client.ts b/pr-checks/api-client.ts index b8f914d815..8f990362b9 100644 --- a/pr-checks/api-client.ts +++ b/pr-checks/api-client.ts @@ -8,3 +8,14 @@ export function getApiClient(token: string): ApiClient { const opts = githubUtils.getOctokitOptions(token); return new githubUtils.GitHub(opts); } + +/** 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; +} diff --git a/pr-checks/post-comment.test.ts b/pr-checks/post-comment.test.ts new file mode 100644 index 0000000000..db1ce178a4 --- /dev/null +++ b/pr-checks/post-comment.test.ts @@ -0,0 +1,344 @@ +#!/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, resolveToken } from "./api-client"; +import { + findExistingComment, + type Options, + parseOptions, + setComment, +} from "./post-comment"; + +/** A baseline set of valid CLI flags, as an array, for use in parseOptions tests. */ +const validFlags = [ + "--body", + "hello", + "--marker", + "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: "marker", + updateOnly: false, + issueId: 42, + repository: { owner: "github", repo: "codeql-action" }, + }); + }); + + await it("enables update-only mode when --update-only is supplied", () => { + const options = parseOptions([...validFlags, "--update-only"]); + assert.equal(options.updateOnly, true); + }); + + await it("rejects a missing --body", () => { + assert.throws( + () => parseOptions(withoutFlag("--body")), + /Missing required flag --body/, + ); + }); + + await it("rejects an empty --body", () => { + assert.throws( + () => parseOptions(withFlag("--body", "")), + /The value for --body cannot be empty/, + ); + }); + + await it("rejects a whitespace-only --body", () => { + assert.throws( + () => parseOptions(withFlag("--body", " \t\n")), + /The value for --body cannot be empty/, + ); + }); + + 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", "")), + /The value for --marker cannot be empty/, + ); + }); + + await it("rejects a whitespace-only --marker", () => { + assert.throws( + () => parseOptions(withFlag("--marker", " \t\n")), + /The value for --marker cannot be empty/, + ); + }); + + await it("rejects a missing --repository", () => { + assert.throws( + () => parseOptions(withoutFlag("--repository")), + /Missing required flag --repository/, + ); + }); + + await it("rejects an empty --repository", () => { + assert.throws( + () => parseOptions(withFlag("--repository", "")), + /The value for --repository cannot be empty/, + ); + }); + + await it("rejects a whitespace-only --repository", () => { + assert.throws( + () => parseOptions(withFlag("--repository", " \t\n")), + /The value for --repository cannot be empty/, + ); + }); + + await it("rejects an invalid --repository value", () => { + assert.throws( + () => parseOptions(withFlag("--repository", "github/codeql/action")), + /Invalid --repository value/, + ); + }); +}); + +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, + "marker", + ); + assert.equal(id, 3); + }); + + 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, + "marker", + ); + assert.equal(id, undefined); + }); +}); + +/** A baseline set of `Options`, for use in `setComment` tests. */ +const baseOptions: Options = { + body: "hello", + marker: "marker", + updateOnly: false, + issueId: 42, + repository: { owner: "owner", repo: "repo" }, +}; + +function assertCommentContent( + comment: { body?: string }, + marker: string, + body: string, +): void { + assert.ok(comment.body?.startsWith(``)); + assert.ok(comment.body?.endsWith(body)); +} + +describe("setComment", async () => { + await it("does not create a comment in update-only mode when none exists", async () => { + const { client, comments } = fakeClient([]); + + await setComment(client, { ...baseOptions, updateOnly: true }); + + // Resolution does not introduce a warning that was never posted. + assert.deepEqual(comments, []); + }); + + await it("leaves unrelated comments unchanged in update-only mode", async () => { + const originalComments = [{ id: 7, body: "unrelated" }]; + const { client, comments } = fakeClient(originalComments); + + await setComment(client, { ...baseOptions, updateOnly: true }); + + // Unrelated comments neither trigger resolution nor get overwritten. + assert.deepEqual(comments, originalComments); + }); + + await it("uses the supplied marker without affecting other comments", async () => { + const marker = "custom-warning"; + const { client, comments } = fakeClient([ + { id: 1, body: "\nunrelated warning" }, + { id: 2, body: "\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: "\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("")), + ); + + 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("")), + ); + + 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(""), + ); + // 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: "\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: "\nold" }, + ]); + const body = + "`CHANGELOG.md` is not modified in this PR. No changelog action is needed."; + + await setComment(client, { ...baseOptions, body, updateOnly: true }); + + // 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..5c39655774 --- /dev/null +++ b/pr-checks/post-comment.ts @@ -0,0 +1,177 @@ +#!/usr/bin/env npx tsx + +/* +Creates or updates a pull request comment identified by a marker. The marker +is supplied via `--marker` (or the `.github/actions/post-comment` action's `marker` input) +and must be non-empty. Supply a plain-text identifier; the CLI wraps it in a +hidden HTML comment, prepends it to `--body`, and uses it to find the comment on later runs. +Supply `--update-only` to suppress creation; omit it to update or create comments. + +Example: +```sh +GH_TOKEN= npx tsx ./pr-checks/post-comment.ts \ + --body "The changelog needs updating." \ + --marker "my-bot" \ + --issue-id 123 \ + --repository github/codeql-action +``` +*/ + +import assert from "node:assert/strict"; +import { parseArgs } from "node:util"; + +import { parseRepositoryNwo, type RepositoryNwo } from "../src/repository"; + +import { type ApiClient, getApiClient, resolveToken } from "./api-client"; + +/** Represents the command-line options. */ +export interface Options { + /** The full comment body. */ + body: string; + /** Plain-text identifier wrapped in a hidden HTML comment. */ + marker: string; + /** Whether to only update a comment if a matching marker exists. */ + updateOnly: boolean; + /** The issue/pull request number to post/update the comment on. */ + issueId: number; + /** The repository to operate on. */ + repository: RepositoryNwo; +} + +function hiddenMarker(marker: string): string { + return ``; +} + +/** 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" }, + "update-only": { type: "boolean", default: false }, + "issue-id": { type: "string" }, + repository: { type: "string" }, + }, + strict: true, + }); + + assert(values.body !== undefined, "Missing required flag --body."); + assert(values.body.trim() !== "", "The value for --body cannot be empty."); + assert(values.marker !== undefined, "Missing required flag --marker."); + assert( + values.marker.trim() !== "", + "The value for --marker cannot be empty.", + ); + assert( + values.repository !== undefined, + "Missing required flag --repository.", + ); + assert( + values.repository.trim() !== "", + "The value for --repository cannot be empty.", + ); + assert( + /^[\w.-]+\/[\w.-]+$/.test(values.repository), + "Invalid --repository value.", + ); + + return { + body: values.body, + marker: values.marker, + updateOnly: values["update-only"], + issueId: Number(values["issue-id"]), + repository: parseRepositoryNwo(values.repository), + }; +} + +/** + * Finds the first comment on the given issue/pull request whose body starts with `marker`, if any. + */ +export async function findExistingComment( + client: ApiClient, + repository: RepositoryNwo, + issueNumber: number, + marker: string, +): Promise { + const formattedMarker = hiddenMarker(marker); + const comments = await client.paginate(client.rest.issues.listComments, { + ...repository, + issue_number: issueNumber, + per_page: 100, + }); + const markedComments = comments.filter((comment) => + comment.body?.startsWith(formattedMarker), + ); + if (markedComments.length > 1) { + console.warn( + `Found ${markedComments.length} comments with marker "${formattedMarker}". ` + + "Updating the first one and leaving the others unchanged. " + + "Use a unique marker for each distinct managed comment to avoid this situation.", + ); + } + return markedComments.at(-1)?.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 = `${hiddenMarker(marker)}\n${options.body}`; + + const existingCommentId = await findExistingComment( + client, + repository, + issueId, + marker, + ); + if (existingCommentId === undefined) { + if (options.updateOnly) { + console.info( + "No existing comment found; skipping creation because --update-only was specified.", + ); + return; + } + 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(); +}