From b6f763daf8e757343ab572fabaa7898c495d19ac Mon Sep 17 00:00:00 2001 From: Dina Berry Date: Wed, 8 Apr 2026 08:54:54 -0700 Subject: [PATCH] feat(cli): init/upgrade parity tests, CI gate, doctor global install UX --- .changeset/init-upgrade-parity.md | 5 + .github/workflows/squad-ci.yml | 90 ++++++++++ packages/squad-cli/src/cli/commands/doctor.ts | 27 +-- packages/squad-cli/src/cli/core/upgrade.ts | 2 +- test/cli/doctor.test.ts | 39 ++++- test/cli/init-upgrade-parity.test.ts | 159 +++++++++++++++++- 6 files changed, 305 insertions(+), 17 deletions(-) create mode 100644 .changeset/init-upgrade-parity.md diff --git a/.changeset/init-upgrade-parity.md b/.changeset/init-upgrade-parity.md new file mode 100644 index 000000000..58ad25cfe --- /dev/null +++ b/.changeset/init-upgrade-parity.md @@ -0,0 +1,5 @@ +--- +'@bradygaster/squad-cli': patch +--- + +Improve doctor UX for global installs — ESM checks show ℹ️ info instead of ⚠️ warn with clearer messaging. Export ENSURE_DIRECTORIES for parity testing. diff --git a/.github/workflows/squad-ci.yml b/.github/workflows/squad-ci.yml index b2720ef74..aef3a302c 100644 --- a/.github/workflows/squad-ci.yml +++ b/.github/workflows/squad-ci.yml @@ -820,3 +820,93 @@ jobs: console.log('✅ All ' + passed + ' subpath exports resolve and import successfully'); " + + # ═══════════════════════════════════════════════════════════════════════ + # Upgrade → Doctor validation (FR-3, Issue #817) + # Verifies that init → upgrade → doctor produces no failures. + # Only runs when init/upgrade/doctor/templates code changes. + # ═══════════════════════════════════════════════════════════════════════ + upgrade-doctor-check: + needs: changes + if: >- + github.event_name == 'pull_request' + && !cancelled() + && (needs.changes.outputs.code == 'true' + || needs.changes.result == 'failure') + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - uses: actions/setup-node@v4 + with: + node-version: 22 + cache: 'npm' + + - name: Check for relevant changes + id: relevant + run: | + BASE="${{ github.event.pull_request.base.sha }}" + HEAD="${{ github.event.pull_request.head.sha }}" + CHANGED=$(git diff --name-only "$BASE"..."$HEAD") + RELEVANT=$(echo "$CHANGED" | grep -E '^packages/squad-cli/src/cli/(commands/doctor|core/init|core/upgrade|core/templates)\.ts$' || true) + if [ -z "$RELEVANT" ]; then + echo "skip=true" >> "$GITHUB_OUTPUT" + echo "No init/upgrade/doctor/templates changes — skipping" + else + echo "skip=false" >> "$GITHUB_OUTPUT" + echo "Relevant files changed:" + echo "$RELEVANT" + fi + + - name: Install dependencies + if: steps.relevant.outputs.skip != 'true' + run: npm install + + - name: Build + if: steps.relevant.outputs.skip != 'true' + run: npm run build + + - name: "🩺 Init → Upgrade → Doctor validation" + if: steps.relevant.outputs.skip != 'true' + run: | + node --input-type=module -e " + import { join } from 'path'; + import { mkdirSync, rmSync, existsSync } from 'fs'; + import { randomBytes } from 'crypto'; + + // Dynamic imports for built CLI modules + const { runInit } = await import('./packages/squad-cli/dist/cli/core/init.js'); + const { runUpgrade } = await import('./packages/squad-cli/dist/cli/core/upgrade.js'); + const { runDoctor } = await import('./packages/squad-cli/dist/cli/commands/doctor.js'); + + const testDir = join(process.cwd(), '.ci-upgrade-doctor-' + randomBytes(4).toString('hex')); + try { + mkdirSync(testDir, { recursive: true }); + + console.log('Step 1: squad init'); + await runInit(testDir); + + console.log('Step 2: squad upgrade'); + await runUpgrade(testDir, { force: true }); + + console.log('Step 3: squad doctor'); + const checks = await runDoctor(testDir); + const failures = checks.filter(c => c.status === 'fail'); + + if (failures.length > 0) { + console.error(''); + console.error('::error::Doctor reported ' + failures.length + ' failure(s) after init + upgrade:'); + failures.forEach(f => console.error(' ❌ ' + f.name + ' — ' + f.message)); + process.exit(1); + } + + console.log('✅ Doctor passed — no failures after init + upgrade (' + checks.length + ' checks)'); + } finally { + if (existsSync(testDir)) { + rmSync(testDir, { recursive: true, force: true }); + } + } + " diff --git a/packages/squad-cli/src/cli/commands/doctor.ts b/packages/squad-cli/src/cli/commands/doctor.ts index 4e1659fd6..031e2239d 100644 --- a/packages/squad-cli/src/cli/commands/doctor.ts +++ b/packages/squad-cli/src/cli/commands/doctor.ts @@ -55,6 +55,15 @@ function tryReadJson(p: string): unknown | undefined { } } +/** + * Detect whether we appear to be running from a global npm install. + * Heuristic: no node_modules directory in the cwd means the CLI was + * likely installed globally (npm i -g) rather than as a project dependency. + */ +export function looksLikeGlobalInstall(cwd: string): boolean { + return !isDirectory(path.join(cwd, 'node_modules')); +} + // ── mode detection ────────────────────────────────────────────────── function detectMode(cwd: string): ModeInfo { @@ -330,20 +339,19 @@ function checkVscodeJsonrpcExports(cwd: string): DoctorCheck { } // Detect whether we're in a local dev context (node_modules exists) or global install - const hasNodeModules = isDirectory(path.join(cwd, 'node_modules')); - if (hasNodeModules) { + if (looksLikeGlobalInstall(cwd)) { return { name: 'vscode-jsonrpc exports field', status: 'warn', - message: 'not found in node_modules — run npm install or check dependencies', + severity: 'info', + message: 'Expected for global CLI installs — ESM patches applied at package install time', }; } return { name: 'vscode-jsonrpc exports field', status: 'warn', - severity: 'info', - message: 'not found in node_modules (expected for global installs)', + message: 'not found in node_modules — run npm install or check dependencies', }; } @@ -386,20 +394,19 @@ function checkCopilotSdkSessionPatch(cwd: string): DoctorCheck { } // Detect whether we're in a local dev context (node_modules exists) or global install - const hasNodeModules = isDirectory(path.join(cwd, 'node_modules')); - if (hasNodeModules) { + if (looksLikeGlobalInstall(cwd)) { return { name: 'copilot-sdk session.js ESM patch', status: 'warn', - message: 'not found in node_modules — run npm install or check dependencies', + severity: 'info', + message: 'Expected for global CLI installs — ESM patches applied at package install time', }; } return { name: 'copilot-sdk session.js ESM patch', status: 'warn', - severity: 'info', - message: 'not found in node_modules (expected for global installs)', + message: 'not found in node_modules — run npm install or check dependencies', }; } diff --git a/packages/squad-cli/src/cli/core/upgrade.ts b/packages/squad-cli/src/cli/core/upgrade.ts index 975fb8f98..fbd176fbd 100644 --- a/packages/squad-cli/src/cli/core/upgrade.ts +++ b/packages/squad-cli/src/cli/core/upgrade.ts @@ -281,7 +281,7 @@ const GITIGNORE_ENTRIES = [ '.squad-workstream', ]; -const ENSURE_DIRECTORIES = [ +export const ENSURE_DIRECTORIES = [ '.squad/identity', '.squad/orchestration-log', '.squad/log', diff --git a/test/cli/doctor.test.ts b/test/cli/doctor.test.ts index 85e56f354..578511eb7 100644 --- a/test/cli/doctor.test.ts +++ b/test/cli/doctor.test.ts @@ -11,7 +11,7 @@ import { mkdir, rm, writeFile } from 'fs/promises'; import { join } from 'path'; import { existsSync } from 'fs'; import { randomBytes } from 'crypto'; -import { runDoctor, getDoctorMode, checkNodeVersion } from '@bradygaster/squad-cli/commands/doctor'; +import { runDoctor, getDoctorMode, checkNodeVersion, looksLikeGlobalInstall } from '@bradygaster/squad-cli/commands/doctor'; import type { DoctorCheck } from '@bradygaster/squad-cli/commands/doctor'; const TEST_ROOT = join(process.cwd(), `.test-doctor-${randomBytes(4).toString('hex')}`); @@ -210,7 +210,7 @@ describe('squad doctor', () => { // ── #565 — Actionable resolution hints in warnings ──────────────── - it('vscode-jsonrpc info says "expected for global installs" when not in node_modules', async () => { + it('vscode-jsonrpc info says "Expected for global CLI installs" when not in node_modules', async () => { await scaffold(TEST_ROOT); const checks = await runDoctor(TEST_ROOT); @@ -218,10 +218,11 @@ describe('squad doctor', () => { expect(jsonrpcCheck).toBeDefined(); expect(jsonrpcCheck?.status).toBe('warn'); expect(jsonrpcCheck?.severity).toBe('info'); - expect(jsonrpcCheck?.message).toContain('expected for global installs'); + expect(jsonrpcCheck?.message).toContain('Expected for global CLI installs'); + expect(jsonrpcCheck?.message).toContain('ESM patches applied at package install time'); }); - it('copilot-sdk info says "expected for global installs" when not in node_modules', async () => { + it('copilot-sdk info says "Expected for global CLI installs" when not in node_modules', async () => { await scaffold(TEST_ROOT); const checks = await runDoctor(TEST_ROOT); @@ -229,7 +230,8 @@ describe('squad doctor', () => { expect(sdkCheck).toBeDefined(); expect(sdkCheck?.status).toBe('warn'); expect(sdkCheck?.severity).toBe('info'); - expect(sdkCheck?.message).toContain('expected for global installs'); + expect(sdkCheck?.message).toContain('Expected for global CLI installs'); + expect(sdkCheck?.message).toContain('ESM patches applied at package install time'); }); it('absolute teamRoot warning includes "Edit .squad/config.json"', async () => { @@ -291,4 +293,31 @@ describe('squad doctor', () => { expect(agentMdCheck?.status).toBe('fail'); expect(agentMdCheck?.message).toContain('squad upgrade'); }); + + // ── #817 — Global install detection ────────────────────────────── + + it('looksLikeGlobalInstall returns true when no node_modules exists', async () => { + await scaffold(TEST_ROOT); + // TEST_ROOT has no node_modules — should detect as global + expect(looksLikeGlobalInstall(TEST_ROOT)).toBe(true); + }); + + it('looksLikeGlobalInstall returns false when node_modules exists', async () => { + await scaffold(TEST_ROOT); + await mkdir(join(TEST_ROOT, 'node_modules'), { recursive: true }); + expect(looksLikeGlobalInstall(TEST_ROOT)).toBe(false); + }); + + it('ESM checks show warn (not info) when node_modules exists but packages missing', async () => { + await scaffold(TEST_ROOT); + // Create node_modules to simulate local install where packages are missing + await mkdir(join(TEST_ROOT, 'node_modules'), { recursive: true }); + + const checks = await runDoctor(TEST_ROOT); + const jsonrpcCheck = checks.find((c: DoctorCheck) => c.name === 'vscode-jsonrpc exports field'); + expect(jsonrpcCheck).toBeDefined(); + expect(jsonrpcCheck?.status).toBe('warn'); + expect(jsonrpcCheck?.severity).toBeUndefined(); + expect(jsonrpcCheck?.message).toContain('run npm install'); + }); }); diff --git a/test/cli/init-upgrade-parity.test.ts b/test/cli/init-upgrade-parity.test.ts index 2a6279518..af367f2c0 100644 --- a/test/cli/init-upgrade-parity.test.ts +++ b/test/cli/init-upgrade-parity.test.ts @@ -6,19 +6,24 @@ * guard against that class of drift. * * @see https://github.com/bradygaster/squad/issues/822 + * @see https://github.com/bradygaster/squad/issues/817 */ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { mkdir, rm, readFile, writeFile } from 'fs/promises'; import { join } from 'path'; -import { existsSync } from 'fs'; +import { existsSync, rmSync } from 'fs'; import { randomBytes } from 'crypto'; import { runInit } from '@bradygaster/squad-cli/core/init'; import { runUpgrade, ensureGitattributes, ensureDirectories, + ensureCastingDefaults, + ENSURE_DIRECTORIES, } from '@bradygaster/squad-cli/core/upgrade'; +import { runDoctor } from '@bradygaster/squad-cli/commands/doctor'; +import type { DoctorCheck } from '@bradygaster/squad-cli/commands/doctor'; const TEST_ROOT = join( process.cwd(), @@ -237,4 +242,156 @@ describe('Init / Upgrade parity', () => { expect(count).toBe(1); } }); + + // ========================================================================= + // FR-2 (#817): Extended parity tests + // ========================================================================= + + // ------------------------------------------------------------------------- + // 8. agents/ is user data, NOT scaffolding — intentionally excluded + // from ENSURE_DIRECTORIES is acceptable (NFR-1) + // ------------------------------------------------------------------------- + it('agents/ IS in ENSURE_DIRECTORIES (infrastructure, not user content)', () => { + // agents/ directory is a structural requirement — doctor checks for it. + // But the *contents* (charter.md, history.md) are user data. + // ENSURE_DIRECTORIES covers the parent directory, which is correct. + expect(ENSURE_DIRECTORIES).toContain('.squad/agents'); + }); + + // ------------------------------------------------------------------------- + // 9. Parametric: each ENSURE_DIRECTORIES entry is recreated after deletion + // ------------------------------------------------------------------------- + it.each(ENSURE_DIRECTORIES)( + 'ensureDirectories recreates %s after deletion', + async (dir) => { + await runInit(TEST_ROOT); + + // Make sure the dir exists first (init or ensureDirectories creates it) + const fullPath = join(TEST_ROOT, dir); + if (!existsSync(fullPath)) { + await mkdir(fullPath, { recursive: true }); + } + expect(existsSync(fullPath)).toBe(true); + + // Delete it + rmSync(fullPath, { recursive: true, force: true }); + expect(existsSync(fullPath)).toBe(false); + + // ensureDirectories should recreate it + const created = ensureDirectories(TEST_ROOT); + expect(existsSync(fullPath)).toBe(true); + expect(created).toContain(dir); + }, + ); + + // ------------------------------------------------------------------------- + // 10. init → delete casting/ → upgrade → verify casting files recreated + // ------------------------------------------------------------------------- + it('upgrade recreates casting/ with default files after deletion', async () => { + await runInit(TEST_ROOT); + + const castingDir = join(TEST_ROOT, '.squad', 'casting'); + expect(existsSync(castingDir)).toBe(true); + + // Delete casting dir entirely + rmSync(castingDir, { recursive: true, force: true }); + expect(existsSync(castingDir)).toBe(false); + + // Upgrade should recreate it with defaults + await runUpgrade(TEST_ROOT, { force: true }); + + expect(existsSync(castingDir)).toBe(true); + for (const file of CASTING_FILES) { + const filePath = join(castingDir, file); + expect(existsSync(filePath)).toBe(true); + + const content = await readFile(filePath, 'utf-8'); + expect(() => JSON.parse(content)).not.toThrow(); + } + }); + + // ------------------------------------------------------------------------- + // 11. upgrade creates sessions/ on a fresh init (init doesn't create it) + // ------------------------------------------------------------------------- + it('upgrade creates .squad/sessions/ even though init does not', async () => { + await runInit(TEST_ROOT); + + const sessionsDir = join(TEST_ROOT, '.squad', 'sessions'); + // init may or may not create sessions/ — the key assertion is + // that after upgrade, it MUST exist + if (existsSync(sessionsDir)) { + rmSync(sessionsDir, { recursive: true, force: true }); + } + expect(existsSync(sessionsDir)).toBe(false); + + await runUpgrade(TEST_ROOT, { force: true }); + + expect(existsSync(sessionsDir)).toBe(true); + }); + + // ------------------------------------------------------------------------- + // 12. Pre-casting repo: upgrade adds missing casting dir + files + // ------------------------------------------------------------------------- + it('upgrade on a pre-casting repo structure adds casting with defaults', async () => { + await runInit(TEST_ROOT); + + // Simulate a pre-casting repo: remove casting entirely + const castingDir = join(TEST_ROOT, '.squad', 'casting'); + if (existsSync(castingDir)) { + rmSync(castingDir, { recursive: true, force: true }); + } + + // Also remove sessions and orchestration-log to simulate older layout + const sessionsDir = join(TEST_ROOT, '.squad', 'sessions'); + if (existsSync(sessionsDir)) { + rmSync(sessionsDir, { recursive: true, force: true }); + } + + await runUpgrade(TEST_ROOT, { force: true }); + + // casting should now exist with all defaults + expect(existsSync(castingDir)).toBe(true); + for (const file of CASTING_FILES) { + expect(existsSync(join(castingDir, file))).toBe(true); + } + + // sessions should also be created + expect(existsSync(sessionsDir)).toBe(true); + }); + + // ------------------------------------------------------------------------- + // 13. After init → upgrade, doctor reports no failures + // ------------------------------------------------------------------------- + it('doctor reports no failures after init + upgrade', async () => { + await runInit(TEST_ROOT); + await runUpgrade(TEST_ROOT, { force: true }); + + const checks = await runDoctor(TEST_ROOT); + const failures = checks.filter((c: DoctorCheck) => c.status === 'fail'); + + // Any failures indicate a parity gap + expect(failures).toEqual([]); + }); + + // ------------------------------------------------------------------------- + // 14. ENSURE_DIRECTORIES covers all critical infrastructure dirs + // Excludes: .squad/plugins, .squad/.scratch (non-critical), + // .squad/decisions (parent of decisions/inbox, created implicitly) + // ------------------------------------------------------------------------- + it('ENSURE_DIRECTORIES covers critical infrastructure', () => { + const expected = [ + '.squad/identity', + '.squad/orchestration-log', + '.squad/log', + '.squad/sessions', + '.squad/decisions/inbox', + '.squad/casting', + '.squad/agents', + '.copilot/skills', + ]; + + for (const dir of expected) { + expect(ENSURE_DIRECTORIES).toContain(dir); + } + }); }); \ No newline at end of file