From 781e76e142916477648fc03877e0a7f1db3f82fd Mon Sep 17 00:00:00 2001 From: Copilot Date: Sat, 13 Jun 2026 15:12:57 +0300 Subject: [PATCH 1/2] fix(prompt): coordinator must probe for squad_state/memory tools before mutating state on non-local backends (closes #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 #1305 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../fix-1305-state-backend-handshake.md | 67 +++++++++++++++ .squad-templates/squad.agent.md | 35 +++++++- .../templates/squad.agent.md.template | 35 +++++++- .../templates/squad.agent.md.template | 35 +++++++- templates/squad.agent.md.template | 35 +++++++- test/state-backend-handshake.test.ts | 85 +++++++++++++++++++ 6 files changed, 284 insertions(+), 8 deletions(-) create mode 100644 .changeset/fix-1305-state-backend-handshake.md create mode 100644 test/state-backend-handshake.test.ts diff --git a/.changeset/fix-1305-state-backend-handshake.md b/.changeset/fix-1305-state-backend-handshake.md new file mode 100644 index 000000000..40af92ac4 --- /dev/null +++ b/.changeset/fix-1305-state-backend-handshake.md @@ -0,0 +1,67 @@ +--- +"@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 backends + +## Symptom + +A coordinator session against a Squad with `stateBackend: "two-layer"` wrote `.squad/decisions.md` + four `.squad/agents/{name}/history.md` files + a `.squad/decisions/inbox/copilot-directive-…md` 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):** 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 use `tool_search_tool_regex` to find them or guess they exist. The `squad_state` MCP server is correctly registered with `tools: ["*"]` and responds to `initialize`/`tools/list` over 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 `stateBackend` resolution at L131. Steps: + +1. If `STATE_BACKEND ∈ {"local", "worktree"}`: file ops on `.squad/` are valid; skip the probe. +2. Otherwise: probe for `squad_state_health` via `tool_search_tool_regex` (or equivalent tool-discovery mechanism). On success, call `squad_state_health` once to confirm the bridge answers. +3. **If the probe fails**: HALT before any state write. Output a precise error to the user (verbatim text in the template): *"Squad's runtime state bridge is missing for backend `{STATE_BACKEND}`. The `squad_state` MCP server in `.mcp.json` is not reachable in this Copilot session. Restart Copilot CLI so `.mcp.json` is loaded, or change `stateBackend` to `local` in `.squad/config.json`."* + +### 2. Replaced soft "if not available" language with a HARD RULE in `### Memory Governance Tools` + +Lists the runtime-owned paths that are FORBIDDEN to write via `create`/`edit`/`write_file` on 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.md` + +Clarified the local-backend carve-out so `STATE_BACKEND ∈ {"local", "worktree"}` users still freely use `create`/`edit`/`write_file` on `.squad/`. + +Also clarified that `memory.*` and `squad_state_*` share the same MCP server (they're aliases in the same registry — see `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` runs against all 4 template mirror targets and asserts (20 tests = 5 assertions × 4 files): + +- Mandatory handshake section exists with "every session" + "before any state mutation" timing +- Probe step exists with `squad_state_health` + tool-discovery mechanism +- HALT step exists with restart-CLI + change-stateBackend remediation +- HARD RULE exists listing the forbidden paths + `create`/`edit`/`write_file` tools by name +- Local/worktree carve-out preserved + +20/20 pass; `npm run lint` clean. + +## 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.md` and `.squad/skills/agent-conduct/SKILL.md` as defense-in-depth for users who skip `squad upgrade`. Worth a small follow-up PR; not blocking. + +Closes #1305 diff --git a/.squad-templates/squad.agent.md b/.squad-templates/squad.agent.md index 744fa2f60..e070fe4a1 100644 --- a/.squad-templates/squad.agent.md +++ b/.squad-templates/squad.agent.md @@ -130,6 +130,18 @@ The `union` merge driver keeps all lines from both sides, which is correct for a **Resolve state backend:** Read `.squad/config.json` (at the resolved TEAM_ROOT) and check the `stateBackend` field. Valid values: `"local"` (default), `"orphan"`, `"two-layer"`. Legacy alias: `"worktree"` maps to `"local"`. Deprecated: `"git-notes"` maps to `"two-layer"` with a deprecation warning. Store as `STATE_BACKEND` and pass it into every spawn prompt. This determines how agents read and write mutable state (history, decisions, logs). Static config (charters, team.md, routing.md) always lives on disk regardless of backend. The `"two-layer"` option combines git-notes (commit-scoped annotations) with orphan branch (permanent state) — see the blog post for the full architecture. +**State-backend handshake — MANDATORY on every session before any state mutation (bradygaster/squad#1305):** + +For all backends EXCEPT `"local"` / `"worktree"`, the runtime owns persistence and you MUST NOT touch `.squad/decisions.md`, `.squad/decisions/inbox/`, `.squad/agents/*/history.md`, `.squad/casting/*.json`, `.squad/identity/*.md`, or `.squad/memory/*` paths via `create` / `edit` / `write_file` tools. Those writes either fail at the pre-commit hook or create phantom state the runtime overwrites at next read — a contract violation that produces silent data loss. + +The `squad_state_*` and `memory.*` tools that own persistence are exposed via the `squad_state` MCP server (declared in `.mcp.json`). Copilot CLI may load MCP tools **lazily** — they are not always advertised in your initial function list at session start. You MUST proactively confirm they are reachable: + +1. If `STATE_BACKEND ∈ {"local", "worktree"}`: file ops on `.squad/` are valid; skip the probe. +2. Otherwise (backend is `orphan`, `two-layer`, or `git-notes`): probe for `squad_state_health` (or any `squad_state_*` / `memory.*` tool) using whatever tool-discovery mechanism your runtime exposes (e.g. `tool_search_tool_regex` in Copilot CLI). If you can locate the tool, call `squad_state_health` once to confirm it answers; on success, treat the bridge as available for the rest of the session. +3. **If the probe fails** (tool not found, or `squad_state_health` errors): **HALT** before any state write. Tell the user verbatim: *"Squad's runtime state bridge is missing for backend `{STATE_BACKEND}`. The `squad_state` MCP server in `.mcp.json` is not reachable in this Copilot session. Restart Copilot CLI so `.mcp.json` is loaded, or change `stateBackend` to `local` in `.squad/config.json`."* — and stop until the user acknowledges. Do not silently fall back to raw file ops. + +This handshake runs **once per session**, not per spawn. Cache the result. + **⚡ Context caching:** After the first message in a session, `team.md`, `routing.md`, and `registry.json` are already in your context. Do NOT re-read them on subsequent messages — you already have the roster, routing rules, and cast names. Only re-read if the user explicitly modifies the team (adds/removes members, changes routing). **Session catch-up (lazy — not on every start):** Do NOT scan logs on every session start. Only provide a catch-up summary when: @@ -272,14 +284,33 @@ The `name` parameter generates the human-readable agent ID shown in the tasks pa ### Memory Governance Tools -When memory tools are available, use them before writing durable memory by hand: +The `memory.*` tools share the same `squad_state` MCP server as `squad_state_*` (they're aliases in the same registry — see `packages/squad-cli/src/cli/commands/state-mcp.ts`). After the state-backend handshake above confirms the bridge is reachable, prefer governed memory tools for durable writes: - Classify candidate memories with `memory.classify`. - Persist approved durable facts, decisions, and policies with `memory.write`. - Search governed memory with `memory.search` before relying only on raw file search. - Promote, delete, and audit governed entries with `memory.promote`, `memory.delete`, and `memory.audit`. -If memory tools are not available, use runtime state tools for durable Squad state when present. In MCP sessions these are exposed as `squad_state_read`, `squad_state_write`, `squad_state_append`, `squad_state_delete`, `squad_state_list`, and `squad_state_health` aliases. Only fall back to local `.squad/` file writes when `STATE_BACKEND` is `worktree`/`local` and no runtime state tool exists. For `git-notes`, `orphan`, or `two-layer`, do not hand-write mutable state; report that the `squad_state` MCP/runtime state bridge is missing. Never claim provider-backed Copilot Memory, semantic indexing, or remote deletion unless a configured tool or CLI bridge performed the operation. External semantic memory is opt-in; forbidden or transient content must not be persisted. +If `memory.*` is not present in the bridge (older Squad versions before the bridge landed) but `squad_state_*` is, use `squad_state_*` directly. Both are governed paths. + +**HARD RULE — Backend contract enforcement:** If `STATE_BACKEND ∈ {"orphan", "two-layer", "git-notes"}` AND the state-backend handshake (above) did NOT confirm reachable tools, you MUST NOT write to ANY of these paths via `create` / `edit` / `write_file`: + +- `.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.md` + +These are runtime-managed paths under non-local backends. Hand-writing creates phantom state. The pre-commit hook will catch it and fail the user; even if it didn't, the runtime overwrites the file at next read. Report the missing bridge and halt instead. + +For `STATE_BACKEND ∈ {"local", "worktree"}`, file writes to `.squad/` are valid because the local backend IS the filesystem. + +**External memory:** Never claim provider-backed Copilot Memory, semantic indexing, or remote deletion unless a configured tool or CLI bridge performed the operation. External semantic memory is opt-in; forbidden or transient content must not be persisted. ### Routing diff --git a/packages/squad-cli/templates/squad.agent.md.template b/packages/squad-cli/templates/squad.agent.md.template index 744fa2f60..e070fe4a1 100644 --- a/packages/squad-cli/templates/squad.agent.md.template +++ b/packages/squad-cli/templates/squad.agent.md.template @@ -130,6 +130,18 @@ The `union` merge driver keeps all lines from both sides, which is correct for a **Resolve state backend:** Read `.squad/config.json` (at the resolved TEAM_ROOT) and check the `stateBackend` field. Valid values: `"local"` (default), `"orphan"`, `"two-layer"`. Legacy alias: `"worktree"` maps to `"local"`. Deprecated: `"git-notes"` maps to `"two-layer"` with a deprecation warning. Store as `STATE_BACKEND` and pass it into every spawn prompt. This determines how agents read and write mutable state (history, decisions, logs). Static config (charters, team.md, routing.md) always lives on disk regardless of backend. The `"two-layer"` option combines git-notes (commit-scoped annotations) with orphan branch (permanent state) — see the blog post for the full architecture. +**State-backend handshake — MANDATORY on every session before any state mutation (bradygaster/squad#1305):** + +For all backends EXCEPT `"local"` / `"worktree"`, the runtime owns persistence and you MUST NOT touch `.squad/decisions.md`, `.squad/decisions/inbox/`, `.squad/agents/*/history.md`, `.squad/casting/*.json`, `.squad/identity/*.md`, or `.squad/memory/*` paths via `create` / `edit` / `write_file` tools. Those writes either fail at the pre-commit hook or create phantom state the runtime overwrites at next read — a contract violation that produces silent data loss. + +The `squad_state_*` and `memory.*` tools that own persistence are exposed via the `squad_state` MCP server (declared in `.mcp.json`). Copilot CLI may load MCP tools **lazily** — they are not always advertised in your initial function list at session start. You MUST proactively confirm they are reachable: + +1. If `STATE_BACKEND ∈ {"local", "worktree"}`: file ops on `.squad/` are valid; skip the probe. +2. Otherwise (backend is `orphan`, `two-layer`, or `git-notes`): probe for `squad_state_health` (or any `squad_state_*` / `memory.*` tool) using whatever tool-discovery mechanism your runtime exposes (e.g. `tool_search_tool_regex` in Copilot CLI). If you can locate the tool, call `squad_state_health` once to confirm it answers; on success, treat the bridge as available for the rest of the session. +3. **If the probe fails** (tool not found, or `squad_state_health` errors): **HALT** before any state write. Tell the user verbatim: *"Squad's runtime state bridge is missing for backend `{STATE_BACKEND}`. The `squad_state` MCP server in `.mcp.json` is not reachable in this Copilot session. Restart Copilot CLI so `.mcp.json` is loaded, or change `stateBackend` to `local` in `.squad/config.json`."* — and stop until the user acknowledges. Do not silently fall back to raw file ops. + +This handshake runs **once per session**, not per spawn. Cache the result. + **⚡ Context caching:** After the first message in a session, `team.md`, `routing.md`, and `registry.json` are already in your context. Do NOT re-read them on subsequent messages — you already have the roster, routing rules, and cast names. Only re-read if the user explicitly modifies the team (adds/removes members, changes routing). **Session catch-up (lazy — not on every start):** Do NOT scan logs on every session start. Only provide a catch-up summary when: @@ -272,14 +284,33 @@ The `name` parameter generates the human-readable agent ID shown in the tasks pa ### Memory Governance Tools -When memory tools are available, use them before writing durable memory by hand: +The `memory.*` tools share the same `squad_state` MCP server as `squad_state_*` (they're aliases in the same registry — see `packages/squad-cli/src/cli/commands/state-mcp.ts`). After the state-backend handshake above confirms the bridge is reachable, prefer governed memory tools for durable writes: - Classify candidate memories with `memory.classify`. - Persist approved durable facts, decisions, and policies with `memory.write`. - Search governed memory with `memory.search` before relying only on raw file search. - Promote, delete, and audit governed entries with `memory.promote`, `memory.delete`, and `memory.audit`. -If memory tools are not available, use runtime state tools for durable Squad state when present. In MCP sessions these are exposed as `squad_state_read`, `squad_state_write`, `squad_state_append`, `squad_state_delete`, `squad_state_list`, and `squad_state_health` aliases. Only fall back to local `.squad/` file writes when `STATE_BACKEND` is `worktree`/`local` and no runtime state tool exists. For `git-notes`, `orphan`, or `two-layer`, do not hand-write mutable state; report that the `squad_state` MCP/runtime state bridge is missing. Never claim provider-backed Copilot Memory, semantic indexing, or remote deletion unless a configured tool or CLI bridge performed the operation. External semantic memory is opt-in; forbidden or transient content must not be persisted. +If `memory.*` is not present in the bridge (older Squad versions before the bridge landed) but `squad_state_*` is, use `squad_state_*` directly. Both are governed paths. + +**HARD RULE — Backend contract enforcement:** If `STATE_BACKEND ∈ {"orphan", "two-layer", "git-notes"}` AND the state-backend handshake (above) did NOT confirm reachable tools, you MUST NOT write to ANY of these paths via `create` / `edit` / `write_file`: + +- `.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.md` + +These are runtime-managed paths under non-local backends. Hand-writing creates phantom state. The pre-commit hook will catch it and fail the user; even if it didn't, the runtime overwrites the file at next read. Report the missing bridge and halt instead. + +For `STATE_BACKEND ∈ {"local", "worktree"}`, file writes to `.squad/` are valid because the local backend IS the filesystem. + +**External memory:** Never claim provider-backed Copilot Memory, semantic indexing, or remote deletion unless a configured tool or CLI bridge performed the operation. External semantic memory is opt-in; forbidden or transient content must not be persisted. ### Routing diff --git a/packages/squad-sdk/templates/squad.agent.md.template b/packages/squad-sdk/templates/squad.agent.md.template index 744fa2f60..e070fe4a1 100644 --- a/packages/squad-sdk/templates/squad.agent.md.template +++ b/packages/squad-sdk/templates/squad.agent.md.template @@ -130,6 +130,18 @@ The `union` merge driver keeps all lines from both sides, which is correct for a **Resolve state backend:** Read `.squad/config.json` (at the resolved TEAM_ROOT) and check the `stateBackend` field. Valid values: `"local"` (default), `"orphan"`, `"two-layer"`. Legacy alias: `"worktree"` maps to `"local"`. Deprecated: `"git-notes"` maps to `"two-layer"` with a deprecation warning. Store as `STATE_BACKEND` and pass it into every spawn prompt. This determines how agents read and write mutable state (history, decisions, logs). Static config (charters, team.md, routing.md) always lives on disk regardless of backend. The `"two-layer"` option combines git-notes (commit-scoped annotations) with orphan branch (permanent state) — see the blog post for the full architecture. +**State-backend handshake — MANDATORY on every session before any state mutation (bradygaster/squad#1305):** + +For all backends EXCEPT `"local"` / `"worktree"`, the runtime owns persistence and you MUST NOT touch `.squad/decisions.md`, `.squad/decisions/inbox/`, `.squad/agents/*/history.md`, `.squad/casting/*.json`, `.squad/identity/*.md`, or `.squad/memory/*` paths via `create` / `edit` / `write_file` tools. Those writes either fail at the pre-commit hook or create phantom state the runtime overwrites at next read — a contract violation that produces silent data loss. + +The `squad_state_*` and `memory.*` tools that own persistence are exposed via the `squad_state` MCP server (declared in `.mcp.json`). Copilot CLI may load MCP tools **lazily** — they are not always advertised in your initial function list at session start. You MUST proactively confirm they are reachable: + +1. If `STATE_BACKEND ∈ {"local", "worktree"}`: file ops on `.squad/` are valid; skip the probe. +2. Otherwise (backend is `orphan`, `two-layer`, or `git-notes`): probe for `squad_state_health` (or any `squad_state_*` / `memory.*` tool) using whatever tool-discovery mechanism your runtime exposes (e.g. `tool_search_tool_regex` in Copilot CLI). If you can locate the tool, call `squad_state_health` once to confirm it answers; on success, treat the bridge as available for the rest of the session. +3. **If the probe fails** (tool not found, or `squad_state_health` errors): **HALT** before any state write. Tell the user verbatim: *"Squad's runtime state bridge is missing for backend `{STATE_BACKEND}`. The `squad_state` MCP server in `.mcp.json` is not reachable in this Copilot session. Restart Copilot CLI so `.mcp.json` is loaded, or change `stateBackend` to `local` in `.squad/config.json`."* — and stop until the user acknowledges. Do not silently fall back to raw file ops. + +This handshake runs **once per session**, not per spawn. Cache the result. + **⚡ Context caching:** After the first message in a session, `team.md`, `routing.md`, and `registry.json` are already in your context. Do NOT re-read them on subsequent messages — you already have the roster, routing rules, and cast names. Only re-read if the user explicitly modifies the team (adds/removes members, changes routing). **Session catch-up (lazy — not on every start):** Do NOT scan logs on every session start. Only provide a catch-up summary when: @@ -272,14 +284,33 @@ The `name` parameter generates the human-readable agent ID shown in the tasks pa ### Memory Governance Tools -When memory tools are available, use them before writing durable memory by hand: +The `memory.*` tools share the same `squad_state` MCP server as `squad_state_*` (they're aliases in the same registry — see `packages/squad-cli/src/cli/commands/state-mcp.ts`). After the state-backend handshake above confirms the bridge is reachable, prefer governed memory tools for durable writes: - Classify candidate memories with `memory.classify`. - Persist approved durable facts, decisions, and policies with `memory.write`. - Search governed memory with `memory.search` before relying only on raw file search. - Promote, delete, and audit governed entries with `memory.promote`, `memory.delete`, and `memory.audit`. -If memory tools are not available, use runtime state tools for durable Squad state when present. In MCP sessions these are exposed as `squad_state_read`, `squad_state_write`, `squad_state_append`, `squad_state_delete`, `squad_state_list`, and `squad_state_health` aliases. Only fall back to local `.squad/` file writes when `STATE_BACKEND` is `worktree`/`local` and no runtime state tool exists. For `git-notes`, `orphan`, or `two-layer`, do not hand-write mutable state; report that the `squad_state` MCP/runtime state bridge is missing. Never claim provider-backed Copilot Memory, semantic indexing, or remote deletion unless a configured tool or CLI bridge performed the operation. External semantic memory is opt-in; forbidden or transient content must not be persisted. +If `memory.*` is not present in the bridge (older Squad versions before the bridge landed) but `squad_state_*` is, use `squad_state_*` directly. Both are governed paths. + +**HARD RULE — Backend contract enforcement:** If `STATE_BACKEND ∈ {"orphan", "two-layer", "git-notes"}` AND the state-backend handshake (above) did NOT confirm reachable tools, you MUST NOT write to ANY of these paths via `create` / `edit` / `write_file`: + +- `.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.md` + +These are runtime-managed paths under non-local backends. Hand-writing creates phantom state. The pre-commit hook will catch it and fail the user; even if it didn't, the runtime overwrites the file at next read. Report the missing bridge and halt instead. + +For `STATE_BACKEND ∈ {"local", "worktree"}`, file writes to `.squad/` are valid because the local backend IS the filesystem. + +**External memory:** Never claim provider-backed Copilot Memory, semantic indexing, or remote deletion unless a configured tool or CLI bridge performed the operation. External semantic memory is opt-in; forbidden or transient content must not be persisted. ### Routing diff --git a/templates/squad.agent.md.template b/templates/squad.agent.md.template index 744fa2f60..e070fe4a1 100644 --- a/templates/squad.agent.md.template +++ b/templates/squad.agent.md.template @@ -130,6 +130,18 @@ The `union` merge driver keeps all lines from both sides, which is correct for a **Resolve state backend:** Read `.squad/config.json` (at the resolved TEAM_ROOT) and check the `stateBackend` field. Valid values: `"local"` (default), `"orphan"`, `"two-layer"`. Legacy alias: `"worktree"` maps to `"local"`. Deprecated: `"git-notes"` maps to `"two-layer"` with a deprecation warning. Store as `STATE_BACKEND` and pass it into every spawn prompt. This determines how agents read and write mutable state (history, decisions, logs). Static config (charters, team.md, routing.md) always lives on disk regardless of backend. The `"two-layer"` option combines git-notes (commit-scoped annotations) with orphan branch (permanent state) — see the blog post for the full architecture. +**State-backend handshake — MANDATORY on every session before any state mutation (bradygaster/squad#1305):** + +For all backends EXCEPT `"local"` / `"worktree"`, the runtime owns persistence and you MUST NOT touch `.squad/decisions.md`, `.squad/decisions/inbox/`, `.squad/agents/*/history.md`, `.squad/casting/*.json`, `.squad/identity/*.md`, or `.squad/memory/*` paths via `create` / `edit` / `write_file` tools. Those writes either fail at the pre-commit hook or create phantom state the runtime overwrites at next read — a contract violation that produces silent data loss. + +The `squad_state_*` and `memory.*` tools that own persistence are exposed via the `squad_state` MCP server (declared in `.mcp.json`). Copilot CLI may load MCP tools **lazily** — they are not always advertised in your initial function list at session start. You MUST proactively confirm they are reachable: + +1. If `STATE_BACKEND ∈ {"local", "worktree"}`: file ops on `.squad/` are valid; skip the probe. +2. Otherwise (backend is `orphan`, `two-layer`, or `git-notes`): probe for `squad_state_health` (or any `squad_state_*` / `memory.*` tool) using whatever tool-discovery mechanism your runtime exposes (e.g. `tool_search_tool_regex` in Copilot CLI). If you can locate the tool, call `squad_state_health` once to confirm it answers; on success, treat the bridge as available for the rest of the session. +3. **If the probe fails** (tool not found, or `squad_state_health` errors): **HALT** before any state write. Tell the user verbatim: *"Squad's runtime state bridge is missing for backend `{STATE_BACKEND}`. The `squad_state` MCP server in `.mcp.json` is not reachable in this Copilot session. Restart Copilot CLI so `.mcp.json` is loaded, or change `stateBackend` to `local` in `.squad/config.json`."* — and stop until the user acknowledges. Do not silently fall back to raw file ops. + +This handshake runs **once per session**, not per spawn. Cache the result. + **⚡ Context caching:** After the first message in a session, `team.md`, `routing.md`, and `registry.json` are already in your context. Do NOT re-read them on subsequent messages — you already have the roster, routing rules, and cast names. Only re-read if the user explicitly modifies the team (adds/removes members, changes routing). **Session catch-up (lazy — not on every start):** Do NOT scan logs on every session start. Only provide a catch-up summary when: @@ -272,14 +284,33 @@ The `name` parameter generates the human-readable agent ID shown in the tasks pa ### Memory Governance Tools -When memory tools are available, use them before writing durable memory by hand: +The `memory.*` tools share the same `squad_state` MCP server as `squad_state_*` (they're aliases in the same registry — see `packages/squad-cli/src/cli/commands/state-mcp.ts`). After the state-backend handshake above confirms the bridge is reachable, prefer governed memory tools for durable writes: - Classify candidate memories with `memory.classify`. - Persist approved durable facts, decisions, and policies with `memory.write`. - Search governed memory with `memory.search` before relying only on raw file search. - Promote, delete, and audit governed entries with `memory.promote`, `memory.delete`, and `memory.audit`. -If memory tools are not available, use runtime state tools for durable Squad state when present. In MCP sessions these are exposed as `squad_state_read`, `squad_state_write`, `squad_state_append`, `squad_state_delete`, `squad_state_list`, and `squad_state_health` aliases. Only fall back to local `.squad/` file writes when `STATE_BACKEND` is `worktree`/`local` and no runtime state tool exists. For `git-notes`, `orphan`, or `two-layer`, do not hand-write mutable state; report that the `squad_state` MCP/runtime state bridge is missing. Never claim provider-backed Copilot Memory, semantic indexing, or remote deletion unless a configured tool or CLI bridge performed the operation. External semantic memory is opt-in; forbidden or transient content must not be persisted. +If `memory.*` is not present in the bridge (older Squad versions before the bridge landed) but `squad_state_*` is, use `squad_state_*` directly. Both are governed paths. + +**HARD RULE — Backend contract enforcement:** If `STATE_BACKEND ∈ {"orphan", "two-layer", "git-notes"}` AND the state-backend handshake (above) did NOT confirm reachable tools, you MUST NOT write to ANY of these paths via `create` / `edit` / `write_file`: + +- `.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.md` + +These are runtime-managed paths under non-local backends. Hand-writing creates phantom state. The pre-commit hook will catch it and fail the user; even if it didn't, the runtime overwrites the file at next read. Report the missing bridge and halt instead. + +For `STATE_BACKEND ∈ {"local", "worktree"}`, file writes to `.squad/` are valid because the local backend IS the filesystem. + +**External memory:** Never claim provider-backed Copilot Memory, semantic indexing, or remote deletion unless a configured tool or CLI bridge performed the operation. External semantic memory is opt-in; forbidden or transient content must not be persisted. ### Routing diff --git a/test/state-backend-handshake.test.ts b/test/state-backend-handshake.test.ts new file mode 100644 index 000000000..05d0a8281 --- /dev/null +++ b/test/state-backend-handshake.test.ts @@ -0,0 +1,85 @@ +/** + * Regression tests for bradygaster/squad#1305 — the canonical squad.agent.md + * template must instruct the coordinator to probe for squad_state and memory + * tools before mutating state on non-local backends, and must hard-refuse + * writes when the bridge isn't reachable. + * + * Background: Copilot CLI loads MCP server tools lazily — they're not + * always advertised in the initial function list. The pre-1305 prompt said + * "when memory tools are available, use them" which models interpreted as + * "if listed" instead of "after probing". That led to a real incident where + * a coordinator session against a two-layer backend wrote .squad/decisions.md + * via raw create/edit tools, hit the pre-commit hook, and treated it as a + * git problem instead of a contract violation. + */ + +import { describe, it, expect } from 'vitest'; +import { readFileSync, existsSync } from 'node:fs'; +import path from 'node:path'; + +const REPO_ROOT = path.resolve(__dirname, '..'); + +const TEMPLATE_TARGETS = [ + '.squad-templates/squad.agent.md', + 'templates/squad.agent.md.template', + 'packages/squad-cli/templates/squad.agent.md.template', + 'packages/squad-sdk/templates/squad.agent.md.template', +]; + +describe('squad.agent.md.template — state-backend handshake (#1305)', () => { + for (const rel of TEMPLATE_TARGETS) { + describe(rel, () => { + const fullPath = path.join(REPO_ROOT, rel); + if (!existsSync(fullPath)) { + it.skip(`(file does not exist in this checkout: ${rel})`, () => {}); + return; + } + const content = readFileSync(fullPath, 'utf-8'); + + it('declares the mandatory state-backend handshake section', () => { + // The handshake must be flagged as MANDATORY and explicitly mention + // every session and pre-state-mutation timing — soft language was + // the original failure mode. + expect(content).toMatch(/State-backend handshake[^\n]*MANDATORY/i); + expect(content).toMatch(/every session/i); + expect(content).toMatch(/before any state mutation/i); + }); + + it('instructs the coordinator to PROBE for squad_state_health on non-local backends', () => { + // The probe is the load-bearing behavioral instruction. The pre-1305 + // prompt said "when memory tools are available" which models read as + // "if listed in my tool block"; the post-1305 prompt explicitly says + // to probe via tool-discovery (e.g. tool_search_tool_regex). + expect(content).toMatch(/squad_state_health/); + expect(content).toMatch(/tool_search_tool_regex|tool-discovery/i); + }); + + it('instructs the coordinator to HALT when the probe fails', () => { + // Halt is the load-bearing safety instruction. Without it, models + // silently fall back to raw file ops and violate the backend contract. + expect(content).toMatch(/HALT/i); + // Must tell the user how to fix it — restart Copilot CLI or change + // stateBackend to local. + expect(content).toMatch(/Restart Copilot CLI/i); + expect(content).toMatch(/stateBackend.*local/i); + }); + + it('declares a HARD RULE forbidding raw file writes to runtime-owned paths under non-local backends', () => { + expect(content).toMatch(/HARD RULE/i); + // The forbidden-paths list must include the high-traffic state files. + expect(content).toMatch(/\.squad\/decisions\.md/); + expect(content).toMatch(/\.squad\/decisions\/inbox/); + 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); + }); + + it('keeps the local/worktree carve-out explicit (file ops valid for local backends)', () => { + // The rule applies ONLY to non-local backends. Local-backend users + // must still be able to use create/edit/write_file on .squad/. + expect(content).toMatch(/local.*worktree|local.*backend.*valid/i); + }); + }); + } +}); From 352bb3b5bf7fdaf46b70d4c4c0364543094aee74 Mon Sep 17 00:00:00 2001 From: Tamir Dresher Date: Sat, 13 Jun 2026 15:38:43 +0300 Subject: [PATCH 2/2] test: tighten handshake-rule regex (split into 3 separate assertions) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer follow-up on #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> --- test/state-backend-handshake.test.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/state-backend-handshake.test.ts b/test/state-backend-handshake.test.ts index 05d0a8281..ed7613512 100644 --- a/test/state-backend-handshake.test.ts +++ b/test/state-backend-handshake.test.ts @@ -71,8 +71,14 @@ describe('squad.agent.md.template — state-backend handshake (#1305)', () => { expect(content).toMatch(/\.squad\/decisions\/inbox/); 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); + // model maps the rule to its actual function inventory. Match each + // tool name with separate assertions so dropping any one of them + // (e.g. forgetting `write_file`) fails the test — the previous + // single-regex form had | precedence so the shorter alternative + // `create\s*/\s*edit` could pass without `write_file`. + expect(content).toMatch(/\bcreate\b/); + expect(content).toMatch(/\bedit\b/); + expect(content).toMatch(/\bwrite_file\b/); }); it('keeps the local/worktree carve-out explicit (file ops valid for local backends)', () => {