diff --git a/.changeset/fix-detect-squad-dir-zero-deps.md b/.changeset/fix-detect-squad-dir-zero-deps.md new file mode 100644 index 000000000..ca9fb1396 --- /dev/null +++ b/.changeset/fix-detect-squad-dir-zero-deps.md @@ -0,0 +1,7 @@ +--- +"@bradygaster/squad-cli": patch +--- + +fix(cli): revert detect-squad-dir to zero-dependency bootstrap + +The StorageProvider refactor (26047dc5) accidentally converted this bootstrap utility from raw node:fs to FSStorageProvider. This file runs before the SDK is loaded and must not depend on @bradygaster/squad-sdk. Adds regression guard test. diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index f1528d90d..8a843118f 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -31,6 +31,46 @@ If you see any of these, STOP immediately and comment on the issue asking for gu - ANY file deletions you didn't explicitly intend - Changes outside the scope of your assigned issue +## Protected Files — Zero External Dependencies + +**Some files MUST only use Node.js built-in modules. Do NOT add npm packages, SDK imports, or any non-`node:*` dependencies to them.** + +These are bootstrap utilities that run **before** the Squad SDK is loaded. If they import SDK code (e.g., `FSStorageProvider`, anything from `squad-sdk`), the CLI breaks at startup. + +### Protected file list +| File | Purpose | +|------|---------| +| `packages/squad-cli/src/cli/core/detect-squad-dir.ts` | Finds `.squad/` directory at startup — runs before SDK init | +| `packages/squad-cli/src/cli/core/errors.ts` | Error classes (`SquadError`, `fatal()`) — used by all CLI entry points | +| `packages/squad-cli/src/cli/core/gh-cli.ts` | GitHub CLI wrapper — uses only `node:child_process` and `node:util` | +| `packages/squad-cli/src/cli/core/output.ts` | Color/emoji console output — pure ANSI codes, zero imports | +| `packages/squad-cli/src/cli/core/history-split.ts` | Separates portable knowledge from project data — pure string logic | + +### Rules +- ❌ **NEVER** convert these files to use `FSStorageProvider`, `StorageProvider`, or any SDK abstraction +- ❌ **NEVER** add `import` or `require` statements referencing packages outside `node:*` built-ins +- ✅ **ONLY** use `node:fs`, `node:path`, `node:child_process`, `node:util`, and other Node.js built-in modules +- ✅ **DO** check this list before sweeping refactors (e.g., "convert all fs calls to StorageProvider") +- ✅ **LOOK** for `— zero dependencies` markers in file headers as a signal + +### Why this matters +Regression tests guard these files (`detect-squad-dir-zero-deps.test.ts` verifies zero external dependencies), but **prevention is better than detection**. A broken bootstrap means the entire CLI fails to start — no helpful error, just a crash. + +> **When adding new bootstrap utilities**, add them to this table and write a matching zero-dependency regression test. + +### SDK/CLI package boundary +The CLI (`squad-cli`) depends on the SDK (`squad-sdk`). Some CLI files run **before** the SDK is fully loaded. The `packages/squad-cli/src/cli/core/` directory contains a mix of early-startup/bootstrap utilities and later SDK-dependent modules, so treat every file in it with extra caution. The protected list above is the authoritative set of zero-dependency bootstrap files. If you need to add SDK imports to another `core/` file, verify it is not in the protected list above and confirm the SDK is loaded at that point in the startup sequence. + +## Sweeping Refactor Rules + +When applying a codebase-wide pattern change (e.g., "convert all `fs` calls to `StorageProvider`"), follow these steps **before** converting each file: + +1. **Check the Protected Files list above.** If the file is listed, do NOT convert it. +2. **Scan for zero-dependency markers.** Look for `— zero dependencies` in the file's header comment. If present, do NOT convert it. +3. **Verify imports resolve.** When adding `import { X } from '@bradygaster/squad-sdk'`, confirm `X` is actually exported from the SDK's barrel file (`packages/squad-sdk/src/index.ts`). Unresolved imports cause build failures. +4. **Never convert ALL files blindly.** Some files have specific constraints documented in their headers or in this instructions file. Read before you refactor. +5. **Test after each logical group.** Don't convert 30 files and then run the build — convert in small batches and verify each one compiles. + ## Team Context Before starting work on any issue: diff --git a/packages/squad-cli/src/cli/core/detect-squad-dir.ts b/packages/squad-cli/src/cli/core/detect-squad-dir.ts index b623d9a81..63692b25f 100644 --- a/packages/squad-cli/src/cli/core/detect-squad-dir.ts +++ b/packages/squad-cli/src/cli/core/detect-squad-dir.ts @@ -2,10 +2,8 @@ * Squad directory detection — zero dependencies */ +import fs from 'node:fs'; import path from 'node:path'; -import { FSStorageProvider } from '@bradygaster/squad-sdk'; - -const storage = new FSStorageProvider(); export interface SquadDirInfo { path: string; @@ -22,10 +20,9 @@ export interface SquadDirInfo { export function resolveWorktreeMainCheckout(dir: string): string | null { const gitPath = path.join(dir, '.git'); try { - if (storage.isDirectorySync(gitPath)) return null; - const content = storage.readSync(gitPath); - if (content === undefined) return null; - const match = content.trim().match(/^gitdir:\s*(.+)$/m); + if (fs.statSync(gitPath).isDirectory()) return null; + const content = fs.readFileSync(gitPath, 'utf-8').trim(); + const match = content.match(/^gitdir:\s*(.+)$/m); if (!match || !match[1]) return null; const worktreeGitDir = path.resolve(dir, match[1].trim()); // worktreeGitDir = /main/.git/worktrees/name @@ -34,7 +31,7 @@ export function resolveWorktreeMainCheckout(dir: string): string | null { const mainGitDir = path.resolve(worktreeGitDir, '..', '..'); const mainCheckout = path.dirname(mainGitDir); // Verify the derived main checkout is a real git repo - if (!storage.existsSync(mainGitDir) || !storage.isDirectorySync(mainGitDir)) { + if (!fs.existsSync(mainGitDir) || !fs.statSync(mainGitDir).isDirectory()) { return null; } return mainCheckout; @@ -53,10 +50,10 @@ export function detectSquadDir(dest: string): SquadDirInfo { const squadDir = path.join(dest, '.squad'); const aiTeamDir = path.join(dest, '.ai-team'); - if (storage.existsSync(squadDir)) { + if (fs.existsSync(squadDir)) { return { path: squadDir, name: '.squad', isLegacy: false }; } - if (storage.existsSync(aiTeamDir)) { + if (fs.existsSync(aiTeamDir)) { return { path: aiTeamDir, name: '.ai-team', isLegacy: true }; } @@ -65,10 +62,10 @@ export function detectSquadDir(dest: string): SquadDirInfo { if (mainCheckout) { const mainSquadDir = path.join(mainCheckout, '.squad'); const mainAiTeamDir = path.join(mainCheckout, '.ai-team'); - if (storage.existsSync(mainSquadDir)) { + if (fs.existsSync(mainSquadDir)) { return { path: mainSquadDir, name: '.squad', isLegacy: false }; } - if (storage.existsSync(mainAiTeamDir)) { + if (fs.existsSync(mainAiTeamDir)) { return { path: mainAiTeamDir, name: '.ai-team', isLegacy: true }; } } diff --git a/test/detect-squad-dir-zero-deps.test.ts b/test/detect-squad-dir-zero-deps.test.ts new file mode 100644 index 000000000..12e8426e2 --- /dev/null +++ b/test/detect-squad-dir-zero-deps.test.ts @@ -0,0 +1,38 @@ +/** + * Regression guard — detect-squad-dir.ts must stay zero-dependency. + * + * The StorageProvider refactor (26047dc5) accidentally converted this + * bootstrap utility to depend on @bradygaster/squad-sdk. This file runs + * before the SDK is loaded, so it must only use node built-ins. + */ + +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; + +const SOURCE_PATH = resolve( + __dirname, + '..', + 'packages', + 'squad-cli', + 'src', + 'cli', + 'core', + 'detect-squad-dir.ts', +); + +describe('detect-squad-dir zero-dependency guard', () => { + const source = readFileSync(SOURCE_PATH, 'utf-8'); + + it('must not import from @bradygaster/squad-sdk', () => { + expect(source).not.toMatch(/from\s+['"]@bradygaster\/squad-sdk(?:\/[^'"]+)?['"]/); + }); + + it('must not require @bradygaster/squad-sdk', () => { + expect(source).not.toMatch(/require\s*\(\s*['"]@bradygaster\/squad-sdk(?:\/[^'"]+)?['"]\s*\)/); + }); + + it('should import from node:fs for filesystem operations', () => { + expect(source).toMatch(/from\s+['"]node:fs['"]/); + }); +});