State-backend v2: ref-level CAS, recursive deleteDir, stub-name collision, regression coverage

Background

This issue tracks the residual concerns from the PR #1200 review that were
intentionally deferred out of that PR to keep its scope reviewable. The
in-PR fixes resolved concerns B, G, H in full and concerns C, D in part. The
items below are the remainder.

Source review: PR #1200 review comment 4621356216

Decision file: .squad/workstreams/active/squad-agents-ai/decisions/inbox/picard-pr1200-followup-scope.md

Priority: Ship before promoting TwoLayerBackend to the default backend
for fleet / multi-session workloads. The single-user single-session case is
unaffected by concern A and can ship today.

Scope

A. Replace bare update-ref / git notes add -f with CAS-safe forms

Problem. The current write path uses:

• git update-ref refs/heads/<X> <newSha> — last writer wins, silently
  clobbers concurrent updates from another process.
• git notes add -f <ref> <sha> — -f is "force overwrite", same
  last-writer-wins behaviour.

Under concurrent writers (squad watch + interactive session, two terminals,
fleet) this can drop state without any error.

Fix.

1. update-ref refs/heads/<X> <new> <old> — compare-and-swap form. Pass
   the SHA read at the start of the write as <old>.
2. Replace git notes add -f with an optimistic loop: read current note
   blob, build new blob, git notes add (no -f) and retry on the
   "already exists" / mismatched-parent error.
3. Wrap both in a retry-on-mismatch loop with jittered exponential
   back-off (50 ms → 800 ms, max 5 attempts).
4. On final give-up, throw a typed StateBackendConcurrencyError so
   callers can decide how to surface it (toast vs. re-queue vs. abort).

Acceptance criteria.

• All write paths in WorktreeBackend, GitNotesBackend,
  OrphanBranchBackend, TwoLayerBackend use CAS / optimistic-loop form.
• Grep for update-ref and notes add -f returns zero hits in
  packages/squad-sdk/src/.
• Regression test from concern I below passes (see I).
• Typed StateBackendConcurrencyError exported and documented.

I. Concurrent-writer regression test

Problem. There is no test that exercises the write path under contention.
Concern A would have been caught by such a test.

Fix. Add a Vitest case to test/state-backend.test.ts:

5 worker promises × 20 appends each → expect 100 distinct entries in final state

For each backend (WorktreeBackend, GitNotesBackend, OrphanBranchBackend,
TwoLayerBackend):

1. Spin up a fresh repo fixture.
2. Launch 5 concurrent append(key, value-N) calls × 20 iterations.
3. After all settle, read the key and assert the resulting list has length
   100 with no duplicates and no losses.
4. Test must be deterministic — use bounded jitter, not real wall clock.

Acceptance criteria.

• Test exists, passes locally, passes in CI.
• Test fails on main HEAD (proves it catches the bug concern A describes)
  and passes after concern A's fix lands.

E. deleteDir must be recursive across nested subtrees

Problem. Current deleteDir(prefix) only removes direct children matching
the prefix. State keys with deeper nesting (e.g. agents/picard/history/2026/06/...)
leave orphan entries behind when their parent dir is deleted.

Fix.

1. Update deleteDir in every backend to walk the full subtree and remove
   every entry whose key starts with prefix + '/'.
2. Add regression test:
   • Write 5 keys: a/b/c, a/b/d, a/b/e/f, a/b/e/g/h, a/other.
   • Call deleteDir('a/b').
   • Assert only a/other remains.

Acceptance criteria.

• All 4 backends pass the recursive-delete regression test.
• No regression in existing deleteDir tests.

F. Rename stateBackend: 'external' stub to 'external-stub'

Problem. The state-backend config currently accepts 'external' as a
sentinel/stub value. PR #1194 plans to introduce a real
stateLocation: 'external' feature with the same string. If both ship the
same name, config parsing collides and silently misroutes state.

Fix.

1. Rename the stub from 'external' to 'external-stub' everywhere in
   packages/squad-sdk/src/state-backend*.ts and types.
2. Add a config-read deprecation warning: when the config still says
   stateBackend: 'external', log
   [deprecation] stateBackend:'external' is renamed to 'external-stub'; please update your config and continue with the stub.
3. Update the upgrade migration in squad-cli (the same machinery used by
   squad upgrade) to rewrite 'external' → 'external-stub' in
   user configs automatically.

Acceptance criteria.

• Land before PR State documentation out of sync with latest release #1194 merges.
• grep -R "stateBackend: 'external'" returns only the deprecation-warning
  test fixture.
• Migration test confirms an old config is rewritten on squad upgrade.

C-residual. Audit other backends for swallowed catches

Problem. The in-PR fix added console.warn('[two-layer] ...') to 3 hot
paths in TwoLayerBackend only. Other backends (WorktreeBackend,
GitNotesBackend, OrphanBranchBackend) may still have silent
catch (e) {} or catch { return undefined } blocks.

Fix.

1. Grep packages/squad-sdk/src/state-backend*.ts for catch blocks that
   do not re-throw, do not log, and do not return a typed error.
2. For each one, add either a structured warn (matching the
   [<backend-name>] tag convention) or a typed thrown error, whichever
   is appropriate for the call path.
3. Document the policy: "no silent catches in state-backend write/read
   paths; every swallow must be deliberate and tagged."

Acceptance criteria.

• Audit-grep returns zero un-tagged silent catches in all backends.
• Policy doc in packages/squad-sdk/src/state-backend.md (or equivalent).

D-residual. Extend per-layer verifyStateBackend contract

Problem. The in-PR fix added per-layer probing for TwoLayerBackend only.
Any future composite backend (e.g. a three-layer cache+notes+orphan stack, or
a remote+local mirror) would re-introduce the same blind-spot.

Fix.

1. Promote per-layer health probing into the StateBackend interface
   itself as an optional verifyLayers?(): Promise<LayerHealth[]> method.
2. verifyStateBackend calls verifyLayers() if present and aggregates
   the per-layer result into its top-level report.
3. Default implementation on the base class returns a single-layer
   result so existing single-layer backends need no change.

Acceptance criteria.

• StateBackend interface gains verifyLayers? (non-breaking).
• TwoLayerBackend.verifyLayers returns 2 entries; single-layer backends
  return 1 entry (or omit the method).
• Startup probe report includes a per-layer breakdown when present.

Sequencing

1. F must land before PR State documentation out of sync with latest release #1194 merges (calendar-driven).
2. A + I are coupled (test in I proves A) and should ship together,
   before promoting TwoLayerBackend to the default for fleet workloads.
3. C-residual, D-residual, E are independent and can ship in any order.

Priority

Ship before promoting TwoLayerBackend to default for fleet workloads.
The single-user single-session path on the current PR is safe. The follow-ups
above matter for: squad watch running concurrently with an interactive
session; two terminals on the same repo; multi-machine fleet usage.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com