Walkthrough

The success aggregate job in .github/workflows/ci.yml—which combined results from compile-and-lint, test-smoke, and test into a single TS CI / Success status check—is removed along with a related comment block that referenced keeping the job name in sync.

Changes

CI Workflow Cleanup

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

• pnpm/pnpm#12453: Adds or adjusts the success aggregate gate in the same .github/workflows/ci.yml, including the conditional naming logic that this PR removes.

• X
• Mastodon
• Reddit
• LinkedIn

• Remove the TS CI / Success aggregate gate job from the workflow.
• Drop stale comments tying compile-and-lint to the removed gate job.
• Rely on individual job contexts for branch-protection requirements going forward.

graph TD
  E(["GitHub push/PR"]) --> C["Compile & Lint"] --> S["Test smoke"] --> T["Test matrix"]
  C --> K[("Status checks")]
  S --> K
  T --> K --> B{"Branch protection"}
  subgraph Legend
    direction LR
    _evt(["Event"]) ~~~ _job["CI job"] ~~~ _chk[("Check context")] ~~~ _bp{"Rule"}
  end

Loading

The following are alternative approaches to this PR:

1. Keep an aggregate gate job (rename or simplify)

• ➕ Branch protection can continue requiring a single stable context
• ➕ Avoids needing to enumerate matrix job contexts in protection rules
• ➖ Extra job logic/maintenance (needs/always/result aggregation)
• ➖ Risk of misconfiguration around duplicate runs and skipped contexts

2. Use a dedicated "required checks" workflow/job that only runs on push

• ➕ Produces a single required context without duplicate PR-run ambiguity
• ➕ Keeps per-job workflows simpler while preserving a stable merge gate
• ➖ Adds another workflow/job and more CI surface area
• ➖ Requires careful wiring so it truly depends on all needed jobs

3. Switch branch protection to require specific job contexts (no aggregate)

• ➕ Simplest CI workflow (no synthetic gate)
• ➕ Required checks map directly to real work and failures
• ➖ Can be brittle if the job set changes (matrix expansions/renames)
• ➖ Requires immediate branch protection updates to avoid blocking merges

Recommendation: Removing the aggregate job is fine if the repository’s branch protection (or ruleset) is updated at the same time to require the real job contexts (e.g., TS CI / Compile & Lint and the relevant TS CI / Test / ... checks). If frequent matrix shape changes are expected, consider retaining a small aggregate gate job (or a push-only gate) to keep a single stable required context.

Other (1) +0 / -30

ci.ymlRemove 'TS CI / Success' aggregate gate and related comments +0/-30

Remove 'TS CI / Success' aggregate gate and related comments

• Deletes the workflow’s aggregate 'success' job that previously acted as a single required branch-protection context. Removes the now-stale comment block and the "keep in sync" note tied to the gate job’s name/condition logic.

.github/workflows/ci.yml

Description

The PR deletes the success aggregate job, so the TS CI / Success check context no longer exists
and any branch protection rule that still requires it will keep PRs unmergeable. Without an
always-running, static-named gate, requiring individual TS CI job/matrix contexts is also brittle
when jobs can skip or the test matrix changes shape across refs.

Code

.github/workflows/ci.yml[L92-116]

-  # Single aggregate gate — the only TS CI context branch protection needs
-  # to require. The Test matrix changes shape per branch (see the excludes
-  # above) and the jobs skip on same-repo PR events, so requiring the
-  # individual per-combination contexts is brittle. This job always runs
-  # and fails only if a dependency actually failed or was cancelled —
-  # skipped dependencies count as a pass.
-  #
-  # The name is `TS CI / Success` on every run that does real work, but a
-  # different name on the same-repo pull_request run that `compile-and-lint`
-  # skips for deduplication. Otherwise that skipped run would report a
-  # green `TS CI / Success` and satisfy branch protection before the real
-  # push run finished — letting a PR merge without CI. The name condition
-  # is `compile-and-lint`'s `if:` verbatim and MUST stay in sync with it.
-  success:
-    name: ${{ (github.event_name == 'push' || github.event.pull_request.head.repo.full_name != github.repository || github.head_ref == 'chore/update-lockfile') && 'TS CI / Success' || 'TS CI / Success (skipped duplicate)' }}
-    if: ${{ always() }}
-    needs:
-      - compile-and-lint
-      - test-smoke
-      - test
-    runs-on: ubuntu-latest
-    steps:
-      - name: Fail if any dependency failed or was cancelled
-        if: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') }}
-        run: exit 1

Evidence

The TS CI workflow no longer defines any success job or TS CI / Success check, while still
containing conditional skipping and a ref-dependent test matrix. The repo’s Rust CI workflow
explicitly retains a success gate because requiring per-job/per-matrix contexts is brittle when
jobs skip or matrix contexts don’t appear, which is the same class of concern now reintroduced for
TS CI if branch protection is not updated carefully.

.github/workflows/ci.yml[8-86]
.github/workflows/ci.yml[9-15]
.github/workflows/ci.yml[54-82]
.github/workflows/pacquet-ci.yml[403-410]

Agent prompt

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

### Issue description
This PR removes the `success` aggregate gate job from `.github/workflows/ci.yml`. If branch protection still requires the `TS CI / Success` status context, PRs will remain blocked because that context will never be reported.

Additionally, this repo already documents (in Rust CI) that requiring individual job/matrix contexts is brittle when jobs skip or the matrix changes shape; TS CI still has skip/matrix-shape variability, so a stable always-running gate (or a carefully chosen stable required-check strategy) is needed.

### Issue Context
- TS CI now only defines `compile-and-lint`, `test-smoke`, and `test` jobs, with `compile-and-lint` conditionally skipped and `test` using ref-dependent excludes.
- Rust CI keeps a `success` gate explicitly to avoid branch-protection brittleness when jobs skip/matrix contexts may not appear.

### Fix Focus Areas
- .github/workflows/ci.yml[8-86]
- .github/workflows/pacquet-ci.yml[403-410]

### Suggested fix options
Pick one:
1) **Re-introduce a TS aggregate gate** (recommended if you want a single required context): add an always-running job (static name) that `needs` the TS jobs and fails when any needed job failed/cancelled.
2) **If intentionally removing the gate**, ensure branch protection is updated to require only check contexts that are guaranteed to be reported in all relevant scenarios (avoid relying on contexts that can be skipped or vary with matrix shape/ref).

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