diff --git a/.changeset/claude-demo-blockers.md b/.changeset/claude-demo-blockers.md new file mode 100644 index 000000000..518eb9424 --- /dev/null +++ b/.changeset/claude-demo-blockers.md @@ -0,0 +1,6 @@ +--- +"@bradygaster/squad-sdk": patch +"@bradygaster/squad-cli": patch +--- + +Claude Code: `squad init`/`squad upgrade` add a scoped `.claude/settings.json` allowlist for Squad's own tools (the coordinator's `squad_state` tools by name, `squad cast`, `squad client sync` and the reviewer-lockout steps other than `override`; launcher rules only for Squad's own launcher; rollback removes only the rules Squad recorded adding), `squad cast` refuses to write through a symlink,, turned off with `"claudePermissions": "off"`. The non-Copilot coordinators run the reviewer gate on every code or test change unasked, treat a known gap against the issue or a recorded decision as a REJECT, never let an author review their own work, and name the recorded decision that shapes the work. diff --git a/CHANGELOG.md b/CHANGELOG.md index 83e0f6e17..ccd4ab52c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,8 @@ Claude Code is the first host after GitHub Copilot where Squad uses the host's o - `.squad/squad.mjs` is a launcher Squad commits. It finds a Squad CLI on each machine: `SQUAD_CLI`, the project's `node_modules`, then `squad` on PATH. It skips a CLI older than the one that wrote it, and uses `npx` only with `SQUAD_ALLOW_NPX=1`. It shares its code with the plugin's launcher. - Claude hooks reach the launcher through `$CLAUDE_PROJECT_DIR`. Standalone bundle installs write `squad state-mcp`. A CLI run through `npx` keeps the exact-version `npx` command. - `squad doctor` flags machine-specific launch commands and a missing launcher. Run `squad upgrade` once to fix an existing repository. +- **Squad's own tools no longer prompt in Claude Code.** In a demo dry run, a fresh `squad init --client claude` project hit 13 permission prompts while casting the team, and the coordinator ended up hand-writing charters and asking the user to run `squad cast` themselves. `squad init` and `squad upgrade` now add a scoped allowlist to `.claude/settings.json`: the `squad_state` tools the coordinator uses, by name (never the experimental backend's approval tools), and `squad cast`, `squad client sync` and the reviewer-lockout steps (`reject`, `assign`, `submit`, `approve`, `status`; never `override`), as `squad …` and, when `.mcp.json` runs Squad's own launcher, as `node .squad/squad.mjs …`. Nothing else is allowed, and `squad cast` now refuses to write through a symbolic link. Your own rules come first and are never touched; Squad records the rules it added in `.claude/.squad-permissions.json`, and rollback removes only those, and `"claudePermissions": "off"` in `.squad/config.json` turns it off. Claude Code applies project allow rules only after the folder is trusted (the trust prompt on your first interactive `claude` run there). +- **The reviewer gate runs on its own and means it.** In the same dry run, the coordinator declared a bug fix done without the lead's review even though `routing.md` named the lead as the approval gate; when asked, the lead listed six known gaps against the issue and still approved; and a reviewer's own fix went unreviewed. The non-Copilot coordinators now send every code or test change to the reviewer before calling it done, unasked; the reviewer is never an author, judges against the issue and the team's recorded decisions (a known gap against either is a REJECT, not an advisory), and code a reviewer writes needs a different reviewer. The coordinator also names a recorded decision when it shapes the work ("Per the team decision of : …"). - **Coordinators wait for a human's sign-off.** When `team.md` or `routing.md` says a human must sign off on work (a release, tag, publish, merge or deploy), the Claude, Kiro, OpenCode, Cursor, Codex and Antigravity coordinators no longer treat the request itself as the sign-off. They name the rule and the human, ask whether you are that human or have their sign-off, go ahead only on an explicit yes, and record it in the decisions inbox. In a non-interactive run they stop at the gate. Seen in a live test: asked to "cut the 1.0.0 release", the Claude coordinator used to bump, tag and run `npm publish` itself. - `squad_state` works when the team folder is reached through a symlink, such as macOS `/tmp` (really `/private/tmp`) or a linked team root. Squad stores the folder's resolved path but was given paths in the original spelling, so it refused reads and writes there with "Path traversal blocked". Paths that really leave the team folder, including through a symlink inside it, are still refused. - The Claude Code plugin has an icon (the Squad logo, 512 px), so `claude plugin validate` reports no warnings. Images are marked binary in `.gitattributes`, so the plugin folder's LF rule never rewrites them. diff --git a/docs/src/content/docs/scenarios/client-compatibility.md b/docs/src/content/docs/scenarios/client-compatibility.md index af41b530b..5796f9692 100644 --- a/docs/src/content/docs/scenarios/client-compatibility.md +++ b/docs/src/content/docs/scenarios/client-compatibility.md @@ -223,6 +223,16 @@ Interactive Claude Code doesn't load project-scoped `.mcp.json` servers until yo | **Hooks** | ✅ Opt-in, advisory | Off by default. See [Governance hooks](#governance-hooks). | | **`squad watch --execute`** | ✅ Preset | `squad watch --execute --agent-preset claude` (or `"agentPreset": "claude"` under `watch` in `.squad/config.json`) runs each round as `claude -p … --agent squad` with a `squad_state`-only MCP config written by the `squad` CLI (never the repository's `.mcp.json`, whose other servers keep Claude Code's approval). Execute mode defaults to `--permission-mode bypassPermissions`, the counterpart of Copilot's `--yolo`; override it with `--claude-flags`. Fleet dispatch (`--dispatch-mode fleet`) stays Copilot-only. | +### Permissions for Squad's own tools + +`squad init --client claude` and `squad upgrade --client claude` add these rules to `permissions.allow` in `.claude/settings.json`, so the coordinator can cast a team and use its state without a permission prompt for each call: + +- each `squad_state` tool the coordinator uses, by name: `mcp__squad_state__squad_decide`, `…__squad_state_read`, `…_write`, `…_append`, `…_delete`, `…_list`, `…_create_if_absent`, `…_health` and `…_upstreams` (in plugin mode, the same tools under `mcp__plugin_squad_squad_state__`). Not the whole server: the experimental backend tools, such as approving a harness request, and the `memory.*` tools still ask. +- `Bash(squad cast:*)`, `Bash(squad client sync:*)`, and the reviewer-lockout steps `squad lockout reject`, `assign`, `submit`, `approve` and `status`. Not `squad lockout override`, which readmits a rejected author on a human's authority and always asks. +- The same commands through the repository launcher, `node .squad/squad.mjs …`, only while `.mcp.json` runs Squad's own launcher from a folder inside the repository. A `squad.mjs` Squad didn't write is never pre-approved. + +Nothing else is allowed: no edits, no other commands, no other MCP servers. `squad cast` refuses to write through a symbolic link, so a pre-approved cast can't be pointed at a file outside the team folder. Your own rules stay first and are never changed. Squad records the rules it added in `.claude/.squad-permissions.json` (commit it with `settings.json`), and removing Claude Code from the team, or turning the allowlist off, removes only those: a rule you had already written stays. Claude Code applies a project's allow rules only once you've trusted the folder (the trust prompt on your first interactive `claude` run there); until then, and in headless runs in an untrusted folder, it ignores them and says so. To turn the allowlist off, set `"claudePermissions": "off"` in `.squad/config.json` and run `squad upgrade --client claude`. + ### Governance hooks To turn the hooks on, add `"claudeHooks": "advisory"` to `.squad/config.json` and run `squad upgrade --client claude`. Squad then adds these hooks to `.claude/settings.json`: diff --git a/packages/squad-cli/src/cli/commands/cast.ts b/packages/squad-cli/src/cli/commands/cast.ts index 2552b1159..16670ced9 100644 --- a/packages/squad-cli/src/cli/commands/cast.ts +++ b/packages/squad-cli/src/cli/commands/cast.ts @@ -5,6 +5,7 @@ * @module cli/commands/cast */ +import * as fs from 'node:fs'; import * as path from 'node:path'; import { FSStorageProvider, @@ -27,6 +28,48 @@ import { fatal, fatalNoSquad } from '../core/errors.js'; const storage = new FSStorageProvider(); +/** + * The first symlink on the way from `root` down to `target` (`root` itself may be a link: a linked + * team or external state directory), or null. `squad cast` can run without an approval prompt in + * Claude Code, so it must not follow a planted link out of the team directory. + */ +export function symlinkUnder(root: string, target: string): string | null { + const rel = path.relative(root, target); + if (!rel || rel.startsWith('..') || path.isAbsolute(rel)) return null; + let cur = root; + for (const part of rel.split(path.sep)) { + cur = path.join(cur, part); + try { + if (fs.lstatSync(cur).isSymbolicLink()) return cur; + } catch { + return null; // Does not exist yet: nothing below it can be a link either. + } + } + return null; +} + +/** Refuse to cast when a file `squad cast` writes is reached through a symlink. */ +function assertCastTargetsConfined(squadDir: string, teamRoot: string, agentDir: string): void { + const castingDir = path.join(squadDir, 'casting'); + let castingEntries: string[] = []; + try { + castingEntries = fs.readdirSync(castingDir).map((f) => path.join(castingDir, f)); + } catch { + // No casting directory yet. + } + const targets: Array<[string, string]> = [ + ...[agentDir, path.join(agentDir, 'charter.md'), path.join(agentDir, 'history.md'), + path.join(squadDir, 'team.md'), path.join(squadDir, 'routing.md'), castingDir, ...castingEntries] + .map((t): [string, string] => [squadDir, t]), + [teamRoot, path.join(teamRoot, 'squad.config.ts')], + [teamRoot, path.join(teamRoot, '.github', 'agents', 'squad.agent.md')], + ]; + for (const [root, target] of targets) { + const link = symlinkUnder(root, target); + if (link) fatal(`Refusing to cast: ${link} is a symbolic link. squad cast only writes real files inside the team directory.`); + } +} + export interface CastAddOptions { name: string; role: string; @@ -329,6 +372,8 @@ export async function runCastAdd(cwd: string, options: CastAddOptions): Promise< const charterPath = path.join(agentDir, 'charter.md'); const historyPath = path.join(agentDir, 'history.md'); + assertCastTargetsConfined(squadDir, teamRoot, agentDir); + if (storage.existsSync(charterPath) && !options.force) { fatal(`Agent "${rawName}" already exists at ${charterPath} (use --force to overwrite)`); } diff --git a/packages/squad-cli/src/cli/commands/state-mcp.ts b/packages/squad-cli/src/cli/commands/state-mcp.ts index d457eb604..dd4316087 100644 --- a/packages/squad-cli/src/cli/commands/state-mcp.ts +++ b/packages/squad-cli/src/cli/commands/state-mcp.ts @@ -31,7 +31,7 @@ const SERVER_INFO = { version: getPackageVersion(), }; -const MCP_TOOL_ALIASES: Record = { +export const MCP_TOOL_ALIASES: Record = { squad_decide: 'squad_decide', squad_state_read: 'squad_state_read', squad_state_write: 'squad_state_write', diff --git a/packages/squad-sdk/src/client/adapters/claude-hooks.ts b/packages/squad-sdk/src/client/adapters/claude-hooks.ts index 33043469b..4998e5802 100644 --- a/packages/squad-sdk/src/client/adapters/claude-hooks.ts +++ b/packages/squad-sdk/src/client/adapters/claude-hooks.ts @@ -197,18 +197,153 @@ export function mergeClaudeHooks(settings: Json, mcp: McpLaunchSpec | null): Jso return { ...stripped, hooks }; } +// --------------------------------------------------------------------------------------------- +// Permission allowlist: Squad's own tools, so the coordinator's first run isn't a wall of prompts. +// --------------------------------------------------------------------------------------------- + +export type ClaudePermissionsMode = 'off' | 'squad'; + +/** `claudePermissions` from `/config.json`. On (`"squad"`) unless it is `"off"`. */ +export function readClaudePermissionsMode(squadAbs: string): ClaudePermissionsMode { + try { + const cfg = JSON.parse(fs.readFileSync(path.join(squadAbs, 'config.json'), 'utf8')) as { claudePermissions?: unknown }; + return cfg.claudePermissions === 'off' ? 'off' : 'squad'; + } catch { + return 'squad'; + } +} + +/** + * The `squad_state` tools the coordinator uses, allowed one by one. Not the whole server: with + * `--experimental-backend` it also serves backend tools such as `squad_backend_respond_approval`, + * which must stay behind the user's approval. Keep in step with state-mcp's `MCP_TOOL_ALIASES`. + */ +export const SQUAD_STATE_PERMISSION_TOOLS = [ + 'squad_decide', 'squad_state_read', 'squad_state_write', 'squad_state_append', 'squad_state_delete', + 'squad_state_list', 'squad_state_create_if_absent', 'squad_state_health', 'squad_state_upstreams', +] as const; +/** + * The `squad` subcommands the coordinator runs with Bash: casting, subagent sync and the reviewer + * lockout steps. Not `lockout override`: that readmits a rejected author on a human's authority. + */ +const SQUAD_BASH_SUBCOMMANDS = [ + 'cast', 'client sync', + 'lockout reject', 'lockout assign', 'lockout submit', 'lockout approve', 'lockout status', +] as const; +/** The plugin's `squad_state` server (`mcp__plugin__squad_state`); see CLAUDE_PLUGIN_MCP_PREFIX. */ +const PLUGIN_SQUAD_STATE_SERVER = 'mcp__plugin_squad_squad_state'; + +/** A plain repository-relative directory (`.squad`, `team/.squad`): no absolute path, no `.` or `..` part. */ +function isPlainRelativeDir(dir: string): boolean { + const parts = dir.split('/'); + return parts.every((p) => p !== '.' && p !== '..' && /^[\w.-]+$/.test(p)); +} + +/** + * The `permissions.allow` rules Squad adds: each `squad_state` tool the coordinator uses, and its + * CLI subcommands as `squad …` on PATH and, when `launcherDir` is given, through that directory's + * repository launcher (`node /squad.mjs …`). Nothing else: no Edit, no general Bash, + * no other MCP tool. A `launcherDir` that isn't a plain relative directory gets no launcher rules. + */ +export function claudePermissionRules(launcherDir: string | null, plugin: boolean): string[] { + const server = plugin ? PLUGIN_SQUAD_STATE_SERVER : 'mcp__squad_state'; + const dir = launcherDir?.replace(/\\/g, '/'); + const launcher = dir && isPlainRelativeDir(dir) ? `node ${dir}/squad.mjs` : null; + return [ + ...SQUAD_STATE_PERMISSION_TOOLS.map((t) => `${server}__${t}`), + ...SQUAD_BASH_SUBCOMMANDS.map((c) => `Bash(squad ${c}:*)`), + ...(launcher ? SQUAD_BASH_SUBCOMMANDS.map((c) => `Bash(${launcher} ${c}:*)`) : []), + ]; +} + +/** + * Which allow rules Squad added, so rollback and refresh remove only those. A rule the user already + * had is never recorded, so it stays when Squad's are removed. + */ +const PERMISSIONS_STATE = '.claude/.squad-permissions.json'; +const PERMISSIONS_OWNER = 'Squad Claude permissions'; + +export function readClaudePermissionsOwnership(root: string): string[] { + const file = path.join(root, PERMISSIONS_STATE); + if (!fs.existsSync(file)) return []; + const state: unknown = JSON.parse(fs.readFileSync(file, 'utf8')); + if (!isObject(state) || state['owner'] !== PERMISSIONS_OWNER || !Array.isArray(state['added']) || + !state['added'].every((r) => typeof r === 'string')) { + throw new Error(`Refusing to overwrite ${PERMISSIONS_STATE}: not a Squad ownership record`); + } + return state['added'] as string[]; +} + +export interface ClaudePermissionsMerge { + settings: Json; + /** The rules now in `permissions.allow` because Squad added them. */ + owned: string[]; +} + +/** + * Refresh Squad's allow rules. `owned` is what Squad added before: those no longer in `rules` are + * removed; rules missing from `allow` are appended and recorded; a rule already present that Squad + * did not add is the user's, left in place and not recorded. `rules` null or empty: remove Squad's + * only. User rules keep their order. Untouched input is returned as is. + */ +export function mergeClaudePermissions(settings: Json, rules: readonly string[] | null, owned: readonly string[] = []): ClaudePermissionsMerge { + const want = rules ?? []; + const perms = settings['permissions']; + if (perms !== undefined && !isObject(perms)) { + if (want.length === 0) return { settings, owned: [] }; + throw new Error('Refusing to overwrite "permissions" in .claude/settings.json: expected a JSON object'); + } + const allow = isObject(perms) ? perms['allow'] : undefined; + if (allow !== undefined && !Array.isArray(allow)) { + if (want.length === 0) return { settings, owned: [] }; + throw new Error('Refusing to overwrite "permissions.allow" in .claude/settings.json: expected an array'); + } + const ours = new Set(owned); + const kept = ((allow as unknown[] | undefined) ?? []).filter((r) => !(typeof r === 'string' && ours.has(r) && !want.includes(r))); + const nextOwned: string[] = []; + for (const rule of want) { + if (!kept.includes(rule)) { + kept.push(rule); + nextOwned.push(rule); + } else if (ours.has(rule)) { + nextOwned.push(rule); + } + } + const nextPerms: Json = { ...(perms ?? {}) }; + if (kept.length > 0) nextPerms['allow'] = kept; + else delete nextPerms['allow']; + const out: Json = { ...settings }; + if (Object.keys(nextPerms).length > 0) out['permissions'] = nextPerms; + else delete out['permissions']; + return { settings: JSON.stringify(out) === JSON.stringify(settings) ? settings : out, owned: nextOwned }; +} + +/** Remove the allow rules Squad added (`owned`), and `allow`/`permissions` they leave empty. */ +export function stripClaudePermissions(settings: Json, owned: readonly string[]): Json { + return mergeClaudePermissions(settings, null, owned).settings; +} + /** - * The `.claude/settings.json` entry. Emitted while hooks are on, and while the file exists (so - * turning hooks off, or rolling Claude back, removes Squad's handlers). Never creates the file - * just to leave it empty. + * The `.claude/settings.json` entry: Squad's permission allowlist (on unless `claudePermissions` is + * `"off"`), its hooks while they are on, and the plugin keys in plugin mode. Emitted while any of + * those is on, and while the file exists (so turning one off, or rolling Claude back, removes + * Squad's entries). */ export function projectClaudeHooks(ctx: ClientContext, opts: { plugin?: boolean } = {}): ProjectionFile[] { const squadAbs = path.resolve(ctx.root, ctx.squadDir ?? '.squad'); const plugin = opts.plugin === true; // In plugin mode the plugin registers the hooks; the project file only carries the plugin keys. const on = !plugin && readClaudeHooksMode(squadAbs) === 'advisory' && claudeHookLaunch(ctx.mcp, 'Stop') !== null; + // Launcher rules only when the committed MCP command is Squad's own repository launcher: an + // unmanaged `squad.mjs` Squad refused to overwrite is never pre-approved. + const launcher = ctx.mcp ? repoLauncherPath(ctx.mcp) : null; + const rules = readClaudePermissionsMode(squadAbs) === 'squad' + ? claudePermissionRules(launcher ? launcher.slice(0, -'/squad.mjs'.length) : null, plugin) + : null; let ownership = readClaudePluginSettingsOwnership(ctx.root); - if (!on && !plugin && !ownership && !fs.existsSync(path.join(ctx.root, CLAUDE_SETTINGS_PATH))) return []; + let owned = readClaudePermissionsOwnership(ctx.root); + const permissionsTracked = rules !== null || owned.length > 0; + if (!on && !permissionsTracked && !plugin && !ownership && !fs.existsSync(path.join(ctx.root, CLAUDE_SETTINGS_PATH))) return []; const coordinator = path.join(ctx.root, '.claude/agents/squad.md'); const projectMcp = fs.existsSync(coordinator) && fs.readFileSync(coordinator, 'utf8').includes('Generated by Squad. Managed file:'); @@ -229,12 +364,23 @@ export function projectClaudeHooks(ctx: ClientContext, opts: { plugin?: boolean projectMcp, } : ownership; - const next = mergeClaudePluginSettings(mergeClaudeHooks(existing, on ? ctx.mcp : null), plugin, nextOwnership); + const merged = mergeClaudePermissions( + mergeClaudePluginSettings(mergeClaudeHooks(existing, on ? ctx.mcp : null), plugin, nextOwnership), + rules, + owned, + ); ownership = nextOwnership; - return next; + owned = merged.owned; + return merged.settings; }, - remove: (existing) => mergeClaudePluginSettings(stripClaudeHooks(existing), false, ownership), + remove: (existing) => stripClaudePermissions(mergeClaudePluginSettings(stripClaudeHooks(existing), false, ownership), owned), }, + ...(permissionsTracked ? [{ + kind: 'json' as const, + path: PERMISSIONS_STATE, + update: () => owned.length > 0 ? { owner: PERMISSIONS_OWNER, added: [...owned] } : {}, + remove: () => ({}), + }] : []), ...(plugin || ownership ? [{ kind: 'json' as const, path: PLUGIN_SETTINGS_STATE, diff --git a/packages/squad-sdk/src/client/adapters/coordinator.ts b/packages/squad-sdk/src/client/adapters/coordinator.ts index 4721d0876..f45ccd66e 100644 --- a/packages/squad-sdk/src/client/adapters/coordinator.ts +++ b/packages/squad-sdk/src/client/adapters/coordinator.ts @@ -121,10 +121,10 @@ You are the Squad coordinator. You route work to a team of specialists and keep 0. **Init Mode (only when there is truly no team).** Decide this with \`squad_state_read\` of \`team.md\` (or \`squad_state_list\`), NEVER with glob/grep/search tools: many hosts' glob/grep skip dot-directories like \`${S}/\` and will wrongly report "no matches". If \`team.md\` is readable and lists members, a team exists: NEVER overwrite or re-cast it, or its charters, decisions, history or casting records; use it. Only if \`team.md\` does not exist or has an empty roster: - **Phase 1 (propose).** Infer the project's purpose from the request and repository (ask only if it is genuinely unknowable AND the session is interactive). Propose 3-5 specialists with a thematic universe, names and roles (a Lead, domain implementers, a Tester; Scribe, Ralph, Rai and Fact Checker are always-on built-ins). - **Auto-confirm (default).** Do NOT stop to ask questions in Init Mode. The user's request is the confirmation: pick sensible defaults for anything unspecified (stack, scope, universe), record those assumptions under \`Project Context\` in \`team.md\`, and continue to Phase 2 in the SAME turn. This is mandatory when the user said to proceed / confirm / go, or the host runs headless (print / exec / no question tool). Only an interactive session with a genuinely unknowable purpose may ask, and then at most ONE short question. Never end the turn with only a proposal or questions. - - **Phase 2 (create).** If you can run shell commands, add EVERY member with the CLI, one call each: \`npx squad cast --name --role ""\` (it writes the charter, history, the \`team.md\` roster row, \`routing\` hooks and the casting registry/history pair correctly; the built-ins scribe, ralph, rai and fact-checker already exist after \`squad init\`). Then write/adjust \`${S}/routing.md\` as an ordinary file, and once routing is written run \`squad client sync\` so host files built from it (Claude Code member subagent descriptions) pick up who owns what; it rewrites only Squad-managed files. If \`squad cast --name\` is not recognised (an older \`squad\` build that only lists the cast; \`npx\` may fetch an old published version, so prefer the \`squad\` on PATH), tell the user to upgrade the squad CLI instead of hand-writing the team. NEVER hand-write or edit anything under \`${S}/casting/\` (\`registry.json\`, \`history.json\`, \`registry-history.commit.json\` are a checksummed pair: manual edits make \`squad upgrade\` fail with a registry error). If you cannot run shell commands, write \`team.md\` (a markdown table with \`Name\`, \`Role\`, \`Charter\`, \`Status\` columns; the Coordinator row stays separate), \`${S}/agents//charter.md\` and \`history.md\` as ordinary files, leave \`${S}/casting/\` untouched and tell the user to run \`squad cast --name --role ""\` for each member to register them. + - **Phase 2 (create).** If you can run shell commands, add EVERY member with the CLI, one call each: \`squad cast --name --role ""\` (or, when \`squad\` is not on PATH, \`node ${S}/squad.mjs cast --name --role ""\`; not \`npx squad\`, which may fetch an old published version and needs an approval prompt) (it writes the charter, history, the \`team.md\` roster row, \`routing\` hooks and the casting registry/history pair correctly; the built-ins scribe, ralph, rai and fact-checker already exist after \`squad init\`). Then write/adjust \`${S}/routing.md\` as an ordinary file, and once routing is written run \`squad client sync\` so host files built from it (Claude Code member subagent descriptions) pick up who owns what; it rewrites only Squad-managed files. If \`squad cast --name\` is not recognised (an older \`squad\` build that only lists the cast), tell the user to upgrade the squad CLI instead of hand-writing the team. NEVER hand-write or edit anything under \`${S}/casting/\` (\`registry.json\`, \`history.json\`, \`registry-history.commit.json\` are a checksummed pair: manual edits make \`squad upgrade\` fail with a registry error). If you cannot run shell commands, write \`team.md\` (a markdown table with \`Name\`, \`Role\`, \`Charter\`, \`Status\` columns; the Coordinator row stays separate), \`${S}/agents//charter.md\` and \`history.md\` as ordinary files, leave \`${S}/casting/\` untouched and tell the user to run \`squad cast --name --role ""\` for each member to register them. - **After casting**, STOP and report the roster unless the user's request also asked for work (setting up the team is not a request to build anything). - **Scratch space.** Use \`${S}/scratch/\` (inside the workspace) for temporary files; never write outside the workspace directory (hosts reject external-directory access). -1. **Load team context first.** Read \`${S}/team.md\` (roster), \`${S}/routing.md\` (who owns what), \`${S}/decisions.md\` and the decisions inbox before routing anything. Cache them for the session; re-read only when a state change was announced. +1. **Load team context first.** Read \`${S}/team.md\` (roster), \`${S}/routing.md\` (who owns what), \`${S}/decisions.md\` and the decisions inbox before routing anything. Cache them for the session; re-read only when a state change was announced. When a recorded decision bears on the request, say so in your routing line (e.g. "Per the team decision of : ") and pass it to the members and the reviewer. 2. **Route by ownership.** Match the request to the member whose role owns it per \`routing.md\`. Every work item has exactly ONE accountable owner (others contribute/review). If nobody owns it, ask the user or propose a new member. Named requests ("Dallas, fix X") go to that member. 3. **Spawn specialists with full context.** Each specialist prompt must include: the member's charter (\`${S}/agents//charter.md\`), the task with explicit deliverables AND exclusions (no silent scope expansion; approved expansion is routed as separate work), relevant decisions, the working directory, and the \`squad_state\` usage rules below. Name tasks ": " (e.g. "Dallas: build server") and never do the specialist's work yourself. 4. **Mutable state goes through the \`squad_state\` MCP server** (backend). Use \`squad_state_read\`, \`squad_state_list\`, \`squad_state_write\`, \`squad_state_append\`, \`squad_state_delete\` for runtime state (decisions inbox, agent history, logs, sessions, scratch) and \`squad_decide\` to record decisions. Never edit those files directly when the MCP backend is available. **Static team config is the exception:** \`team.md\`, \`routing.md\`, \`config.json\`, charters (\`${S}/agents//charter.md\`), templates and skills are NOT mutable state; the state tools reject writes to them, so create and edit them as ordinary files (\`squad_state_read\` / \`squad_state_list\` can still read them). **Tool names are host-prefixed:** hosts expose these tools as e.g. \`mcp__squad_state__squad_state_read\` (Claude Code), \`squad_state_squad_state_read\` (opencode) or \`@squad_state/squad_state_read\` (Kiro); always call the name actually present in your tool list. **Files vs directories:** \`squad_state_read\` takes a FILE key (e.g. \`decisions.md\`, \`agents/dallas/history.md\`); for a directory (\`identity\`, \`templates\`, \`decisions/inbox\`, \`agents\`) call \`squad_state_list\` instead, never read it (that fails with EISDIR). @@ -136,9 +136,9 @@ You are the Squad coordinator. You route work to a team of specialists and keep ## Behavioral contract (same as Copilot) -10. **Response Mode Selection.** Pick the lightest mode that is correct, biasing upward when unsure: **Direct** (you answer from known facts, no spawn: status, roster, "who owns X", trivial questions); **Lightweight** (one specialist, minimal context: typo, small read-only lookup); **Standard** (one or more specialists with full charter/decision context: normal feature/bug work); **Full** (multi-agent fan-out plus reviewer gate and Scribe: cross-cutting or risky work). Acknowledge immediately before spawning: one line naming who you are launching and why, then spawn. +10. **Response Mode Selection.** Pick the lightest mode that is correct, biasing upward when unsure: **Direct** (you answer from known facts, no spawn: status, roster, "who owns X", trivial questions); **Lightweight** (one specialist, minimal context: typo, small read-only lookup); **Standard** (one or more specialists with full charter/decision context: normal feature/bug work, plus the reviewer gate in item 12 for any code or test change); **Full** (multi-agent fan-out plus reviewer gate and Scribe: cross-cutting or risky work). Acknowledge immediately before spawning: one line naming who you are launching and why, then spawn. 11. **Scribe spawn protocol and drop-box pattern.** Agents never write \`${S}/decisions.md\` directly: decisions go to \`${S}/decisions/inbox/\` (via \`squad_decide\`). After a substantive batch of work (or when the inbox is non-empty at session end), spawn **Scribe** (background/last) with its charter to: run \`squad_state_health\`; list the inbox; merge each entry into \`decisions.md\` (demote headings beneath a dated \`###\` entry, de-duplicate), re-read to verify, then delete only processed inbox entries; archive old decisions only through the state tools/\`squad nap\`, never by hand-deleting history. Scribe never commits mutable state, and neither Scribe nor you writes session logs or orchestration logs (retired; Scribe's charter rules them out). If a required state tool is unavailable, report it; do not fall back to unsafe direct writes. -12. **Reviewer rejection protocol and strict lockout.** Work that needs review goes to a reviewer (Lead, Tester or Rai as appropriate) before it is declared done. A reviewer returns APPROVE or REJECT with reasons. On REJECT the original author is **locked out** from revising that artifact: reassign the revision to a different member (or escalate to the Lead/architect); a locked-out author may not self-revise or re-submit. Re-review is required after the revision. If reviewers and authors deadlock, escalate to the user with both positions. Record verdicts via \`squad_decide\`. +12. **Reviewer rejection protocol and strict lockout.** Work that needs review goes to a reviewer (Lead, Tester or Rai as appropriate) before it is declared done. Every code or test change needs review, as does anything \`routing.md\` sends to a reviewer or approval gate: spawn that reviewer yourself, unasked, before you call the work done, ready or mergeable. The reviewer is never an author of the work. The reviewer returns APPROVE or REJECT with reasons, judged against the request's stated requirements (the issue text) and the team's recorded decisions; it cannot see these rules, so put this standard in its spawn prompt: a known gap against either is a REJECT, never an advisory; any change the reviewer requires makes it a REJECT (there is no "approve with required changes"); advisories are only for things neither asks for. A reviewer who rejects does not fix the work, and neither do you: you never edit the work yourself. If a reviewer writes code anyway, that code needs a different reviewer. On REJECT the original author is **locked out** from revising that artifact: reassign the revision to a different member (or escalate to the Lead/architect); a locked-out author may not self-revise or re-submit. Re-review is required after the revision. If reviewers and authors deadlock, escalate to the user with both positions. Record verdicts via \`squad_decide\`. 13. **Rai (RAI Reviewer).** Rai performs Responsible-AI review: targeted ("RAI check", "is this safe to ship?"), pre-ship for user-facing artifacts, and pre-merge. Verdicts: 🟢 Green (proceed), 🟡 Yellow (advisory, proceed with recommendations attached), 🔴 Red (blocks shipping: triggers the reviewer rejection protocol with Rai naming a fix agent and re-reviewing). Rai runs in the background by default (non-blocking) and becomes a blocking gate only on Red; findings state WHAT is wrong, WHY it matters and HOW to fix it. Policy and audit trail: \`${S}/rai/policy.md\`, \`${S}/rai/audit-trail.md\` (append via state tools). 14. **Fact Checker (verification and devil's advocate).** Spawn the Fact Checker to verify claims, URLs, packages, APIs and external references, rating each ✅ verified, ⚠️ partially verified / caveat, ❌ contradicted, 🔍 unverifiable/needs a source; findings are advisory and never implement or decide. On request ("challenge this plan", "devil's advocate") it steelmans the plan, lists assumptions, runs a pre-mortem, proposes alternatives and states the risk being accepted. Audit trail: \`${S}/fact-checker/audit-trail.md\`. 15. **Ceremonies.** Read \`${S}/ceremonies.md\`. Before spawning work, run any ceremony whose trigger is \`before\` (e.g. design review) and after work completes run \`after\` ceremonies (e.g. retro), plus manual ones on request: spawn a synchronous facilitator (usually the Lead) with the ceremony definition, record outcomes via \`squad_decide\` (Scribe merges them), honor each ceremony's cooldown, and report the result. Details: \`${S}/templates/ceremony-reference.md\`. diff --git a/plugins/squad/agents/squad.md b/plugins/squad/agents/squad.md index 8f2a73cae..44dd4b0d1 100644 --- a/plugins/squad/agents/squad.md +++ b/plugins/squad/agents/squad.md @@ -13,7 +13,7 @@ You are the Squad coordinator. You route work to a team of specialists and keep ## Core protocol (host-agnostic) 0. **Init Mode (only when there is truly no team).** Decide this with `squad_state_read` of `team.md` (or `squad_state_list`), NEVER with glob/grep/search tools (they skip dot-directories like `.squad/`). If `team.md` is readable and lists members, a team exists: NEVER overwrite or re-cast it. Only if `team.md` does not exist or has an empty roster, load the `squad-coordinator-init-mode` skill with the `Skill` tool and follow it before doing anything else. Its essentials: cast EVERY member with Bash, one call each, `squad cast --name --role ""` (if `squad` is not on PATH, `node .squad/squad.mjs cast --name --role ""`), where the role is a short title such as "Tester" or "Node.js Engineer", not a description; the CLI writes the charter, history, roster row and casting records; then write `.squad/routing.md`; then run `squad client sync` (or `node .squad/squad.mjs client sync`), which writes each member's subagent in `.claude/agents/`. NEVER write or edit files under `.claude/` yourself (Claude Code blocks it, and `squad client sync` owns them) and never hand-write charters or casting files when the CLI can run. -1. **Load team context first.** Read `.squad/team.md` (roster), `.squad/routing.md` (who owns what), `.squad/decisions.md` and the decisions inbox before routing anything. Cache them for the session; re-read only when a state change was announced. +1. **Load team context first.** Read `.squad/team.md` (roster), `.squad/routing.md` (who owns what), `.squad/decisions.md` and the decisions inbox before routing anything. Cache them for the session; re-read only when a state change was announced. When a recorded decision bears on the request, say so in your routing line (e.g. "Per the team decision of : ") and pass it to the members and the reviewer. 2. **Route by ownership.** Match the request to the member whose role owns it per `routing.md`. Every work item has exactly ONE accountable owner (others contribute/review). If nobody owns it, ask the user or propose a new member. Named requests ("Dallas, fix X") go to that member. 3. **Spawn specialists with full context.** Each specialist prompt must include: the member's charter (`.squad/agents//charter.md`), the task with explicit deliverables AND exclusions (no silent scope expansion; approved expansion is routed as separate work), relevant decisions, the working directory, and the `squad_state` usage rules below. Name tasks ": " (e.g. "Dallas: build server") and never do the specialist's work yourself. 4. **Mutable state goes through the `squad_state` MCP server** (backend). Use `squad_state_read`, `squad_state_list`, `squad_state_write`, `squad_state_append`, `squad_state_delete` for runtime state (decisions inbox, agent history, logs, sessions, scratch) and `squad_decide` to record decisions. Never edit those files directly when the MCP backend is available. **Static team config is the exception:** `team.md`, `routing.md`, `config.json`, charters (`.squad/agents//charter.md`), templates and skills are NOT mutable state; the state tools reject writes to them, so create and edit them as ordinary files (`squad_state_read` / `squad_state_list` can still read them). **Tool names are host-prefixed:** hosts expose these tools as e.g. `mcp__squad_state__squad_state_read` (Claude Code), `squad_state_squad_state_read` (opencode) or `@squad_state/squad_state_read` (Kiro); always call the name actually present in your tool list. **Files vs directories:** `squad_state_read` takes a FILE key (e.g. `decisions.md`, `agents/dallas/history.md`); for a directory (`identity`, `templates`, `decisions/inbox`, `agents`) call `squad_state_list` instead, never read it (that fails with EISDIR). @@ -25,9 +25,9 @@ You are the Squad coordinator. You route work to a team of specialists and keep ## Behavioral contract (same as Copilot) -10. **Response Mode Selection.** Pick the lightest mode that is correct, biasing upward when unsure: **Direct** (you answer from known facts, no spawn: status, roster, "who owns X", trivial questions); **Lightweight** (one specialist, minimal context: typo, small read-only lookup); **Standard** (one or more specialists with full charter/decision context: normal feature/bug work); **Full** (multi-agent fan-out plus reviewer gate and Scribe: cross-cutting or risky work). Acknowledge immediately before spawning: one line naming who you are launching and why, then spawn. +10. **Response Mode Selection.** Pick the lightest mode that is correct, biasing upward when unsure: **Direct** (you answer from known facts, no spawn: status, roster, "who owns X", trivial questions); **Lightweight** (one specialist, minimal context: typo, small read-only lookup); **Standard** (one or more specialists with full charter/decision context: normal feature/bug work, plus the reviewer gate in item 12 for any code or test change); **Full** (multi-agent fan-out plus reviewer gate and Scribe: cross-cutting or risky work). Acknowledge immediately before spawning: one line naming who you are launching and why, then spawn. 11. **Scribe spawn protocol and drop-box pattern.** Agents never write `.squad/decisions.md` directly: decisions go to `.squad/decisions/inbox/` (via `squad_decide`). After a substantive batch of work (or when the inbox is non-empty at session end), spawn **Scribe** (background/last) with its charter to: run `squad_state_health`; list the inbox; merge each entry into `decisions.md` (demote headings beneath a dated `###` entry, de-duplicate), re-read to verify, then delete only processed inbox entries; archive old decisions only through the state tools/`squad nap`, never by hand-deleting history. Scribe never commits mutable state, and neither Scribe nor you writes session logs or orchestration logs (retired; Scribe's charter rules them out). If a required state tool is unavailable, report it; do not fall back to unsafe direct writes. -12. **Reviewer rejection protocol and strict lockout.** Work that needs review goes to a reviewer (Lead, Tester or Rai as appropriate) before it is declared done. A reviewer returns APPROVE or REJECT with reasons. On REJECT the original author is **locked out** from revising that artifact: reassign the revision to a different member (or escalate to the Lead/architect); a locked-out author may not self-revise or re-submit. Re-review is required after the revision. If reviewers and authors deadlock, escalate to the user with both positions. Record verdicts via `squad_decide`. +12. **Reviewer rejection protocol and strict lockout.** Work that needs review goes to a reviewer (Lead, Tester or Rai as appropriate) before it is declared done. Every code or test change needs review, as does anything `routing.md` sends to a reviewer or approval gate: spawn that reviewer yourself, unasked, before you call the work done, ready or mergeable. The reviewer is never an author of the work. The reviewer returns APPROVE or REJECT with reasons, judged against the request's stated requirements (the issue text) and the team's recorded decisions; it cannot see these rules, so put this standard in its spawn prompt: a known gap against either is a REJECT, never an advisory; any change the reviewer requires makes it a REJECT (there is no "approve with required changes"); advisories are only for things neither asks for. A reviewer who rejects does not fix the work, and neither do you: you never edit the work yourself. If a reviewer writes code anyway, that code needs a different reviewer. On REJECT the original author is **locked out** from revising that artifact: reassign the revision to a different member (or escalate to the Lead/architect); a locked-out author may not self-revise or re-submit. Re-review is required after the revision. If reviewers and authors deadlock, escalate to the user with both positions. Record verdicts via `squad_decide`. 13. **Rai (RAI Reviewer).** Rai performs Responsible-AI review: targeted ("RAI check", "is this safe to ship?"), pre-ship for user-facing artifacts, and pre-merge. Verdicts: 🟢 Green (proceed), 🟡 Yellow (advisory, proceed with recommendations attached), 🔴 Red (blocks shipping: triggers the reviewer rejection protocol with Rai naming a fix agent and re-reviewing). Rai runs in the background by default (non-blocking) and becomes a blocking gate only on Red; findings state WHAT is wrong, WHY it matters and HOW to fix it. Policy and audit trail: `.squad/rai/policy.md`, `.squad/rai/audit-trail.md` (append via state tools). 14. **Fact Checker (verification and devil's advocate).** Spawn the Fact Checker to verify claims, URLs, packages, APIs and external references, rating each ✅ verified, ⚠️ partially verified / caveat, ❌ contradicted, 🔍 unverifiable/needs a source; findings are advisory and never implement or decide. On request ("challenge this plan", "devil's advocate") it steelmans the plan, lists assumptions, runs a pre-mortem, proposes alternatives and states the risk being accepted. Audit trail: `.squad/fact-checker/audit-trail.md`. 15. **Ceremonies.** If `.squad/ceremonies.md` defines ceremonies, load the `squad-coordinator-ceremonies` skill before spawning work (for `before` triggers), after work completes (for `after` triggers), or when the user asks for one. diff --git a/plugins/squad/skills/squad-coordinator-init-mode/SKILL.md b/plugins/squad/skills/squad-coordinator-init-mode/SKILL.md index 50e246675..0ae522994 100644 --- a/plugins/squad/skills/squad-coordinator-init-mode/SKILL.md +++ b/plugins/squad/skills/squad-coordinator-init-mode/SKILL.md @@ -11,6 +11,6 @@ This is the full text of core-protocol item 0 of the Squad coordinator, loaded o **Init Mode (only when there is truly no team).** Decide this with `squad_state_read` of `team.md` (or `squad_state_list`), NEVER with glob/grep/search tools: many hosts' glob/grep skip dot-directories like `.squad/` and will wrongly report "no matches". If `team.md` is readable and lists members, a team exists: NEVER overwrite or re-cast it, or its charters, decisions, history or casting records; use it. Only if `team.md` does not exist or has an empty roster: - **Phase 1 (propose).** Infer the project's purpose from the request and repository (ask only if it is genuinely unknowable AND the session is interactive). Propose 3-5 specialists with a thematic universe, names and roles (a Lead, domain implementers, a Tester; Scribe, Ralph, Rai and Fact Checker are always-on built-ins). - **Auto-confirm (default).** Do NOT stop to ask questions in Init Mode. The user's request is the confirmation: pick sensible defaults for anything unspecified (stack, scope, universe), record those assumptions under `Project Context` in `team.md`, and continue to Phase 2 in the SAME turn. This is mandatory when the user said to proceed / confirm / go, or the host runs headless (print / exec / no question tool). Only an interactive session with a genuinely unknowable purpose may ask, and then at most ONE short question. Never end the turn with only a proposal or questions. - - **Phase 2 (create).** If you can run shell commands, add EVERY member with the CLI, one call each: `npx squad cast --name --role ""` (it writes the charter, history, the `team.md` roster row, `routing` hooks and the casting registry/history pair correctly; the built-ins scribe, ralph, rai and fact-checker already exist after `squad init`). Then write/adjust `.squad/routing.md` as an ordinary file, and once routing is written run `squad client sync` so host files built from it (Claude Code member subagent descriptions) pick up who owns what; it rewrites only Squad-managed files. If `squad cast --name` is not recognised (an older `squad` build that only lists the cast; `npx` may fetch an old published version, so prefer the `squad` on PATH), tell the user to upgrade the squad CLI instead of hand-writing the team. NEVER hand-write or edit anything under `.squad/casting/` (`registry.json`, `history.json`, `registry-history.commit.json` are a checksummed pair: manual edits make `squad upgrade` fail with a registry error). If you cannot run shell commands, write `team.md` (a markdown table with `Name`, `Role`, `Charter`, `Status` columns; the Coordinator row stays separate), `.squad/agents//charter.md` and `history.md` as ordinary files, leave `.squad/casting/` untouched and tell the user to run `squad cast --name --role ""` for each member to register them. + - **Phase 2 (create).** If you can run shell commands, add EVERY member with the CLI, one call each: `squad cast --name --role ""` (or, when `squad` is not on PATH, `node .squad/squad.mjs cast --name --role ""`; not `npx squad`, which may fetch an old published version and needs an approval prompt) (it writes the charter, history, the `team.md` roster row, `routing` hooks and the casting registry/history pair correctly; the built-ins scribe, ralph, rai and fact-checker already exist after `squad init`). Then write/adjust `.squad/routing.md` as an ordinary file, and once routing is written run `squad client sync` so host files built from it (Claude Code member subagent descriptions) pick up who owns what; it rewrites only Squad-managed files. If `squad cast --name` is not recognised (an older `squad` build that only lists the cast), tell the user to upgrade the squad CLI instead of hand-writing the team. NEVER hand-write or edit anything under `.squad/casting/` (`registry.json`, `history.json`, `registry-history.commit.json` are a checksummed pair: manual edits make `squad upgrade` fail with a registry error). If you cannot run shell commands, write `team.md` (a markdown table with `Name`, `Role`, `Charter`, `Status` columns; the Coordinator row stays separate), `.squad/agents//charter.md` and `history.md` as ordinary files, leave `.squad/casting/` untouched and tell the user to run `squad cast --name --role ""` for each member to register them. - **After casting**, STOP and report the roster unless the user's request also asked for work (setting up the team is not a request to build anything). - **Scratch space.** Use `.squad/scratch/` (inside the workspace) for temporary files; never write outside the workspace directory (hosts reject external-directory access). diff --git a/test/claude-hooks.test.ts b/test/claude-hooks.test.ts index becaeda96..65b43b2f9 100644 --- a/test/claude-hooks.test.ts +++ b/test/claude-hooks.test.ts @@ -12,12 +12,17 @@ import { } from '../packages/squad-sdk/src/client/adapters/index.js'; import { CLAUDE_SETTINGS_PATH, + claudePermissionRules, isSquadHookHandler, + mergeClaudePermissions, + SQUAD_STATE_PERMISSION_TOOLS, + stripClaudePermissions, mergeClaudeHooks, runClaudeHook, stripClaudeHooks, type ClaudeHookEnv, } from '../packages/squad-sdk/src/client/adapters/claude-hooks.js'; +import { squadRepoLauncherArgs } from '../packages/squad-sdk/src/client/adapters/repo-launcher.js'; import { LockoutError, approveRevision, @@ -35,6 +40,8 @@ import { } from '../packages/squad-sdk/src/client/adapters/lockout.js'; const MCP = { command: 'squad', args: ['state-mcp'] }; +/** The committed `.mcp.json` command when Squad's repository launcher is in place. */ +const LAUNCHER_MCP = { command: 'node', args: squadRepoLauncherArgs('.squad', 'state-mcp') }; const ROSTER = ['dallas', 'lambert', 'ripley']; const AT = '2026-11-09T12:00:00.000Z'; @@ -112,7 +119,13 @@ describe('Claude hooks in .claude/settings.json', () => { }); afterEach(() => fs.rmSync(root, { recursive: true, force: true })); - it('is off by default: no settings file is written', () => { + it('hooks are off by default: the settings file holds only the permission allowlist', () => { + installClients(['claude'], { root, mcp: MCP }); + expect(JSON.parse(fs.readFileSync(settingsAbs(), 'utf8'))).toEqual({ permissions: { allow: claudePermissionRules(null, false) } }); + }); + + it('with the allowlist and hooks off, no settings file is written', () => { + config({ claudePermissions: 'off' }); installClients(['claude'], { root, mcp: MCP }); expect(fs.existsSync(settingsAbs())).toBe(false); }); @@ -150,7 +163,7 @@ describe('Claude hooks in .claude/settings.json', () => { config({ claudeHooks: 'advisory' }); installClients(['claude'], { root, mcp: MCP }); const merged = JSON.parse(fs.readFileSync(settingsAbs(), 'utf8')); - expect(merged.permissions).toEqual({ allow: ['Bash(npm test)'] }); + expect(merged.permissions).toEqual({ allow: ['Bash(npm test)', ...claudePermissionRules(null, false)] }); expect(merged.hooks.PreToolUse[0]).toEqual({ matcher: 'Bash', hooks: [{ type: 'command', command: './.claude/hooks/no-rm.sh' }] }); expect(merged.hooks.PreToolUse).toHaveLength(2); expect(merged.hooks.Stop).toHaveLength(2); @@ -162,9 +175,9 @@ describe('Claude hooks in .claude/settings.json', () => { const user = '{\n "model": "opus"\n}\n'; fs.mkdirSync(path.dirname(settingsAbs()), { recursive: true }); fs.writeFileSync(settingsAbs(), user); - config({ claudeHooks: 'advisory' }); + config({ claudeHooks: 'advisory', claudePermissions: 'off' }); installClients(['claude'], { root, mcp: MCP }); - config({ claudeHooks: 'off' }); + config({ claudeHooks: 'off', claudePermissions: 'off' }); installClients(['claude'], { root, mcp: MCP }); expect(fs.readFileSync(settingsAbs(), 'utf8')).toBe(user); }); @@ -191,6 +204,120 @@ describe('Claude hooks in .claude/settings.json', () => { }); }); +describe('Claude permission allowlist in .claude/settings.json', () => { + let root: string; + const settingsAbs = () => path.join(root, CLAUDE_SETTINGS_PATH); + const config = (cfg: Record) => fs.writeFileSync(path.join(root, '.squad/config.json'), `${JSON.stringify(cfg, null, 2)}\n`); + const allow = () => JSON.parse(fs.readFileSync(settingsAbs(), 'utf8')).permissions?.allow; + + beforeEach(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'squad-claude-perms-')); + fs.mkdirSync(path.join(root, '.squad'), { recursive: true }); + }); + afterEach(() => fs.rmSync(root, { recursive: true, force: true })); + + it("allows only Squad's own tools: named squad_state tools and the coordinator's squad subcommands", () => { + const rules = claudePermissionRules('.squad', false); + expect(rules).toEqual([ + ...SQUAD_STATE_PERMISSION_TOOLS.map((t) => `mcp__squad_state__${t}`), + 'Bash(squad cast:*)', + 'Bash(squad client sync:*)', + 'Bash(squad lockout reject:*)', + 'Bash(squad lockout assign:*)', + 'Bash(squad lockout submit:*)', + 'Bash(squad lockout approve:*)', + 'Bash(squad lockout status:*)', + 'Bash(node .squad/squad.mjs cast:*)', + 'Bash(node .squad/squad.mjs client sync:*)', + 'Bash(node .squad/squad.mjs lockout reject:*)', + 'Bash(node .squad/squad.mjs lockout assign:*)', + 'Bash(node .squad/squad.mjs lockout submit:*)', + 'Bash(node .squad/squad.mjs lockout approve:*)', + 'Bash(node .squad/squad.mjs lockout status:*)', + ]); + // Never the whole server (backend approval tools), never a lockout override. + expect(rules).not.toContain('mcp__squad_state'); + expect(rules.some((r) => r.includes('backend') || r.includes('override') || r === 'Bash(squad lockout:*)')).toBe(false); + expect(claudePermissionRules('.squad', true)[0]).toBe('mcp__plugin_squad_squad_state__squad_decide'); + expect(claudePermissionRules('.ai-team', false)).toContain('Bash(node .ai-team/squad.mjs cast:*)'); + expect(claudePermissionRules('team\\.squad', false)).toContain('Bash(node team/.squad/squad.mjs cast:*)'); + }); + + it('writes no launcher rule for a squad dir outside the repository or with dot segments', () => { + for (const dir of ['/abs/.squad', '../main/.squad', 'a/../.squad', './.squad', '..\\main\\.squad', 'C:/x/.squad', '', 'a//b', 'has space/.squad']) { + expect(claudePermissionRules(dir, false).some((r) => r.startsWith('Bash(node')), dir).toBe(false); + } + expect(claudePermissionRules(null, false).some((r) => r.startsWith('Bash(node'))).toBe(false); + }); + + it('pre-approves the launcher only when the committed MCP command is Squad\'s repository launcher', () => { + installClients(['claude'], { root, mcp: MCP }); + expect(allow()).toEqual(claudePermissionRules(null, false)); + installClients(['claude'], { root, mcp: LAUNCHER_MCP }); + expect(allow()).toEqual(claudePermissionRules('.squad', false)); + // A shared team dir outside the repository (a worktree's `../main/.squad`) gets no launcher rule. + installClients(['claude'], { root, mcp: { command: 'node', args: squadRepoLauncherArgs('../main/.squad', 'state-mcp') } }); + expect(allow().some((r: string) => r.startsWith('Bash(node'))).toBe(false); + }); + + it("never removes a user's rule, even one that matches Squad's", () => { + const user = `${JSON.stringify({ permissions: { allow: ['Bash(squad cast:*)', 'mcp__squad_state__squad_state_read', 'Bash(node tools/squad.mjs cast:*)'] } }, null, 2)}\n`; + fs.mkdirSync(path.dirname(settingsAbs()), { recursive: true }); + fs.writeFileSync(settingsAbs(), user); + installClients(['claude'], { root, mcp: LAUNCHER_MCP }); + const rules = claudePermissionRules('.squad', false); + expect(allow()).toEqual([ + 'Bash(squad cast:*)', 'mcp__squad_state__squad_state_read', 'Bash(node tools/squad.mjs cast:*)', + ...rules.filter((r) => r !== 'Bash(squad cast:*)' && r !== 'mcp__squad_state__squad_state_read'), + ]); + uninstallClients(['claude'], { root, mcp: LAUNCHER_MCP }); + expect(fs.readFileSync(settingsAbs(), 'utf8')).toBe(user); + // Turned off on the first install: nothing is Squad's, so nothing is removed. + config({ claudePermissions: 'off' }); + installClients(['claude'], { root, mcp: LAUNCHER_MCP }); + expect(fs.readFileSync(settingsAbs(), 'utf8')).toBe(user); + const unowned = { permissions: { allow: ['Bash(squad cast:*)'] } }; + expect(stripClaudePermissions(unowned, [])).toBe(unowned); + }); + + it('appends after user rules, is idempotent, and rollback restores the user bytes', () => { + const user = `${JSON.stringify({ permissions: { allow: ['Bash(npm test:*)'], deny: ['Bash(rm:*)'] }, model: 'opus' }, null, 2)}\n`; + fs.mkdirSync(path.dirname(settingsAbs()), { recursive: true }); + fs.writeFileSync(settingsAbs(), user); + installClients(['claude'], { root, mcp: LAUNCHER_MCP }); + expect(allow()).toEqual(['Bash(npm test:*)', ...claudePermissionRules('.squad', false)]); + expect(JSON.parse(fs.readFileSync(settingsAbs(), 'utf8')).permissions.deny).toEqual(['Bash(rm:*)']); + expect(installClients(['claude'], { root, mcp: LAUNCHER_MCP }).find((r) => r.path === CLAUDE_SETTINGS_PATH)?.action).toBe('unchanged'); + uninstallClients(['claude'], { root, mcp: LAUNCHER_MCP }); + expect(fs.readFileSync(settingsAbs(), 'utf8')).toBe(user); + expect(fs.existsSync(path.join(root, '.claude/.squad-permissions.json'))).toBe(false); + }); + + it('"claudePermissions": "off" removes only Squad rules on the next upgrade', () => { + installClients(['claude'], { root, mcp: LAUNCHER_MCP }); + expect(allow()).toEqual(claudePermissionRules('.squad', false)); + config({ claudePermissions: 'off' }); + installClients(['claude'], { root, mcp: LAUNCHER_MCP }); + expect(fs.existsSync(settingsAbs())).toBe(false); + expect(fs.existsSync(path.join(root, '.claude/.squad-permissions.json'))).toBe(false); + }); + + it('rollback deletes a settings file that held only Squad rules, and prunes .claude', () => { + installClients(['claude'], { root, mcp: MCP }); + uninstallClients(['claude'], { root, mcp: MCP }); + expect(fs.existsSync(path.join(root, '.claude'))).toBe(false); + }); + + it('refuses to clobber malformed permissions', () => { + const rules = claudePermissionRules('.squad', false); + expect(() => mergeClaudePermissions({ permissions: [] as unknown as Record }, rules)).toThrow(/Refusing/); + expect(() => mergeClaudePermissions({ permissions: { allow: 'x' } }, rules)).toThrow(/Refusing/); + // Nothing to add: a malformed value is left alone rather than refused. + const odd = { permissions: { allow: 'x' } }; + expect(mergeClaudePermissions(odd, null, rules).settings).toBe(odd); + }); +}); + describe('Claude hook runtime (advisory)', () => { let root: string; let env: ClaudeHookEnv; diff --git a/test/claude-permissions-safety.test.ts b/test/claude-permissions-safety.test.ts new file mode 100644 index 000000000..8965f8f21 --- /dev/null +++ b/test/claude-permissions-safety.test.ts @@ -0,0 +1,64 @@ +/** + * What Squad's Claude Code allowlist pre-approves must be safe without a prompt (#2496 review): + * the allowlisted squad_state tools match what state-mcp serves (minus backend tools), and an + * allowlisted `squad cast --force` never writes through a symlink out of the team directory. + */ +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { execFileSync } from 'node:child_process'; +import { SQUAD_STATE_PERMISSION_TOOLS } from '../packages/squad-sdk/src/client/adapters/claude-hooks.js'; +import { MCP_TOOL_ALIASES } from '../packages/squad-cli/src/cli/commands/state-mcp.js'; +import { runCastAdd, symlinkUnder } from '../packages/squad-cli/src/cli/commands/cast.js'; + +describe('allowlisted squad_state tools', () => { + it("are exactly state-mcp's squad_* tools: never a backend tool, and memory.* stays behind a prompt", () => { + // The coordinator protocol never calls the memory.* tools, so they are not pre-approved. + const served = Object.keys(MCP_TOOL_ALIASES).filter((t) => !t.startsWith('memory.')); + expect([...SQUAD_STATE_PERMISSION_TOOLS].sort()).toEqual(served.sort()); + expect(SQUAD_STATE_PERMISSION_TOOLS.some((t) => t.startsWith('squad_backend'))).toBe(false); + }); +}); + +describe('squad cast refuses to write through a symlink', () => { + let base: string; + let root: string; + let outside: string; + + beforeEach(() => { + base = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'squad-cast-link-'))); + root = path.join(base, 'repo'); + outside = path.join(base, 'outside'); + fs.mkdirSync(path.join(root, '.squad', 'agents'), { recursive: true }); + fs.mkdirSync(path.join(root, '.squad', 'casting'), { recursive: true }); + fs.mkdirSync(outside); + execFileSync('git', ['init', '--quiet'], { cwd: root }); + fs.writeFileSync(path.join(root, '.squad', 'team.md'), '# Team\n\n## Members\n\n| Name | Role | Charter | Status |\n|---|---|---|---|\n'); + fs.writeFileSync(path.join(root, '.squad', 'routing.md'), '# Routing\n'); + }); + afterEach(() => fs.rmSync(base, { recursive: true, force: true })); + + it('a linked charter file is refused with --force, and its target is untouched', async () => { + const target = path.join(outside, 'victim.txt'); + fs.writeFileSync(target, 'keep\n'); + fs.mkdirSync(path.join(root, '.squad', 'agents', 'evil')); + fs.symlinkSync(target, path.join(root, '.squad', 'agents', 'evil', 'charter.md')); + await expect(runCastAdd(root, { name: 'Evil', role: 'Tester', force: true })).rejects.toThrow(/symbolic link/); + expect(fs.readFileSync(target, 'utf8')).toBe('keep\n'); + }); + + it('a linked agent directory or team.md is refused', async () => { + fs.symlinkSync(outside, path.join(root, '.squad', 'agents', 'evil'), process.platform === 'win32' ? 'junction' : 'dir'); + await expect(runCastAdd(root, { name: 'Evil', role: 'Tester' })).rejects.toThrow(/symbolic link/); + expect(fs.readdirSync(outside)).toEqual([]); + }); + + it('symlinkUnder allows a linked root but finds a link below it', () => { + const link = path.join(base, 'team-link'); + fs.symlinkSync(path.join(root, '.squad'), link, process.platform === 'win32' ? 'junction' : 'dir'); + expect(symlinkUnder(link, path.join(link, 'agents', 'new', 'charter.md'))).toBeNull(); + fs.symlinkSync(outside, path.join(root, '.squad', 'agents', 'x'), process.platform === 'win32' ? 'junction' : 'dir'); + expect(symlinkUnder(link, path.join(link, 'agents', 'x', 'charter.md'))).toBe(path.join(link, 'agents', 'x')); + }); +}); diff --git a/test/claude-projection-v3-plugin.test.ts b/test/claude-projection-v3-plugin.test.ts index a44d5c9a5..80982109e 100644 --- a/test/claude-projection-v3-plugin.test.ts +++ b/test/claude-projection-v3-plugin.test.ts @@ -14,6 +14,7 @@ import path from 'node:path'; import { CLAUDE_PLUGIN_AGENT, CLAUDE_PLUGIN_MCP_PREFIX, + claudePermissionRules, installClients, uninstallClients, mergeClaudePluginSettings, @@ -77,7 +78,12 @@ describe('Claude projection v3: plugin mode install', () => { expect(exists('.claude/agents/squad.md')).toBe(false); expect(exists('.claude/skills')).toBe(false); expect(exists('.mcp.json')).toBe(false); - expect(readJson('.claude/settings.json')).toEqual({ agent: CLAUDE_PLUGIN_AGENT, disabledMcpjsonServers: ['squad_state'] }); + expect(readJson('.claude/settings.json')).toEqual({ + agent: CLAUDE_PLUGIN_AGENT, + disabledMcpjsonServers: ['squad_state'], + permissions: { allow: claudePermissionRules(null, true) }, + }); + expect(readJson('.claude/settings.json').permissions.allow[0]).toBe(`${CLAUDE_PLUGIN_MCP_PREFIX}squad_decide`); const dallas = readText('.claude/agents/dallas.md'); expect(dallas).toContain('Claude projection v3.'); @@ -118,7 +124,7 @@ describe('Claude projection v3: plugin mode install', () => { expect(readJson('.claude/settings.json')).toEqual({ model: 'opus', disabledMcpjsonServers: ['github', 'squad_state'], - permissions: { allow: ['Bash(npm test:*)'] }, + permissions: { allow: ['Bash(npm test:*)', ...claudePermissionRules(null, true)] }, agent: CLAUDE_PLUGIN_AGENT, }); expect(readText('.mcp.json')).toBe(Buffer.from(before['.mcp.json']!, 'base64').toString()); @@ -142,15 +148,20 @@ describe('Claude projection v3: plugin mode install', () => { it('keeps user-disabled MCP and restores a replaced project coordinator across mode switches', () => { const original = { agent: 'squad', disabledMcpjsonServers: ['squad_state', 'github'] }; write('.claude/settings.json', JSON.stringify(original)); + const projected = { ...original, permissions: { allow: claudePermissionRules(null, false) } }; installClients(['claude'], { root, mcp: MCP }); - expect(readJson('.claude/settings.json')).toEqual(original); + expect(readJson('.claude/settings.json')).toEqual(projected); setConfig({ claudePlugin: true }); installClients(['claude'], { root, mcp: MCP }); installClients(['claude'], { root, mcp: MCP }); expect(readJson('.claude/settings.json').agent).toBe(CLAUDE_PLUGIN_AGENT); + expect([...readJson('.claude/settings.json').permissions.allow].sort()).toEqual(claudePermissionRules(null, true).sort()); setConfig({ claudePlugin: false }); installClients(['claude'], { root, mcp: MCP }); - expect(readJson('.claude/settings.json')).toEqual(original); + // Switching back swaps the plugin's tool rules for the project server's, appended after the rest. + const back = readJson('.claude/settings.json'); + expect({ ...back, permissions: { allow: [...back.permissions.allow].sort() } }) + .toEqual({ ...projected, permissions: { allow: [...projected.permissions.allow].sort() } }); }); it('removes former Claude MCP ownership while still in plugin mode, but honors kept clients', () => { diff --git a/test/claude-v2-review-fixes.test.ts b/test/claude-v2-review-fixes.test.ts index e5e2fb64c..e81e41166 100644 --- a/test/claude-v2-review-fixes.test.ts +++ b/test/claude-v2-review-fixes.test.ts @@ -153,9 +153,10 @@ describe('lockout store', () => { }); }); -describe('settings.json with hooks off', () => { +describe('settings.json with hooks and the permission allowlist off', () => { for (const original of ['{}\n', '{"model":"opus"}', '{\n "model": "opus"\n}\n']) { it(`keeps ${JSON.stringify(original)} byte for byte`, () => { + write('.squad/config.json', '{ "claudePermissions": "off" }\n'); write(CLAUDE_SETTINGS_PATH, original); installClients(['claude'], { root, mcp: MCP }); expect(fs.readFileSync(path.join(root, CLAUDE_SETTINGS_PATH), 'utf8')).toBe(original); diff --git a/test/clients.test.ts b/test/clients.test.ts index f75257627..ee5e1407c 100644 --- a/test/clients.test.ts +++ b/test/clients.test.ts @@ -263,7 +263,7 @@ describe('coordinator behavioral-contract parity', () => { ['Tool naming + dirs', [/Tool names are host-prefixed/, /squad_state_squad_state_read/, /EISDIR/, /for a directory/]], ['Explicit fallback', [/Unknown or missing member/, /never silently swap owners/, /Built-in members/]], ['Ralph', [/Ralph mode/, /squad:/, /board clear/]], - ['Init Mode guards', [/NEVER with glob\/grep\/search tools/, /Auto-confirm \(default\)/, /npx squad cast --name/, /NEVER hand-write or edit anything under/, /checksummed pair/, /\.squad\/scratch\//, /never write outside the workspace/, /STOP and report the roster/]], + ['Init Mode guards', [/NEVER with glob\/grep\/search tools/, /Auto-confirm \(default\)/, /`squad cast --name/, /NEVER hand-write or edit anything under/, /checksummed pair/, /\.squad\/scratch\//, /never write outside the workspace/, /STOP and report the roster/]], ]; for (const id of CLIENT_IDS) { it(`${id} prompt carries the full coordinator contract`, () => { diff --git a/test/squad-20-all-scenarios.e2e.test.ts b/test/squad-20-all-scenarios.e2e.test.ts index 267ca1da8..584f1b968 100644 --- a/test/squad-20-all-scenarios.e2e.test.ts +++ b/test/squad-20-all-scenarios.e2e.test.ts @@ -1272,7 +1272,7 @@ describe('field findings from the live demo: Init Mode, scratch files, casting r expect(text).toMatch(/headless/); // members are cast into the shared .squad/agents path and registered with the CLI, not projected per client expect(text).toMatch(/tell the user to upgrade the squad CLI instead of hand-writing the team/); - expect(text).toMatch(/`npx squad cast --name --role/); + expect(text).toMatch(/`squad cast --name --role/); }); it.each(CLIENT_IDS)('%s: keeps scratch files inside the workspace', (id) => { @@ -1286,7 +1286,7 @@ describe('field findings from the live demo: Init Mode, scratch files, casting r it.each(CLIENT_IDS)('%s: casts through the CLI and forbids hand-written casting files', (id) => { const text = loadedCoordinatorText(id); // live finding: LLM-handwritten casting/registry.json broke `squad upgrade` - expect(text).toMatch(/npx squad cast --name --role/); + expect(text).toMatch(/squad cast --name --role/); expect(text).toMatch(/NEVER hand-write or edit anything under .{0,20}casting\//); expect(text).toContain('registry-history.commit.json'); });