Add CHANGELOG.md modification check and warning comment by mario-campos · Pull Request #4217 · github/codeql-action

mario-campos · 2026-10-08T15:48:14Z

This PR adds a PR check for non-release/mergeback/update-bundle PRs. The check posts a sticky PR comment if CHANGELOG.md has been modified in the PR, warning the author to instead direct their change to a change-note.

Testing

In order to test this Action and its script, I forked this repository at dsp-testing/mario-campos-post-comment-test and modified the CHANGELOG.md file in a PR.

In dsp-testing#2, a comment was successfully created.

In dsp-testing#3, I modified CHANGELOG.md to invoke the comment, and then subsequently reverted the change to successfully invoke the comment deletion.

Risk assessment

For internal use only. Please select the risk level of this change:

Low risk: Changes are fully under feature flags, or have been fully tested and validated in pre-production environments and are highly observable, or are documentation or test only.

Which use cases does this change impact?

Workflow types:

N/A

Products:

N/A

Environments:

Test repository - This change will be tested on a test repository before merging.
Testing/None - This change does not impact any CodeQL workflows in production.

How did/will you validate this change?

None - I am not validating these changes.

If something goes wrong after this change is released, what are the mitigation and rollback strategies?

Development/testing only - This change cannot cause any failures in production.

How will you know if something goes wrong after this change is released?

N/A

Are there any special considerations for merging or releasing this change?

No special considerations - This change can be merged at any time.

Merge / deployment checklist

Confirm this change is backwards compatible with existing workflows.
Consider adding a changelog entry for this change.
Confirm the readme and docs have been updated if necessary.

mbg

Couple of initial comments on this draft. I like that this is taking advantage of the setup that the existing fetch-base provides.

I haven't checked if the changelog-check step behaves as expected yet either. One thing I did note though related to it is that the fetch-base step is skipped if github.event.pull_request.user.login != 'dependabot[bot]'. That isn't new in this PR (and we probably shouldn't address it here), but it might be better to check github.triggering_actor instead of github.event.pull_request.user.login. The justification in the comment is that Dependabot doesn't have permission, but it is possible for us to push commits to Dependabot PR branches or re-run workflows on it.

Also agree that it would be good to not spam the comment and instead replace an existing one. It's fairly easy to do that with gh or in a script by leaving a comment () in the comment that's posted and then looking for that.

… change warning

Copilot

🟡 Changes recommended

Shell command substitution currently prevents warning creation, and comment lifecycle and exclusion logic need correction.

3 open findings

Stale warning persists when changelog changes are reverted · New
Title-only exemption suppresses warnings for ordinary pull requests · New
Unescaped backticks trigger command substitution and fail the warning step · New

What changed in this PR

Adds a PR check that warns contributors to use change notes instead of editing CHANGELOG.md directly.

Changes:

Detects CHANGELOG.md modifications.
Adds a sticky warning comment with automation exclusions.
Consolidates PR comment posting into one job.

File	Description
`.github/workflows/pr-checks.yml`	Detects changelog edits and posts PR warnings.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Refactor conditions for PR title checks in workflow. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

mario-campos · 2026-10-08T18:00:38Z

@copilot Are there unresolved findings?

Copilot · 2026-10-08T18:01:37Z

@copilot Are there unresolved findings?

Yes—one valid finding remains: the warning step is gated on changelog-changed == 'true', so if a later commit reverts the CHANGELOG.md change, the step never runs and the existing warning comment stays stale. It should also run when the file is unchanged and remove any existing marker comment. The shell backticks are escaped, and the automation exclusion checks now include branch criteria.

…dified

Copilot

🟢 Approval recommended

The workflow correctly handles comment creation, deduplication, cleanup, and automation exemptions.

0 open findings

3 resolved since last review

Unescaped backticks trigger command substitution and fail the warning step
Title-only exemption suppresses warnings for ordinary pull requests
Stale warning persists when changelog changes are reverted

🧠 Review effort: Balanced

mbg

Thanks for changing this to move away from pulling in another action and also moving the step that posts or updates the comment out of other-checks and into (now) post-pr-comments.

I added a few more comments, but the main one is about avoiding more, lengthy-ish shell scripts. It shouldn't be hard to implement in TS in pr-checks and I have included some relevant pointers in my detailed comment about this.

Co-authored-by: Michael B. Gale <mbg@github.com>

Copilot

🟡 Changes recommended

Input-validation, test-coverage, link, and workflow-bootstrap issues remain unresolved.

5 open findings

Avoid resolving the local action from the PR base SHA · New
Reject empty or whitespace-only comment markers · New
Validate issue numbers as complete positive integers · New
Use a repository-qualified URL for change-note instructions · New
Add tests for comment operation dispatch branches · New

🧠 Review effort: Balanced

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Update link in PR checks workflow to point to the correct README. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

…tability

mario-campos · 2026-10-12T16:51:30Z

Note: the failed check is expected, because it's a consequence of introducing an Action and simoultanously changing the workflow to call said Action in the PR's base ref. For this PR, it's a not possible to do both, but it shan't be a problem once merged.

mbg

I have added some detailed comments with some major points of feedback, but they are all broadly aligned with what we discussed elsewhere already about this script/action being too over-engineered.

mbg · 2026-10-14T15:27:37Z

+          action-condition: ${{ needs.other-checks.outputs.changelog-changed }}
+          action-if-true: upsert
+          action-if-false: delete


Rather than making this the responsibility of the post-comment action and thus adding complexity there, this could be achieved with a single action input and if conditions on two post-comment steps in the workflow. The step with the delete action wouldn't need the body so there'd be no meaningful duplication.

mbg · 2026-10-14T15:29:27Z

+        # 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: <!-- post-comment:${{ github.workflow }}:${{ github.job }}:${{ github.action }} -->


It would probably make sense to make this an input rather than generate it automatically. Although this might be convenient, it leads to issues if we e.g. wanted to have two post-comment steps in one job or it might not have the expected behaviour if we want post-comment steps across different workflows/jobs that should edit the same comment.

mbg · 2026-10-14T15:30:12Z

+  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


See my other comment, but this seems to be shifting functionality that already exists in workflows into this action and thus adds unnecessary complexity.

mbg · 2026-10-14T15:30:39Z

+inputs:
+  body:
+    description: "Full comment markdown."
+    required: true


Optional for the delete action, if you keep it.

mbg · 2026-10-14T15:34:05Z

+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;


See my other comments where I advocate for a single action input: I'd remove none, make insert and upsert the same (depending on whether a tag was specified or not), and possibly remove delete. I was quite surprised to find that, on your second test PR, there was no trace of the comment. Rather than deleting it, it might be better to edit it to a message that e.g. acknowledges that there's no problem anymore.

That simplifies this script (there's only one action) and makes it more visible that a problem was resolved.

mbg · 2026-10-14T15:36:10Z

+/**
+ * Validates and parses a required CLI flag's raw string `value`, delegating the actual
+ * value-specific validation/conversion to `parse`.
+ */
+function parseFlag<T>(
+  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}`);
+  }
+}


We are currently using Node's parseArgs module (which this script also uses). The validation options there are quite limited, but I am not sure that it makes sense to implement this little argument parsing/validation library in this script. At a minimum, make this something that the pr-checks scripts can share, but really if you see a need for this, we can add a dependency on a more feature-rich library for this kind of thing.

Add CHANGELOG.md modification check and warning comment

b2b77a8

github-actions Bot added the size/XS Should be very easy to review label Oct 8, 2026

Gate comment-post step on successful fetch-base

fbd33ac

mbg reviewed Oct 8, 2026

View reviewed changes

Comment thread .github/workflows/pr-checks.yml Outdated

Comment thread .github/workflows/pr-checks.yml Outdated

mario-campos added 5 commits October 8, 2026 11:16

Use gh api instead of third-party Action

43ac64c

Move post-comment step to post-pr-comments job

f7aa9dc

Skip CHANGELOG.md modification warning for release and backport PRs

780e947

Exclude "Mergeback" and "Update default bundle" PRs from CHANGELOG.md…

ae75d46

… change warning

Use a sticky comment to warn about CHANGELOG.md changes

6e4d3fb

github-actions Bot added size/S Should be easy to review and removed size/XS Should be very easy to review labels Oct 8, 2026

Update CHANGELOG.md warning message to reference change-notes directory

d97a656

mario-campos marked this pull request as ready for review October 8, 2026 17:43

mario-campos requested a review from a team as a code owner October 8, 2026 17:43

Copilot AI balanced review requested due to automatic review settings October 8, 2026 17:43

Copilot started reviewing on behalf of mario-campos October 8, 2026 17:43

View session

Copilot AI reviewed Oct 8, 2026

View reviewed changes

Comment thread .github/workflows/pr-checks.yml Outdated

Comment thread .github/workflows/pr-checks.yml Outdated

Comment thread .github/workflows/pr-checks.yml Outdated

mario-campos and others added 2 commits October 8, 2026 12:55

Escape backticks PR comment body

ccdf690

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Refine exclusion conditions for CHANGELOG-modification comment

928548a

Refactor conditions for PR title checks in workflow. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot started work on behalf of mario-campos October 8, 2026 18:01

View session

Copilot finished work on behalf of mario-campos October 8, 2026 18:01

Delete CHANGELOG.md-modification comment if CHANGELOG.md no longer mo…

0707124

…dified

mario-campos requested a balanced review from Copilot October 8, 2026 18:12

Copilot started reviewing on behalf of mario-campos October 8, 2026 18:12

View session

Copilot AI reviewed Oct 8, 2026

View reviewed changes

mbg reviewed Oct 8, 2026

View reviewed changes

Comment thread .github/workflows/pr-checks.yml Outdated

Comment thread .github/workflows/pr-checks.yml Outdated

Comment thread .github/workflows/pr-checks.yml Outdated

mario-campos and others added 2 commits October 8, 2026 15:19

Format all file references as inline Markdown code spans

27d37d9

Co-authored-by: Michael B. Gale <mbg@github.com>

Simplify if condition of CHANGELOG-comment-post step

9fe741e

mario-campos added 2 commits October 8, 2026 23:02

Add script and Action for managing PR comments

8c8b082

Replace bash/run step with local Action post-comment

14cefaa

github-actions Bot added size/L May be hard to review and removed size/S Should be easy to review labels Oct 9, 2026

mario-campos requested a balanced review from Copilot October 9, 2026 04:07

Copilot started reviewing on behalf of mario-campos October 9, 2026 04:07

View session

Copilot AI reviewed Oct 9, 2026

View reviewed changes

Comment thread .github/workflows/pr-checks.yml

Comment thread pr-checks/post-comment.ts Outdated

Comment thread pr-checks/post-comment.ts Outdated

Comment thread .github/workflows/pr-checks.yml Outdated

Comment thread pr-checks/post-comment.ts

mario-campos and others added 4 commits October 9, 2026 10:35

Improve issue ID parsing and validation logic

4d464b0

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Use HTTP link to unreleased-change-notes/README.md

3c452a1

Update link in PR checks workflow to point to the correct README. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Reject empty --marker

f1419b8

Refactor comment handling logic into performAction for improved tes…

e7dadca

…tability

github-actions Bot added size/XL May be very hard to review and removed size/L May be hard to review labels Oct 9, 2026

mario-campos requested a review from mbg October 12, 2026 16:58

mbg requested changes Oct 14, 2026

View reviewed changes

Conversation

mario-campos commented Oct 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing

Risk assessment

Which use cases does this change impact?

How did/will you validate this change?

If something goes wrong after this change is released, what are the mitigation and rollback strategies?

How will you know if something goes wrong after this change is released?

Are there any special considerations for merging or releasing this change?

Merge / deployment checklist

Uh oh!

mbg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mario-campos commented Oct 8, 2026

Uh oh!

Copilot AI commented Oct 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

mbg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mario-campos commented Oct 12, 2026

Uh oh!

mbg left a comment

Choose a reason for hiding this comment

Uh oh!

mbg Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

mbg Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

mbg Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

mbg Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

mbg Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

mbg Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mario-campos commented Oct 8, 2026 •

edited

Loading