ci(readiness): scope concurrency to bound PR refreshes by tamirdresher · Pull Request #2423 · github/squad

tamirdresher · 2026-11-08T19:21:57Z

What

Scopes the Squad PR Readiness writer's concurrency to bound PR refreshes. .github/workflows/squad-pr-readiness.yml drops its workflow-level group, keyed by head repository and branch, and splits into two jobs:

bind, named readiness-bind, validates the run, exposes the bound PR as job outputs, and joins no group.
readiness runs only for a bound PR. It joins squad-pr-readiness-<PR number>-<head SHA> with cancel-in-progress.

scripts/pr-readiness.mjs now writes the checklist only while the PR's head is still the bound head.

Working as github-integration-dev (GitHub Integration Engineer).

Why

#2409 moved readiness to a workflow_run writer with a workflow-level group keyed by head branch. Every delivery joins that group before any job's if: is evaluated. So:

Runs whose job is skipped can cancel a real checklist refresh: fork-PR and push runs of Squad CI, failed or cancelled requests, and Dependabot runs.
Two open PRs from one head branch to different bases (dev, main, insider) cancel each other.

Before #2409 the group was keyed per PR, so neither could happen. Copilot code review found this on #2409 (thread).

Closes #2420

How

Bind first, deduplicate only bound runs:
- bind keeps the existing gating if:, the claim download and the binding step. It publishes bound, pr_number, head_sha, base_ref, draft and labels as job outputs.
- readiness has needs: bind and if: needs.bind.outputs.bound == 'true'. Only then does it join its job-level group, keyed by PR number and head SHA.
- The checklist's PR_* inputs now come only from needs.bind.outputs.*.
Stale snapshots (from Copilot's review of this PR): bind checks the head outside any group. A delivery can therefore bind head A and reach readiness after head B was pushed and refreshed.
- run() compares PR_HEAD_SHA with the live PR head twice: when it first reads the PR, before any CI query, and again with a fresh read just before writing.
- On a mismatch it logs both SHAs, writes nothing and exits 0. If it can't read the head before writing, it fails without writing.
- Keying the group by head SHA means a late run for A can't cancel or replace B's refresh. Refreshes of one head still dedupe.
- Rejected alternatives:
  - cancel-in-progress: false: a late job could still replace B's pending job.
  - A queue: key: actionlint 1.7.12, which squad-workflow-lint.yml pins, rejects it.
Check-run names: workflow_run check runs land on the default-branch tip, which is the head commit of any dev → main PR. So readiness-bind joins readiness in SELF_CHECK_NAMES (scripts/pr-readiness.mjs). Otherwise a running or failing bind job, which a fork can trigger with a malformed claim, would show in that PR's "CI passing" item. Required checks are unaffected.
Checkouts: each job checks out only the script it runs, from the default branch, with persist-credentials: false. scripts/pr-readiness.mjs imports only Node built-ins, so the single-file sparse checkout is enough.
Tests (test/pull-request-workflow-split.test.ts): the existing contracts are now job-aware:
- each job has one default-branch checkout;
- dependent jobs wait for the binding;
- the bound inputs are exact.
New assertions:
- there's no workflow-level group, and the only group is the one on readiness, keyed by PR number and head SHA;
- every writer job's check-run name is in SELF_CHECK_NAMES;
- a small simulation computes which group each delivery joins:
  - 6 deliveries join none: a fork CI run, a push CI run on dev, a Dependabot run, a cancelled request, an unbound request, and a forged claim;
  - PRs from one head branch to different bases, a fork PR, and a dev → main PR each get their own group;
  - a late run for an older head doesn't join the current head's group;
  - runs of the same PR head still share a group.
Tests (test/pr-readiness.test.ts): out-of-order completion with a mocked fetch:
- a late run for an older head writes nothing;
- a head that moves while the checks run blocks the write;
- an unreadable head before writing fails without writing;
- a matching head still writes.
Behavior notes:
- A newer bound run for the same PR head cancels the older readiness job, and that check-run name is excluded from "CI passing".
- A run for an older head is no longer cancelled by a newer head. It stops at the first head check, which costs a few API calls.
- The head check and the write aren't atomic, because GitHub has no conditional update for issue comments. A stale write would need that one request to outlast the newer head's whole request, bind and readiness chain.
- Each delivery that isn't skipped now runs a short bind job even when superseded, at a small runner-time cost.
Out of scope:
- squad-impact.yml and squad-repo-health-report.yml keep branch-keyed groups. Each has one trigger and only dev as base, so neither case applies (see PR readiness: workflow-level concurrency lets skipped runs cancel checklist refreshes #2420).
- Dropping pull-requests: write from the bind job (least privilege) is left for later; the existing test forbids job-level permissions.
- The base ref and labels still come from the bind snapshot (PR readiness: base ref and labels come from the bind snapshot instead of the live PR #2424).

Validation (Windows, Node 27; branch rebased on dev):

Command	Result
`npx vitest run test/pull-request-workflow-split.test.ts test/workflow-run-pr-binding.test.ts test/pr-readiness.test.ts`	✅ 3 files, 202 passed
The same split suite with `.github/workflows/squad-pr-readiness.yml` restored from `dev`	❌ as expected: 9 failed, 40 passed. The messages name the workflow and group, e.g. `squad-pr-readiness.yml: a Squad CI run of a fork PR, which lists no PR joined "Squad PR Readiness-contributor/squad-feature"`
The three suites with the stale-snapshot fix reverted (workflow and script from `80b88597`, new tests kept)	❌ as expected: 5 failed, 197 passed. The messages name both SHAs, e.g. `the run bound to aaaa… wrote while PR #42 is at bbbb…`, and `the late run bound to aaaa… joins "squad-pr-readiness-7", the group of the refresh for current head bbbb…`
Targeted mutations of the fixed workflow and script. First round (12): dropping the posting job's `if:` or `needs`, a branch-keyed group, `cancel-in-progress: false`, a group on `bind`, re-adding the workflow-level group, `persist-credentials: true`, a checkout `ref:`, `PR_NUMBER` from the payload, a bind output bypassing the binding, removing `readiness-bind` from `SELF_CHECK_NAMES`, and others. Stale-snapshot round: dropping either head check, writing when the pre-write read fails, a silent skip, treating every head as stale, failing instead of exiting 0, and a group keyed only by PR or only by SHA	✅ each turns the suite red
`node scripts/pr-readiness.mjs` with a stubbed `fetch`	✅ a stale bound head prints `Skipping stale readiness snapshot for PR #42: bound head aaa… is not the live head bbb…; nothing was written.`, writes nothing and exits 0; a matching head creates the comment
`node --test test/ci-concurrency.test.cjs`	✅ 20 passed
`actionlint` 1.7.12 with shellcheck 0.10.0 over `.github/workflows/*.yml` (`.github/actionlint.yaml`)	✅ exit 0, including type checks of the `needs.bind.outputs.*` references
`npx eslint test/pr-readiness.test.ts test/pull-request-workflow-split.test.ts scripts/pr-readiness.mjs`	✅ clean

Not run locally: npm run build, the full npm test, and npm run lint. No package source changed, and CI runs them.

⚠️ Quick Check

If SDK/CLI source files changed: completed the applicable Changeset step below (npx changeset add / .changeset/*.md, direct CHANGELOG.md entry for maintainers, or skip-changelog label for no user-facing changes): not applicable, no SDK or CLI files changed

PR Readiness Checklist

The PR readiness bot will validate these automatically after push.
Check each item before requesting review. See CONTRIBUTING.md for full details.

Branch & Commit

Branch created from dev (not main)
Branch is up to date with dev (git fetch upstream && git rebase upstream/dev)
Verified diff contains only intended changes (git diff --cached --stat): 4 files, no deletions
PR is not in draft mode (mark ready when checks pass)
Commit history is clean (squash fixups before review): 2 commits, the change and the review fix; squash-merged

Build & Test

npm run build passes: not run locally; no package source changed, and CI builds
npm test passes (all tests green): the targeted suites above; CI runs the full suite
npm run lint passes (type check clean): not run; no TypeScript under packages/ changed
npm run lint:eslint passes: ran ESLint on the changed test files and the script
For migration PRs (>20 files): include test output summary in PR description: N/A (4 files)

Changeset

Changeset added via npx changeset add (if packages/squad-sdk/src/ or packages/squad-cli/src/ changed): not needed; only a workflow, a repo script and tests changed
Or direct CHANGELOG.md entry (maintainers only — write-protected for external contributors)
Or skip-changelog label applied (if no user-facing changes)

Docs

README section updated (if new feature/module): N/A
Docs feature page (if new user-facing capability): N/A

Exports

package.json subpath exports updated (if new module): N/A

Breaking Changes

None. The readiness comment and checklist format are unchanged. A run whose bound head is no longer the PR's head now writes nothing. A new check-run name, readiness-bind, appears on the default-branch tip and is excluded from "CI passing".

Waivers

None.

Squad PR Readiness used a workflow-level concurrency group keyed by head repository and branch. Every workflow_run delivery joins that group before any job's if: is evaluated. So skipped runs (fork and push runs of Squad CI, cancelled requests) cancelled real checklist refreshes, and PRs from one head branch to different bases cancelled each other. Split the writer into two jobs. bind joins no group and exposes the binding as job outputs. readiness runs only for a bound PR and joins squad-pr-readiness-<PR number> with cancel-in-progress. The bind job is named readiness-bind and added to SELF_CHECK_NAMES, because workflow_run check runs land on the dev tip, which is the head of any PR from dev. The split tests now check each job's checkout, require dependent jobs to wait for the binding, and simulate which group each delivery joins. Closes #2420 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-11-08T19:22:20Z

🟡 Impact Analysis — PR #2423

Risk tier: 🟡 MEDIUM

📊 Summary

Metric	Count
Files changed	4
Files added	0
Files modified	4
Files deleted	0
Modules touched	3

🎯 Risk Factors

4 files changed (≤5 → LOW)
3 modules touched (2-4 → MEDIUM)

📦 Modules Affected

ci-workflows (1 file)

.github/workflows/squad-pr-readiness.yml

scripts (1 file)

scripts/pr-readiness.mjs

tests (2 files)

test/pr-readiness.test.ts
test/pull-request-workflow-split.test.ts

This report is generated automatically for every PR. See #733 for details.

github-actions · 2026-11-08T19:22:24Z

🛫 PR Readiness Check

ℹ️ This comment updates on each push. Last checked: commit a15b25c

PR Scope: 🔧 Infrastructure

⚠️ 3 item(s) to address before review

Status	Check	Details
❌	Single commit	2 commits — consider squashing before review
✅	Not in draft	Ready for review
✅	Branch up to date	Up to date with dev
❌	Copilot review	No Copilot review yet — it may still be processing
✅	Changeset present	No source files changed — changeset not required
✅	Scope clean	No .squad/ or docs/proposals/ files
✅	No merge conflicts	No merge conflicts
✅	Copilot threads resolved	All 1 Copilot thread(s) resolved
❌	CI passing	5 check(s) failing: samples-build, sdk-exports-validation, test, Policy Gates, gh-aw strict compile

Files Changed (4 files, +448 −38)

File	+/−
`.github/workflows/squad-pr-readiness.yml`	+48 −13
`scripts/pr-readiness.mjs`	+46 −3
`test/pr-readiness.test.ts`	+107 −4
`test/pull-request-workflow-split.test.ts`	+247 −18

Total: +448 −38

This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.

github-actions · 2026-11-08T19:22:30Z

🏗️ Architectural Review

Authoritative evidence: scanners from the PR base commit, posted by the default-branch workflow_run reporter.
Results for head commit a15b25c5f39621894a4d02a4e4551f9fd74f8d96.

⚠️ Architectural review: 1 info.

Severity	Category	Finding	Files
ℹ️ info	template-sync	Template files changed in .github/workflows/ but not in other template locations. If these templates should stay in sync, consider updating the others too.	`Changed: .github/workflows/`, `Unchanged: templates/, .squad-templates/, packages/squad-cli/templates/`

Automated architectural review — informational only.

github-actions · 2026-11-08T19:22:31Z

🔒 Security Review

Authoritative evidence: scanners from the PR base commit, posted by the default-branch workflow_run reporter.
Results for head commit a15b25c5f39621894a4d02a4e4551f9fd74f8d96.

🔒 Security review: 1 warning(s).

Severity	Category	Finding	Location
🟡 warning	pii-env-var	Environment variable with sensitive name pattern (CREDENTIALS) — ensure this isn't hardcoded.	`.github/workflows/squad-pr-readiness.yml:111`

Automated security review — informational only.

Copilot

🟡 Changes recommended

The posting path does not revalidate the bound SHA, allowing a late job for an older PR head to overwrite the current head’s failing checklist with stale passing results.

1 open issue

Prevent stale bind results from overwriting the latest PR checklist · New

What changed in this PR

Moves PR readiness deduplication behind PR binding to prevent skipped deliveries and different PRs from cancelling each other’s checklist refreshes, addressing #2420.

Changes:

Splits binding and posting into separate jobs, with PR-keyed concurrency only on the bound posting job.
Excludes the new readiness-bind check run from the checklist’s CI evaluation.
Extends workflow tests for job dependencies, bound inputs, and concurrency isolation.

File	Description
`test/pull-request-workflow-split.test.ts`	Adds job-aware contracts and concurrency simulations.
`scripts/pr-readiness.mjs`	Excludes the bind job’s check-run name from CI evaluation.
`.github/workflows/squad-pr-readiness.yml`	Separates binding from posting and applies PR-scoped concurrency.

_{Review effort: Max (auto)}

The bind job checks the PR head outside the posting job's concurrency group. A delivery could therefore bind head A, reach the posting job after head B was pushed and refreshed, and overwrite B's checklist with A's: scripts/pr-readiness.mjs queried CI for A without comparing it with the live PR head. run() now compares PR_HEAD_SHA with the live PR head when it first reads the PR, before any CI query, and again just before it writes. On a mismatch it logs one line naming both SHAs, writes nothing and exits 0. If the PR cannot be read before writing, it fails without writing. The posting job's group is now keyed by PR number and head SHA. Keyed by PR alone, such a late job could cancel the current head's refresh (cancel-in-progress) or replace it while pending, then skip as stale, so no checklist was posted for the current head. Refreshes of one head still share a group, so the newer one wins. Tests cover out-of-order completion: a late run, a head that moves while the checks run, an unreadable head, and a matching head that still writes. The concurrency simulation checks that a late run bound to an older head joins a different group from the current head's refresh. Closes #2420 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

🔵 Needs a closer look

The privileged workflow_run writer refactor relies on GitHub concurrency and check-run placement behavior that can only be validated through post-merge runs on the default branch.

No open issues 🎉

1 resolved since last review

Prevent stale bind results from overwriting the latest PR checklist

_{Review effort: Max (auto)}

…#2436) #2409 moved the impact, Post Repo Health findings and heartbeat writers onto workflow_run jobs. Their check runs attach to the default-branch tip, not the PR head, and squad-impact.yml / squad-repo-health-report.yml cancel superseded runs, so a quick push on any PR leaves a `cancelled` check on dev. checkCIStatus() treats cancelled/failure as failing CI, so every dev-to-main PR reported "check(s) failing: impact" based on unrelated PRs' runs. readiness-bind was already excluded for this reason (#2423); this adds the remaining workflow_run writer names. Co-authored-by: Copilot <Copilot@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

tamirdresher marked this pull request as ready for review November 8, 2026 19:33

tamirdresher requested a review from bradygaster as a code owner November 8, 2026 19:33

Copilot AI auto review requested due to automatic review settings November 8, 2026 19:33

Copilot started reviewing on behalf of tamirdresher November 8, 2026 19:34

View session

Copilot stopped reviewing on behalf of tamirdresher due to an error November 8, 2026 19:35
Copilot stopped unexpectedly while processing your request.

Copilot started reviewing on behalf of tamirdresher November 8, 2026 19:36

View session

Copilot AI reviewed Nov 8, 2026

View reviewed changes

Comment thread .github/workflows/squad-pr-readiness.yml Resolved

tamirdresher mentioned this pull request Nov 8, 2026

PR readiness: base ref and labels come from the bind snapshot instead of the live PR #2424

Open

tamirdresher requested an auto review from Copilot November 8, 2026 20:27

Copilot started reviewing on behalf of tamirdresher November 8, 2026 20:28

View session

Copilot AI reviewed Nov 8, 2026

View reviewed changes

tamirdresher merged commit 0444041 into dev Nov 8, 2026
36 of 41 checks passed

tamirdresher mentioned this pull request Nov 9, 2026

fix(readiness): exclude workflow_run writer checks from CI evaluation #2436

Merged

Navigation Menu

Repository navigation

ci(readiness): scope concurrency to bound PR refreshes - #2423

ci(readiness): scope concurrency to bound PR refreshes#2423
tamirdresher merged 2 commits into
devfrom
squad/2420-readiness-concurrency

tamirdresher commented Nov 8, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Nov 8, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Nov 8, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Nov 8, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Nov 8, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

tamirdresher commented Nov 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

How

⚠️ Quick Check

PR Readiness Checklist

Branch & Commit

Build & Test

Changeset

Docs

Exports

Breaking Changes

Waivers

Uh oh!

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

Uh oh!

🟡 Impact Analysis — PR #2423

📊 Summary

🎯 Risk Factors

📦 Modules Affected

Uh oh!

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

Uh oh!

🛫 PR Readiness Check

⚠️ 3 item(s) to address before review

Files Changed (4 files, +448 −38)

Uh oh!

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

Uh oh!

🏗️ Architectural Review

Uh oh!

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

Uh oh!

🔒 Security Review

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tamirdresher commented Nov 8, 2026 •

edited

Loading

github-actions Bot commented Nov 8, 2026 •

edited

Loading

github-actions Bot commented Nov 8, 2026 •

edited

Loading

github-actions Bot commented Nov 8, 2026 •

edited

Loading

github-actions Bot commented Nov 8, 2026 •

edited

Loading