fix(prompt): coordinator must probe for squad_state/memory tools before mutating state (closes #1305) - #1306
Merged
tamirdresher merged 2 commits intoJun 13, 2026
Conversation
…re mutating state on non-local backends (closes bradygaster#1305) Symptom: a coordinator session against a Squad with stateBackend two-layer wrote .squad/decisions.md and history files via raw create/edit tools, never calling any squad_state or memory tool. The pre-commit hook caught the contract violation; the agent treated it as a 'git problem' instead of the symptom it was. Root cause: two failure modes stacked. Mechanical (Copilot CLI): MCP server tools are loaded lazily, not always advertised in the model's initial function list. Squad cannot fix this server-side; out of scope. Behavioral (squad.agent.md): pre-1305 prompt said 'when memory tools are available, use them' which models read as 'if listed' instead of 'after probing'. No hard refusal clause when the agent is about to violate the state-backend contract. Two changes to .squad-templates/squad.agent.md (synced to 4 mirror targets): 1. New 'State-backend handshake' section (MANDATORY, every session): - Skip for local/worktree backends - For orphan/two-layer/git-notes: probe via tool_search_tool_regex for squad_state_health; call it to confirm the bridge answers - On probe fail: HALT and tell the user verbatim how to fix (restart Copilot CLI or change stateBackend to local) 2. Replaced soft 'if not available' language with a HARD RULE in Memory Governance Tools section. Lists 10 forbidden paths (decisions.md, decisions/inbox, agents history, casting JSON, identity, memory, orchestration-log, log, rai+fact-checker audit-trail) that MUST NOT be written via create/edit/write_file on non-local backends when the bridge isn't reachable. Preserved the local/worktree carve-out. Also clarified that memory.* and squad_state_* share the same MCP server (same registry in packages/squad-cli/src/cli/commands/state- mcp.ts), so models stop treating them as separate availability checks. Tests: new test/state-backend-handshake.test.ts asserts 5 invariants against all 4 template mirror targets (20 tests total): handshake mandatory + every-session timing + squad_state_health probe + HALT remediation + HARD RULE with forbidden paths + local-backend carve- out. 20/20 pass. npm run lint clean. Out of scope: server-side MCP tool preload (Copilot CLI feature request), skill reinforcement in init-mode/agent-conduct (small follow-up). Closes bradygaster#1305 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
🛫 PR Readiness Check
PR Scope: 🔧 Infrastructure
|
| Status | Check | Details |
|---|---|---|
| ❌ | Single commit | 2 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 | 0 active Copilot thread(s) resolved (1 outdated skipped) |
| ❌ | CI passing | 8 check(s) still running |
Files Changed (6 files, +290 −8)
| File | +/− |
|---|---|
.changeset/fix-1305-state-backend-handshake.md |
+67 −0 |
.squad-templates/squad.agent.md |
+33 −2 |
packages/squad-cli/templates/squad.agent.md.template |
+33 −2 |
packages/squad-sdk/templates/squad.agent.md.template |
+33 −2 |
templates/squad.agent.md.template |
+33 −2 |
test/state-backend-handshake.test.ts |
+91 −0 |
Total: +290 −8
This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.
Contributor
🟠 Impact Analysis — PR #1306Risk tier: 🟠 HIGH 📊 Summary
🎯 Risk Factors
📦 Modules Affectedroot (2 files)
squad-cli (1 file)
squad-sdk (1 file)
templates (1 file)
tests (1 file)
This report is generated automatically for every PR. See #733 for details. |
tamirdresher
pushed a commit
to tamirdresher/squad
that referenced
this pull request
Jun 13, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the canonical squad.agent.md template (and all mirror targets) to enforce a mandatory per-session “state-backend handshake” that probes for squad_state_* / memory.* tools before any state mutation on non-local backends, and to hard-refuse raw .squad/ writes when the state bridge isn’t reachable—addressing the contract-violation scenario in #1305.
Changes:
- Added a mandatory “State-backend handshake” section that requires probing for
squad_state_healthvia tool discovery and halting with a precise remediation message when unavailable. - Strengthened “Memory Governance Tools” guidance with a hard rule forbidding raw writes to runtime-owned
.squad/paths on non-local backends when the bridge is not confirmed reachable. - Added a regression test ensuring the handshake + hard-rule language is present across all mirrored template targets, plus a changeset.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
test/state-backend-handshake.test.ts |
Adds regression coverage to enforce presence of handshake/probe/halt/hard-rule language across all template mirrors. |
templates/squad.agent.md.template |
Adds the handshake section and hard-rule backend contract enforcement text in the distributed template. |
packages/squad-cli/templates/squad.agent.md.template |
Mirrors the handshake and hard-rule changes for the CLI template target. |
packages/squad-sdk/templates/squad.agent.md.template |
Mirrors the handshake and hard-rule changes for the SDK template target. |
.squad-templates/squad.agent.md |
Updates the canonical source template that all other targets must mirror. |
.changeset/fix-1305-state-backend-handshake.md |
Publishes the release notes/versioning intent for the CLI/SDK minor bump tied to #1305. |
| expect(content).toMatch(/\.squad\/agents\/\*\/history\.md/); | ||
| // And must call out the create/edit/write_file tools by name so the | ||
| // model maps the rule to its actual function inventory. | ||
| expect(content).toMatch(/create.*edit.*write_file|create\s*\/\s*edit/i); |
Reviewer follow-up on bradygaster#1306: the regex '/create.*edit.*write_file|create\s*\/\s*edit/i' has '|' operator precedence ambiguity — the alternation binds at the top, so the shorter branch 'create\s*/\s*edit' could match the rule paragraph even if 'write_file' was missing entirely. The test was meant to ensure all three tool names appear in the HARD RULE list; this fix replaces it with three separate \b-anchored assertions, one per tool, so dropping any one fails the test. 20/20 handshake tests still pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
|
Reviewer thread addressed in commit 352bb3b:
20/20 handshake tests still pass. |
tamirdresher
added a commit
to tamirdresher/squad
that referenced
this pull request
Jun 13, 2026
tamirdresher
added a commit
to tamirdresher/squad
that referenced
this pull request
Jun 13, 2026
bradygaster
approved these changes
Jun 13, 2026
bradygaster
left a comment
bradygaster
left a comment
Owner
There was a problem hiding this comment.
✅ Flight approves. State-backend handshake rule is explicit, the non-local write ban is correctly scoped, template mirrors are synced, and checks are green.
tamirdresher
added a commit
that referenced
this pull request
Jun 13, 2026
…ough the cross-squad skill (#1307) * fix(coordinator): route "spawn a squad" / "another squad" prompts through the cross-squad skill A coordinator initialised by `squad init` saw prompts like "spawn two squads of designers and devs" and fanned out raw `task` agents inside its own context, treating "squad" as generic English for "team / group". It never invoked the bundled `cross-squad` or `cross-squad-communication` skills, so the peer-squad delegation protocol (registry / manifest / sync CLI / git-async / GH-issue patterns) was bypassed entirely. Two structural holes in squad.agent.md allowed this: 1. The Routing table had no row mapping "spawn a squad" phrasing to the Squad-PRODUCT concept (only "upgrade squad" / "squad commands" rows covered Squad-as-a-product vocabulary). 2. The Skill-aware-routing block was process discipline ("check skill directories by domain relevance") with no hard "if the user's word matches a skill name, MUST load the skill" trigger. This fix: - Adds a new routing-table row for the squad-spawning vocabulary ("spawn a squad", "another squad", "two squads", "second squad", "fan out to squads", "delegate to a squad"). Action: invoke the skill tool on cross-squad AND cross-squad-communication BEFORE any task spawn, then delegate via Pattern 0/1/2/3. - Adds a "Hard trigger — keyword-to-skill match" paragraph at the top of the Skill-aware-routing block. If any word in the user's request matches an installed skill name (squad → cross-squad, reflect → reflect, ceremony → matching ceremony skill, fact-check → fact-checking, release → release-process), the coordinator MUST invoke the skill tool to fully load that skill before designing its approach. Includes a "failure mode this rule closes" pointer so the guard survives future paraphrasing. - Strengthens cross-squad/SKILL.md with a Read-this-FIRST callout above the existing Context paragraph, so even a coordinator that skips the routing-table row still hits the trigger when it does eventually load the skill. - Adds a regression test (template-sync.test.ts) that asserts the row + the hard-trigger paragraph + the worked example are present in every mirrored copy of squad.agent.md (5 locations). All 4 mirrors re-synced via `scripts/sync-templates.mjs --sync`. Verified: 223/223 template-sync tests pass. Composability: disjoint from #1292/#1293/#1295/#1298/#1300/#1301/#1302/ #1303/#1304/#1306 — only touches squad.agent.md mirrors + cross-squad/ SKILL.md + the template-sync test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(coordinator): strengthen squad-spawn disambiguation — ask_user on ambiguity + anti-patterns Real-world failure (2026-06-13): even AFTER the routing-table row and hard-trigger paragraph from b5d05fb landed, a peer-squad coordinator *still* did ad-hoc `task` fan-out for "spawn two squads of engineers and QAs". Self-diagnosis surfaced four contributing failure modes: 1. Prior-session anchoring (saw earlier `reviews/squad-alpha/` folders and matched the pattern without re-evaluating user intent). 2. Ambiguous wording, lazy interpretation (silently picked the cheaper option instead of asking). 3. Coordinator doctrine biases toward `task` fan-out (the existing Eager Execution / Parallel Fan-Out section pulled the coordinator back even after it had loaded `cross-squad`). 4. Cost/overhead instinct ("two real squads for a 30-line app feels disproportionate" — judged silently instead of surfacing the trade-off). The original PR #1307 fix closed modes 1 and 3 mechanically (forces the skill to load) but left modes 2 and 4 open (didn't dictate what to DO with that knowledge). This commit closes them: A. squad.agent.md routing row — added two explicit clauses: - "**Default = literal Squad install.** Calling `task` sub-agents 'squad-alpha' / 'squad-beta' does NOT make them squads — that is the explicit anti-pattern." - "**If the request is ambiguous** ... you MUST `ask_user` with a 2-choice prompt — and never silently pick the cheaper option." No escape hatch. The coordinator can no longer rationalise the downgrade as a judgment call. B. cross-squad/SKILL.md — added a full `## Disambiguation: 'squad' vs ad-hoc agents` section with: - Default-behaviour table mapping common phrasings to expected coordinator actions (real squads vs ad-hoc agents vs ambiguous). - ask_user 2-choice protocol verbatim (heavier/persistent vs lighter/ephemeral) so the coordinator has the exact prompt shape. - Four named anti-patterns drawn directly from the observed failure: * Naming task agents "squad-alpha" doesn't make them squads * Prior-session anchoring (pattern is a hint, not a contract) * Silent cheaper-option pick (judgment call belongs to the user) * Loading the skill but doing task fan-out anyway (disambiguation rule OVERRIDES generic fan-out doctrine when "squad" was the trigger) - Sharpened `description:` so the squad skill-aware-router has better natural-language hooks. - `triggers:` frontmatter array (Copilot CLI ignores `triggers:` per sdk/index.js decompile, but the squad coordinator's skill-aware routing system uses natural-language matching against frontmatter + content, so documenting the phrases here helps that matcher fire). C. Regression tests in test/template-sync.test.ts: - Routing row must mention `ask_user` + "anti-pattern" (new × 5 mirrors = 5 assertions). - cross-squad/SKILL.md must have `## Disambiguation` section, default-behaviour rule, ask_user requirement, the squad-alpha anti-pattern, and triggers: frontmatter (5 new × 3 mirrors = 15 assertions). - 20 new assertions total; 243/243 template-sync tests pass. cross-squad/SKILL.md mirrored to packages/squad-cli/templates/skills/ and packages/squad-sdk/templates/skills/ (byte-identical). squad.agent.md re-synced to all 4 mirrors via scripts/sync-templates.mjs. Composability: still disjoint from all other open PRs. Pure additions to two files (squad.agent.md row, cross-squad/SKILL.md content) plus mirrors + tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Tamir Dresher <tamirdresher@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced Jun 13, 2026
feat(prompt): slim squad.agent.md by extracting 3 sections to satellite skills (#1308 phase 1)
#1311
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
"@bradygaster/squad-cli": minor
"@bradygaster/squad-sdk": minor
Fix #1305: coordinator must probe for
squad_state/memory.*tools before writing state, and hard-refuse writes when the bridge isn't reachable on non-local backendsSymptom
A coordinator session against a Squad with
stateBackend: "two-layer"wrote.squad/decisions.md+ four.squad/agents/{name}/history.mdfiles + a.squad/decisions/inbox/copilot-directive-…mdvia rawcreate/edittools — never calling anysquad_state_*ormemory.*tool. The pre-commit hook caught the contract violation; the agent treated it as a "git problem" instead of the symptom it was.Root cause (two failure modes stacked)
Mechanical (Copilot CLI): Copilot CLI loads MCP server tools lazily — they're registered (via
.mcp.json) but not always advertised in the model's initial function list. Models have to usetool_search_tool_regexto find them or guess they exist. Thesquad_stateMCP server is correctly registered withtools: ["*"]and responds toinitialize/tools/listover stdio — but the model's tool block at session start may not include them. This is a Copilot CLI architectural choice (lazy discovery vs. preload), not something Squad can fix server-side.Behavioral (squad.agent.md): The pre-1305 prompt said "When memory tools are available, use them before writing durable memory by hand" and "If memory tools are not available, fall back to squad_decide or squad_state_write". Models read
"available"as"listed in my tool prompt"instead of"after probing to find out". There was no hard refusal clause for the case where the agent is about to violate the state-backend contract.Fix
Two changes to
.squad-templates/squad.agent.md(synced to all 4 mirror targets):1. New "State-backend handshake" section (MANDATORY, runs once per session)
Inserted right after the
stateBackendresolution at L131. Steps:STATE_BACKEND ∈ {"local", "worktree"}: file ops on.squad/are valid; skip the probe.squad_state_healthviatool_search_tool_regex(or equivalent tool-discovery mechanism). On success, callsquad_state_healthonce to confirm the bridge answers.{STATE_BACKEND}. Thesquad_stateMCP server in.mcp.jsonis not reachable in this Copilot session. Restart Copilot CLI so.mcp.jsonis loaded, or changestateBackendtolocalin.squad/config.json."2. Replaced soft "if not available" language with a HARD RULE in
### Memory Governance ToolsLists the runtime-owned paths that are FORBIDDEN to write via
create/edit/write_fileon non-local backends when the bridge isn't reachable:.squad/decisions.md.squad/decisions/inbox/**.squad/agents/*/history.md.squad/casting/*.json.squad/identity/*.md.squad/memory/**.squad/orchestration-log/**.squad/log/**.squad/rai/audit-trail.md.squad/fact-checker/audit-trail.mdClarified the local-backend carve-out so
STATE_BACKEND ∈ {"local", "worktree"}users still freely usecreate/edit/write_fileon.squad/.Also clarified that
memory.*andsquad_state_*share the same MCP server (they're aliases in the same registry — seepackages/squad-cli/src/cli/commands/state-mcp.ts) so models stop treating them as separate availability checks.Tests
New
test/state-backend-handshake.test.tsruns against all 4 template mirror targets and asserts (20 tests = 5 assertions × 4 files):squad_state_health+ tool-discovery mechanismcreate/edit/write_filetools by name20/20 pass;
npm run lintclean.What's NOT in scope (filed/tracked separately)
Server-side fix (auto-preload MCP tools): The mechanical root cause is a Copilot CLI feature — preload MCP server tools into the model's function list at session start instead of behind lazy discovery. Out of Squad's control. If maintainers agree it's worth pursuing, file as a feature request against
github/copilot-cli. With this PR's prompt-level enforcement in place, the server-side fix becomes a nice-to-have rather than load-bearing.Skill reinforcement (init-mode, agent-conduct): Could repeat the rules in
.squad/skills/init-mode/SKILL.mdand.squad/skills/agent-conduct/SKILL.mdas defense-in-depth for users who skipsquad upgrade. Worth a small follow-up PR; not blocking.Closes #1305