Skip to content

ci(changeset): make coverage gaps a visible non-blocking check#170

Merged
PaulNewling merged 1 commit into
v4-betafrom
paulnewling/changeset-coverage-diagnostic-job
May 28, 2026
Merged

ci(changeset): make coverage gaps a visible non-blocking check#170
PaulNewling merged 1 commit into
v4-betafrom
paulnewling/changeset-coverage-diagnostic-job

Conversation

@PaulNewling
Copy link
Copy Markdown
Collaborator

@PaulNewling PaulNewling commented May 28, 2026

Problem

check-coverage ran as a step inside check-changesets with continue-on-error: true. A coverage gap exits 1, but GitHub forces the step conclusion to success (only outcome is failure). So a real gap showed only as an ::error:: annotation on an otherwise-green run — reviewers miss it.

Change

Move check-coverage into its own changeset coverage (diagnostic) job:

  • the step drops continue-on-error → a gap turns the job red;
  • the job sets continue-on-error: true → the run stays green and the merge is not blocked.

Net: a gap surfaces as a red changeset coverage (diagnostic) check, while never blocking. The native changeset status gate in check-changesets is unchanged.

Scope

Only node-simple-pnpm.yaml consumes check-coverage; node-matrix-pnpm.yaml has no coverage step. Keep the changeset coverage (diagnostic) check out of required status checks so it stays non-blocking.

Verification

Canary against platforma-open/antibody-sequence-liabilities#63 (model covered by a changeset, software package edited without one) — expect a red diagnostic job with a green run.

Greptile Summary

This PR extracts the check-coverage step from check-changesets into a new top-level changeset-coverage job. The key insight is that step-level continue-on-error: true hides failures by forcing the step's conclusion to success, while job-level continue-on-error: true keeps the workflow run green but surfaces the job's check as red — giving reviewers a visible signal without blocking the merge.

  • The new changeset-coverage job correctly restricts to pull_request/merge_group events, needs only metadata, and removes continue-on-error from the step level so a gap fails the job.
  • No downstream jobs reference changeset-coverage, keeping it purely diagnostic as intended.
  • The duplicated setup (checkout, pnpm prepare, install) is unavoidable with GitHub Actions job isolation and adds ~1–2 min of CI time per PR.

Confidence Score: 4/5

Safe to merge; the change correctly moves a diagnostic check into its own job and the core changeset gate is untouched.

The restructuring is sound — job-level continue-on-error: true achieves the visible-but-non-blocking goal. The one open question is whether continue-on-error should remain unconditional on merge_group events, since every other preflight job in this workflow conditionally enforces on merge_group. If coverage is intended to be permanently advisory even in the merge queue, the current code is correct as-is.

.github/workflows/node-simple-pnpm.yaml — specifically the changeset-coverage job's continue-on-error value and the duplicated pnpm setup steps.

Important Files Changed

Filename Overview
.github/workflows/node-simple-pnpm.yaml Moves check-coverage from a step inside check-changesets to a new standalone changeset-coverage job with job-level continue-on-error: true, correctly making coverage gaps surface as a red check without blocking the workflow run or merge.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    metadata([metadata]) --> check-changesets
    metadata --> changeset-coverage

    check-changesets["check-changesets\n(blocking on merge_group)"]
    check-changesets --> step_status["pnpm changeset status"]

    changeset-coverage["changeset-coverage\n(continue-on-error: true)"]
    changeset-coverage -->|if: pull_request OR merge_group| setup["checkout + pnpm prepare + pnpm install"]
    setup --> step_coverage["check-coverage action"]

    step_status -->|fail| job_cs_red["❌ check-changesets\n(blocks merge)"]
    step_status -->|pass| job_cs_green["✅ check-changesets\n(gate cleared)"]

    step_coverage -->|fail| job_cov_red["🔴 changeset coverage check = red\n(workflow stays green)"]
    step_coverage -->|pass| job_cov_green["✅ changeset coverage check = green"]
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
.github/workflows/node-simple-pnpm.yaml:593-594
The `changeset-coverage` job has `continue-on-error: true` hardcoded, unlike every sibling preflight job which conditionally enforces on `merge_group`. On a `merge_group` event a coverage gap is therefore fully silent to the merge queue — the queue sees the job's **conclusion** as `success`. If the team ever wants coverage to be enforced in the merge queue, the existing pattern used by `check-changesets` and `preflight-*` jobs would need to be adopted here.

```suggestion
    continue-on-error: ${{ github.event_name != 'merge_group' }}
    if: github.event_name == 'pull_request' || github.event_name == 'merge_group'
```

Reviews (1): Last reviewed commit: "ci(changeset): make coverage gaps a visi..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

check-coverage ran as a step with continue-on-error: true, so a gap
exited 1 but the step was reported success — the gap showed only as an
annotation on a green run, which reviewers miss.

Move it to its own 'changeset coverage (diagnostic)' job: the step drops
continue-on-error (a gap turns the job red), the job sets
continue-on-error: true (run stays green, merge not blocked). The native
'changeset status' gate in check-changesets is unchanged.

Only node-simple-pnpm.yaml uses check-coverage.
@PaulNewling PaulNewling marked this pull request as ready for review May 28, 2026 19:33
@PaulNewling PaulNewling merged commit 66d444e into v4-beta May 28, 2026
1 check passed
@PaulNewling PaulNewling deleted the paulnewling/changeset-coverage-diagnostic-job branch May 28, 2026 19:35
Comment on lines +593 to +594
continue-on-error: true
if: github.event_name == 'pull_request' || github.event_name == 'merge_group'
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 The changeset-coverage job has continue-on-error: true hardcoded, unlike every sibling preflight job which conditionally enforces on merge_group. On a merge_group event a coverage gap is therefore fully silent to the merge queue — the queue sees the job's conclusion as success. If the team ever wants coverage to be enforced in the merge queue, the existing pattern used by check-changesets and preflight-* jobs would need to be adopted here.

Suggested change
continue-on-error: true
if: github.event_name == 'pull_request' || github.event_name == 'merge_group'
continue-on-error: ${{ github.event_name != 'merge_group' }}
if: github.event_name == 'pull_request' || github.event_name == 'merge_group'
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/node-simple-pnpm.yaml
Line: 593-594

Comment:
The `changeset-coverage` job has `continue-on-error: true` hardcoded, unlike every sibling preflight job which conditionally enforces on `merge_group`. On a `merge_group` event a coverage gap is therefore fully silent to the merge queue — the queue sees the job's **conclusion** as `success`. If the team ever wants coverage to be enforced in the merge queue, the existing pattern used by `check-changesets` and `preflight-*` jobs would need to be adopted here.

```suggestion
    continue-on-error: ${{ github.event_name != 'merge_group' }}
    if: github.event_name == 'pull_request' || github.event_name == 'merge_group'
```

How can I resolve this? If you propose a fix, please make it concise.

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.

1 participant