fix(ci): lint individual commits on PRs to catch invalid prefixes early by ralphbean · Pull Request #2514 · fullsend-ai/fullsend

ralphbean · 2026-06-22T15:57:14Z

Summary

The merge queue uses merge (not squash), so individual commit messages matter
commit-lint previously only checked the PR title on pull_request events, deferring per-commit linting to merge_group
Invalid prefixes (like style on PR docs(security): add audit log integrity to threat model #2010) only surfaced when the merge queue rejected the PR — too late for contributors to fix easily
Now commit-lint checks individual commits on pull_request events too, giving early feedback

Test plan

This PR's own commit-lint passes (single commit with valid fix(ci): prefix)
Open a PR with a commit using an invalid prefix (e.g., style) — commit-lint should fail on the PR, not wait for the merge queue

The merge queue uses merge (not squash), so individual commit messages matter. Previously, commit-lint only checked the PR title on pull_request events and deferred per-commit linting to merge_group. This meant invalid prefixes (like `style`) only surfaced when the merge queue rejected the PR — too late for contributors to fix easily. Now commit-lint checks individual commits on pull_request events too, giving early feedback before the merge queue. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>

qodo-code-review · 2026-06-22T15:58:04Z

PR Summary by Qodo

Fix CI commit-lint to lint individual PR commits early
🐞 Bug fix ⚙️ Configuration changes 🕐 10-20 Minutes

Description

• Run per-commit gitlint on pull_request events to fail fast on invalid commit prefixes.
• Keep PR-title linting for pull_request, and commit-range linting for push/merge_group.
• Derive the commit range from the triggering event (push/merge_group/pull_request) via a single
 case statement.

Diagram

graph TD
  GH{{"GitHub event"}} --> CL["commit-lint job"] --> PRT["Lint PR title (PR only)"] --> SEL{"Select commit range"} --> LOOP["Iterate commits (no merges)"] --> GL["gitlint commit subjects"]

  subgraph Legend
    direction LR
    _ext{{"External trigger"}} ~~~ _job["CI job/step"] ~~~ _dec{"Decision"}
  end

High-Level Assessment

The following are alternative approaches to this PR:

1. Use a dedicated commit-message lint action

➕ Less custom bash/range logic to maintain
➕ Often has built-in PR/commit enumeration support
➖ May not match existing .gitlint rules/ignores exactly
➖ Adds external action dependency and upgrade surface area

2. Enforce squash-merge only (make PR title the single source of truth)

➕ Simplifies linting to PR title only
➕ Avoids per-commit policy enforcement overhead
➖ Conflicts with merge queue behavior (merge commits) described in the PR context
➖ Changes repo workflow expectations for contributors

3. Fetch commit list via GitHub API instead of git ranges

➕ Avoids edge cases where base/head SHAs aren’t present locally
➕ Can precisely target PR commits regardless of checkout state
➖ More code/complexity (API auth, pagination) than current approach
➖ Still needs mapping to local gitlint invocation per commit

Recommendation: The PR’s approach (event-aware SHA range selection and local git rev-list linting) is the best fit given the existing gitlint setup and the need to fail fast on pull_request. The main thing to watch is ensuring the checkout (fetch-depth: 0) continues to provide both PR base/head SHAs; if that ever changes, the GitHub API approach becomes the most robust fallback.

Files changed (1) +12 / -8

Other (1) +12 / -8

lint.ymlLint individual PR commits by selecting a pull_request SHA range +12/-8

Lint individual PR commits by selecting a pull_request SHA range

• Updates the commit-lint job to lint individual commits on 'pull_request' events in addition to 'push' and 'merge_group'. Adds PR base/head SHAs to the environment and replaces the if/else range selection with a 'case' statement to compute the correct commit range per event.

.github/workflows/lint.yml

github-actions · 2026-06-22T15:58:57Z

Site preview

Preview: https://c74247a4-site.fullsend-ai.workers.dev

Commit: 136c662dfc2aa89192aa5d5f0557a356ee27f051

qodo-code-review · 2026-06-22T16:00:33Z

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 51 rules

✅ Skills: writing-user-docs, writing-adrs

1. Silent rev-list failure 🐞 Bug ☼ Reliability

Description

The Lint commits step expands $(git rev-list ...) without fail-fast mode or explicit error
checking, so if RANGE is an invalid revspec (e.g., PUSH_BEFORE is all-zero or a SHA isn’t
available locally), the loop can run zero times and the job still succeeds—skipping commit linting.
This undermines the purpose of adding early per-commit checks on pull_request events.

Code

.github/workflows/lint.yml[R86-94]

+          case "${EVENT_NAME}" in
+            push)          RANGE="${PUSH_BEFORE}..${PUSH_AFTER}" ;;
+            merge_group)   RANGE="${MQ_BASE}..${MQ_HEAD}" ;;
+            pull_request)  RANGE="${PR_BASE}..${PR_HEAD}" ;;
+            *)             echo "Unknown event: ${EVENT_NAME}"; exit 1 ;;
+          esac

          FAILED=false
          for sha in $(git rev-list --no-merges "${RANGE}"); do

Relevance

⭐⭐⭐ High
Team previously fixed silent workflow skips by adding explicit error handling/fail-fast in CI
scripts (e.g., #2398, #191).
PR-#2398
PR-#191
PR-#1565

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The workflow’s shell script does not enable fail-fast behavior and does not check git rev-list’s
exit code, so a failing git rev-list can result in an empty list and a passing job. This is the
same reliability failure mode as prior accepted workflow issues where missing error handling caused
silent skipping.

.github/workflows/lint.yml[76-103]
PR-#2398

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The workflow uses `for sha in $(git rev-list ...)` without `set -e` / `set -o pipefail` or an explicit exit-status check. If `git rev-list` fails due to an invalid range, the command substitution expands to empty, the loop is skipped, and the step can still exit 0.

### Issue Context
This PR makes per-commit linting run on `pull_request` events as well, increasing the importance of not silently skipping commit enumeration.

### Fix Focus Areas
- .github/workflows/lint.yml[85-103]

### Suggested fix approach
- Add `set -euo pipefail` at the start of the `run:` block, OR explicitly validate the range before looping, e.g.:
 - compute `RANGE`
 - run `COMMITS=$(git rev-list --no-merges "${RANGE}")` with `if ! ...; then echo ::error::...; exit 1; fi`
- Optionally handle the known all-zero `before` edge case for pushes by switching to a single-SHA range when `PUSH_BEFORE` is `0000000000000000000000000000000000000000`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

fullsend-ai-review · 2026-06-22T16:03:25Z

🤖 Finished Review · ✅ Success · Started 4:03 PM UTC · Completed 4:16 PM UTC
Commit: d3e9d46 · View workflow run →

codecov · 2026-06-22T16:05:01Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

fullsend-ai-review · 2026-06-22T16:16:29Z

Review

Findings

High

[protected-path] .github/workflows/lint.yml — This PR modifies a file under .github/, which is a protected path requiring human approval. No linked issue provides authorization for this change. While the PR description clearly explains the motivation (catching invalid commit prefixes earlier by linting individual commits on pull_request events), protected-path changes require a linked issue for traceability.
Remediation: File an issue describing the problem (invalid commit prefixes only caught at merge-queue time) and link it to this PR. Human approval is required regardless.

Previous run

Review

Findings

High

[protected-path] .github/workflows/lint.yml — This PR modifies a file under .github/, which is a protected path requiring human approval. No linked issue provides authorization for this change. While the PR description explains the motivation (catching invalid commit prefixes earlier), protected-path changes require a linked issue for traceability.
Remediation: File an issue describing the problem (invalid commit prefixes only caught at merge-queue time) and link it to this PR. Human approval is required regardless.

Low

[edge-case] .github/workflows/lint.yml:84 — The pull_request range uses github.event.pull_request.base.sha..github.event.pull_request.head.sha. The base SHA is the tip of the target branch at event time, not the merge-base. If main has diverged significantly from the PR's fork point, git rev-list --no-merges could include commits the author did not write, leading to spurious lint failures. Unlikely in practice with typical PR workflows.
[scope-alignment] .github/workflows/lint.yml — The PR title uses fix(ci): but per COMMITS.md, fix is for bug fixes visible to users and ci is for CI/CD pipeline changes. This change modifies when validation runs (CI timing), which is better characterized as a ci: or ci(commit-lint): prefix.

Labels: PR modifies CI workflow (.github/workflows/lint.yml)

fullsend-ai-review

See the review comment for full details.

fullsend-ai-review · 2026-06-22T16:16:31Z

          MQ_BASE: ${{ github.event.merge_group.base_sha }}
          MQ_HEAD: ${{ github.event.merge_group.head_sha }}
+          PR_BASE: ${{ github.event.pull_request.base.sha }}
+          PR_HEAD: ${{ github.event.pull_request.head.sha }}


[low] edge-case

The pull_request range uses github.event.pull_request.base.sha..github.event.pull_request.head.sha. The base SHA is the tip of the target branch at event time, not the merge-base. If main has diverged significantly from the PR fork point, git rev-list --no-merges could include commits the author did not write, leading to spurious lint failures. Unlikely in practice with typical PR workflows.

fullsend-ai-review · 2026-06-24T16:22:34Z

🤖 Finished Review · ✅ Success · Started 4:22 PM UTC · Completed 4:32 PM UTC
Commit: 136c662 · View workflow run →

fullsend-ai-review

See the review comment for full details.

fullsend-ai-retro · 2026-06-24T16:37:10Z

🤖 Finished Retro · ✅ Success · Started 4:37 PM UTC · Completed 4:45 PM UTC
Commit: 136c662 · View workflow run →

fullsend-ai-retro · 2026-06-24T16:45:54Z

Retro: PR #2514 — `fix(ci): lint individual commits on PRs`

Timeline

2026-06-22 15:57 — ralphbean opens PR fix(ci): lint individual commits on PRs to catch invalid prefixes early #2514, a 1-file change to .github/workflows/lint.yml adding per-commit linting on pull_request events.
2026-06-22 16:03 — First review agent run completes (run 27965975494). Posts "Finished Review, Success."
2026-06-22 16:16 — Review agent files CHANGES_REQUESTED verdict (~13 min after "Success" comment). Flags: protected-path (high), edge-case (low), scope-alignment (low).
2026-06-24 11:17–11:31 — Two human reviewers (ifireball, ascerra) approve with no comments.
2026-06-24 15:58 — PR added to merge queue. Removed ~10 min later (16:08).
2026-06-24 16:19 — New commit pushed. Auto-merge enabled. Second review dispatched (run 28113064349).
2026-06-24 16:22:34 — Review agent posts "Finished Review, Success."
2026-06-24 16:22:37 — Auto-merge adds PR to merge queue (3 seconds after "Success").
2026-06-24 16:32:01 — Review agent's post-script files CHANGES_REQUESTED (~9.5 min after "Success").
2026-06-24 16:32:34 — PR merges despite unresolved CHANGES_REQUESTED.

What went well

Review agent findings were all valid: protected-path was mechanically correct, edge-case was technically insightful, scope-alignment was accurate per COMMITS.md.
Human reviewers correctly overrode the protected-path block by providing explicit approvals.

Proposals

1 proposal filed (see below). Several candidate proposals were skipped because they overlap with existing open issues:

Scope-alignment under-rated — covered by #2105 (escalate prefix mismatches to CHANGES_REQUESTED).
Redundant review runs — covered by #1452, #893, and 10+ other deduplication issues.
Cancel reviews on merge — covered by #2388.

Proposals filed

Review status comment should not finalize until post-script completes (in fullsend-ai/fullsend)

github-actions Bot deployed to site-preview June 22, 2026 15:58 View deployment

ralphbean mentioned this pull request Jun 22, 2026

docs(security): add audit log integrity to threat model #2010

Merged

fullsend-ai-review Bot suggested changes Jun 22, 2026

View reviewed changes

fullsend-ai-review Bot added the component/ci CI pipelines and checks label Jun 22, 2026

ifireball approved these changes Jun 24, 2026

View reviewed changes

ascerra approved these changes Jun 24, 2026

View reviewed changes

ralphbean added this pull request to the merge queue Jun 24, 2026

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jun 24, 2026

Merge branch 'main' into fix/commit-lint-pr-commits

136c662

ralphbean enabled auto-merge June 24, 2026 16:19

github-actions Bot deployed to site-preview June 24, 2026 16:20 View deployment

ralphbean added this pull request to the merge queue Jun 24, 2026

fullsend-ai-review Bot suggested changes Jun 24, 2026

View reviewed changes

Merged via the queue into main with commit abf8a4c Jun 24, 2026
14 checks passed

ralphbean deleted the fix/commit-lint-pr-commits branch June 24, 2026 16:32

This was referenced Jun 24, 2026

fix(ci): skip e2e/functional tests in merge queue for irrelevant paths #2617

Open

e2e: CreateIssue call in admin_test.go lacks retry for transient 401s #2618

Open

fullsend-ai-retro Bot mentioned this pull request Jun 24, 2026

Review status comment should not finalize until post-script completes #2619

Open

Conversation

ralphbean commented Jun 22, 2026

Summary

Test plan

Uh oh!

qodo-code-review Bot commented Jun 22, 2026

PR Summary by Qodo

Uh oh!

github-actions Bot commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Site preview

Uh oh!

qodo-code-review Bot commented Jun 22, 2026

Code Review by Qodo

Uh oh!

fullsend-ai-review Bot commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Jun 22, 2026

Codecov Report

Uh oh!

fullsend-ai-review Bot commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review

Findings

High

Review

Findings

High

Low

Uh oh!

fullsend-ai-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

fullsend-ai-review Bot Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fullsend-ai-retro Bot commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-retro Bot commented Jun 24, 2026

Retro: PR #2514 — fix(ci): lint individual commits on PRs

Timeline

What went well

Proposals

Proposals filed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Jun 22, 2026 •

edited

Loading

fullsend-ai-review Bot commented Jun 22, 2026 •

edited

Loading

fullsend-ai-review Bot commented Jun 22, 2026 •

edited

Loading

fullsend-ai-review Bot commented Jun 24, 2026 •

edited

Loading

fullsend-ai-retro Bot commented Jun 24, 2026 •

edited

Loading

Retro: PR #2514 — `fix(ci): lint individual commits on PRs`