From 3b398c848ea2d0271d51d5b7bdeadf9a6dc83df0 Mon Sep 17 00:00:00 2001 From: Brady Gaster Date: Fri, 13 Nov 2026 09:39:09 -0800 Subject: [PATCH 1/3] fix(claude): allow Squad's own tools; enforce the reviewer gate From the Squad 2.0 demo dry run: - squad init/upgrade --client claude add a scoped permissions.allow to .claude/settings.json: the squad_state MCP server and squad cast, squad client sync, squad lockout (on PATH and via .squad/squad.mjs). Opt out with "claudePermissions": "off". User rules are never touched; rollback strips only Squad's rules. - Non-Copilot coordinators run the reviewer gate on every code or test change unasked; the reviewer is never an author, judges against the issue and recorded decisions (a known gap is a REJECT), and code a reviewer writes needs another reviewer. The coordinator names the recorded decision that shapes the work. Co-Authored-By: Claude Opus 5.5 --- .changeset/claude-demo-blockers.md | 6 ++ CHANGELOG.md | 2 + .../docs/scenarios/client-compatibility.md | 9 ++ .../src/client/adapters/claude-hooks.ts | 91 +++++++++++++++++-- .../src/client/adapters/coordinator.ts | 6 +- plugins/squad/agents/squad.md | 6 +- test/claude-hooks.test.ts | 87 +++++++++++++++++- test/claude-projection-v3-plugin.test.ts | 16 +++- test/claude-v2-review-fixes.test.ts | 3 +- 9 files changed, 205 insertions(+), 21 deletions(-) create mode 100644 .changeset/claude-demo-blockers.md diff --git a/.changeset/claude-demo-blockers.md b/.changeset/claude-demo-blockers.md new file mode 100644 index 000000000..d6f6ce91a --- /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 `squad_state` server and `squad cast`, `squad client sync`, `squad lockout`), 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..514026a3e 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` MCP server (`mcp__squad_state`, or the plugin's server in plugin mode) and `squad cast`, `squad client sync` and `squad lockout`, as `squad …` and as `node .squad/squad.mjs …`. Nothing else is allowed. Your own rules come first and are never touched, rollback removes only Squad's rules, 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..9d513648c 100644 --- a/docs/src/content/docs/scenarios/client-compatibility.md +++ b/docs/src/content/docs/scenarios/client-compatibility.md @@ -223,6 +223,15 @@ 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: + +- `mcp__squad_state`: every tool of Squad's `squad_state` MCP server (in plugin mode, the plugin's `mcp__plugin_squad_squad_state` instead) +- `Bash(squad cast:*)`, `Bash(squad client sync:*)` and `Bash(squad lockout:*)`, plus the same three through the repository launcher, `node .squad/squad.mjs …` + +Nothing else is allowed: no edits, no other commands, no other MCP servers. Your own rules stay first and are never changed, and removing Claude Code from the team removes only Squad's rules. 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-sdk/src/client/adapters/claude-hooks.ts b/packages/squad-sdk/src/client/adapters/claude-hooks.ts index 33043469b..d7f91cd2e 100644 --- a/packages/squad-sdk/src/client/adapters/claude-hooks.ts +++ b/packages/squad-sdk/src/client/adapters/claude-hooks.ts @@ -197,18 +197,94 @@ 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` subcommands the coordinator runs with Bash: casting, subagent sync, reviewer lockouts. */ +const SQUAD_BASH_SUBCOMMANDS = ['cast', 'client sync', 'lockout'] 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'; +const SQUAD_PERMISSION_RULE = new RegExp( + `^(?:mcp__squad_state|${PLUGIN_SQUAD_STATE_SERVER}|Bash\\((?:squad|node (?:[\\w.-]+/)+squad\\.mjs) (?:${SQUAD_BASH_SUBCOMMANDS.join('|')}):\\*\\))$`, +); + +/** + * The `permissions.allow` rules Squad adds: every tool of its `squad_state` server (a server-wide + * `mcp__` rule), and the coordinator's three CLI subcommands, both as `squad …` on PATH and + * through the repository launcher. Nothing else: no Edit, no general Bash, no other MCP server. + */ +export function claudePermissionRules(squadDir: string, plugin: boolean): string[] { + const launcher = `node ${squadDir.replace(/\\/g, '/').replace(/\/+$/, '')}/squad.mjs`; + return [ + plugin ? PLUGIN_SQUAD_STATE_SERVER : 'mcp__squad_state', + ...SQUAD_BASH_SUBCOMMANDS.map((c) => `Bash(squad ${c}:*)`), + ...SQUAD_BASH_SUBCOMMANDS.map((c) => `Bash(${launcher} ${c}:*)`), + ].filter((r) => SQUAD_PERMISSION_RULE.test(r)); +} + +export function isSquadPermissionRule(rule: unknown): boolean { + return typeof rule === 'string' && SQUAD_PERMISSION_RULE.test(rule); +} + +/** Remove Squad's allow rules (and `allow`/`permissions` they leave empty). Untouched input is returned as is. */ +export function stripClaudePermissions(settings: Json): Json { + const perms = settings['permissions']; + if (!isObject(perms) || !Array.isArray(perms['allow'])) return settings; + const allow = perms['allow'] as unknown[]; + const kept = allow.filter((r) => !isSquadPermissionRule(r)); + if (kept.length === allow.length) return settings; + 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 out; +} + +/** Refresh Squad's allow rules: strip the old ones, then append `rules` (none: strip only). User rules keep their order. */ +export function mergeClaudePermissions(settings: Json, rules: readonly string[] | null): Json { + const stripped = stripClaudePermissions(settings); + if (!rules || rules.length === 0) return stripped; + const perms = stripped['permissions']; + if (perms !== undefined && !isObject(perms)) { + 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)) { + throw new Error('Refusing to overwrite "permissions.allow" in .claude/settings.json: expected an array'); + } + const out: Json = { ...stripped, permissions: { ...(perms ?? {}), allow: [...((allow as unknown[] | undefined) ?? []), ...rules] } }; + return JSON.stringify(out) === JSON.stringify(settings) ? settings : out; +} + /** - * 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; + const rules = readClaudePermissionsMode(squadAbs) === 'squad' ? claudePermissionRules(ctx.squadDir ?? '.squad', plugin) : null; let ownership = readClaudePluginSettingsOwnership(ctx.root); - if (!on && !plugin && !ownership && !fs.existsSync(path.join(ctx.root, CLAUDE_SETTINGS_PATH))) return []; + if (!on && !rules && !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,11 +305,14 @@ export function projectClaudeHooks(ctx: ClientContext, opts: { plugin?: boolean projectMcp, } : ownership; - const next = mergeClaudePluginSettings(mergeClaudeHooks(existing, on ? ctx.mcp : null), plugin, nextOwnership); + const next = mergeClaudePermissions( + mergeClaudePluginSettings(mergeClaudeHooks(existing, on ? ctx.mcp : null), plugin, nextOwnership), + rules, + ); ownership = nextOwnership; return next; }, - remove: (existing) => mergeClaudePluginSettings(stripClaudeHooks(existing), false, ownership), + remove: (existing) => stripClaudePermissions(mergeClaudePluginSettings(stripClaudeHooks(existing), false, ownership)), }, ...(plugin || ownership ? [{ kind: 'json' as const, diff --git a/packages/squad-sdk/src/client/adapters/coordinator.ts b/packages/squad-sdk/src/client/adapters/coordinator.ts index 4721d0876..440ad1366 100644 --- a/packages/squad-sdk/src/client/adapters/coordinator.ts +++ b/packages/squad-sdk/src/client/adapters/coordinator.ts @@ -124,7 +124,7 @@ You are the Squad coordinator. You route work to a team of specialists and keep - **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. - **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. A reviewer returns APPROVE or REJECT with reasons, judged against the request's stated requirements (the issue text) and the team's recorded decisions: a known gap against either is a REJECT, never an advisory on an APPROVE; advisories are for things neither asks for. A reviewer who rejects does not fix the work; 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..23f9793ca 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. A reviewer returns APPROVE or REJECT with reasons, judged against the request's stated requirements (the issue text) and the team's recorded decisions: a known gap against either is a REJECT, never an advisory on an APPROVE; advisories are for things neither asks for. A reviewer who rejects does not fix the work; 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/test/claude-hooks.test.ts b/test/claude-hooks.test.ts index becaeda96..1b0cc92c8 100644 --- a/test/claude-hooks.test.ts +++ b/test/claude-hooks.test.ts @@ -12,7 +12,11 @@ import { } from '../packages/squad-sdk/src/client/adapters/index.js'; import { CLAUDE_SETTINGS_PATH, + claudePermissionRules, isSquadHookHandler, + isSquadPermissionRule, + mergeClaudePermissions, + stripClaudePermissions, mergeClaudeHooks, runClaudeHook, stripClaudeHooks, @@ -112,7 +116,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('.squad', 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 +160,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('.squad', 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 +172,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 +201,75 @@ 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: the squad_state server and three squad subcommands", () => { + expect(claudePermissionRules('.squad', false)).toEqual([ + 'mcp__squad_state', + 'Bash(squad cast:*)', + 'Bash(squad client sync:*)', + 'Bash(squad lockout:*)', + 'Bash(node .squad/squad.mjs cast:*)', + 'Bash(node .squad/squad.mjs client sync:*)', + 'Bash(node .squad/squad.mjs lockout:*)', + ]); + expect(claudePermissionRules('.squad', true)[0]).toBe('mcp__plugin_squad_squad_state'); + expect(claudePermissionRules('.ai-team', false)).toContain('Bash(node .ai-team/squad.mjs cast:*)'); + // A squad dir that would widen or escape the rule shape is left out rather than written. + expect(claudePermissionRules('/abs/.squad', false).some((r) => r.includes('/abs/'))).toBe(false); + }); + + it('never recognises a user rule as Squad-owned', () => { + for (const rule of ['Bash(squad:*)', 'Bash(squad init:*)', 'mcp__squad_state__squad_state_write', 'Bash(npm test:*)', 'mcp__github', 'Edit']) { + expect(isSquadPermissionRule(rule)).toBe(false); + } + const user = { permissions: { allow: ['Bash(squad:*)'], deny: ['Bash(rm:*)'] } }; + expect(stripClaudePermissions(user)).toBe(user); + }); + + 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: 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: MCP }).find((r) => r.path === CLAUDE_SETTINGS_PATH)?.action).toBe('unchanged'); + uninstallClients(['claude'], { root, mcp: MCP }); + expect(fs.readFileSync(settingsAbs(), 'utf8')).toBe(user); + }); + + it('"claudePermissions": "off" removes only Squad rules on the next upgrade', () => { + installClients(['claude'], { root, mcp: MCP }); + expect(allow()).toEqual(claudePermissionRules('.squad', false)); + config({ claudePermissions: 'off' }); + installClients(['claude'], { root, mcp: MCP }); + expect(fs.existsSync(settingsAbs())).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/); + }); +}); + describe('Claude hook runtime (advisory)', () => { let root: string; let env: ClaudeHookEnv; diff --git a/test/claude-projection-v3-plugin.test.ts b/test/claude-projection-v3-plugin.test.ts index a44d5c9a5..2567875ff 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('.squad', true) }, + }); + expect(readJson('.claude/settings.json').permissions.allow[0]).toBe(CLAUDE_PLUGIN_MCP_PREFIX.replace(/__$/, '')); 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('.squad', true)] }, agent: CLAUDE_PLUGIN_AGENT, }); expect(readText('.mcp.json')).toBe(Buffer.from(before['.mcp.json']!, 'base64').toString()); @@ -142,15 +148,17 @@ 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('.squad', 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).toEqual(claudePermissionRules('.squad', true)); setConfig({ claudePlugin: false }); installClients(['claude'], { root, mcp: MCP }); - expect(readJson('.claude/settings.json')).toEqual(original); + expect(readJson('.claude/settings.json')).toEqual(projected); }); 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); From 6b3d2238ddef2003e274a833df8b79f935fcd555 Mon Sep 17 00:00:00 2001 From: Brady Gaster Date: Fri, 13 Nov 2026 10:40:34 -0800 Subject: [PATCH 2/3] fix(coordinator): reviewer standard goes in the reviewer's prompt; no "approve with required changes" Live run: the lead reviewed unasked but returned "APPROVED with one required fix", and the coordinator then edited the code itself with no re-review. The reviewer subagent never sees the coordinator's rules, so the coordinator now puts the verdict standard in its spawn prompt; a required change is a REJECT; the coordinator never edits the work. Co-Authored-By: Claude Opus 5.5 --- packages/squad-sdk/src/client/adapters/coordinator.ts | 2 +- plugins/squad/agents/squad.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/squad-sdk/src/client/adapters/coordinator.ts b/packages/squad-sdk/src/client/adapters/coordinator.ts index 440ad1366..2f058ed82 100644 --- a/packages/squad-sdk/src/client/adapters/coordinator.ts +++ b/packages/squad-sdk/src/client/adapters/coordinator.ts @@ -138,7 +138,7 @@ You are the Squad coordinator. You route work to a team of specialists and keep 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. 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. A reviewer returns APPROVE or REJECT with reasons, judged against the request's stated requirements (the issue text) and the team's recorded decisions: a known gap against either is a REJECT, never an advisory on an APPROVE; advisories are for things neither asks for. A reviewer who rejects does not fix the work; 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\`. +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 23f9793ca..44dd4b0d1 100644 --- a/plugins/squad/agents/squad.md +++ b/plugins/squad/agents/squad.md @@ -27,7 +27,7 @@ You are the Squad coordinator. You route work to a team of specialists and keep 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. 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. A reviewer returns APPROVE or REJECT with reasons, judged against the request's stated requirements (the issue text) and the team's recorded decisions: a known gap against either is a REJECT, never an advisory on an APPROVE; advisories are for things neither asks for. A reviewer who rejects does not fix the work; 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`. +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. From 8726f072fedc336468462e91f26e2827406cafa8 Mon Sep 17 00:00:00 2001 From: Brady Gaster Date: Fri, 13 Nov 2026 11:47:28 -0800 Subject: [PATCH 3/3] fix(claude): narrow the permission allowlist; record what Squad adds Review fixes for the Claude Code allowlist: - squad_state tools are allowed by name, never the whole server, so the experimental backend's approval tools (and memory.*) still prompt. - lockout: only reject/assign/submit/approve/status; never override. - Launcher rules only when .mcp.json runs Squad's own repository launcher from a plain relative folder (no absolute path, no . or .. part). - Squad records the rules it adds in .claude/.squad-permissions.json and removes only those; a matching rule the user already had stays. Removes the rule-shape regex (CodeQL polynomial regex). - squad cast refuses to write through a symbolic link, so a pre-approved `cast --force` cannot overwrite a file outside the team folder. - Init Mode casts with `squad cast` / `node .squad/squad.mjs cast`, not `npx squad cast`, matching the allowlist. Co-Authored-By: Claude Opus 5.5 --- .changeset/claude-demo-blockers.md | 2 +- CHANGELOG.md | 2 +- .../docs/scenarios/client-compatibility.md | 7 +- packages/squad-cli/src/cli/commands/cast.ts | 45 ++++++ .../squad-cli/src/cli/commands/state-mcp.ts | 2 +- .../src/client/adapters/claude-hooks.ts | 149 +++++++++++++----- .../src/client/adapters/coordinator.ts | 2 +- .../squad-coordinator-init-mode/SKILL.md | 2 +- test/claude-hooks.test.ts | 90 ++++++++--- test/claude-permissions-safety.test.ts | 64 ++++++++ test/claude-projection-v3-plugin.test.ts | 15 +- test/clients.test.ts | 2 +- test/squad-20-all-scenarios.e2e.test.ts | 4 +- 13 files changed, 307 insertions(+), 79 deletions(-) create mode 100644 test/claude-permissions-safety.test.ts diff --git a/.changeset/claude-demo-blockers.md b/.changeset/claude-demo-blockers.md index d6f6ce91a..518eb9424 100644 --- a/.changeset/claude-demo-blockers.md +++ b/.changeset/claude-demo-blockers.md @@ -3,4 +3,4 @@ "@bradygaster/squad-cli": patch --- -Claude Code: `squad init`/`squad upgrade` add a scoped `.claude/settings.json` allowlist for Squad's own tools (the `squad_state` server and `squad cast`, `squad client sync`, `squad lockout`), 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. +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 514026a3e..ccd4ab52c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,7 +78,7 @@ 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` MCP server (`mcp__squad_state`, or the plugin's server in plugin mode) and `squad cast`, `squad client sync` and `squad lockout`, as `squad …` and as `node .squad/squad.mjs …`. Nothing else is allowed. Your own rules come first and are never touched, rollback removes only Squad's rules, 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). +- **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. diff --git a/docs/src/content/docs/scenarios/client-compatibility.md b/docs/src/content/docs/scenarios/client-compatibility.md index 9d513648c..5796f9692 100644 --- a/docs/src/content/docs/scenarios/client-compatibility.md +++ b/docs/src/content/docs/scenarios/client-compatibility.md @@ -227,10 +227,11 @@ Interactive Claude Code doesn't load project-scoped `.mcp.json` servers until yo `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: -- `mcp__squad_state`: every tool of Squad's `squad_state` MCP server (in plugin mode, the plugin's `mcp__plugin_squad_squad_state` instead) -- `Bash(squad cast:*)`, `Bash(squad client sync:*)` and `Bash(squad lockout:*)`, plus the same three through the repository launcher, `node .squad/squad.mjs …` +- 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. Your own rules stay first and are never changed, and removing Claude Code from the team removes only Squad's rules. 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`. +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 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 d7f91cd2e..4998e5802 100644 --- a/packages/squad-sdk/src/client/adapters/claude-hooks.ts +++ b/packages/squad-sdk/src/client/adapters/claude-hooks.ts @@ -213,62 +213,114 @@ export function readClaudePermissionsMode(squadAbs: string): ClaudePermissionsMo } } -/** The `squad` subcommands the coordinator runs with Bash: casting, subagent sync, reviewer lockouts. */ -const SQUAD_BASH_SUBCOMMANDS = ['cast', 'client sync', 'lockout'] as const; +/** + * 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'; -const SQUAD_PERMISSION_RULE = new RegExp( - `^(?:mcp__squad_state|${PLUGIN_SQUAD_STATE_SERVER}|Bash\\((?:squad|node (?:[\\w.-]+/)+squad\\.mjs) (?:${SQUAD_BASH_SUBCOMMANDS.join('|')}):\\*\\))$`, -); + +/** 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: every tool of its `squad_state` server (a server-wide - * `mcp__` rule), and the coordinator's three CLI subcommands, both as `squad …` on PATH and - * through the repository launcher. Nothing else: no Edit, no general Bash, no other MCP server. + * 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(squadDir: string, plugin: boolean): string[] { - const launcher = `node ${squadDir.replace(/\\/g, '/').replace(/\/+$/, '')}/squad.mjs`; +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 [ - plugin ? PLUGIN_SQUAD_STATE_SERVER : 'mcp__squad_state', + ...SQUAD_STATE_PERMISSION_TOOLS.map((t) => `${server}__${t}`), ...SQUAD_BASH_SUBCOMMANDS.map((c) => `Bash(squad ${c}:*)`), - ...SQUAD_BASH_SUBCOMMANDS.map((c) => `Bash(${launcher} ${c}:*)`), - ].filter((r) => SQUAD_PERMISSION_RULE.test(r)); + ...(launcher ? SQUAD_BASH_SUBCOMMANDS.map((c) => `Bash(${launcher} ${c}:*)`) : []), + ]; } -export function isSquadPermissionRule(rule: unknown): boolean { - return typeof rule === 'string' && SQUAD_PERMISSION_RULE.test(rule); +/** + * 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[]; } -/** Remove Squad's allow rules (and `allow`/`permissions` they leave empty). Untouched input is returned as is. */ -export function stripClaudePermissions(settings: Json): Json { - const perms = settings['permissions']; - if (!isObject(perms) || !Array.isArray(perms['allow'])) return settings; - const allow = perms['allow'] as unknown[]; - const kept = allow.filter((r) => !isSquadPermissionRule(r)); - if (kept.length === allow.length) return settings; - 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 out; +export interface ClaudePermissionsMerge { + settings: Json; + /** The rules now in `permissions.allow` because Squad added them. */ + owned: string[]; } -/** Refresh Squad's allow rules: strip the old ones, then append `rules` (none: strip only). User rules keep their order. */ -export function mergeClaudePermissions(settings: Json, rules: readonly string[] | null): Json { - const stripped = stripClaudePermissions(settings); - if (!rules || rules.length === 0) return stripped; - const perms = stripped['permissions']; +/** + * 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 out: Json = { ...stripped, permissions: { ...(perms ?? {}), allow: [...((allow as unknown[] | undefined) ?? []), ...rules] } }; - return JSON.stringify(out) === JSON.stringify(settings) ? settings : out; + 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; } /** @@ -282,9 +334,16 @@ export function projectClaudeHooks(ctx: ClientContext, opts: { plugin?: boolean 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; - const rules = readClaudePermissionsMode(squadAbs) === 'squad' ? claudePermissionRules(ctx.squadDir ?? '.squad', plugin) : 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 && !rules && !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:'); @@ -305,15 +364,23 @@ export function projectClaudeHooks(ctx: ClientContext, opts: { plugin?: boolean projectMcp, } : ownership; - const next = mergeClaudePermissions( + 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) => stripClaudePermissions(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 2f058ed82..f45ccd66e 100644 --- a/packages/squad-sdk/src/client/adapters/coordinator.ts +++ b/packages/squad-sdk/src/client/adapters/coordinator.ts @@ -121,7 +121,7 @@ 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. 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. 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 1b0cc92c8..65b43b2f9 100644 --- a/test/claude-hooks.test.ts +++ b/test/claude-hooks.test.ts @@ -14,14 +14,15 @@ import { CLAUDE_SETTINGS_PATH, claudePermissionRules, isSquadHookHandler, - isSquadPermissionRule, 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, @@ -39,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'; @@ -118,7 +121,7 @@ describe('Claude hooks in .claude/settings.json', () => { 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('.squad', false) } }); + 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', () => { @@ -160,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)', ...claudePermissionRules('.squad', false)] }); + 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); @@ -213,48 +216,90 @@ describe('Claude permission allowlist in .claude/settings.json', () => { }); afterEach(() => fs.rmSync(root, { recursive: true, force: true })); - it("allows only Squad's own tools: the squad_state server and three squad subcommands", () => { - expect(claudePermissionRules('.squad', false)).toEqual([ - 'mcp__squad_state', + 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:*)', + '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:*)', + '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:*)', ]); - expect(claudePermissionRules('.squad', true)[0]).toBe('mcp__plugin_squad_squad_state'); + // 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:*)'); - // A squad dir that would widen or escape the rule shape is left out rather than written. - expect(claudePermissionRules('/abs/.squad', false).some((r) => r.includes('/abs/'))).toBe(false); + expect(claudePermissionRules('team\\.squad', false)).toContain('Bash(node team/.squad/squad.mjs cast:*)'); }); - it('never recognises a user rule as Squad-owned', () => { - for (const rule of ['Bash(squad:*)', 'Bash(squad init:*)', 'mcp__squad_state__squad_state_write', 'Bash(npm test:*)', 'mcp__github', 'Edit']) { - expect(isSquadPermissionRule(rule)).toBe(false); + 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); } - const user = { permissions: { allow: ['Bash(squad:*)'], deny: ['Bash(rm:*)'] } }; - expect(stripClaudePermissions(user)).toBe(user); + 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: MCP }); + 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: MCP }).find((r) => r.path === CLAUDE_SETTINGS_PATH)?.action).toBe('unchanged'); - uninstallClients(['claude'], { root, mcp: MCP }); + 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: MCP }); + installClients(['claude'], { root, mcp: LAUNCHER_MCP }); expect(allow()).toEqual(claudePermissionRules('.squad', false)); config({ claudePermissions: 'off' }); - installClients(['claude'], { root, mcp: MCP }); + 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', () => { @@ -267,6 +312,9 @@ describe('Claude permission allowlist in .claude/settings.json', () => { 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); }); }); 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 2567875ff..80982109e 100644 --- a/test/claude-projection-v3-plugin.test.ts +++ b/test/claude-projection-v3-plugin.test.ts @@ -81,9 +81,9 @@ describe('Claude projection v3: plugin mode install', () => { expect(readJson('.claude/settings.json')).toEqual({ agent: CLAUDE_PLUGIN_AGENT, disabledMcpjsonServers: ['squad_state'], - permissions: { allow: claudePermissionRules('.squad', true) }, + permissions: { allow: claudePermissionRules(null, true) }, }); - expect(readJson('.claude/settings.json').permissions.allow[0]).toBe(CLAUDE_PLUGIN_MCP_PREFIX.replace(/__$/, '')); + 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.'); @@ -124,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:*)', ...claudePermissionRules('.squad', true)] }, + permissions: { allow: ['Bash(npm test:*)', ...claudePermissionRules(null, true)] }, agent: CLAUDE_PLUGIN_AGENT, }); expect(readText('.mcp.json')).toBe(Buffer.from(before['.mcp.json']!, 'base64').toString()); @@ -148,17 +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('.squad', false) } }; + const projected = { ...original, permissions: { allow: claudePermissionRules(null, false) } }; installClients(['claude'], { root, mcp: MCP }); 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).toEqual(claudePermissionRules('.squad', true)); + 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(projected); + // 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/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'); });