Skip to content

ci: validate commit messages instead of PR title - #18462

Open
Christopher Co (christopherco) wants to merge 1 commit into
microsoft:4.0from
christopherco:chrco/fix-conventional-commit-checker
Open

ci: validate commit messages instead of PR title#18462
Christopher Co (christopherco) wants to merge 1 commit into
microsoft:4.0from
christopherco:chrco/fix-conventional-commit-checker

Conversation

@christopherco

@christopherco Christopher Co (christopherco) commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

We rebase-merge, so every commit enters the permanent 4.0 history. The
old check only validated the PR title, which could hide non-conventional
commits behind a conventional title (or fail a well-formed branch behind a
descriptive one).

Validate the header of every commit instead. A github-script step reads
the PR commits via the API and checks each against the Conventional Commit
types in CONTRIBUTING.md, allowing optional scopes and breaking-change
markers. On failure it comments the offending commits and how to fix them,
and removes the comment once all are valid.

The workflow stays on pull_request_target so it can comment on fork PRs,
but only reads commit metadata through the API and never checks out or
runs PR code. Commit subjects are HTML-encoded before display, the count
is checked against the PR total to fail closed on the 250-commit API cap,
and all actions remain SHA-pinned.

Fixes: AB#22437

Copilot AI balanced review requested due to automatic review settings August 15, 2026 04:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Replaces PR-title linting with per-commit Conventional Commit validation for rebase-merged 4.0 history.

Changes:

  • Validates all PR commit headers through the GitHub API.
  • Posts sanitized diagnostics and remediation guidance.
  • Clarifies commit and PR-title requirements.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
CONTRIBUTING.md Documents per-commit CI validation.
.github/workflows/check-pr-title.yml Removes PR-title validation.
.github/workflows/check-commit-messages.yml Adds commit-message validation and feedback.

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

Comment thread .github/workflows/check-commit-messages.yml
Comment thread .github/workflows/check-commit-messages.yml
Comment thread .github/workflows/check-commit-messages.yml Outdated
Comment thread .github/workflows/check-commit-messages.yml
Copilot AI review requested due to automatic review settings August 15, 2026 06:10
@christopherco
Christopher Co (christopherco) force-pushed the chrco/fix-conventional-commit-checker branch from 668ea1c to 7508c37 Compare August 15, 2026 06:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (3)

.github/workflows/check-commit-messages.yml:17

  • pull_request_target loads workflow definitions only from the repository's default branch, but the default-branch workflow listing still contains check-pr-title.yml and not this new file. As a result, merging this change into 4.0 will neither start this validator nor retire the title validator. Add a default-branch stub for this check (and remove/replace the old default-branch workflow), delegating to a trusted reusable workflow on 4.0, as done by .github/workflows/check-rendered-specs-stub.yml:1-38.
  pull_request_target: # zizmor: ignore[dangerous-triggers]

.github/workflows/check-commit-messages.yml:125

  • This builds the comment from every invalid commit, but GitHub rejects issue/PR comments over 65,536 characters. With up to 250 commits and each 200-character subject expanding into HTML entities, list can exceed that limit several times over, so the sticky-comment step fails and users receive none of the promised details. Cap the displayed entries and report how many were omitted.
            const list = invalid
              .map((c) => `- \`${c.sha}\` <code>${encodeSubject(c.subject)}</code>`)
              .join('\n');

.github/workflows/check-commit-messages.yml:166

  • A missing output is treated as an empty string by Actions expressions, so this deletion also runs when validate_commits crashes before setting error_message (for example, on an API failure). That removes the previous diagnostic even though validation did not succeed. Require the validation step to have succeeded before deleting the sticky comment.
      - if: steps.validate_commits.outputs.error_message == ''

Copilot AI review requested due to automatic review settings August 15, 2026 06:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (3)

.github/workflows/check-commit-messages.yml:59

  • A whitespace-only subject such as feat: passes this regex because .+ accepts spaces, even though the documented format requires a non-empty short summary. Require the first summary character to be non-whitespace and anchor the match so the whole header is validated.
              ': .+'                          // ': ' followed by a non-empty summary

.github/workflows/check-commit-messages.yml:94

  • This per-subject limit does not bound the final comment size after encoding. With up to 250 invalid commits, disallowed or non-ASCII characters expand into numeric entities, so the generated body can grow to hundreds of kilobytes and exceed GitHub's 65,536-character issue-comment limit; the sticky-comment step then fails instead of reporting the offending commits. Cap the aggregate rendered list (while preserving every SHA and marking truncated subject previews), or choose a per-entry budget derived from the API limit.
              const firstLine = String(text || '').split('\n')[0].slice(0, 200);

.github/workflows/check-commit-messages.yml:141

  • Changing the sticky-comment header leaves existing pr-title-lint-error comments behind on open PRs. Those comments continue telling contributors that titles must use Conventional Commits, which directly contradicts the new policy; moreover, title edits no longer trigger this workflow. Add a migration cleanup for the legacy header and ensure an event can run it for existing PRs (for example, temporarily retain edited).
          header: commit-message-lint-error

@christopherco
Christopher Co (christopherco) force-pushed the chrco/fix-conventional-commit-checker branch from 7508c37 to 8266e2e Compare August 15, 2026 06:24
We rebase-merge, so every commit enters the permanent 4.0 history. The
old check only validated the PR title, which could hide non-conventional
commits behind a conventional title (or fail a well-formed branch behind a
descriptive one).

Validate the header of every commit instead. A github-script step reads
the PR commits via the API and checks each against the Conventional Commit
types in CONTRIBUTING.md, allowing optional scopes and breaking-change
markers. On failure it comments the offending commits and how to fix them,
and removes the comment once all are valid.

The workflow stays on pull_request_target so it can comment on fork PRs,
but only reads commit metadata through the API and never checks out or
runs PR code. Commit subjects are HTML-encoded before display, the count
is checked against the PR total to fail closed on the 250-commit API cap,
and all actions remain SHA-pinned.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 886c4e30-30d0-448a-9180-5c4a0ee8018c
Copilot AI review requested due to automatic review settings August 15, 2026 07:15
@christopherco
Christopher Co (christopherco) force-pushed the chrco/fix-conventional-commit-checker branch from 8266e2e to 34b1610 Compare August 15, 2026 07:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (2)

.github/workflows/check-commit-messages.yml:171

  • The removed workflow used the sticky-comment header pr-title-lint-error, but this replacement only deletes commit-message-lint-error. Existing open PRs with a title-lint comment will therefore retain a bot message claiming their title must follow Conventional Commits, directly contradicting the new policy. Delete the legacy header as part of this migration.
      # Delete the previous comment once every commit message is valid.
      - if: steps.validate_commits.outputs.error_message == ''
        uses: marocchino/sticky-pull-request-comment@5770ad5eb8f42dd2c4f34da00c94c5381e49af88 # v3.0.5
        with:
          header: commit-message-lint-error
          delete: true

.github/workflows/check-commit-messages.yml:60

  • The pattern treats whitespace as a non-empty summary, so a commit such as fix: passes even though it has no <short summary> as required by CONTRIBUTING.md. Require the first summary character to be non-whitespace and anchor the header to the end of the subject.
              ': .+'                          // ': ' followed by a non-empty summary

@christopherco
Christopher Co (christopherco) marked this pull request as ready for review August 15, 2026 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants