Simplify post-comment to a single set-comment mode

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Mario Campos
2026-10-28 16:00:37 -05:00
parent ae23865548
commit 94488778fd
4 changed files with 76 additions and 160 deletions

View File

@@ -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"

View File

@@ -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: "<!-- post-comment:CHANGELOG.md -->"
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 }}

View File

@@ -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",
"<!-- marker -->",
"--action",
"insert",
"--issue-id",
"42",
"--repository",
@@ -52,22 +50,11 @@ describe("parseOptions", async () => {
assert.deepEqual(options, {
body: "hello",
marker: "<!-- 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: "<!-- 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 = "<!-- custom-warning -->";
const { client, calls } = fakeClient([
{ id: 1, body: "<!-- marker -->\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 = "<!-- custom-warning -->";
const { client, calls } = fakeClient([
{ id: 1, body: "<!-- marker -->\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: "<!-- marker -->\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: "<!-- marker -->\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, []);
});
});

View File

@@ -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. `<!-- my-bot -->`) 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<void> {
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<number> {
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;
}