Skip to content

fix(claude): allow Squad's own tools; enforce the reviewer gate (demo dry-run blockers) - #2496

Open
bradygaster wants to merge 3 commits into
devfrom
squad/demo-blockers
Open

bradygaster wants to merge 3 commits into
devfrom
squad/demo-blockers

Conversation

@bradygaster

@bradygaster bradygaster commented Nov 13, 2026 •

Copy link
Copy Markdown
Collaborator

Found in a dry run of the Squad 2.0 Claude Code demo (cast a team → fix an issue → next-day recall → continue in Copilot CLI) on a scratch repo. Part of the remaining-work checklist #2469.

Problems

  1. Permission prompts on first run. With default permissions, a fresh squad init --client claude project got 13 denials while casting: every squad_state call and every squad cast / squad client sync. The coordinator hand-wrote the charters instead and ended by asking the user to run squad cast themselves.
  2. The reviewer gate was unreliable. 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 to review, the lead listed six known gaps against the issue's stated requirement ("GitHub's slug rules") and still approved. Told to apply a new rule, the reviewer wrote the fix himself and nobody reviewed it.

Fix

  • Scoped allowlist (claude-hooks.ts). squad init and squad upgrade add these to permissions.allow in .claude/settings.json:

    • the nine squad_state tools the coordinator uses, by name (mcp__squad_state__squad_decide, …__squad_state_read, …; mcp__plugin_squad_squad_state__… in plugin mode). Not the whole server: the experimental backend's tools (such as squad_backend_respond_approval) and memory.* still prompt.
    • Bash(squad cast:*), Bash(squad client sync:*), and squad lockout reject|assign|submit|approve|status. Never lockout override.
    • the same commands as node <squad dir>/squad.mjs …, only when .mcp.json runs Squad's own repository launcher from a plain relative folder (no absolute path, no . or .. part).

    Nothing else. Squad records the rules it adds in .claude/.squad-permissions.json. Rollback and "claudePermissions": "off" remove only those, so a matching rule the user already had stays. User rules stay first and are never changed. Malformed permissions values are refused, not overwritten.

  • squad cast refuses to write through a symbolic link below the team root, so a pre-approved cast --force can't overwrite a file outside the team folder.

  • Init Mode casts with squad cast (or node .squad/squad.mjs cast), not npx squad cast, matching the allowlist.

  • Reviewer gate (shared digest items 1, 10 and 12; all non-Copilot hosts). Copilot's snapshot is unchanged.

    • Every code or test change, and anything routing.md sends to an approval gate, goes to the reviewer unasked, before the work is called done.
    • The reviewer is never an author of the work.
    • The verdict is judged against the issue and the recorded decisions: a known gap against either is a REJECT, not an advisory.
    • Code a reviewer writes needs a different reviewer.
    • The routing line names a recorded decision when it shapes the work, so the memory is visible.

Policy note for reviewers

This turns on a default that writes project permission rules. Until now Squad didn't touch .claude/settings.json unless hooks or the plugin were enabled. The rules only cover Squad's own state server and three CLI subcommands, and there's an opt-out. Push back if you'd rather make it opt-in.

Tests

  • New Claude permission allowlist suite in test/claude-hooks.test.ts: exact rule set; user rules never matched; append order, idempotence, byte-exact rollback; opt-out; pruning .claude; malformed input.
  • The existing tests that asserted "no settings.json by default" now set claudePermissions: "off" or expect the rules. The plugin-mode tests also cover the switch from plugin to project and back.
  • Full suite: 13,527 passed. One later full run hit two load timeouts in the unrelated gh-aw tests; those 32 files pass on their own.
  • scripts/build-claude-plugin.mjs --check --version 2.0.0-preview.4 passes. Only plugins/squad/agents/squad.md changed.

Live verification: partial

  • With this build, a fresh project's casting run had no squad_state denials. The Bash rules were still ignored, and Claude Code says why: "Ignoring 7 permissions.allow entries from .claude/settings.json: this workspace has not been trusted." Project allow rules apply only after the interactive trust prompt. The docs now say so.
  • Not verified live yet: casting in a trusted folder with no prompts, and the new reviewer-gate wording. The reference machine ran out of Claude usage mid-run.

🤖 Generated with Claude Code

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 <noreply@anthropic.com>
Copilot AI dynamic review requested due to automatic review settings November 13, 2026 17:39
@github-actions

github-actions Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

🛫 PR Readiness Check

ℹ️ This comment updates on each push. Last checked: commit 8726f07

PR Scope: 📦🔧 Mixed (product + infrastructure)

⚠️ 3 item(s) to address before review

Status Check Details
❌ Single commit 3 commits — consider squashing before review
✅ Not in draft Ready for review
✅ Branch up to date Up to date with dev
❌ Copilot review No Copilot review yet — it may still be processing
✅ Changeset present Changeset file found
✅ Scope clean No .squad/ or docs/proposals/ files
✅ No merge conflicts No merge conflicts
✅ Copilot threads resolved 1 active Copilot thread(s) resolved (6 outdated skipped)
❌ CI passing 5 check(s) failing: samples-build, test, docs-quality, sdk-exports-validation, gh-aw strict compile

Files Changed (15 files, +440 −28)

File +/−
.changeset/claude-demo-blockers.md +6 −0
CHANGELOG.md +2 −0
docs/src/content/docs/scenarios/client-compatibility.md +10 −0
packages/squad-cli/src/cli/commands/cast.ts +45 −0
packages/squad-cli/src/cli/commands/state-mcp.ts +1 −1
packages/squad-sdk/src/client/adapters/claude-hooks.ts +153 −7
packages/squad-sdk/src/client/adapters/coordinator.ts +4 −4
plugins/squad/agents/squad.md +3 −3
plugins/squad/skills/squad-coordinator-init-mode/SKILL.md +1 −1
test/claude-hooks.test.ts +131 −4
test/claude-permissions-safety.test.ts +64 −0
test/claude-projection-v3-plugin.test.ts +15 −4
test/claude-v2-review-fixes.test.ts +2 −1
test/clients.test.ts +1 −1
test/squad-20-all-scenarios.e2e.test.ts +2 −2

Total: +440 −28


This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.

@github-actions

github-actions Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

🟠 Impact Analysis — PR #2496

Risk tier: 🟠 HIGH

📊 Summary

Metric Count
Files changed 15
Files added 2
Files modified 13
Files deleted 0
Modules touched 5

🎯 Risk Factors

  • 15 files changed (6-20 → MEDIUM)
  • 5 modules touched (5-8 → HIGH)

📦 Modules Affected

docs (1 file)
  • docs/src/content/docs/scenarios/client-compatibility.md
root (4 files)
  • .changeset/claude-demo-blockers.md
  • CHANGELOG.md
  • plugins/squad/agents/squad.md
  • plugins/squad/skills/squad-coordinator-init-mode/SKILL.md
squad-cli (2 files)
  • packages/squad-cli/src/cli/commands/cast.ts
  • packages/squad-cli/src/cli/commands/state-mcp.ts
squad-sdk (2 files)
  • packages/squad-sdk/src/client/adapters/claude-hooks.ts
  • packages/squad-sdk/src/client/adapters/coordinator.ts
tests (6 files)
  • test/claude-hooks.test.ts
  • test/claude-permissions-safety.test.ts
  • test/claude-projection-v3-plugin.test.ts
  • test/claude-v2-review-fixes.test.ts
  • test/clients.test.ts
  • test/squad-20-all-scenarios.e2e.test.ts

This report is generated automatically for every PR. See #733 for details.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved permission flaws allow agents to bypass required human approval, execute unvalidated launchers, and overwrite files outside the workspace.

7 open issues
What changed in this PR

This PR reduces first-run permission prompts in Claude Code and strengthens reviewer requirements for non-Copilot hosts, addressing demo blockers tracked in #2469.

Changes:

  • Add default permission rules for Squad’s state server and selected CLI commands, with opt-out and cleanup support.
  • Require independent review against issue requirements and recorded team decisions.
  • Update plugin output, permission tests, documentation, and release notes.
File Description
test/​claude-v2-review-fixes.test.ts Preserves settings-byte checks with permissions disabled.
test/​claude-projection-v3-plugin.test.ts Covers permission rules across plugin/project mode switches.
test/​claude-hooks.test.ts Tests permission generation, merging, rollback, opt-out, and malformed input.
plugins/​squad/​agents/​squad.md Updates the generated reviewer and decision-routing protocol.
packages/​squad-sdk/​src/​client/​adapters/​coordinator.ts Strengthens shared review gates and decision-aware routing.
packages/​squad-sdk/​src/​client/​adapters/​claude-hooks.ts Generates, merges, and removes Squad permission rules.
docs/​src/​content/​docs/​scenarios/​client-compatibility.md Documents permission defaults, workspace trust, and opt-out.
CHANGELOG.md Records permission and reviewer-protocol changes.
.changeset/​claude-demo-blockers.md Adds SDK and CLI patch release notes.

Review effort: Max (dynamic)

… "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 <noreply@anthropic.com>
@bradygaster

bradygaster commented Nov 13, 2026 •

Copy link
Copy Markdown
Collaborator Author

Live verification (reviewer gate)

Scratch repo with a realistic bug (mdlinks #14: anchor links always reported broken). Fresh squad init --client claude, cast a team, then only "Team, fix issue #14 (ISSUE-14.md). Work on a branch." No mention of review anywhere.

Run 1, after the first commit. The lead reviewed unasked ("Waiting on Mercer's verdict before declaring it done"). But the verdict was "APPROVED with one required fix", and then the coordinator applied that fix itself, with no re-review. Cause: the reviewer is a subagent and never sees the coordinator's rules.

Run 2, after 6b3d223. The coordinator now puts the standard in the reviewer's prompt, a required change is a REJECT, and the coordinator never edits the work. Without being asked, the team ran the full protocol:

lockout reject issue-14-slug --author Vasco --reviewer Mercer --reason "githubSlug regex strips underscores…"
lockout assign issue-14-slug --successor Amelia
squad_decide: Mercer REJECTED issue #14 fix … Vasco locked out, Amelia assigned
spawn amelia  → fix
lockout submit issue-14-slug --author Amelia
spawn mercer  → re-review
lockout approve issue-14-slug --reviewer Mercer

.squad/governance/lockouts.json ends as state: cleared, lockedAuthors: [vasco], successor: amelia. 10/10 tests pass. 485 s, $2.71.

Not fixed by this PR: the approved slug still diverges from GitHub on edge cases (Café → caf, Foo Bar → foo-bar) that the issue text doesn't spell out. Recording a decision of "match GitHub exactly" covers that, as in the earlier dry run.

Live verification (allowlist)

  • Casting with this build: 0 denials of squad_state.
  • The Bash(squad …) rules apply only once the folder is trusted. A no-flags run in a trusted folder is still pending.

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 <noreply@anthropic.com>
*/
const SQUAD_BASH_SUBCOMMANDS = [
'cast', 'client sync',
'lockout reject', 'lockout assign', 'lockout submit', 'lockout approve', 'lockout status',

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants