feat(sdk,cli): external state storage — .squad/ outside working tree (#792)#797
feat(sdk,cli): external state storage — .squad/ outside working tree (#792)#797tamirdresher merged 12 commits intodevfrom
Conversation
…ree (#792) - Add stateLocation: 'external' to SquadDirConfig - resolveExternalStateDir() + deriveProjectKey() utilities - resolveSquadPaths() honors external state location - squad externalize / internalize commands - State survives branch switches, invisible to git status - 9 new tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🛫 PR Readiness Check
|
| Status | Check | Details |
|---|---|---|
| ❌ | Single commit | 12 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 | 2 unresolved Copilot thread(s) — fix and resolve before merging |
| ❌ | CI passing | 16 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 SDK + CLI support for storing .squad/ state outside the repo working tree (under a platform-specific global Squad directory), intended to survive branch switches and avoid git status/PR pollution.
Changes:
- SDK: introduces
stateLocation: 'external'in.squad/config.jsonand resolves state paths to{globalSquadDir}/projects/{projectKey}/. - SDK: adds
resolveExternalStateDir()andderiveProjectKey()and exports them from the SDK barrel. - CLI: adds
externalize/internalizecommand implementation to move state in/out, plus tests + changeset.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
test/external-state.test.ts |
Adds tests for project key derivation, external dir resolution, and config parsing. |
packages/squad-sdk/src/resolution.ts |
Adds external state location support to config parsing + path resolution; adds new helpers. |
packages/squad-sdk/src/index.ts |
Exports new SDK APIs (resolveExternalStateDir, deriveProjectKey). |
packages/squad-cli/src/cli/commands/externalize.ts |
Implements state migration commands and gitignore helper. |
.changeset/external-state.md |
Release notes for the new external state storage feature. |
packages/squad-sdk/src/resolution.ts
Outdated
| /** | ||
| * Where squad state is stored. | ||
| * - 'local' (default): state lives in `.squad/` inside the repo | ||
| * - 'external': state lives in `~/.squad/projects/{projectKey}/`, only a | ||
| * thin config.json marker remains in the repo. Survives branch switches, | ||
| * invisible to `git status`, never pollutes PRs. | ||
| */ | ||
| stateLocation?: 'local' | 'external'; |
There was a problem hiding this comment.
The stateLocation docstring says external state lives in ~/.squad/projects/{projectKey}/, but resolveExternalStateDir() actually uses resolveGlobalSquadPath() (e.g. %APPDATA%/squad, ~/Library/Application Support/squad, $XDG_CONFIG_HOME/squad). Please update the docs here to describe {globalSquadDir}/projects/{projectKey}/ to avoid misleading users.
packages/squad-sdk/src/resolution.ts
Outdated
| if (config && config.stateLocation === 'external') { | ||
| // External mode: state lives in ~/.squad/projects/{projectKey}/ | ||
| const projectRoot = path.resolve(projectDir, '..'); | ||
| const projectKey = config.projectKey || deriveProjectKey(projectRoot); | ||
| const externalDir = resolveExternalStateDir(projectKey); | ||
| return { | ||
| mode: 'remote', | ||
| projectDir: externalDir, | ||
| teamDir: externalDir, | ||
| personalDir: resolvePersonalSquadDir(), | ||
| config, | ||
| name, | ||
| isLegacy, | ||
| }; |
There was a problem hiding this comment.
In external mode, resolveSquadPaths() redirects state to the external directory, but resolveSquad() still returns the in-repo .squad/ marker directory. Several CLI code paths use resolveSquad() as the state root (e.g. nap), which will now point at the thin marker and miss the real state. Consider either teaching resolveSquad() to detect stateLocation: 'external' and return the external dir, or updating call sites to consistently use resolveSquadPaths() when they need the state root.
packages/squad-sdk/src/resolution.ts
Outdated
| export function resolveExternalStateDir(projectKey: string, create: boolean = true): string { | ||
| const globalDir = resolveGlobalSquadPath(); | ||
| const stateDir = path.join(globalDir, 'projects', projectKey); | ||
| if (create && !storage.existsSync(stateDir)) { | ||
| storage.mkdirSync(stateDir, { recursive: true }); | ||
| } | ||
| return stateDir; |
There was a problem hiding this comment.
resolveExternalStateDir(projectKey) uses path.join(globalDir, 'projects', projectKey) without validating projectKey. If projectKey comes from .squad/config.json, a malicious value containing path separators or .. could escape the intended directory and cause reads/writes outside the Squad global dir. Please sanitize/validate projectKey (e.g., enforce the same slug rules as deriveProjectKey and reject any .. / path separators) before joining.
packages/squad-sdk/src/resolution.ts
Outdated
| * Uses the basename of the repo directory, lowercased and sanitized. | ||
| */ | ||
| export function deriveProjectKey(repoRoot: string): string { | ||
| const basename = path.basename(repoRoot); |
There was a problem hiding this comment.
deriveProjectKey() uses path.basename(repoRoot), which is platform-dependent. On non-Windows platforms, passing a Windows-style path like C:\Users\...\repo will treat backslashes as normal characters and produce an incorrect basename, causing the included test case to fail on Linux/macOS CI. Consider using path.win32.basename() when the input contains backslashes / a drive prefix (or normalizing separators first) so the function is robust across platforms.
| const basename = path.basename(repoRoot); | |
| const isWindowsStylePath = repoRoot.includes('\\') || /^[a-zA-Z]:/.test(repoRoot); | |
| const basename = isWindowsStylePath ? path.win32.basename(repoRoot) : path.basename(repoRoot); |
| describe('resolveExternalStateDir', () => { | ||
| it('creates the projects directory', () => { | ||
| const dir = resolveExternalStateDir('test-project-123'); | ||
| expect(existsSync(dir)).toBe(true); | ||
| expect(dir).toContain('projects'); | ||
| expect(dir).toContain('test-project-123'); | ||
|
|
||
| // Cleanup | ||
| rmSync(dir, { recursive: true, force: true }); | ||
| }); |
There was a problem hiding this comment.
These tests call resolveExternalStateDir() which resolves under the real user global config directory (XDG_CONFIG_HOME/APPDATA/homedir) and will create/delete directories outside the test temp root. Please isolate tests by setting XDG_CONFIG_HOME (and APPDATA on win32) to TEST_ROOT in a beforeEach, then restore env var(s) in afterEach, so the suite never mutates real user state.
| /** | ||
| * squad externalize — move .squad/ state out of the working tree. | ||
| * | ||
| * Moves all squad state to `~/.squad/projects/{projectKey}/` and writes | ||
| * a thin `.squad/config.json` marker in the repo. After externalization: | ||
| * - State survives branch switches (not tied to the working tree) | ||
| * - State is invisible to `git status` and never pollutes PRs | ||
| * - A `.squad/config.json` marker lets the walk-up resolver find state | ||
| * | ||
| * To restore local state, run `squad internalize`. | ||
| * | ||
| * @module cli/commands/externalize | ||
| */ |
There was a problem hiding this comment.
This command module is added, but it doesn't appear to be wired into the CLI entry router (no cmd === 'externalize' / internalize handling), so users won't be able to invoke it. Please add routing in packages/squad-cli/src/cli-entry.ts (and update help output if applicable) so the feature is reachable.
| if (storage.existsSync(dest)) { | ||
| // Merge: copy contents over (don't clobber existing) | ||
| copyDirRecursive(src, dest); | ||
| } else { | ||
| copyDirRecursive(src, dest); | ||
| } |
There was a problem hiding this comment.
The comment says the directory move is a "merge" that "don't clobber existing", but copyDirRecursive() ultimately calls storage.writeSync(destPath, content) which will overwrite existing files. Either adjust the comment to match behavior, or change the copy logic to skip/rename existing files when dest already exists.
| if (storage.existsSync(dest)) { | |
| // Merge: copy contents over (don't clobber existing) | |
| copyDirRecursive(src, dest); | |
| } else { | |
| copyDirRecursive(src, dest); | |
| } | |
| // Copy contents into the external state directory. If destination files | |
| // already exist, conflicting files may be overwritten by copyDirRecursive(). | |
| copyDirRecursive(src, dest); |
| // Write thin config.json marker | ||
| const config = { | ||
| version: 1, | ||
| teamRoot: '.', | ||
| projectKey: key, | ||
| stateLocation: 'external', | ||
| }; | ||
| storage.writeSync( | ||
| path.join(squadDir, 'config.json'), |
There was a problem hiding this comment.
runExternalize() unconditionally writes a new .squad/config.json object and will discard any existing config fields (e.g. consult mode flags, extractionDisabled, or an existing remote teamRoot). Please read and merge the existing config (preserving unknown fields) or fail fast with a clear message when a non-default config is already present, to avoid breaking existing setups.
| // Write thin config.json marker | |
| const config = { | |
| version: 1, | |
| teamRoot: '.', | |
| projectKey: key, | |
| stateLocation: 'external', | |
| }; | |
| storage.writeSync( | |
| path.join(squadDir, 'config.json'), | |
| // Write thin config.json marker without discarding unrelated existing config. | |
| const configPath = path.join(squadDir, 'config.json'); | |
| let existingConfig: Record<string, unknown> = {}; | |
| if (storage.existsSync(configPath)) { | |
| const existingConfigRaw = storage.readSync(configPath); | |
| if (existingConfigRaw !== null && existingConfigRaw.trim() !== '') { | |
| try { | |
| const parsed = JSON.parse(existingConfigRaw) as unknown; | |
| if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { | |
| fatal('.squad/config.json must contain a JSON object before it can be externalized safely.'); | |
| } | |
| existingConfig = parsed as Record<string, unknown>; | |
| } catch { | |
| fatal('Failed to parse existing .squad/config.json. Fix or remove the file before running `squad externalize`.'); | |
| } | |
| } | |
| } | |
| if ( | |
| Object.prototype.hasOwnProperty.call(existingConfig, 'teamRoot') && | |
| existingConfig.teamRoot !== '.' | |
| ) { | |
| fatal('Refusing to overwrite existing .squad/config.json: `teamRoot` is already set to a non-default value. Update it manually or remove the conflicting config before running `squad externalize`.'); | |
| } | |
| if ( | |
| Object.prototype.hasOwnProperty.call(existingConfig, 'projectKey') && | |
| existingConfig.projectKey !== key | |
| ) { | |
| fatal(`Refusing to overwrite existing .squad/config.json: \`projectKey\` is already set to "${String(existingConfig.projectKey)}", not "${key}".`); | |
| } | |
| if ( | |
| Object.prototype.hasOwnProperty.call(existingConfig, 'stateLocation') && | |
| existingConfig.stateLocation !== 'external' | |
| ) { | |
| fatal(`Refusing to overwrite existing .squad/config.json: \`stateLocation\` is already set to "${String(existingConfig.stateLocation)}", not "external".`); | |
| } | |
| const config = { | |
| ...existingConfig, | |
| version: 1, | |
| teamRoot: '.', | |
| projectKey: key, | |
| stateLocation: 'external', | |
| }; | |
| storage.writeSync( | |
| configPath, |
| // Remove the external marker from config.json | ||
| const updatedConfig = { ...config }; | ||
| delete updatedConfig.stateLocation; | ||
| storage.writeSync(configPath, JSON.stringify(updatedConfig, null, 2) + '\n'); | ||
|
|
There was a problem hiding this comment.
runInternalize() only deletes stateLocation but leaves teamRoot (written as '.' during externalize) in config.json. That means loadDirConfig() will still succeed and resolveSquadPaths() will treat the repo as remote mode, resolving teamDir to the project root instead of .squad/, which can break agent discovery and other state lookups after internalization. Consider removing the config.json marker entirely (restoring true local mode) or restoring the pre-externalize config.
.changeset/external-state.md
Outdated
| - `resolveExternalStateDir()` resolves state to `~/.squad/projects/{projectKey}/` | ||
| - `deriveProjectKey()` generates a stable key from the repo path | ||
| - `resolveSquadPaths()` honors external state location | ||
| - `squad externalize` moves state out, `squad internalize` moves it back | ||
| - State survives branch switches, invisible to `git status`, never pollutes PRs | ||
| - Thin `.squad/config.json` marker stays in repo (gitignored) | ||
| - 8 new tests |
There was a problem hiding this comment.
Release note details look inconsistent with the implementation:
- The path is described as
~/.squad/projects/{projectKey}/, but the code resolves under the platform-specific global squad dir (resolveGlobalSquadPath()), not~/.squad. - This changeset says "8 new tests", but the PR description and the added test file include 9 test cases.
Please update this changeset text to match reality so published notes aren’t misleading.
| - `resolveExternalStateDir()` resolves state to `~/.squad/projects/{projectKey}/` | |
| - `deriveProjectKey()` generates a stable key from the repo path | |
| - `resolveSquadPaths()` honors external state location | |
| - `squad externalize` moves state out, `squad internalize` moves it back | |
| - State survives branch switches, invisible to `git status`, never pollutes PRs | |
| - Thin `.squad/config.json` marker stays in repo (gitignored) | |
| - 8 new tests | |
| - `resolveExternalStateDir()` resolves state under the platform-specific global Squad directory using `resolveGlobalSquadPath()` | |
| - `deriveProjectKey()` generates a stable key from the repo path | |
| - `resolveSquadPaths()` honors external state location | |
| - `squad externalize` moves state out, `squad internalize` moves it back | |
| - State survives branch switches, invisible to `git status`, never pollutes PRs | |
| - Thin `.squad/config.json` marker stays in repo (gitignored) | |
| - 9 new tests |
- Fix doc comment: ~/ → {globalSquadDir}/ for accuracy
- Add path traversal protection: sanitize projectKey, reject ../
- Cross-platform deriveProjectKey: use path.win32.basename for Windows paths
- Preserve existing config.json fields during externalize (merge, don't overwrite)
- Delete config.json on internalize (restore true local mode)
- Remove dead code branch in directory copy
- Isolate tests from real user state (redirect env vars to TEST_ROOT)
- Add 3 security tests (path traversal, empty key, sanitization)
- Fix changeset: correct test count (12) and path description
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes unwired-command CI gate — externalize.ts was added but never imported in cli-entry.ts, making it unreachable for users. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Update JSDoc in externalize.ts to match platform-specific paths from resolveGlobalSquadPath() - Add comment on resolveSquad() explaining it intentionally returns the in-repo marker dir - Sanitize projectKey at CLI injection point with replace(/[\/\\\.]/g, '_') Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
# Conflicts: # packages/squad-sdk/src/index.ts # packages/squad-sdk/src/resolution.ts
🟡 Impact Analysis — PR #797Risk tier: 🟡 MEDIUM 📊 Summary
🎯 Risk Factors
📦 Modules Affectedroot (1 file)
squad-cli (2 files)
squad-sdk (2 files)
tests (1 file)
|
🏗️ Dina Review: REQUEST CHANGESExternal state storage — .squad/ outside working tree Good architecture overall — state isolation is excellent, security (path traversal protection) is solid, migration is reversible. 5 of 10 Copilot comments are already addressed. 2 critical issues to fix:
Minor: Docstring at resolution.ts:44 says ~/.squad/\ instead of {globalSquadDir}/. Once these are fixed, this is a strong addition. The external state pattern is the right approach for keeping PRs clean. |
|
I saw this PR and jumped on it to test (most of my issues have been around where squad and project files live and contamination related). I think it's a great idea - any work towards standardizing how Squad handles this is fantastic. I'm not sure as I'm still learning but is it possible that there's some overlap or feature duplication with the "squad link" command and related dual root features? |
… preserve config - Add resolveExternalStateDir() with path traversal validation and projectKey sanitization (rejects empty, '..', normalizes separators) - Add deriveProjectKey() for stable project key derivation from paths - Add stateLocation field to SquadDirConfig interface and loadDirConfig() - Export both new functions from SDK index - Fix runInternalize() to remove teamRoot and projectKey alongside stateLocation (preserves unrelated config fields like consult/stateBackend) - Docstrings reference platform-specific paths (%APPDATA%, ~/.config, etc.) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🏗️ Architectural Review
Automated architectural review — informational only. |
|
once all copilot's suggestions are implemented this lgtm |
…ster/squad into squad/792-external-state
What
Adds external state storage — move
.squad/state outside the working tree to a platform-specific global directory, surviving branch switches and staying invisible togit status.Why
.squad/in the working tree is destroyed on branch switch, pollutes PRs and diffs. Some users don't want squad state in version control. Closes #792How
stateLocation: 'external'option in.squad/config.jsonresolveExternalStateDir()resolves state under global Squad dir with path traversal protectionderiveProjectKey()generates stable cross-platform key from repo pathresolveSquadPaths()honors external modesquad externalize/squad internalizecommands wired into CLI entryTesting
npm run buildpassesnpm testpasses (12 new tests — key derivation, dir resolution, config parsing, path traversal security)Docs
Exports
resolveExternalStateDirandderiveProjectKeyadded to SDK barrelBreaking Changes
None — opt-in feature. Default behavior unchanged.
Waivers
None