diff --git a/.github/workflows/squad-cli-pin-drift.yml b/.github/workflows/squad-cli-pin-drift.yml new file mode 100644 index 000000000..fc491a4dc --- /dev/null +++ b/.github/workflows/squad-cli-pin-drift.yml @@ -0,0 +1,147 @@ +# Squad CLI pin drift guard (#1825) +# +# `workflows/shared/squad.md` pins the Squad CLI version that new-repo activation +# installs. That pin decays silently with every npm release: it was introduced +# correct on 2026-08-07 referencing 0.11.0, 0.12.0 published on 2026-08-13, and the +# pin then sat stale for 8 days with nobody touching it. `git log -S` shows the line +# had been modified exactly once, ever — at introduction. It was caught only because +# an agent happened to run a cold start against a throwaway repo (fixed in PR #1818). +# +# The pin itself is correct and 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 absence of a drift mechanism was the defect, not the pin. +# +# This job compares the pin against the **published** npm dist-tag. It deliberately +# does NOT read `packages/squad-cli/package.json`: that is the next unreleased version +# (0.13.0 at time of writing) and resolves to E404 on npm, so automating from it would +# drive straight into the failure PR #1818 just fixed. A guard that reads the same +# source as the thing it guards is decoration (.squad/decisions.md:604) — so the two +# sources here are independent by construction. +# +# On drift it opens an issue rather than only failing, so the alert arrives with a fix +# path attached instead of an unexplained red run. + +name: Squad CLI Pin Drift + +on: + schedule: + # Daily at 07:15 UTC. Off the hour to avoid the scheduler's peak-minute backlog. + - cron: '15 7 * * *' + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: squad-cli-pin-drift + cancel-in-progress: false + +jobs: + check-pin: + name: Compare activation pin against published npm release + runs-on: ubuntu-latest + permissions: + contents: read + issues: write + steps: + - name: Checkout + uses: actions/checkout@v6.0.0 + + - name: Compare pin against npm dist-tag + id: compare + env: + PIN_FILE: workflows/shared/squad.md + NPM_PACKAGE: '@bradygaster/squad-cli' + run: | + set -euo pipefail + + # The pin as activation actually resolves it: the literal fallback in + # `vars.SQUAD_CLI_VERSION || ''`. Extracted with sed rather than a + # PCRE grep because the escaping stays readable, and an extractor nobody can + # read is an extractor nobody will notice has stopped matching. + pinned="$(sed -n "s/.*SQUAD_CLI_VERSION: .*|| '\([^']*\)'.*/\1/p" "$PIN_FILE" | head -1)" + if [ -z "$pinned" ]; then + echo "::error file=$PIN_FILE::Could not locate the SQUAD_CLI_VERSION pin. If the pin moved or changed shape, update this guard — a guard that silently stops finding its target is worse than no guard." + exit 1 + fi + + latest="$(npm view "$NPM_PACKAGE" dist-tags.latest 2>/dev/null || true)" + if [ -z "$latest" ]; then + echo "::error::Could not read dist-tags.latest for $NPM_PACKAGE from npm." + exit 1 + fi + + echo "pinned=$pinned" >> "$GITHUB_OUTPUT" + echo "latest=$latest" >> "$GITHUB_OUTPUT" + echo "Pinned in $PIN_FILE : $pinned" + echo "Published npm latest : $latest" + + if [ "$pinned" = "$latest" ]; then + echo "drift=false" >> "$GITHUB_OUTPUT" + echo "✅ Activation pin matches the published release." + else + echo "drift=true" >> "$GITHUB_OUTPUT" + echo "::warning::Activation pin ($pinned) is behind the published release ($latest)." + fi + + - name: Open drift issue + if: steps.compare.outputs.drift == 'true' + env: + GH_TOKEN: ${{ github.token }} + PINNED: ${{ steps.compare.outputs.pinned }} + LATEST: ${{ steps.compare.outputs.latest }} + TITLE: 'Squad CLI activation pin is behind the published release' + run: | + set -euo pipefail + + # Only one open issue at a time: this runs daily, and a guard that files a + # duplicate every morning trains people to ignore it. + 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 + fi + + # Built with printf rather than a heredoc: heredoc bodies must sit at column 0, + # which terminates the YAML block scalar this script lives in. + # + # shellcheck disable=SC2016 + # SC2016 ("expressions don't expand in single quotes") fires on the markdown + # BACKTICKS below -- shellcheck reads ` as command substitution, and inside + # single quotes it correctly notes it will not expand. That is exactly what we + # want: these are literal code spans in the issue body, not shell expansions. + # Every real value ($PINNED, $LATEST) is passed as a printf ARGUMENT, never + # interpolated into the format string. Scoped to this block, not the file, and + # `test/squad-cli-pin.test.ts` asserts no single-quoted literal here contains a + # `$name` expansion -- so this suppression cannot mask a genuine SC2016 defect. + { + printf '%s\n\n' 'The Squad CLI version pinned for new-repo activation is behind the published npm release.' + printf '%s\n' '| | Version |' + printf '%s\n' '|---|---|' + printf '| Pinned in `workflows/shared/squad.md` | `%s` |\n' "$PINNED" + printf '| Published npm `dist-tags.latest` | `%s` |\n\n' "$LATEST" + printf 'New-repo activation installs `%s`, so anything that shipped in `%s` is missing from a cold start.\n\n' "$PINNED" "$LATEST" + printf '%s\n\n' '### Fix' + printf 'Update **both** literals in `workflows/shared/squad.md` to `%s`:\n\n' "$LATEST" + printf '%s\n' '1. the `Default is .` 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.' + printf '%s\n' '> Do **not** copy the version from the CLI package manifest in this repo. That is the' + printf '%s\n\n' '> next *unreleased* version and resolves to E404 on npm — the exact failure PR #1818 fixed.' + printf '%s\n' 'Filed automatically by `.github/workflows/squad-cli-pin-drift.yml` (#1825).' + } > "$RUNNER_TEMP/drift-body.md" + + gh issue create --title "$TITLE" --body-file "$RUNNER_TEMP/drift-body.md" + + - name: Fail the run on drift + if: steps.compare.outputs.drift == 'true' + env: + PINNED: ${{ steps.compare.outputs.pinned }} + LATEST: ${{ steps.compare.outputs.latest }} + run: | + # Values pass through env rather than expression interpolation, per the + # shell-input contract in workflows/squad.md: interpolation splices text into + # the script before the shell parses it, so the quoting that would contain a + # hostile value never gets a chance to apply. + echo "::error::Activation pin $PINNED != published $LATEST. See the issue filed by the previous step." + exit 1 diff --git a/test/squad-cli-pin.test.ts b/test/squad-cli-pin.test.ts new file mode 100644 index 000000000..1a7e9667f --- /dev/null +++ b/test/squad-cli-pin.test.ts @@ -0,0 +1,157 @@ +/** + * Squad CLI activation pin consistency (#1825) + * + * `workflows/shared/squad.md` states the pinned CLI version twice: once as prose in + * the header comment that documents `vars.SQUAD_CLI_VERSION`, and once as the literal + * fallback that activation actually resolves. Two copies of a version number drift + * against each other, and the prose copy is the one a human reads when deciding + * whether activation is current — so a stale comment misinforms precisely the person + * trying to verify the pin. + * + * This suite is the offline half of the guard. The online half + * (`.github/workflows/squad-cli-pin-drift.yml`) compares the pin against npm's + * published `dist-tags.latest` on a schedule; it cannot run here because it needs the + * network, and a network call in the unit suite fails closed on an offline machine. + * + * Split that way, each half checks something the other structurally cannot: + * - here: the file agrees with itself, on every PR, deterministically + * - there: the file agrees with reality, daily + * + * Neither reads `packages/squad-cli/package.json`. That holds the next *unreleased* + * version — 0.13.0 while this was written, which resolves to E404 on npm — so deriving + * the pin from it reproduces the exact breakage PR #1818 fixed. + */ + +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; + +const PIN_FILE = join(process.cwd(), 'workflows', 'shared', 'squad.md'); +const DRIFT_WORKFLOW = join( + process.cwd(), + '.github', + 'workflows', + 'squad-cli-pin-drift.yml', +); + +function readPinFile(): string { + return readFileSync(PIN_FILE, 'utf8'); +} + +/** The literal activation resolves: `vars.SQUAD_CLI_VERSION || ''`. */ +function effectivePin(source: string): string | undefined { + return source.match(/SQUAD_CLI_VERSION:\s*\$\{\{\s*vars\.SQUAD_CLI_VERSION\s*\|\|\s*'([^']+)'/)?.[1]; +} + +/** The version quoted in the header comment that documents the default. */ +function documentedPin(source: string): string | undefined { + return source.match(/^#\s*Default is ([0-9][^\s.]*(?:\.[^\s.]+)*)\.\s*$/m)?.[1]; +} + +describe('Squad CLI activation pin (#1825)', () => { + it('states a resolvable pin', () => { + const pin = effectivePin(readPinFile()); + + // If this stops matching, the drift workflow's extraction has almost certainly + // stopped matching too — and it would report "no drift" against a pin it never + // found. Failing here makes that visible on the PR that reshapes the line. + expect(pin, 'could not locate the SQUAD_CLI_VERSION fallback literal').toBeDefined(); + expect(pin).toMatch(/^\d+\.\d+\.\d+/); + }); + + it('documents the same version it resolves', () => { + const source = readPinFile(); + + expect(documentedPin(source), 'header comment does not document a default version').toBeDefined(); + expect(documentedPin(source)).toBe(effectivePin(source)); + }); + + it('does not carry a second, unreachable default', () => { + const source = readPinFile(); + const pin = effectivePin(source); + + // `SQUAD_CLI_VERSION` is set from `vars.X || ''`, which always yields a + // non-empty string, so a shell `${SQUAD_CLI_VERSION:-}` fallback can never + // fire. It is unreachable code whose only real effect is to be a third place the + // version can go stale — and being unreachable, it goes stale invisibly. + expect(source).not.toMatch(/\$\{SQUAD_CLI_VERSION:-/); + + // Belt and braces: no *other* copy of the version string in an npx invocation. + const npxLines = source.split(/\r?\n/).filter((l) => l.includes('squad-cli@')); + for (const line of npxLines) { + expect(line, `npx line hardcodes a version instead of using the env var: ${line.trim()}`) + .not.toContain(`squad-cli@${pin}`); + } + }); + + it('keeps the drift guard pointed at npm rather than the repo', () => { + const workflow = readFileSync(DRIFT_WORKFLOW, 'utf8'); + + // Comments are stripped first. The header comment names the trap deliberately, to + // explain why the guard avoids it — documenting a hazard should not trip the check + // that enforces it. What must not appear is the path in *executable* lines. + const executable = workflow + .split(/\r?\n/) + .filter((l) => !l.trimStart().startsWith('#')) + .join('\n'); + + // The trap this issue was filed to avoid: the CLI package manifest holds the next + // 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/); + }); + + it('passes values into shell through env, not expression interpolation', () => { + const workflow = readFileSync(DRIFT_WORKFLOW, 'utf8'); + + // Per the shell-input contract in workflows/squad.md: `${{ }}` is substituted into + // the script text *before* the shell parses it, so quoting cannot contain a hostile + // value — the value has already become code. Only `env:` mapping is safe. + // + // This guard also catches a subtler mistake made while writing this workflow: a + // literal `${{ ... }}` intended as documentation inside an issue body was silently + // evaluated by Actions instead of printed. + const lines = workflow.split(/\r?\n/); + const offenders: string[] = []; + let inRun = false; + let runIndent = 0; + + for (const line of lines) { + const indent = line.length - line.trimStart().length; + if (inRun && line.trim() !== '' && indent <= runIndent) inRun = false; + if (inRun && line.includes('${{')) offenders.push(line.trim()); + if (/^\s*run:\s*\|/.test(line)) { + inRun = true; + runIndent = indent; + } + } + + expect(offenders, `expression interpolation inside run: blocks:\n${offenders.join('\n')}`) + .toEqual([]); + }); + + it('keeps its SC2016 suppression honest', () => { + const workflow = readFileSync(DRIFT_WORKFLOW, 'utf8'); + + // The issue-body builder disables SC2016 because shellcheck reads the markdown + // BACKTICKS in its printf format strings as command substitution and, correctly, + // reports that they will not expand inside single quotes. That is the intended + // behaviour — they are literal code spans in the rendered issue. + // + // But SC2016 also catches a real defect class: writing '$VAR' in single quotes + // while expecting expansion, which silently emits the literal text "$VAR". The + // suppression would hide that too. This test restores exactly that coverage, so + // the disable removes the false positives without removing the check. + // + // Values must reach printf as ARGUMENTS ("$PINNED"), never inside the format string. + const singleQuoted = [...workflow.matchAll(/'([^'\n]*)'/g)].map(m => m[1]); + const offenders = singleQuoted.filter(s => /\$[A-Za-z_{]/.test(s)); + + expect( + offenders, + 'single-quoted literals containing a $expansion — these will emit the literal text, ' + + `not the value, and SC2016 is suppressed here so shellcheck will not say so:\n${offenders.join('\n')}`, + ).toEqual([]); + }); +}); diff --git a/workflows/shared/squad.md b/workflows/shared/squad.md index 0f7264724..465d008c6 100644 --- a/workflows/shared/squad.md +++ b/workflows/shared/squad.md @@ -87,7 +87,7 @@ jobs: echo "✓ Existing squad team detected with roster entries — skipping init." else echo "No existing squad team found — running squad init." - npx --yes "@bradygaster/squad-cli@${SQUAD_CLI_VERSION:-0.12.0}" init --preset default --state-backend local + npx --yes "@bradygaster/squad-cli@${SQUAD_CLI_VERSION}" init --preset default --state-backend local fi - name: Upload Squad state artifact