feat(cli): squad upgrade --self to update the CLI package#802
feat(cli): squad upgrade --self to update the CLI package#802tamirdresher merged 13 commits intodevfrom
Conversation
- --self installs @bradygaster/squad-cli@latest (stable) - --self --insider installs @bradygaster/squad-cli@insider (prerelease) - Auto-continues with repo upgrade after self-install - Detects package manager (npm/pnpm/yarn) - 4 new tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🛫 PR Readiness Check
|
| Status | Check | Details |
|---|---|---|
| ❌ | Single commit | 13 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 | Changeset file found |
| ✅ | Scope clean | No .squad/ or docs/proposals/ files |
| ✅ | No merge conflicts | No merge conflicts |
| ❌ | Copilot threads resolved | 1 unresolved Copilot thread(s) — fix and resolve before merging |
| ❌ | CI passing | 14 check(s) still running |
This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.
There was a problem hiding this comment.
Pull request overview
Adds a new CLI upgrade mode that can update the globally installed Squad CLI package itself (squad upgrade --self), optionally using an --insider prerelease tag, and then proceeds with the normal repo template/skills upgrade flow.
Changes:
- Added
--self/--insideroptions to theupgradecommand and wired them fromcli-entry.tsintorunUpgrade(). - Implemented a self-upgrade routine in
packages/squad-cli/src/cli/core/upgrade.tsthat runs a global install via npm/pnpm/yarn and reports version changes. - Added a changeset and a new test file covering flag wiring/help text (mostly via source assertions).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
packages/squad-cli/src/cli/core/upgrade.ts |
Implements the self-upgrade install flow and threads insider into runUpgrade() options. |
packages/squad-cli/src/cli-entry.ts |
Adds help text and parses --self / --insider flags for the upgrade subcommand. |
test/self-upgrade.test.ts |
Adds tests that assert wiring/help text primarily by reading source files. |
.changeset/self-upgrade.md |
Publishes a minor changeset describing the new CLI self-upgrade capability. |
| // After self-upgrade, the new CLI will have new templates. | ||
| // Continue with the regular upgrade to apply them to the repo. | ||
| info('Continuing with repo upgrade using the new CLI version...'); | ||
| console.log(); |
There was a problem hiding this comment.
After --self installs the new global package, this process continues running the old in-memory code (e.g., TEMPLATE_MANIFEST and upgrade logic). If the upgraded CLI version introduces new manifest entries or changes upgrade behavior, the “continue with repo upgrade” step can’t apply those changes reliably. Consider re-execing the newly installed CLI (e.g., spawn npx ${PACKAGE_NAME}@${tag} upgrade ... and exit) or printing an explicit instruction to rerun squad upgrade with the new version instead of continuing in-process.
| // After self-upgrade, the new CLI will have new templates. | |
| // Continue with the regular upgrade to apply them to the repo. | |
| info('Continuing with repo upgrade using the new CLI version...'); | |
| console.log(); | |
| // Re-run the CLI in a fresh process so the repo upgrade uses the newly | |
| // installed code instead of this process's already-loaded modules. | |
| const rerunArgs = process.argv.slice(1).filter((arg) => arg !== '--self'); | |
| info('Re-running upgrade with the newly installed CLI version...'); | |
| console.log(); | |
| execFileSync(process.execPath, rerunArgs, { | |
| stdio: 'inherit', | |
| }); | |
| process.exit(0); |
|
|
||
| try { | ||
| if (isPnpm) { | ||
| execFileSync('pnpm', ['add', '-g', `${PACKAGE_NAME}@${tag}`], { | ||
| stdio: 'inherit', | ||
| timeout: 120_000, | ||
| }); | ||
| } else if (isYarn) { | ||
| execFileSync('yarn', ['global', 'add', `${PACKAGE_NAME}@${tag}`], { | ||
| stdio: 'inherit', | ||
| timeout: 120_000, | ||
| }); | ||
| } else { | ||
| // Default: npm | ||
| execFileSync('npm', ['install', '-g', `${PACKAGE_NAME}@${tag}`], { | ||
| stdio: 'inherit', | ||
| timeout: 120_000, | ||
| }); | ||
| } | ||
| } catch (err) { | ||
| const msg = (err as Error).message; | ||
| if (msg.includes('EACCES') || msg.includes('permission')) { | ||
| fatal( | ||
| `Permission denied. Try:\n` + | ||
| ` sudo npm install -g ${PACKAGE_NAME}@${tag}\n` + | ||
| `Or use npx: npx ${PACKAGE_NAME}@${tag} upgrade` | ||
| ); | ||
| } | ||
| fatal(`Self-upgrade failed: ${msg}`); | ||
| } | ||
|
|
||
| // Read the new version after install | ||
| try { | ||
| const newVersionOutput = execFileSync('npx', ['--yes', PACKAGE_NAME, '--version'], { |
There was a problem hiding this comment.
The permission-denied handling relies on substring checks against err.message and always suggests sudo npm ... / npx ... even when the selected installer is pnpm or yarn. It’d be more reliable to branch on the error’s structured fields (code/errno/status, and captured stderr) and tailor the remediation to the actual command being run (or include pnpm/yarn equivalents).
| try { | |
| if (isPnpm) { | |
| execFileSync('pnpm', ['add', '-g', `${PACKAGE_NAME}@${tag}`], { | |
| stdio: 'inherit', | |
| timeout: 120_000, | |
| }); | |
| } else if (isYarn) { | |
| execFileSync('yarn', ['global', 'add', `${PACKAGE_NAME}@${tag}`], { | |
| stdio: 'inherit', | |
| timeout: 120_000, | |
| }); | |
| } else { | |
| // Default: npm | |
| execFileSync('npm', ['install', '-g', `${PACKAGE_NAME}@${tag}`], { | |
| stdio: 'inherit', | |
| timeout: 120_000, | |
| }); | |
| } | |
| } catch (err) { | |
| const msg = (err as Error).message; | |
| if (msg.includes('EACCES') || msg.includes('permission')) { | |
| fatal( | |
| `Permission denied. Try:\n` + | |
| ` sudo npm install -g ${PACKAGE_NAME}@${tag}\n` + | |
| `Or use npx: npx ${PACKAGE_NAME}@${tag} upgrade` | |
| ); | |
| } | |
| fatal(`Self-upgrade failed: ${msg}`); | |
| } | |
| // Read the new version after install | |
| try { | |
| const newVersionOutput = execFileSync('npx', ['--yes', PACKAGE_NAME, '--version'], { | |
| const installerCommand = isPnpm ? 'pnpm' : isYarn ? 'yarn' : 'npm'; | |
| const installerArgs = isPnpm | |
| ? ['add', '-g', `${PACKAGE_NAME}@${tag}`] | |
| : isYarn | |
| ? ['global', 'add', `${PACKAGE_NAME}@${tag}`] | |
| : ['install', '-g', `${PACKAGE_NAME}@${tag}`]; | |
| const runnerCommand = isPnpm ? 'pnpm' : isYarn ? 'yarn' : 'npx'; | |
| const runnerArgs = isPnpm | |
| ? ['dlx', PACKAGE_NAME, '--version'] | |
| : isYarn | |
| ? ['dlx', PACKAGE_NAME, '--version'] | |
| : ['--yes', PACKAGE_NAME, '--version']; | |
| try { | |
| const installOutput = execFileSync(installerCommand, installerArgs, { | |
| encoding: 'utf-8', | |
| stdio: ['pipe', 'pipe', 'pipe'], | |
| timeout: 120_000, | |
| }); | |
| if (installOutput) { | |
| process.stdout.write(installOutput); | |
| } | |
| } catch (err) { | |
| const childErr = err as Error & { | |
| code?: string | number; | |
| errno?: string | number; | |
| status?: number | null; | |
| stderr?: string | Buffer; | |
| stdout?: string | Buffer; | |
| }; | |
| const msg = childErr.message ?? 'Unknown error'; | |
| const stderr = | |
| typeof childErr.stderr === 'string' | |
| ? childErr.stderr | |
| : childErr.stderr?.toString() ?? ''; | |
| const stdout = | |
| typeof childErr.stdout === 'string' | |
| ? childErr.stdout | |
| : childErr.stdout?.toString() ?? ''; | |
| if (stdout) { | |
| process.stdout.write(stdout); | |
| } | |
| if (stderr) { | |
| process.stderr.write(stderr); | |
| } | |
| const errorText = `${msg}\n${stderr}`.toLowerCase(); | |
| const isPermissionDenied = | |
| childErr.code === 'EACCES' || | |
| childErr.errno === 'EACCES' || | |
| childErr.status === 126 || | |
| errorText.includes('eacces') || | |
| errorText.includes('permission denied') || | |
| errorText.includes('operation not permitted'); | |
| if (isPermissionDenied) { | |
| const installHelp = `${installerCommand} ${installerArgs.join(' ')}`; | |
| const elevateHelp = `sudo ${installHelp}`; | |
| const runHelp = `${runnerCommand} ${isPnpm || isYarn ? `dlx ${PACKAGE_NAME} upgrade` : `${PACKAGE_NAME}@${tag} upgrade`}`; | |
| fatal( | |
| `Permission denied while running "${installHelp}". Try:\n` + | |
| ` ${elevateHelp}\n` + | |
| `Or run without a global install:\n` + | |
| ` ${runHelp}` | |
| ); | |
| } | |
| fatal(`Self-upgrade failed: ${stderr.trim() || msg}`); | |
| } | |
| // Read the new version after install | |
| try { | |
| const newVersionOutput = execFileSync(runnerCommand, runnerArgs, { |
| const migrateDir = args.includes('--migrate-directory'); | ||
| const selfUpgrade = args.includes('--self'); | ||
| const forceUpgrade = args.includes('--force'); | ||
| const insiderUpgrade = args.includes('--insider'); | ||
| const dest = hasGlobal ? (await lazySquadSdk()).resolveGlobalSquadPath() : process.cwd(); | ||
|
|
||
| // Handle --migrate-directory flag | ||
| if (migrateDir) { | ||
| await migrateDirectory(dest); | ||
| // Continue with regular upgrade after migration | ||
| } | ||
|
|
||
| // Run upgrade | ||
| await runUpgrade(dest, { | ||
| migrateDirectory: migrateDir, | ||
| self: selfUpgrade, | ||
| force: forceUpgrade | ||
| force: forceUpgrade, | ||
| insider: insiderUpgrade, | ||
| }); |
There was a problem hiding this comment.
--insider is parsed and passed into runUpgrade even when --self is not provided. In that case the flag is silently ignored (since insider is only used for self-upgrade), which can confuse users. Consider validating that --insider requires --self (fatal/warn), or define/implement a non-self meaning for insider upgrades.
| import { describe, it, expect, vi } from 'vitest'; | ||
|
|
||
| // We test the UpgradeOptions interface and the self-upgrade code path structure. | ||
| // Actual npm install is not tested (would modify the system), but we verify: | ||
| // - The option is wired correctly | ||
| // - The function signature accepts insider flag | ||
| // - The package name constant is correct | ||
|
|
||
| describe('squad upgrade --self', () => { | ||
| it('UpgradeOptions includes self and insider flags', async () => { | ||
| const { runUpgrade } = await import('../packages/squad-cli/src/cli/core/upgrade.js'); | ||
| // Verify the function accepts the options without type error | ||
| expect(typeof runUpgrade).toBe('function'); | ||
| }); | ||
|
|
||
| it('cli-entry parses --self and --insider flags', async () => { | ||
| // Read the cli-entry source and verify flag parsing | ||
| const fs = await import('node:fs'); | ||
| const path = await import('node:path'); | ||
| const entrySource = fs.readFileSync( | ||
| path.join(process.cwd(), 'packages', 'squad-cli', 'src', 'cli-entry.ts'), | ||
| 'utf-8', | ||
| ); | ||
| expect(entrySource).toContain("args.includes('--self')"); | ||
| expect(entrySource).toContain("args.includes('--insider')"); | ||
| expect(entrySource).toContain('insider: insiderUpgrade'); | ||
| }); | ||
|
|
||
| it('help text documents --self and --insider', async () => { | ||
| const fs = await import('node:fs'); | ||
| const path = await import('node:path'); | ||
| const entrySource = fs.readFileSync( | ||
| path.join(process.cwd(), 'packages', 'squad-cli', 'src', 'cli-entry.ts'), | ||
| 'utf-8', | ||
| ); | ||
| expect(entrySource).toContain('--self (upgrade the CLI package itself)'); | ||
| expect(entrySource).toContain('--self --insider'); | ||
| }); | ||
|
|
||
| it('upgrade module references correct npm package name', async () => { | ||
| const fs = await import('node:fs'); | ||
| const path = await import('node:path'); | ||
| const upgradeSource = fs.readFileSync( | ||
| path.join(process.cwd(), 'packages', 'squad-cli', 'src', 'cli', 'core', 'upgrade.ts'), | ||
| 'utf-8', | ||
| ); | ||
| expect(upgradeSource).toContain("@bradygaster/squad-cli"); | ||
| expect(upgradeSource).toContain("'insider'"); | ||
| expect(upgradeSource).toContain("'latest'"); |
There was a problem hiding this comment.
These tests mostly assert on source-text substrings and don’t exercise the new --self behavior (e.g., that runUpgrade(..., { self: true, insider: true }) invokes the correct execFileSync command/tag). This makes the tests brittle to refactors and leaves the new code path effectively untested; consider mocking node:child_process and asserting the self-upgrade flow/arguments instead (and use expectTypeOf if you want to validate UpgradeOptions shape).
| import { describe, it, expect, vi } from 'vitest'; | |
| // We test the UpgradeOptions interface and the self-upgrade code path structure. | |
| // Actual npm install is not tested (would modify the system), but we verify: | |
| // - The option is wired correctly | |
| // - The function signature accepts insider flag | |
| // - The package name constant is correct | |
| describe('squad upgrade --self', () => { | |
| it('UpgradeOptions includes self and insider flags', async () => { | |
| const { runUpgrade } = await import('../packages/squad-cli/src/cli/core/upgrade.js'); | |
| // Verify the function accepts the options without type error | |
| expect(typeof runUpgrade).toBe('function'); | |
| }); | |
| it('cli-entry parses --self and --insider flags', async () => { | |
| // Read the cli-entry source and verify flag parsing | |
| const fs = await import('node:fs'); | |
| const path = await import('node:path'); | |
| const entrySource = fs.readFileSync( | |
| path.join(process.cwd(), 'packages', 'squad-cli', 'src', 'cli-entry.ts'), | |
| 'utf-8', | |
| ); | |
| expect(entrySource).toContain("args.includes('--self')"); | |
| expect(entrySource).toContain("args.includes('--insider')"); | |
| expect(entrySource).toContain('insider: insiderUpgrade'); | |
| }); | |
| it('help text documents --self and --insider', async () => { | |
| const fs = await import('node:fs'); | |
| const path = await import('node:path'); | |
| const entrySource = fs.readFileSync( | |
| path.join(process.cwd(), 'packages', 'squad-cli', 'src', 'cli-entry.ts'), | |
| 'utf-8', | |
| ); | |
| expect(entrySource).toContain('--self (upgrade the CLI package itself)'); | |
| expect(entrySource).toContain('--self --insider'); | |
| }); | |
| it('upgrade module references correct npm package name', async () => { | |
| const fs = await import('node:fs'); | |
| const path = await import('node:path'); | |
| const upgradeSource = fs.readFileSync( | |
| path.join(process.cwd(), 'packages', 'squad-cli', 'src', 'cli', 'core', 'upgrade.ts'), | |
| 'utf-8', | |
| ); | |
| expect(upgradeSource).toContain("@bradygaster/squad-cli"); | |
| expect(upgradeSource).toContain("'insider'"); | |
| expect(upgradeSource).toContain("'latest'"); | |
| import { beforeEach, describe, expect, expectTypeOf, it, vi } from 'vitest'; | |
| const childProcessMocks = vi.hoisted(() => ({ | |
| execFileSync: vi.fn(), | |
| })); | |
| vi.mock('node:child_process', () => ({ | |
| execFileSync: childProcessMocks.execFileSync, | |
| })); | |
| describe('squad upgrade --self', () => { | |
| beforeEach(() => { | |
| vi.resetModules(); | |
| childProcessMocks.execFileSync.mockReset(); | |
| }); | |
| it('UpgradeOptions includes self and insider flags', async () => { | |
| const { runUpgrade } = await import('../packages/squad-cli/src/cli/core/upgrade.js'); | |
| expectTypeOf(runUpgrade).toBeFunction(); | |
| expectTypeOf<Parameters<typeof runUpgrade>[1]>().toMatchTypeOf<{ | |
| self?: boolean; | |
| insider?: boolean; | |
| }>(); | |
| }); | |
| it('runs self-upgrade with the insider tag', async () => { | |
| const { runUpgrade } = await import('../packages/squad-cli/src/cli/core/upgrade.js'); | |
| await runUpgrade(process.cwd(), { self: true, insider: true }); | |
| expect(childProcessMocks.execFileSync).toHaveBeenCalledTimes(1); | |
| const [command, args] = childProcessMocks.execFileSync.mock.calls[0] ?? []; | |
| expect(typeof command).toBe('string'); | |
| expect(Array.isArray(args)).toBe(true); | |
| expect(args).toEqual( | |
| expect.arrayContaining(['install', '-g', '@bradygaster/squad-cli@insider']), | |
| ); | |
| }); | |
| it('runs self-upgrade with the latest tag by default', async () => { | |
| const { runUpgrade } = await import('../packages/squad-cli/src/cli/core/upgrade.js'); | |
| await runUpgrade(process.cwd(), { self: true }); | |
| expect(childProcessMocks.execFileSync).toHaveBeenCalledTimes(1); | |
| const [command, args] = childProcessMocks.execFileSync.mock.calls[0] ?? []; | |
| expect(typeof command).toBe('string'); | |
| expect(Array.isArray(args)).toBe(true); | |
| expect(args).toEqual( | |
| expect.arrayContaining(['install', '-g', '@bradygaster/squad-cli@latest']), | |
| ); |
- cleanup.md: automated housekeeping watch capability - scratch-dir.md: .squad/.scratch/ temp file management - external-state.md: squad externalize/internalize commands - self-upgrade.md: squad upgrade --self/--insider - built-in-roles.md: add fact-checker role (13th engineering role) - skills.md: document 8 built-in skills shipped with init/upgrade - README.md: add externalize/internalize + upgrade --self to command table (15→17) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🏗️ Architectural Review
Automated architectural review — informational only. |
- Add restart message after --self upgrade (stale code warning) - Fix permission handling: only suggest sudo for npm, not pnpm/yarn - Warn when --insider used without --self - Add test verifying selfUpgradeCli is called with correct args - Export selfUpgradeCli, call it from cli-entry before runUpgrade (exit early) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
# Conflicts: # packages/squad-cli/src/cli-entry.ts # test/cli/upgrade.test.ts
🟡 Impact Analysis — PR #802Risk tier: 🟡 MEDIUM 📊 Summary
🎯 Risk Factors
📦 Modules Affecteddocs (6 files)
root (2 files)
squad-cli (2 files)
tests (1 file)
This report is generated automatically for every PR. See #733 for details. |
🏗️ Dina Review: APPROVE WITH FOLLOW-UPsquad upgrade --self — CLI self-update Core functionality is safe and working. Self-upgrade exits immediately (avoids stale code issues). Permission handling is functional. Comment #3 (--insider\ without --self) is already addressed in code. 120s timeout protection is good. Follow-up items (non-blocking, create issues):
✅ Ready to merge. Follow-up issues recommended for rollback and integrity. |
… validation - Check err.code === 'EACCES' instead of substring matching on err.message - Use detected installer name (npm/pnpm/yarn) in sudo suggestion - Improve --insider without --self warning with actionable message Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…er/squad into squad/798-self-upgrade
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…li-entry Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
What
Adds
squad upgrade --selfto update the Squad CLI package itself, with optional--insiderflag for prerelease builds.Why
Users must manually run
npm install -g @bradygaster/squad-cli@latestto update. This should be a single command. Closes #801How
selfUpgradeCli()function inupgrade.tsruns npm/pnpm/yarn global install--selfinstalls@bradygaster/squad-cli@latest(stable)--self --insiderinstalls@bradygaster/squad-cli@insider(prerelease)npm_config_user_agentTesting
npm run buildpassesnpm testpasses (4 new tests — type validation, flag parsing, help text, package name)Docs
Exports
N/A
Breaking Changes
None — new flag on existing command.
Waivers
None