From 94488778fdc9296be5ba13464f1784bf439495e1 Mon Sep 17 00:00:00 2001 From: Mario Campos Date: Wed, 28 Oct 2026 16:00:37 -0500 Subject: [PATCH] Simplify post-comment to a single set-comment mode Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/actions/post-comment/action.yml | 11 +-- .github/workflows/pr-checks.yml | 13 ++- pr-checks/post-comment.test.ts | 112 ++++++++---------------- pr-checks/post-comment.ts | 100 ++++++--------------- 4 files changed, 76 insertions(+), 160 deletions(-) diff --git a/.github/actions/post-comment/action.yml b/.github/actions/post-comment/action.yml index b8bffb3d5..951fd1704 100644 --- a/.github/actions/post-comment/action.yml +++ b/.github/actions/post-comment/action.yml @@ -1,17 +1,14 @@ name: "Post comment" -description: "Create, update, or delete a sticky PR comment identified by a caller-provided marker." +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. Not needed when deleting a comment." - required: false - action: - description: "Action to perform. One of: none, insert, upsert, delete." + description: "Full comment markdown." 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." + 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." @@ -26,11 +23,9 @@ runs: MARKER: ${{ inputs.marker }} GH_TOKEN: ${{ inputs.token }} ISSUE_ID: ${{ inputs.issue-id || github.event.pull_request.number }} - ACTION: ${{ inputs.action }} run: | npx tsx ./pr-checks/post-comment.ts \ --body "$BODY" \ --marker "$MARKER" \ - --action "$ACTION" \ --issue-id "$ISSUE_ID" \ --repository "$GITHUB_REPOSITORY" diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 1f1c79020..fb3f6aba2 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -305,11 +305,10 @@ jobs: 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: upsert token: ${{ secrets.GITHUB_TOKEN }} - - name: Delete the warning about modifying CHANGELOG.md - id: delete-changelog-warning + - name: Mark the CHANGELOG.md concern as resolved + id: resolve-changelog-warning if: >- needs.other-checks.outputs.changelog-changed != 'true' && !startsWith(github.event.pull_request.head.ref, 'update-v') && @@ -319,5 +318,11 @@ jobs: uses: ./.github/actions/post-comment with: marker: "" - action: delete + 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 index cda92de9b..de840427d 100644 --- a/pr-checks/post-comment.test.ts +++ b/pr-checks/post-comment.test.ts @@ -12,8 +12,8 @@ import { findExistingComment, type Options, parseOptions, - performAction, resolveToken, + setComment, } from "./post-comment"; /** A baseline set of valid CLI flags, as an array, for use in parseOptions tests. */ @@ -22,8 +22,6 @@ const validFlags = [ "hello", "--marker", "", - "--action", - "insert", "--issue-id", "42", "--repository", @@ -52,22 +50,11 @@ describe("parseOptions", async () => { assert.deepEqual(options, { body: "hello", marker: "", - action: "insert", issueId: 42, repository: { owner: "github", repo: "codeql-action" }, }); }); - await it("parses a delete action without a body", () => { - const options = parseOptions([ - ...withoutFlag("--body"), - "--action", - "delete", - ]); - assert.equal(options.action, "delete"); - assert.equal(options.body, ""); - }); - await it("rejects a missing --marker", () => { assert.throws( () => parseOptions(withoutFlag("--marker")), @@ -124,8 +111,7 @@ function fakeClient(existingComments: Array<{ id: number; body?: string }>) { const calls: { createComment: unknown[]; updateComment: unknown[]; - deleteComment: unknown[]; - } = { createComment: [], updateComment: [], deleteComment: [] }; + } = { createComment: [], updateComment: [] }; const client = { paginate: async () => existingComments, @@ -138,9 +124,6 @@ function fakeClient(existingComments: Array<{ id: number; body?: string }>) { updateComment: async (params: unknown) => { calls.updateComment.push(params); }, - deleteComment: async (params: unknown) => { - calls.deleteComment.push(params); - }, }, }, }; @@ -176,56 +159,38 @@ describe("findExistingComment", async () => { }); }); -/** A baseline set of `Options`, for use in `performAction` tests. */ +/** A baseline set of `Options`, for use in `setComment` tests. */ const baseOptions: Options = { body: "hello", marker: "", - action: "insert", issueId: 42, repository: { owner: "owner", repo: "repo" }, }; -describe("performAction", async () => { - for (const action of ["upsert", "delete"] as const) { - await it(`uses the supplied marker for '${action}' without affecting other comments`, async () => { - const marker = ""; - const { client, calls } = fakeClient([ - { id: 1, body: "\nunrelated warning" }, - { id: 2, body: `${marker}\nold warning` }, - ]); - const options = parseOptions(withFlag("--marker", marker)); - await performAction(client, action, options); - assert.deepEqual(calls, { - createComment: [], - updateComment: - action === "upsert" - ? [ - { - ...options.repository, - comment_id: 2, - body: `${marker}\nhello`, - }, - ] - : [], - deleteComment: - action === "delete" ? [{ ...options.repository, comment_id: 2 }] : [], - }); - }); - } - - await it("does nothing for 'none'", async () => { - const { client, calls } = fakeClient([]); - await performAction(client, "none", baseOptions); +describe("setComment", async () => { + await it("uses the supplied marker without affecting other comments", async () => { + const marker = ""; + const { client, calls } = fakeClient([ + { id: 1, body: "\nunrelated warning" }, + { id: 2, body: `${marker}\nold warning` }, + ]); + const options = parseOptions(withFlag("--marker", marker)); + await setComment(client, options); assert.deepEqual(calls, { createComment: [], - updateComment: [], - deleteComment: [], + updateComment: [ + { + ...options.repository, + comment_id: 2, + body: `${marker}\nhello`, + }, + ], }); }); - await it("creates a new comment for 'insert', with the marker prepended", async () => { + await it("creates a new comment with the marker prepended", async () => { const { client, calls } = fakeClient([]); - await performAction(client, "insert", baseOptions); + await setComment(client, baseOptions); assert.deepEqual(calls.createComment, [ { owner: "owner", @@ -235,12 +200,11 @@ describe("performAction", async () => { }, ]); assert.deepEqual(calls.updateComment, []); - assert.deepEqual(calls.deleteComment, []); }); - await it("creates a new comment for 'upsert' when no existing comment is found", async () => { + await it("creates a new comment when no existing comment matches", async () => { const { client, calls } = fakeClient([{ id: 1, body: "unrelated" }]); - await performAction(client, "upsert", baseOptions); + await setComment(client, baseOptions); assert.deepEqual(calls.createComment, [ { owner: "owner", @@ -252,11 +216,11 @@ describe("performAction", async () => { assert.deepEqual(calls.updateComment, []); }); - await it("updates the existing comment for 'upsert' when one is found", async () => { + await it("updates the existing comment when one is found", async () => { const { client, calls } = fakeClient([ { id: 7, body: "\nold" }, ]); - await performAction(client, "upsert", baseOptions); + await setComment(client, baseOptions); assert.deepEqual(calls.updateComment, [ { owner: "owner", @@ -268,24 +232,20 @@ describe("performAction", async () => { 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 () => { + await it("replaces a warning with the resolved state using the same marker", async () => { const { client, calls } = fakeClient([ { id: 9, body: "\nold" }, ]); - const options = parseOptions([ - ...withoutFlag("--body"), - "--action", - "delete", - ]); - await performAction(client, options.action, options); - assert.deepEqual(calls.deleteComment, [ - { owner: "github", repo: "codeql-action", comment_id: 9 }, + const body = + "`CHANGELOG.md` is not modified in this PR. No changelog action is needed."; + await setComment(client, { ...baseOptions, body }); + assert.deepEqual(calls.updateComment, [ + { + ...baseOptions.repository, + comment_id: 9, + body: `${baseOptions.marker}\n${body}`, + }, ]); + assert.deepEqual(calls.createComment, []); }); }); diff --git a/pr-checks/post-comment.ts b/pr-checks/post-comment.ts index 602bf2056..3f8abe40b 100644 --- a/pr-checks/post-comment.ts +++ b/pr-checks/post-comment.ts @@ -1,15 +1,14 @@ #!/usr/bin/env npx tsx /* -Generic CLI for creating, updating, or deleting a "sticky" PR comment identified by a hidden +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 so that -`upsert`/`delete` actions know which comment to operate on, and is automatically prepended to +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. @@ -20,20 +19,13 @@ 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 action to perform. */ - action: Action; - /** The issue/pull request number to post/update/delete the comment on. */ + /** The issue/pull request number to post/update the comment on. */ issueId: number; /** The repository to operate on. */ repository: { owner: string; repo: string }; @@ -44,9 +36,8 @@ export function parseOptions(argv?: string[]): Options { const { values } = parseArgs({ args: argv, options: { - body: { type: "string", default: "" }, + body: { type: "string" }, marker: { type: "string" }, - action: { type: "string" }, "issue-id": { type: "string" }, repository: { type: "string" }, }, @@ -57,9 +48,8 @@ export function parseOptions(argv?: string[]): Options { const [owner, repo] = values.repository!.split("/"); return { - body: values.body, + body: values.body!, marker: values.marker!, - action: values.action as Action, issueId: Number(values["issue-id"]), repository: { owner, repo }, }; @@ -94,79 +84,45 @@ export async function findExistingComment( } /** - * Performs the supplied `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`. + * Creates or updates a marked comment using `client` to talk to the GitHub API. */ -export async function performAction( +export async function setComment( 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; + 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(); - console.info(`Action: ${options.action}`); const token = resolveToken(process.env); const client = getApiClient(token); - await performAction(client, options.action, options); + await setComment(client, options); return 0; }