ci: collapse Canvas (Next.js) to single job with conditional steps
Supersedes PR #2321's two-jobs-sharing-a-name approach, which didn't actually clear branch-protection's required-check evaluation. Live test on PR #2314: GraphQL `isRequired` confirmed BOTH check runs under "Canvas (Next.js)" name (one SUCCESS via no-op, one SKIPPED via real job) registered, and the SKIPPED one kept mergeStateStatus = BLOCKED despite the SUCCESS sibling. Branch protection's "set of matching contexts" semantic is stricter than the durable feedback memory documented — at least one passing isn't enough; SKIPPED counts as not-passed regardless. Real fix: ONE job that always runs (no job-level `if:`), with all real work gated on the path filter via per-step `if:`. Produces exactly one "Canvas (Next.js)" check run per commit, always SUCCEEDS, regardless of which paths changed. Costs ~10s of always-on CI runtime per PR — negligible vs. the manual-merge cost when the BLOCKED state catches us. This same anti-pattern probably affects Platform (Go) (`platform` filter), Python Lint & Test (`python` filter), and Shellcheck (E2E scripts) (`scripts` filter) — all required, all path-gated. PR-A and PR-2321 merged because they happened to trigger every filter; PR-B only missed canvas. File a follow-up issue to apply the same single-job-conditional-steps pattern across those required jobs to remove the latent merge-blocker. Updates feedback memory: branch_protection_check_name_parity is wrong about "two jobs sharing name + at-least-one-success works." Need to correct the note.
This commit is contained in:
parent
da17753dec
commit
e22a56d351
54
.github/workflows/ci.yml
vendored
54
.github/workflows/ci.yml
vendored
@ -178,42 +178,44 @@ jobs:
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Path-filter no-op shadow for Canvas (Next.js).
|
||||
# Canvas (Next.js) is a required check on staging branch protection.
|
||||
# The job ALWAYS runs (no `if:` gate at job level) so branch protection
|
||||
# always sees a SUCCESS conclusion. Inside, every meaningful step is
|
||||
# gated on `needs.changes.outputs.canvas == 'true'` — when the PR
|
||||
# doesn't touch canvas/**, the job spins up, skips all real work, and
|
||||
# exits clean in ~10s.
|
||||
#
|
||||
# Branch protection on staging requires a "Canvas (Next.js)" check.
|
||||
# When a PR doesn't touch canvas/** paths, the real canvas-build job
|
||||
# below is skipped via `if:`, and GitHub reports its conclusion as
|
||||
# SKIPPED — which branch protection treats as not-passed → merge
|
||||
# BLOCKED on every workspace-server-only or migration-only PR.
|
||||
#
|
||||
# Pattern (per durable feedback memory: branch_protection_check_name_parity):
|
||||
# split into a real job + a no-op shadow that share the same `name:`.
|
||||
# Exactly one runs per PR; both report the same check context, and at
|
||||
# least one always reports SUCCESS, satisfying the required check.
|
||||
canvas-build-noop:
|
||||
name: Canvas (Next.js)
|
||||
needs: changes
|
||||
if: needs.changes.outputs.canvas != 'true'
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- run: echo "No canvas/** changes in this PR — Canvas (Next.js) skip is intentional, satisfying required-check via this no-op."
|
||||
|
||||
# Why not a job-level `if:` + a no-op shadow:
|
||||
# An earlier version (PR #2321) tried two jobs sharing the same `name:`
|
||||
# — one with `if: == 'true'`, the other with `if: != 'true'`. Branch
|
||||
# protection sees BOTH check runs and treats the SKIPPED one as
|
||||
# not-passed even when the other reports SUCCESS — so the merge stays
|
||||
# BLOCKED. Single-job-with-conditional-steps is the only shape that
|
||||
# produces exactly one Canvas (Next.js) check run per commit AND
|
||||
# always SUCCEEDS, regardless of which paths changed.
|
||||
canvas-build:
|
||||
name: Canvas (Next.js)
|
||||
needs: changes
|
||||
if: needs.changes.outputs.canvas == 'true'
|
||||
runs-on: ubuntu-latest
|
||||
defaults:
|
||||
run:
|
||||
working-directory: canvas
|
||||
steps:
|
||||
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
|
||||
- uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
|
||||
- if: needs.changes.outputs.canvas != 'true'
|
||||
working-directory: .
|
||||
run: echo "No canvas/** changes — skipping real build steps; this job always runs to satisfy the required-check name on branch protection."
|
||||
- if: needs.changes.outputs.canvas == 'true'
|
||||
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
|
||||
- if: needs.changes.outputs.canvas == 'true'
|
||||
uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
|
||||
with:
|
||||
node-version: '22'
|
||||
- run: rm -f package-lock.json && npm install
|
||||
- run: npm run build
|
||||
- name: Run tests with coverage
|
||||
- if: needs.changes.outputs.canvas == 'true'
|
||||
run: rm -f package-lock.json && npm install
|
||||
- if: needs.changes.outputs.canvas == 'true'
|
||||
run: npm run build
|
||||
- if: needs.changes.outputs.canvas == 'true'
|
||||
name: Run tests with coverage
|
||||
# Coverage instrumentation is configured in canvas/vitest.config.ts
|
||||
# (provider: v8, reporters: text + html + json-summary). Step 2 of
|
||||
# #1815 — wires coverage into CI so we get a baseline visible on
|
||||
@ -224,7 +226,7 @@ jobs:
|
||||
# thresholds + a hard gate" — this PR ships the observability half.
|
||||
run: npx vitest run --coverage
|
||||
- name: Upload coverage summary as artifact
|
||||
if: always()
|
||||
if: needs.changes.outputs.canvas == 'true' && always()
|
||||
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
|
||||
with:
|
||||
name: canvas-coverage-${{ github.run_id }}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user