fix(ci): guard the activation CLI pin against silent release drift - #1855
Merged
Conversation
The activation pin in workflows/shared/squad.md decays every npm release with nothing detecting it. It was introduced correct on 2026-08-07 at 0.11.0, 0.12.0 published on 2026-08-13, and the pin sat stale 8 days until a cold start against a throwaway repo happened to surface it (PR #1818). `git log -S` shows the line had been modified exactly once, ever -- at introduction. The pin stays: activation runs on an unrestricted-network job and hands state to the sandboxed agent job, so a bad point release mid-hop is expensive to debug. The missing drift mechanism was the defect. Two halves, each checking what the other structurally cannot: - offline, every PR: the file agrees with itself - online, daily: the file agrees with npm's published dist-tag Neither reads packages/squad-cli/package.json, which holds the next unreleased version (0.13.0, E404 on npm) -- the trap that would reproduce #1818. Also removes an unreachable `${SQUAD_CLI_VERSION:-0.12.0}` shell fallback that could never fire, since the env value is always non-empty. Being unreachable, it would have gone stale invisibly. Closes #1825 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 20afe6d2-444e-414e-8a39-e67ab67ca6df
Contributor
🟡 Impact Analysis — PR #1855Risk tier: 🟡 MEDIUM 📊 Summary
🎯 Risk Factors
📦 Modules Affectedci-workflows (1 file)
root (1 file)
tests (1 file)
This report is generated automatically for every PR. See #733 for details. |
Contributor
🛫 PR Readiness Check
PR Scope: 🔧 Infrastructure
|
| Status | Check | Details |
|---|---|---|
| ❌ | Single commit | 3 commits — consider squashing before review |
| ✅ | Not in draft | Ready for review |
| ❌ | Branch up to date | dev is 1 commit(s) ahead — rebase recommended |
| ❌ | 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 | 3 unresolved Copilot thread(s) — fix and resolve before merging |
| ✅ | CI passing | All checks passing |
Files Changed (3 files, +305 −1)
| File | +/− |
|---|---|
.github/workflows/squad-cli-pin-drift.yml |
+147 −0 |
test/squad-cli-pin.test.ts |
+157 −0 |
workflows/shared/squad.md |
+1 −1 |
Total: +305 −1
This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.
Contributor
🏗️ Architectural Review
Automated architectural review — informational only. |
Contributor
🔒 Security Review🔒 Security review: 1 info.
Automated security review — informational only. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Pin checkout to an immutable SHA and strengthen both test assertions.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds CI safeguards to detect drift between the Squad CLI pin and npm releases.
Changes:
- Removes the unreachable CLI version fallback.
- Adds offline pin-consistency tests.
- Adds scheduled/manual npm drift detection that files an issue and fails on mismatch.
File summaries
| File | Summary |
|---|---|
workflows/shared/squad.md |
Uses the configured CLI version directly. |
test/squad-cli-pin.test.ts |
Adds pin consistency and workflow safety tests; two moderate issues remain in test coverage. |
.github/workflows/squad-cli-pin-drift.yml |
Detects npm drift and reports failures; checkout must be pinned to an immutable commit SHA (critical). |
Review details
Suppressed comments (5)
.github/workflows/squad-cli-pin-drift.yml:102
- This de-duplication never closes or refreshes the issue it finds. After a maintainer fixes the pin, the generated issue remains open; a later release creates the same drift, hits this branch, and suppresses the only current alert/body. Close the matching issue when
drift=false, or only de-duplicate an issue whose recorded values match the current drift.
existing="$(gh issue list --state open --search "$TITLE in:title" --json number --jq '.[0].number // empty')"
if [ -n "$existing" ]; then
echo "Drift issue already open: #$existing — not filing a duplicate."
exit 0
.github/workflows/squad-cli-pin-drift.yml:62
- The tests parse the workflow with separate regexes but never execute the
sedextraction (or the equality branch) used by this step. A mutation of this command or comparison can therefore leave all five tests green while the scheduled guard fails to detect drift; add a deterministic test that runs the same extracted script with stubbednpm/gh, or factor the comparison into a testable helper.
pinned="$(sed -n "s/.*SQUAD_CLI_VERSION: .*|| '\([^']*\)'.*/\1/p" "$PIN_FILE" | head -1)"
.github/workflows/squad-cli-pin-drift.yml:118
docs/src/content/docs/guide/gh-aw.md:145is another copy of this default. If someone follows the generated issue and updates only the two literals listed here, the public guide continues to advertise the old version, leaving user-facing instructions stale after the next release; include the guide row in the remediation (or in the consistency check).
printf 'Update **both** literals in `workflows/shared/squad.md` to `%s`:\n\n' "$LATEST"
printf '%s\n' '1. the `Default is <version>.` line in the header comment'
printf '%s\n\n' '2. the `SQUAD_CLI_VERSION` fallback literal in the activation `env:` block'
printf '%s\n\n' '`test/squad-cli-pin.test.ts` fails if those two disagree, so they cannot be updated by halves.'
.github/workflows/squad-cli-pin-drift.yml:30
workflow_dispatchcan be launched against an arbitrary branch, and checkout withoutreffollows the selected ref. A manual run from a stale feature branch will read that branch's oldsquad.md, compare it with current npm, and create a false drift issue even when the default branch is in sync. Restrict manual runs to the default branch or explicitly check outgithub.event.repository.default_branchbefore extracting the pin.
workflow_dispatch:
test/squad-cli-pin.test.ts:59
- This only matches the three-component prefix, so values such as
0.12.0garbagepass the test even though they are not npm versions andnpxcannot resolve them. Since this is the offline guard for a resolvable fallback, anchor the assertion to a complete semver shape (including optional prerelease/build metadata).
expect(pin).toMatch(/^\d+\.\d+\.\d+/);
- Files reviewed: 3/3 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| issues: write | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v6.0.0 |
Comment on lines
+82
to
+83
| expect(line, `npx line hardcodes a version instead of using the env var: ${line.trim()}`) | ||
| .not.toContain(`squad-cli@${pin}`); |
| // unreleased version, so a guard reading it would compare the repo to itself and | ||
| // then "fix" the pin to something npm cannot install. | ||
| expect(executable).not.toMatch(/packages\/squad-cli\/package\.json/); | ||
| expect(executable).toMatch(/dist-tags\.latest/); |
CI lints with shellcheck 0.10.0 pinned; my workstation had no shellcheck at all, so local actionlint silently skipped its shellcheck integration and the drift workflow looked clean. Installed shellcheck 0.10.0 locally, reproduced the CI failure exactly, then fixed it. SC2016 fires on the markdown BACKTICKS in the issue-body printf format strings: shellcheck reads ` as command substitution and correctly reports it will not expand inside single quotes. That is the intent -- they are literal code spans in the rendered issue, and every real value ($PINNED, $LATEST) is already passed as a printf argument rather than interpolated into a format. Suppressed by exact code, scoped to that block with a written rationale, not file-wide and not by widening .github/actionlint.yaml -- same standard applied in #1827. SC2016 also catches a genuine defect: '$VAR' in single quotes when expansion was intended, which emits the literal text. The disable would hide that too, so `test/squad-cli-pin.test.ts` now asserts no single-quoted literal in the workflow contains a $expansion. Mutation-verified: planting `version is $LATEST` inside the suppressed block leaves actionlint CLEAN -- shellcheck is blind to it, as designed -- while the new test fails. The suppression removes the false positives without removing the check. 6/6 pass restored. Refs #1825 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 20afe6d2-444e-414e-8a39-e67ab67ca6df
This was referenced Aug 24, 2026
bradygaster
added a commit
that referenced
this pull request
Aug 24, 2026
The pin in `workflows/shared/squad.md` decides which Squad CLI a brand-new repository installs during activation. Nothing about publishing touched it, so every release silently made it stale -- it once ran 8 days behind and was caught only because someone happened to cold-start a throwaway repo (PR #1818). PR #1855 added the daily backstop that *detects* that state. This adds the half that prevents it: after `publish-cli` succeeds, the pin moves to the version that same run made installable, and the change arrives as a pull request against `dev`. Both halves are load-bearing. Detection alone leaves a red build with no fix path; prevention alone re-breaks the moment someone publishes out-of-band. Three things worth knowing: - The version is confirmed against two independent sources before anything is rewritten -- the version this run published, and npm's `dist-tags.latest`. The bumper never reads `packages/squad-cli/package.json`, which holds the next *unreleased* version and is exactly the E404 that PR #1818 fixed by hand. - There was a third copy of the pin nobody was guarding, in `docs/src/content/docs/guide/gh-aw.md`. Neither the drift guard nor the existing tests looked at it, so it could sit stale while everything reported green. It is now bumped with the other two and asserted equal on every pull request. - The rewrite is a script rather than an inline `run:` block. Its patterns contain backticks and pipes that are hostile to shell quoting -- the SC2016 problem PR #1855 had to suppress -- and as a file it can be executed by the test suite. That last point is what makes this guard non-decorative: `squad-cli-pin.test.ts` runs the real bumper against the version already pinned. That is an identity rewrite, so it touches nothing, but it proves all three patterns still match. A pattern that quietly stops matching would turn the bump into a no-op, which is the same silent decay this issue was filed about. Verified by mutation: reshaping the pin, drifting the docs copy, and retargeting the pull request at `main` each turn the suite red, and the bumper fails closed with a named error when a site goes missing or is duplicated. Closes #1825 Co-authored-by: brady gaster <bradygaster@github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #1825 — delivers the CI backstop half. Does not close the issue; the release-time bump remains (see the end).
The pin is correct today — that is exactly the problem
workflows/shared/squad.mdpins0.12.0. npmdist-tags.latestis0.12.0. Nothing to bump.That is the point. The pin was also correct the day it was introduced, and it went stale eight days later without anyone touching it —
git log -Sshows the line has been modified exactly once, ever, at introduction. Fixing the value would fix nothing; the defect is that nothing notices when the value goes wrong.Two halves, deliberately split
Each checks something the other structurally cannot:
test/squad-cli-pin.test.ts.github/workflows/squad-cli-pin-drift.ymlworkflow_dispatchThe network check cannot live in the unit suite (it fails closed on an offline machine); the self-consistency check cannot live in the scheduled job (drift would only be caught the morning after the PR merged).
The trap, respected
The issue warns in bold: do not derive the pin from
packages/squad-cli/package.json. It reads0.13.0— unpublished,E404on npm — so automating from it drives straight into the failure PR #1818 just fixed.The guard reads npm
dist-tags.latest, a genuinely independent source. A test asserts the workflow never reads the in-repo manifest in an executable line, so a future "simplification" cannot quietly reintroduce it. (Comment lines are exempt — the header names the trap deliberately, and documenting a hazard shouldn't trip the check that enforces it.)On drift it opens an issue and then fails, so the alert arrives with a fix path rather than an unexplained red run. It de-dupes against an already-open issue: a daily job that files a duplicate every morning trains people to ignore it.
An unreachable third copy, removed
The version appeared three times. The third was dead:
npx --yes "@bradygaster/squad-cli@${SQUAD_CLI_VERSION:-0.12.0}" ...SQUAD_CLI_VERSIONcomes fromvars.SQUAD_CLI_VERSION || '0.12.0', which always yields a non-empty string, so:-can never fire. It was a third place the version could rot — and being unreachable, it would rot invisibly. Now${SQUAD_CLI_VERSION}: if it ever were empty,npxfails loudly instead of silently installing a stale version.Two literals remain (header comment, env fallback) and a test asserts they are equal, so they cannot be updated by halves — the comment is what a human reads when checking whether activation is current, so a stale one misinforms exactly the person trying to verify the pin.
Verification
actionlint: exit 0.gh aw compile --strict: exit 0; emitted lock carries the corrected line.0.12.0. Reshaping the pin line yields empty and exits 1, rather than silently reporting "no drift" against a pin it never found.Mutation-tested — 5 tests, four mutations, one failure each, no cross-firing:
:-fallbackrun:blockTwo bugs the guards caught in this PR's own code
Worth recording, because both were silent:
printf.${{ ... }}written as documentation inside the issue body would have been evaluated by Actions before the shell ever saw it, printing a resolved value instead of the template. That is the same class the shell-input contract inworkflows/squad.mdexists to prevent, so the final step now passes step outputs throughenv:and a test forbids interpolation insiderun:blocks.Remaining on #1825
The release-time bump — the pin moving as part of publishing rather than waiting for the next morning's guard. That belongs in the release flow (five candidate workflows) and is a separate change with a different owner. This PR delivers the backstop that catches drift either way, including out-of-band releases, so the issue stays open for that half rather than being closed half-done.