diff --git a/.changeset/1947-create-if-absent.md b/.changeset/1947-create-if-absent.md new file mode 100644 index 000000000..922235d31 --- /dev/null +++ b/.changeset/1947-create-if-absent.md @@ -0,0 +1,13 @@ +--- +"@bradygaster/squad-sdk": minor +"@bradygaster/squad-cli": minor +--- + +Add atomic repository-scoped `createIfAbsent` operation to all state backends (local, git-notes, orphan-branch, two-layer), `StorageProvider` (FSStorageProvider, InMemoryStorageProvider, SQLiteStorageProvider), `StateBackendStorageAdapter`, and `ToolRegistry`/state-mcp MCP surface (`squad_state_create_if_absent`). + +- Exactly one concurrent creator succeeds; all others receive `StateKeyConflictError` +- Existing content is never overwritten +- Two-layer backend is fail-closed for disagreement/failure between layers +- New typed errors: `StateKeyConflictError`, `StateBackendUncertaintyError` (exported from `@bradygaster/squad-sdk`) +- Local backend uses O_CREAT|O_EXCL (filesystem exclusive creation); git-native backends use CAS loops +- MCP tool `squad_state_create_if_absent` appears in `tools/list` with `conflict`/`uncertainty` failure types diff --git a/.squad-templates/scribe-charter.md b/.squad-templates/scribe-charter.md index 0300782c3..a6ff283cb 100644 --- a/.squad-templates/scribe-charter.md +++ b/.squad-templates/scribe-charter.md @@ -77,7 +77,14 @@ perform these checks, **stop and report** rather than proceeding with an unverif **Worktree awareness:** Use the `TEAM ROOT` provided in the spawn prompt to resolve all `.squad/` paths. If no TEAM ROOT is given, run `git rev-parse --show-toplevel` as fallback. Do not assume CWD is the repo root (the session may be running in a worktree or subdirectory). -**State backend awareness:** Check `STATE_BACKEND` from the spawn prompt. Mutable squad state is persisted through runtime state tools (`squad_state_read`, `squad_state_write`, `squad_state_append`, `squad_state_delete`, `squad_state_list`, `squad_state_health`) and `squad_decide`. Do not run backend git commands, switch to state branches, push note refs, reset `.squad/`, or commit mutable state by hand. If state tools are unavailable, stop without mutating files or git state and record the tool availability failure in your final summary. +**State backend awareness:** Check `STATE_BACKEND` from the spawn prompt. Mutable squad state is persisted through runtime state tools (`squad_state_read`, `squad_state_write`, `squad_state_append`, `squad_state_create_if_absent`, `squad_state_delete`, `squad_state_list`, `squad_state_health`) and `squad_decide`. Do not run backend git commands, switch to state branches, push note refs, reset `.squad/`, or commit mutable state by hand. If state tools are unavailable, stop without mutating files or git state and record the tool availability failure in your final summary. + +**Exclusive canonical artifacts:** When exactly one canonical artifact must exist — a retrospective, a session log, a claim marker — create it with `squad_state_create_if_absent`, never `squad_state_write`. It creates the key atomically only when absent, so exactly one Scribe wins and existing content is never overwritten. Handle its two failure shapes explicitly: + +- `error: "conflict"` — another Scribe already created the canonical artifact. Do **not** retry as a create and do **not** overwrite. Read the winner's content with `squad_state_read` and append to it if you have something to add. +- `error: "uncertainty"` — the outcome is unknown. Do **not** assume success and do **not** write over the key. Stop and report the uncertainty in your final summary. + +Never emulate this with `squad_state_read` followed by `squad_state_write`; that check-then-write pattern is racy and silently destroys a concurrent Scribe's canonical artifact. After every substantial work session: diff --git a/.squad-templates/spawn-reference.md b/.squad-templates/spawn-reference.md index f036d1216..56c362a7a 100644 --- a/.squad-templates/spawn-reference.md +++ b/.squad-templates/spawn-reference.md @@ -123,6 +123,11 @@ prompt: | whenever they are available: - `squad_state_read` / `squad_state_list` for decisions, history, logs, and inbox entries - `squad_state_write` / `squad_state_append` for durable updates + - `squad_state_create_if_absent` when exactly one canonical artifact must exist + (retrospectives, session logs, claim markers). It creates the key atomically + only when absent and never overwrites; on `error: "conflict"` another agent + already won, and on `error: "uncertainty"` the outcome is unknown — do not + assume success and do not overwrite in either case. - `squad_state_delete` after Scribe merges inbox entries - `squad_state_health` when diagnosing backend availability - `squad_decide` for team-relevant decisions diff --git a/.squad/skills/coordinator-source-of-truth/SKILL.md b/.squad/skills/coordinator-source-of-truth/SKILL.md index 3a992ba0a..b6948cb9b 100644 --- a/.squad/skills/coordinator-source-of-truth/SKILL.md +++ b/.squad/skills/coordinator-source-of-truth/SKILL.md @@ -11,7 +11,7 @@ source: "Extracted from squad.agent.md as part of the slimming effort (bradygast ## State backend note -Files below marked as **"Derived / append-only"** are **mutable state** — agents access them with runtime state tools (`squad_state_read`, `squad_state_write`, `squad_state_append`, `squad_state_delete`, `squad_state_list`). The runtime decides whether the configured backend stores them on disk, git-native state, or an external provider. Files marked as **"Authoritative"** are **static config** and always live on disk regardless of backend. +Files below marked as **"Derived / append-only"** are **mutable state** — agents access them with runtime state tools (`squad_state_read`, `squad_state_write`, `squad_state_append`, `squad_state_create_if_absent`, `squad_state_delete`, `squad_state_list`). The runtime decides whether the configured backend stores them on disk, git-native state, or an external provider. Files marked as **"Authoritative"** are **static config** and always live on disk regardless of backend. ## File hierarchy diff --git a/docs/src/content/docs/features/state-backends.md b/docs/src/content/docs/features/state-backends.md index 19cc8ff43..b815a32d6 100644 --- a/docs/src/content/docs/features/state-backends.md +++ b/docs/src/content/docs/features/state-backends.md @@ -171,12 +171,135 @@ interface StateBackend { list(relativeDir: string): string[]; delete(relativePath: string): boolean; append(relativePath: string, content: string): void; + createIfAbsent(relativePath: string, content: string): void; readonly name: string; } ``` +--- +## Atomic Create-if-Absent + +### Overview + +`createIfAbsent(key, content)` creates a state key **only when it does not already exist**. Exactly one concurrent caller receives `void` (success); all others receive a typed `StateKeyConflictError`. Existing content is **never overwritten**. + +This is the correct primitive for workflows where multiple agents might race to register a session, claim a work item, or initialize shared state — any scenario where exactly one creator must win. + +### Repository Scope + +The operation is repository-scoped and verified. Before any git-native create, +the backend asserts that its target directory **is the working-tree root of its +own repository**, by comparing the realpath of that directory against +`git rev-parse --show-toplevel`. + +This matters because `git rev-parse --git-dir` alone is not enough: Git walks up +the directory tree, so a non-repository directory nested inside a repository +silently resolves to the *enclosing* repository. Without the root check, a +create aimed at a non-repository path would escape its intended scope and write +state into a repository the caller never named. + +Any failure, mismatch, bare repository, or unresolvable path fails closed with +`StateBackendUncertaintyError`. There is no silent fallback. + +### Typed Error Contracts + +| Thrown | Meaning | What to do | +|--------|---------|------------| +| `StateKeyConflictError` | Key **definitely** already exists. Another creator won. | Read the winner's content; do not retry as a create. | +| `StateBackendUncertaintyError` | Outcome **unknown** (write failed after exclusive open, CAS retries exhausted, or two-layer disagreement). | Do NOT assume success. Inspect before retrying. | + +Neither error is success-shaped. A conflict or uncertainty always surfaces as a thrown error, never as a return value. + +### SDK Usage + +```typescript +import { + StateKeyConflictError, + StateBackendUncertaintyError, +} from '@bradygaster/squad-sdk'; + +try { + backend.createIfAbsent('sessions/retro-2024-12.md', '# Retro\n'); + // Success: this process is the sole creator +} catch (err) { + if (err instanceof StateKeyConflictError) { + // Another process already created this key — read their content + const content = backend.read(err.key); + } else if (err instanceof StateBackendUncertaintyError) { + // Outcome unknown — do NOT assume success; inspect before retrying + console.error('Uncertain create:', err.message); + } else { + throw err; + } +} +``` + +### Per-Backend Guarantees + +| Backend | Atomicity mechanism | Conflict detection | Uncertainty condition | +|---------|--------------------|--------------------|----------------------| +| **Local** | `open(path, 'wx')` — O_CREAT\|O_EXCL (POSIX + NTFS) | EEXIST on open | Write failure after exclusive open | +| **Orphan branch** | CAS loop: `update-ref` with expected SHA | Key present at snapshot or at new HEAD after CAS loss | CAS retry limit exhausted with key still absent | +| **Git notes** | CAS loop: `update-ref` with expected notes-tree SHA | Key present at snapshot or at new HEAD after CAS loss | CAS retry limit exhausted | +| **Two-layer** | Orphan is authoritative; notes mirrors | Orphan conflict → `StateKeyConflictError` | Orphan success + notes failure/conflict → `StateBackendUncertaintyError` | + +**No process-local locks are used.** Atomicity is enforced by OS-level exclusive file creation (local backend) or Git's `update-ref` compare-and-swap (git-native backends). + +### Two-Layer Fail-Closed Contract + +The two-layer backend uses a **fail-closed** policy for disagreement between layers: + +- Orphan layer is authoritative. If it reports conflict → `StateKeyConflictError`. +- If orphan succeeds but the notes layer fails or reports conflict (disagreement) → `StateBackendUncertaintyError`. + +The caller must not assume success on uncertainty. Silent partial success would allow multiple creators to each believe they won. + +### MCP Tool Registration + +The operation is exposed as the `squad_state_create_if_absent` MCP tool and appears in `tools/list` on state-mcp sessions. + +**Tool name:** `squad_state_create_if_absent` +**Parameters:** `key` (string, relative to `.squad/`), `content` (string) + +Result shapes: +- `{ resultType: 'success' }` — key created +- `{ resultType: 'failure', error: 'conflict' }` — key already existed +- `{ resultType: 'failure', error: 'uncertainty' }` — outcome unknown +- `{ resultType: 'failure', error: }` — other error (invalid key, etc.) + +Only mutable state key prefixes are permitted (`sessions/`, `decisions/inbox/`, `log/`, `orchestration-log/`, `.scratch/`, `identity/`, `agents/*/history.md`, etc.). Static configuration keys (`team.md`, `routing.md`, `config.json`, etc.) are rejected. + +### Migration Guidance + +If you are currently using `write()` to initialize a key that should only be created once: + +```typescript +// Before (unsafe — unconditionally overwrites): +if (!backend.exists('sessions/init.md')) { + backend.write('sessions/init.md', initialContent); +} + +// After (safe — exactly one concurrent caller succeeds): +try { + backend.createIfAbsent('sessions/init.md', initialContent); +} catch (err) { + if (!(err instanceof StateKeyConflictError)) throw err; +} +``` + +The check-then-write pattern above has a TOCTOU race; `createIfAbsent` closes it. + +### Version + +`createIfAbsent` and the `squad_state_create_if_absent` MCP tool first ship in +`@bradygaster/squad-sdk` and `@bradygaster/squad-cli` **0.14.0** (the next minor +release after 0.13.1). Consumers pinned to `0.12.0` or `0.13.x` must upgrade the +pin to `0.14.0`, add `squad_state_create_if_absent` to their MCP tool allowlist, +and restart the state MCP server before the tool appears in `tools/list`. + --- ## Security + State backends include hardening against common injection attacks: - **Path traversal:** `..` segments are rejected - **Null byte injection:** `\0` characters are rejected diff --git a/docs/src/content/docs/features/storage-provider.md b/docs/src/content/docs/features/storage-provider.md index 1888262d3..46a47a141 100644 --- a/docs/src/content/docs/features/storage-provider.md +++ b/docs/src/content/docs/features/storage-provider.md @@ -31,11 +31,12 @@ All of Squad's data — sessions, decisions, agent memories, event logs — flow - **Production** can use SQLite, cloud storage, or a database. - **Multi-team deployments** can route different squads to different backends. -The interface is minimal — just 12 core async methods[^1]: +The interface is minimal — just 13 core async methods[^1]: ```typescript read(filePath: string): Promise write(filePath: string, data: string): Promise +createIfAbsent(filePath: string, data: string): Promise append(filePath: string, data: string): Promise exists(filePath: string): Promise list(dirPath: string): Promise @@ -48,6 +49,12 @@ copy(srcPath: string, destPath: string): Promise stat(targetPath: string): Promise ``` +`createIfAbsent` is the only conditional operation: it must create the key +**atomically and only when absent**, reject with `StateKeyConflictError` when the +key already exists, and reject with `StateBackendUncertaintyError` when the +outcome cannot be determined. It must never overwrite existing content. +See [State Backends → Atomic Create-if-Absent](/features/state-backends/#atomic-create-if-absent). + --- ## Built-in Providers @@ -113,6 +120,14 @@ export class MyCustomStorageProvider implements StorageProvider { // Create parent directories as needed } + async createIfAbsent(filePath: string, data: string): Promise { + // Atomically create ONLY if absent — never overwrite. + // Throw StateKeyConflictError if the key already exists. + // Throw StateBackendUncertaintyError if the outcome is unknown. + // Use a native conditional primitive (O_EXCL open, INSERT OR IGNORE, + // If-None-Match: *, conditional PUT) — a read-then-write check is racy. + } + async append(filePath: string, data: string): Promise { // Append to a file, creating it if missing } diff --git a/packages/squad-cli/src/cli/commands/state-mcp.ts b/packages/squad-cli/src/cli/commands/state-mcp.ts index b6800c3f9..22790193d 100644 --- a/packages/squad-cli/src/cli/commands/state-mcp.ts +++ b/packages/squad-cli/src/cli/commands/state-mcp.ts @@ -31,6 +31,7 @@ const MCP_TOOL_ALIASES: Record = { squad_state_append: 'squad_state_append', squad_state_delete: 'squad_state_delete', squad_state_list: 'squad_state_list', + squad_state_create_if_absent: 'squad_state_create_if_absent', squad_state_health: 'squad_state_health', 'memory.classify': 'memory.classify', 'memory.write': 'memory.write', diff --git a/packages/squad-cli/templates/scribe-charter.md b/packages/squad-cli/templates/scribe-charter.md index 0300782c3..a6ff283cb 100644 --- a/packages/squad-cli/templates/scribe-charter.md +++ b/packages/squad-cli/templates/scribe-charter.md @@ -77,7 +77,14 @@ perform these checks, **stop and report** rather than proceeding with an unverif **Worktree awareness:** Use the `TEAM ROOT` provided in the spawn prompt to resolve all `.squad/` paths. If no TEAM ROOT is given, run `git rev-parse --show-toplevel` as fallback. Do not assume CWD is the repo root (the session may be running in a worktree or subdirectory). -**State backend awareness:** Check `STATE_BACKEND` from the spawn prompt. Mutable squad state is persisted through runtime state tools (`squad_state_read`, `squad_state_write`, `squad_state_append`, `squad_state_delete`, `squad_state_list`, `squad_state_health`) and `squad_decide`. Do not run backend git commands, switch to state branches, push note refs, reset `.squad/`, or commit mutable state by hand. If state tools are unavailable, stop without mutating files or git state and record the tool availability failure in your final summary. +**State backend awareness:** Check `STATE_BACKEND` from the spawn prompt. Mutable squad state is persisted through runtime state tools (`squad_state_read`, `squad_state_write`, `squad_state_append`, `squad_state_create_if_absent`, `squad_state_delete`, `squad_state_list`, `squad_state_health`) and `squad_decide`. Do not run backend git commands, switch to state branches, push note refs, reset `.squad/`, or commit mutable state by hand. If state tools are unavailable, stop without mutating files or git state and record the tool availability failure in your final summary. + +**Exclusive canonical artifacts:** When exactly one canonical artifact must exist — a retrospective, a session log, a claim marker — create it with `squad_state_create_if_absent`, never `squad_state_write`. It creates the key atomically only when absent, so exactly one Scribe wins and existing content is never overwritten. Handle its two failure shapes explicitly: + +- `error: "conflict"` — another Scribe already created the canonical artifact. Do **not** retry as a create and do **not** overwrite. Read the winner's content with `squad_state_read` and append to it if you have something to add. +- `error: "uncertainty"` — the outcome is unknown. Do **not** assume success and do **not** write over the key. Stop and report the uncertainty in your final summary. + +Never emulate this with `squad_state_read` followed by `squad_state_write`; that check-then-write pattern is racy and silently destroys a concurrent Scribe's canonical artifact. After every substantial work session: diff --git a/packages/squad-cli/templates/skills/coordinator-source-of-truth/SKILL.md b/packages/squad-cli/templates/skills/coordinator-source-of-truth/SKILL.md index 3a992ba0a..b6948cb9b 100644 --- a/packages/squad-cli/templates/skills/coordinator-source-of-truth/SKILL.md +++ b/packages/squad-cli/templates/skills/coordinator-source-of-truth/SKILL.md @@ -11,7 +11,7 @@ source: "Extracted from squad.agent.md as part of the slimming effort (bradygast ## State backend note -Files below marked as **"Derived / append-only"** are **mutable state** — agents access them with runtime state tools (`squad_state_read`, `squad_state_write`, `squad_state_append`, `squad_state_delete`, `squad_state_list`). The runtime decides whether the configured backend stores them on disk, git-native state, or an external provider. Files marked as **"Authoritative"** are **static config** and always live on disk regardless of backend. +Files below marked as **"Derived / append-only"** are **mutable state** — agents access them with runtime state tools (`squad_state_read`, `squad_state_write`, `squad_state_append`, `squad_state_create_if_absent`, `squad_state_delete`, `squad_state_list`). The runtime decides whether the configured backend stores them on disk, git-native state, or an external provider. Files marked as **"Authoritative"** are **static config** and always live on disk regardless of backend. ## File hierarchy diff --git a/packages/squad-cli/templates/spawn-reference.md b/packages/squad-cli/templates/spawn-reference.md index f036d1216..56c362a7a 100644 --- a/packages/squad-cli/templates/spawn-reference.md +++ b/packages/squad-cli/templates/spawn-reference.md @@ -123,6 +123,11 @@ prompt: | whenever they are available: - `squad_state_read` / `squad_state_list` for decisions, history, logs, and inbox entries - `squad_state_write` / `squad_state_append` for durable updates + - `squad_state_create_if_absent` when exactly one canonical artifact must exist + (retrospectives, session logs, claim markers). It creates the key atomically + only when absent and never overwrites; on `error: "conflict"` another agent + already won, and on `error: "uncertainty"` the outcome is unknown — do not + assume success and do not overwrite in either case. - `squad_state_delete` after Scribe merges inbox entries - `squad_state_health` when diagnosing backend availability - `squad_decide` for team-relevant decisions diff --git a/packages/squad-sdk/src/state-backend.ts b/packages/squad-sdk/src/state-backend.ts index 09eda75e6..44bd2e4b9 100644 --- a/packages/squad-sdk/src/state-backend.ts +++ b/packages/squad-sdk/src/state-backend.ts @@ -9,9 +9,11 @@ */ import { execFileSync } from 'node:child_process'; +import { openSync, writeSync as fsWriteSync, closeSync, mkdirSync as fsMkdirSync, unlinkSync as fsUnlinkSync, realpathSync as fsRealpathSync } from 'node:fs'; import path from 'node:path'; import { FSStorageProvider } from './storage/fs-storage-provider.js'; import type { StorageProvider, StorageStats } from './storage/storage-provider.js'; +import { StateKeyConflictError, StateBackendUncertaintyError } from './storage/storage-error.js'; const storage = new FSStorageProvider(); @@ -119,9 +121,23 @@ function isExpectedMissing(err: unknown): boolean { export type StateBackendType = 'local' | 'external-stub' | 'orphan' | 'two-layer'; +// Re-export for callers who import directly from state-backend. +export { StateKeyConflictError, StateBackendUncertaintyError } from './storage/storage-error.js'; + export interface StateBackend { read(relativePath: string): string | undefined; write(relativePath: string, content: string): void; + /** + * Atomically create a key only when absent. Returns void on success + * (this caller is the sole creator). Throws {@link StateKeyConflictError} + * if the key already exists. Throws {@link StateBackendUncertaintyError} + * if the outcome cannot be determined with certainty. Never overwrites. + * + * Repository identity is verified at backend construction. If the repository + * is inaccessible or ambiguous at operation time, the method fails with + * {@link StateBackendUncertaintyError} rather than silently expanding scope. + */ + createIfAbsent(relativePath: string, content: string): void; exists(relativePath: string): boolean; list(relativeDir: string): string[]; delete(relativePath: string): boolean; @@ -220,6 +236,62 @@ function gitExecOrThrow(args: string[], cwd: string): string { } } +/** + * Fail-closed repository identity check for atomic create operations. + * + * `git rev-parse --git-dir` is NOT sufficient on its own: git walks up the + * directory tree, so a non-repository directory nested inside a repository + * resolves to the *enclosing* repository. Using that would silently widen the + * operation's scope and write state into a repository the caller never named. + * + * This asserts that `cwd` is the working-tree root of a real repository by + * comparing the realpath of `cwd` against `git rev-parse --show-toplevel`. + * Any failure, mismatch, or bare/ambiguous repository fails closed with + * {@link StateBackendUncertaintyError} — never a silent fallback. + */ +function assertRepositoryIdentity(cwd: string, operation: string): void { + let toplevel: string; + try { + toplevel = gitExecOrThrow(['rev-parse', '--show-toplevel'], cwd); + } catch (err: unknown) { + const msg = err instanceof Error ? err.message : String(err); + throw new StateBackendUncertaintyError(operation, `repository inaccessible: ${msg}`); + } + + if (!toplevel) { + throw new StateBackendUncertaintyError( + operation, + 'repository identity ambiguous: git reported no working-tree root', + ); + } + + let actual: string; + let expected: string; + try { + actual = fsRealpathSync(path.resolve(toplevel)); + expected = fsRealpathSync(path.resolve(cwd)); + } catch (err: unknown) { + const code = (err as NodeJS.ErrnoException).code ?? 'UNKNOWN'; + throw new StateBackendUncertaintyError( + operation, + `repository identity unresolvable: realpath failed (${code})`, + ); + } + + // Case-insensitive compare on Windows/macOS; git and Node can disagree on drive-letter case. + const same = process.platform === 'linux' + ? actual === expected + : actual.toLowerCase() === expected.toLowerCase(); + + if (!same) { + throw new StateBackendUncertaintyError( + operation, + 'repository identity mismatch: the target directory is not the root of its own repository ' + + '(it resolves to an enclosing repository), so the create would escape its intended scope', + ); + } +} + // ── Optimistic concurrency (compare-and-swap) ─────────────────────── /** Maximum CAS retry attempts before surfacing as concurrency error. */ @@ -324,6 +396,45 @@ export class WorktreeBackend implements StateBackend { const key = normalizeKey(relativePath); storage.writeSync(path.join(this.root, key), content); } + createIfAbsent(relativePath: string, content: string): void { + const key = normalizeKey(relativePath); + const fullPath = path.join(this.root, key); + // Ensure parent directory before attempting exclusive open. + try { + fsMkdirSync(path.dirname(fullPath), { recursive: true }); + } catch (mkdirErr: unknown) { + throw new StateBackendUncertaintyError( + 'local:createIfAbsent', + `mkdir failed for "${key}": ${(mkdirErr as NodeJS.ErrnoException).code ?? 'UNKNOWN'}`, + ); + } + // 'wx' = O_WRONLY | O_CREAT | O_EXCL — atomic on POSIX and Windows. + let fd: number; + try { + fd = openSync(fullPath, 'wx'); + } catch (openErr: unknown) { + const code = (openErr as NodeJS.ErrnoException).code; + if (code === 'EEXIST') throw new StateKeyConflictError(key); + throw new StateBackendUncertaintyError( + 'local:createIfAbsent', + `exclusive open failed for "${key}": ${code ?? 'UNKNOWN'}`, + ); + } + try { + const buf = Buffer.from(content, 'utf-8'); + fsWriteSync(fd, buf); + } catch (writeErr: unknown) { + // Write failed after exclusive open: clean up the empty file so the key + // does not appear to exist with partial/empty content. + try { fsUnlinkSync(fullPath); } catch { /* best-effort cleanup */ } + throw new StateBackendUncertaintyError( + 'local:createIfAbsent', + `write failed after exclusive open for "${key}": ${(writeErr as NodeJS.ErrnoException).code ?? 'UNKNOWN'}`, + ); + } finally { + try { closeSync(fd); } catch { /* best-effort */ } + } + } exists(relativePath: string): boolean { const key = normalizeKey(relativePath); return storage.existsSync(path.join(this.root, key)); @@ -541,6 +652,53 @@ export class GitNotesBackend implements StateBackend { }); }, `git-notes:append(${relativePath})`); } + + createIfAbsent(relativePath: string, content: string): void { + const key = normalizeKey(relativePath); + // A conflict is a normal, expected outcome under contention — not an + // infrastructure fault. It is reported out of the breaker as a value so + // repeated legitimate conflicts cannot trip the circuit and degrade a + // typed conflict into a generic "circuit OPEN" error. + const conflict = this.breaker.execute(() => { + // Verify the git repo is still accessible AND that this backend is scoped + // to its own repository root before starting. + assertRepositoryIdentity(this.cwd, `git-notes:createIfAbsent(${key})`); + + let lastStderr = ''; + for (let attempt = 0; attempt < CAS_MAX_ATTEMPTS; attempt++) { + // (1) Snapshot the ref and read the blob at THAT exact snapshot. + const oldRefSha = this.readNotesRef(); + const blob = this.loadBlobAt(oldRefSha); + + // (2) Check key existence against the snapshot, not the live tip. + if (Object.hasOwn(blob, key)) return true; + + // (3) Write the key and attempt atomic CAS. + blob[key] = content; + const writeResult = this.atomicSaveBlob(blob, oldRefSha); + if (writeResult.ok) return false; + + // (4) CAS lost. Re-read at the new tip to distinguish conflict from race. + lastStderr = writeResult.stderr; + const newRefSha = this.readNotesRef(); + const newBlob = this.loadBlobAt(newRefSha); + if (Object.hasOwn(newBlob, key)) { + // Someone else created the key between our read and CAS. + return true; + } + // Key still absent after CAS loss — another writer changed something + // else. Backoff and retry. + if (attempt < CAS_MAX_ATTEMPTS - 1) sleepSync(jitteredBackoffMs(attempt)); + } + // All retries exhausted and key is still absent — uncertain outcome. + throw new StateBackendUncertaintyError( + `git-notes:createIfAbsent(${key})`, + `CAS retry exhausted (${CAS_MAX_ATTEMPTS} attempts): ${lastStderr || 'ref moved between read and write'}`, + ); + }, `git-notes:createIfAbsent(${relativePath})`); + + if (conflict) throw new StateKeyConflictError(key); + } } export class OrphanBranchBackend implements StateBackend { @@ -705,6 +863,89 @@ export class OrphanBranchBackend implements StateBackend { this.write(relativePath, existing + content); } + createIfAbsent(relativePath: string, content: string): void { + const conflictKey = normalizeKey(relativePath); + // Conflicts are expected under contention and are returned as a value, not + // thrown, so they never count as circuit-breaker failures. + const conflict = this.breaker.execute(() => { + const key = conflictKey; + // Verify repository is accessible AND that this backend is scoped to its + // own repository root — never an enclosing repository. + assertRepositoryIdentity(this.cwd, `orphan:createIfAbsent(${key})`); + + this.ensureBranch(); + + // Hash the content blob once outside the CAS loop. + let blobHash: string; + try { + blobHash = gitExecWithInputAndRetry(['hash-object', '-w', '--stdin'], this.cwd, content); + } catch (err: unknown) { + const msg = err instanceof Error ? err.message : String(err); + throw new StateBackendUncertaintyError( + `orphan:createIfAbsent(${key})`, + `failed to hash content: ${msg}`, + ); + } + + const segments = key.split('/'); + let lastStderr = ''; + + for (let attempt = 0; attempt < CAS_MAX_ATTEMPTS; attempt++) { + // (1) Snapshot the current branch head. + const parentCommit = gitExecMaybeMissing(['rev-parse', '--verify', `refs/heads/${this.branch}`], this.cwd); + + // (2) Check key existence at this exact snapshot. + const existsAtSnapshot = parentCommit + ? gitExecMaybeMissing(['cat-file', '-t', `${parentCommit}:${key}`], this.cwd) !== null + : false; + if (existsAtSnapshot) return true; + + // (3) Get the current tree. + let currentTree: string; + if (parentCommit) { + const treeResult = gitExecMaybeMissing(['rev-parse', `${parentCommit}^{tree}`], this.cwd); + currentTree = treeResult ?? gitExecWithInputAndRetry(['mktree'], this.cwd, ''); + } else { + try { currentTree = gitExecWithInputAndRetry(['mktree'], this.cwd, ''); } + catch (err: unknown) { + const msg = err instanceof Error ? err.message : String(err); + throw new StateBackendUncertaintyError(`orphan:createIfAbsent(${key})`, `mktree failed: ${msg}`); + } + } + + // (4) Build new tree with key added. + const newTree = this.updateTree(currentTree, segments, blobHash); + let newCommit: string; + try { + const parentArgs = parentCommit ? ['-p', parentCommit] : []; + newCommit = gitExecWithRetry(['commit-tree', newTree, ...parentArgs, '-m', `Create-if-absent ${key}`], this.cwd); + } catch (err: unknown) { + const msg = err instanceof Error ? err.message : String(err); + throw new StateBackendUncertaintyError(`orphan:createIfAbsent(${key})`, `commit-tree failed: ${msg}`); + } + + // (5) CAS: update-ref expected=snapshot. + const writeResult = tryUpdateRef(`refs/heads/${this.branch}`, newCommit, parentCommit, this.cwd); + if (writeResult.ok) return false; + + // (6) CAS lost — re-check if key now exists. + lastStderr = writeResult.stderr; + const newParent = gitExecMaybeMissing(['rev-parse', '--verify', `refs/heads/${this.branch}`], this.cwd); + if (newParent && gitExecMaybeMissing(['cat-file', '-t', `${newParent}:${key}`], this.cwd) !== null) { + // A concurrent writer created the key. + return true; + } + if (attempt < CAS_MAX_ATTEMPTS - 1) sleepSync(jitteredBackoffMs(attempt)); + } + throw new StateBackendUncertaintyError( + `orphan:createIfAbsent(${conflictKey})`, + `CAS retry exhausted (${CAS_MAX_ATTEMPTS} attempts): ${lastStderr || 'ref moved between read and write'}`, + ); + }, `orphan:createIfAbsent(${relativePath})`); + + if (conflict) throw new StateKeyConflictError(conflictKey); + } + private removeFromTree(baseTree: string, pathSegments: string[]): string { if (pathSegments.length === 0) throw new Error('orphan backend: empty path segments'); if (pathSegments.length === 1) { @@ -798,6 +1039,9 @@ export class StateBackendStorageAdapter implements StorageProvider { constructor(private backend: StateBackend, private squadDir: string) {} // ── Async operations ───────────────────────────────────────────────────── + async createIfAbsent(filePath: string, data: string): Promise { + this.backend.createIfAbsent(this.toRelative(filePath), data); + } async read(filePath: string): Promise { return this.backend.read(this.toRelative(filePath)); } @@ -993,7 +1237,37 @@ export class TwoLayerBackend implements StateBackend { } /** - * Read a single git-notes payload as parsed JSON. + * Atomically create a key only when absent across both layers. + * + * **Fail-closed contract for disagreement and failure:** + * - If orphan already has the key → {@link StateKeyConflictError}. + * - If orphan createIfAbsent succeeds but notes disagrees (reports conflict or + * any other error) → {@link StateBackendUncertaintyError}. The caller must + * NOT assume success and should investigate before retrying. + * + * Rationale: a two-layer disagreement indicates stale cross-layer state that + * cannot be resolved without manual inspection. Silently returning success + * would allow multiple creators to each believe they won. + */ + createIfAbsent(key: string, value: string): void { + // Step 1: attempt atomic create in the authoritative orphan layer. + // Throws StateKeyConflictError or StateBackendUncertaintyError on failure. + this.orphan.createIfAbsent(key, value); + + // Step 2: attempt create in the notes annotation layer (fail-closed). + try { + this.notes.createIfAbsent(key, value); + } catch (notesErr: unknown) { + // Any notes failure after orphan success is a disagreement — fail closed. + const msg = notesErr instanceof Error ? notesErr.message : String(notesErr); + throw new StateBackendUncertaintyError( + `two-layer:createIfAbsent(${key})`, + `orphan succeeded but notes layer failed: ${msg}`, + ); + } + } + + /** * * Returns `null` if no note exists on the given commit for the given ref, * or if the note body is not valid JSON. diff --git a/packages/squad-sdk/src/storage/fs-storage-provider.ts b/packages/squad-sdk/src/storage/fs-storage-provider.ts index 765fa69fa..be3103521 100644 --- a/packages/squad-sdk/src/storage/fs-storage-provider.ts +++ b/packages/squad-sdk/src/storage/fs-storage-provider.ts @@ -1,8 +1,8 @@ -import { readFile, writeFile, appendFile, access, readdir, unlink, mkdir as fsMkdir, realpath, rm, stat as fsStat, rename as fsRename, copyFile } from 'fs/promises'; +import { readFile, writeFile, appendFile, access, readdir, unlink, mkdir as fsMkdir, realpath, rm, stat as fsStat, rename as fsRename, copyFile, open } from 'fs/promises'; import { readFileSync, writeFileSync, appendFileSync, existsSync as fsExistsSync, mkdirSync as fsMkdirSync, realpathSync, readdirSync, statSync, unlinkSync, renameSync as fsRenameSync, copyFileSync, rmSync } from 'fs'; import { dirname, resolve, sep } from 'path'; import type { StorageProvider, StorageStats } from './storage-provider.js'; -import { StorageError } from './storage-error.js'; +import { StorageError, StateKeyConflictError, StateBackendUncertaintyError } from './storage-error.js'; /** * FSStorageProvider — Node.js `fs` implementation of StorageProvider. @@ -127,6 +127,49 @@ export class FSStorageProvider implements StorageProvider { } } + async createIfAbsent(filePath: string, data: string): Promise { + const safePath = await this.assertSafePath(filePath); + // Create parent dirs before attempting exclusive open so mkdir failure + // is distinguished from the key-already-exists conflict. + try { + await fsMkdir(dirname(safePath), { recursive: true }); + } catch (mkdirErr: unknown) { + throw new StateBackendUncertaintyError( + 'createIfAbsent', + `mkdir failed for "${filePath}": ${(mkdirErr as NodeJS.ErrnoException).code ?? 'UNKNOWN'}`, + ); + } + let fh: import('fs/promises').FileHandle | undefined; + try { + // 'wx' = O_WRONLY | O_CREAT | O_EXCL — fails atomically with EEXIST + // if the file already exists, with no race between check and create. + fh = await open(safePath, 'wx'); + } catch (openErr: unknown) { + const code = (openErr as NodeJS.ErrnoException).code; + if (code === 'EEXIST') { + throw new StateKeyConflictError(filePath); + } + throw new StateBackendUncertaintyError( + 'createIfAbsent', + `exclusive open failed for "${filePath}": ${code ?? 'UNKNOWN'}`, + ); + } + try { + await fh.writeFile(data, 'utf-8'); + } catch (writeErr: unknown) { + // Write failed after exclusive open: state is uncertain (file exists but + // may be empty or partial). Attempt cleanup; if cleanup fails, the + // uncertainty persists but we still surface the original error. + try { await unlink(safePath); } catch { /* best-effort cleanup */ } + throw new StateBackendUncertaintyError( + 'createIfAbsent', + `write failed after exclusive open for "${filePath}": ${(writeErr as NodeJS.ErrnoException).code ?? 'UNKNOWN'}`, + ); + } finally { + try { await fh.close(); } catch { /* best-effort */ } + } + } + async read(filePath: string): Promise { const safePath = await this.assertSafePath(filePath); try { diff --git a/packages/squad-sdk/src/storage/in-memory-storage-provider.ts b/packages/squad-sdk/src/storage/in-memory-storage-provider.ts index 80e6025ab..0053249cb 100644 --- a/packages/squad-sdk/src/storage/in-memory-storage-provider.ts +++ b/packages/squad-sdk/src/storage/in-memory-storage-provider.ts @@ -1,5 +1,6 @@ import { posix } from 'path'; import type { StorageProvider, StorageStats } from './storage-provider.js'; +import { StateKeyConflictError } from './storage-error.js'; /** * InMemoryStorageProvider — test-friendly StorageProvider backed by a Map. @@ -18,6 +19,17 @@ export class InMemoryStorageProvider implements StorageProvider { return posix.normalize(p.replace(/\\/g, '/')).replace(/\/+$/, ''); } + async createIfAbsent(filePath: string, data: string): Promise { + const key = this.norm(filePath); + // Single-process: Map.has check and set are effectively atomic within + // a single event-loop turn (no async I/O between check and write). + if (this.files.has(key)) { + throw new StateKeyConflictError(filePath); + } + this.files.set(key, data); + this.mtimes.set(key, Date.now()); + } + async read(filePath: string): Promise { return this.readSync(filePath); } diff --git a/packages/squad-sdk/src/storage/index.ts b/packages/squad-sdk/src/storage/index.ts index 026726286..829673d64 100644 --- a/packages/squad-sdk/src/storage/index.ts +++ b/packages/squad-sdk/src/storage/index.ts @@ -2,4 +2,4 @@ export type { StorageProvider, StorageStats } from './storage-provider.js'; export { FSStorageProvider } from './fs-storage-provider.js'; export { InMemoryStorageProvider } from './in-memory-storage-provider.js'; export { SQLiteStorageProvider } from './sqlite-storage-provider.js'; -export { StorageError } from './storage-error.js'; +export { StorageError, StateKeyConflictError, StateBackendUncertaintyError } from './storage-error.js'; diff --git a/packages/squad-sdk/src/storage/sqlite-storage-provider.ts b/packages/squad-sdk/src/storage/sqlite-storage-provider.ts index a58178127..f5eb59284 100644 --- a/packages/squad-sdk/src/storage/sqlite-storage-provider.ts +++ b/packages/squad-sdk/src/storage/sqlite-storage-provider.ts @@ -2,6 +2,7 @@ import { posix } from 'path'; import { readFileSync, writeFileSync, existsSync, mkdirSync as fsMkdirSync, renameSync } from 'fs'; import { dirname } from 'path'; import type { StorageProvider, StorageStats } from './storage-provider.js'; +import { StateKeyConflictError } from './storage-error.js'; // sql.js types — loaded dynamically type SqlJsStatic = typeof import('sql.js'); @@ -137,6 +138,20 @@ export class SQLiteStorageProvider implements StorageProvider { // ── Async interface ───────────────────────────────────────────────────── + async createIfAbsent(filePath: string, data: string): Promise { + const db = await this.ready(); + const key = this.norm(filePath); + // INSERT OR IGNORE: if row already exists, no rows are modified. + db.run( + `INSERT OR IGNORE INTO files (path, content, updated_at) VALUES (?, ?, ?)`, + [key, data, this.now()], + ); + if (db.getRowsModified() === 0) { + throw new StateKeyConflictError(filePath); + } + this.persist(); + } + async read(filePath: string): Promise { await this.ready(); return this.readSync(filePath); diff --git a/packages/squad-sdk/src/storage/storage-error.ts b/packages/squad-sdk/src/storage/storage-error.ts index 42ea82e79..580e321b6 100644 --- a/packages/squad-sdk/src/storage/storage-error.ts +++ b/packages/squad-sdk/src/storage/storage-error.ts @@ -1,5 +1,48 @@ import { basename } from 'path'; +/** + * Thrown by `createIfAbsent` when the key already exists in the target backend. + * + * Exactly one concurrent creator receives `void`; every other concurrent caller + * receives this error. Content under the key is never overwritten. + * + * @example + * ```ts + * try { + * await storage.createIfAbsent('sessions/retro.md', content); + * } catch (err) { + * if (err instanceof StateKeyConflictError) { + * // Another process already created this key; read the winner's content. + * } + * } + * ``` + */ +export class StateKeyConflictError extends Error { + readonly name = 'StateKeyConflictError'; + constructor(public readonly key: string) { + super(`State key already exists: ${key}`); + } +} + +/** + * Thrown by `createIfAbsent` when the backend cannot determine with certainty + * whether the key was created or not (e.g. write failed after exclusive open, + * or CAS retry exhausted with key still absent). + * + * Distinct from {@link StateKeyConflictError}: that error means the key + * definitely existed; this error means the outcome is unknown. Callers should + * NOT assume success and should treat the operation as failed. + */ +export class StateBackendUncertaintyError extends Error { + readonly name = 'StateBackendUncertaintyError'; + constructor( + public readonly operation: string, + public readonly reason: string, + ) { + super(`State backend uncertainty on '${operation}': ${reason}`); + } +} + /** * Sanitized storage error that strips internal filesystem paths from error messages. * diff --git a/packages/squad-sdk/src/storage/storage-provider.ts b/packages/squad-sdk/src/storage/storage-provider.ts index 0a488c8b2..68347297c 100644 --- a/packages/squad-sdk/src/storage/storage-provider.ts +++ b/packages/squad-sdk/src/storage/storage-provider.ts @@ -13,6 +13,10 @@ * SQLiteStorageProvider throw plain `Error` for invalid operations. Callers * that need provider-agnostic error handling should catch `Error` and inspect * `.code` only when the value is a `StorageError`. + * + * **Atomic create:** `createIfAbsent` creates a key only when absent and + * throws `StateKeyConflictError` (key already exists) or + * `StateBackendUncertaintyError` (outcome unknown). It never overwrites. */ /** Metadata returned by stat(). */ @@ -26,6 +30,24 @@ export interface StorageStats { } export interface StorageProvider { + /** + * Atomically create a file with the given data only if it does not already + * exist. Resolves with `void` on success (this caller is the sole creator). + * + * Throws {@link StateKeyConflictError} if the file already exists. + * Throws {@link StateBackendUncertaintyError} if the outcome cannot be + * determined (e.g. write failed after exclusive open, lock lost after CAS). + * + * **Never** overwrites existing content. Unconditional writes continue to + * use `write()` as before. + * + * Repository scope is verified by the provider at construction time (rootDir + * for FSStorageProvider; git repository root for git-backed providers). + * Operations against an ambiguous or inaccessible repository fail with + * `StateBackendUncertaintyError`. + */ + createIfAbsent(filePath: string, data: string): Promise; + /** * Read the full contents of a file as a UTF-8 string. * Returns `undefined` if the file does not exist (ENOENT). diff --git a/packages/squad-sdk/src/tools/index.ts b/packages/squad-sdk/src/tools/index.ts index 817eccd9a..3f0723d63 100644 --- a/packages/squad-sdk/src/tools/index.ts +++ b/packages/squad-sdk/src/tools/index.ts @@ -16,6 +16,7 @@ import type { SquadTool, SquadToolResult } from '../adapter/types.js'; import { trace, SpanStatusCode } from '../runtime/otel-api.js'; import type { StorageProvider } from '../storage/storage-provider.js'; import { FSStorageProvider } from '../storage/fs-storage-provider.js'; +import { StateKeyConflictError, StateBackendUncertaintyError } from '../storage/storage-error.js'; import type { SquadState } from '../state/squad-state.js'; import { validateStateKey } from '../state-backend.js'; import { spawnParallel, type FanOutDependencies } from '../coordinator/fan-out.js'; @@ -112,6 +113,11 @@ export interface StateListRequest { dir?: string; } +export interface StateCreateIfAbsentRequest { + key: string; + content: string; +} + export interface StatusQuery { /** Filter by agent name */ agentName?: string; @@ -1203,6 +1209,67 @@ export class ToolRegistry { }, }); + // squad_state_create_if_absent: Atomic create-if-absent + const stateCreateIfAbsent = defineTool({ + name: 'squad_state_create_if_absent', + description: [ + 'Atomically create a mutable Squad state key only when it does not already exist.', + 'Returns success to exactly one concurrent creator; all others receive a conflict error.', + 'Never overwrites existing content.', + 'Throws a typed conflict when the key already exists and a typed uncertainty error when the', + 'outcome cannot be determined. Use squad_state_write for unconditional writes.', + 'Keys are relative to .squad/; only mutable state keys are permitted.', + ].join(' '), + parameters: { + type: 'object', + properties: { + key: { type: 'string', description: 'State key relative to .squad/' }, + content: { type: 'string', description: 'Content to store if the key is absent' }, + }, + required: ['key', 'content'], + }, + handler: async (args) => { + if ((args as unknown as Record)['content'] == null || + typeof (args as unknown as Record)['content'] !== 'string') { + return { + textResultForLlm: 'Failed to create state: content is required and must be a string', + resultType: 'failure' as const, + error: 'content is required', + }; + } + try { + const key = normalizeStateToolKey(args.key); + validateMutableStateToolKey(key); + await this.storage.createIfAbsent(path.join(this.squadRoot, key), args.content); + return { + textResultForLlm: `State created: ${key}`, + resultType: 'success', + toolTelemetry: { key }, + }; + } catch (error) { + if (error instanceof StateKeyConflictError) { + return { + textResultForLlm: `State key already exists (conflict): ${sanitizeErrorForLlm(error, this.squadRoot)}`, + resultType: 'failure', + error: 'conflict', + }; + } + if (error instanceof StateBackendUncertaintyError) { + return { + textResultForLlm: `State create outcome uncertain: ${sanitizeErrorForLlm(error, this.squadRoot)}`, + resultType: 'failure', + error: 'uncertainty', + }; + } + return { + textResultForLlm: `Failed to create state: ${sanitizeErrorForLlm(error, this.squadRoot)}`, + resultType: 'failure', + error: String(error), + }; + } + }, + }); + // Register all tools this.tools.set('squad_route', squadRoute); this.tools.set('squad_decide', squadDecide); @@ -1212,6 +1279,7 @@ export class ToolRegistry { this.tools.set('squad_state_append', stateAppend); this.tools.set('squad_state_delete', stateDelete); this.tools.set('squad_state_list', stateList); + this.tools.set('squad_state_create_if_absent', stateCreateIfAbsent); this.tools.set('squad_state_health', stateHealth); this.tools.set('memory.classify', memoryClassify); this.tools.set('memory.write', memoryWrite); diff --git a/packages/squad-sdk/templates/scribe-charter.md b/packages/squad-sdk/templates/scribe-charter.md index 0300782c3..a6ff283cb 100644 --- a/packages/squad-sdk/templates/scribe-charter.md +++ b/packages/squad-sdk/templates/scribe-charter.md @@ -77,7 +77,14 @@ perform these checks, **stop and report** rather than proceeding with an unverif **Worktree awareness:** Use the `TEAM ROOT` provided in the spawn prompt to resolve all `.squad/` paths. If no TEAM ROOT is given, run `git rev-parse --show-toplevel` as fallback. Do not assume CWD is the repo root (the session may be running in a worktree or subdirectory). -**State backend awareness:** Check `STATE_BACKEND` from the spawn prompt. Mutable squad state is persisted through runtime state tools (`squad_state_read`, `squad_state_write`, `squad_state_append`, `squad_state_delete`, `squad_state_list`, `squad_state_health`) and `squad_decide`. Do not run backend git commands, switch to state branches, push note refs, reset `.squad/`, or commit mutable state by hand. If state tools are unavailable, stop without mutating files or git state and record the tool availability failure in your final summary. +**State backend awareness:** Check `STATE_BACKEND` from the spawn prompt. Mutable squad state is persisted through runtime state tools (`squad_state_read`, `squad_state_write`, `squad_state_append`, `squad_state_create_if_absent`, `squad_state_delete`, `squad_state_list`, `squad_state_health`) and `squad_decide`. Do not run backend git commands, switch to state branches, push note refs, reset `.squad/`, or commit mutable state by hand. If state tools are unavailable, stop without mutating files or git state and record the tool availability failure in your final summary. + +**Exclusive canonical artifacts:** When exactly one canonical artifact must exist — a retrospective, a session log, a claim marker — create it with `squad_state_create_if_absent`, never `squad_state_write`. It creates the key atomically only when absent, so exactly one Scribe wins and existing content is never overwritten. Handle its two failure shapes explicitly: + +- `error: "conflict"` — another Scribe already created the canonical artifact. Do **not** retry as a create and do **not** overwrite. Read the winner's content with `squad_state_read` and append to it if you have something to add. +- `error: "uncertainty"` — the outcome is unknown. Do **not** assume success and do **not** write over the key. Stop and report the uncertainty in your final summary. + +Never emulate this with `squad_state_read` followed by `squad_state_write`; that check-then-write pattern is racy and silently destroys a concurrent Scribe's canonical artifact. After every substantial work session: diff --git a/packages/squad-sdk/templates/skills/coordinator-source-of-truth/SKILL.md b/packages/squad-sdk/templates/skills/coordinator-source-of-truth/SKILL.md index 3a992ba0a..b6948cb9b 100644 --- a/packages/squad-sdk/templates/skills/coordinator-source-of-truth/SKILL.md +++ b/packages/squad-sdk/templates/skills/coordinator-source-of-truth/SKILL.md @@ -11,7 +11,7 @@ source: "Extracted from squad.agent.md as part of the slimming effort (bradygast ## State backend note -Files below marked as **"Derived / append-only"** are **mutable state** — agents access them with runtime state tools (`squad_state_read`, `squad_state_write`, `squad_state_append`, `squad_state_delete`, `squad_state_list`). The runtime decides whether the configured backend stores them on disk, git-native state, or an external provider. Files marked as **"Authoritative"** are **static config** and always live on disk regardless of backend. +Files below marked as **"Derived / append-only"** are **mutable state** — agents access them with runtime state tools (`squad_state_read`, `squad_state_write`, `squad_state_append`, `squad_state_create_if_absent`, `squad_state_delete`, `squad_state_list`). The runtime decides whether the configured backend stores them on disk, git-native state, or an external provider. Files marked as **"Authoritative"** are **static config** and always live on disk regardless of backend. ## File hierarchy diff --git a/packages/squad-sdk/templates/spawn-reference.md b/packages/squad-sdk/templates/spawn-reference.md index f036d1216..56c362a7a 100644 --- a/packages/squad-sdk/templates/spawn-reference.md +++ b/packages/squad-sdk/templates/spawn-reference.md @@ -123,6 +123,11 @@ prompt: | whenever they are available: - `squad_state_read` / `squad_state_list` for decisions, history, logs, and inbox entries - `squad_state_write` / `squad_state_append` for durable updates + - `squad_state_create_if_absent` when exactly one canonical artifact must exist + (retrospectives, session logs, claim markers). It creates the key atomically + only when absent and never overwrites; on `error: "conflict"` another agent + already won, and on `error: "uncertainty"` the outcome is unknown — do not + assume success and do not overwrite in either case. - `squad_state_delete` after Scribe merges inbox entries - `squad_state_health` when diagnosing backend availability - `squad_decide` for team-relevant decisions diff --git a/samples/storage-provider-azure/azure-blob-storage-provider.ts b/samples/storage-provider-azure/azure-blob-storage-provider.ts index db61b240d..3fb6c6144 100644 --- a/samples/storage-provider-azure/azure-blob-storage-provider.ts +++ b/samples/storage-provider-azure/azure-blob-storage-provider.ts @@ -1,5 +1,6 @@ import { ContainerClient } from '@azure/storage-blob'; import type { StorageProvider, StorageStats } from '@bradygaster/squad-sdk'; +import { StateKeyConflictError, StateBackendUncertaintyError } from '@bradygaster/squad-sdk'; /** * Azure Blob Storage implementation of StorageProvider. @@ -31,7 +32,7 @@ export class AzureBlobStorageProvider implements StorageProvider { return norm.endsWith('/') ? norm : `${norm}/`; } - // ── Async methods (12) ──────────────────────────────────────────────── + // ── Async methods (13) ──────────────────────────────────────────────── async read(filePath: string): Promise { const blobName = this.normalizePath(filePath); @@ -67,6 +68,36 @@ export class AzureBlobStorageProvider implements StorageProvider { await this.write(filePath, existing + data); } + /** + * Atomically create a blob only when absent. + * + * Uses the Azure `If-None-Match: *` conditional header, which makes the + * service reject the upload with HTTP 409 (BlobAlreadyExists) / 412 when + * the blob already exists. Exactly one concurrent creator wins; existing + * content is never overwritten. A read-then-write check would be racy. + */ + async createIfAbsent(filePath: string, data: string): Promise { + const blobName = this.normalizePath(filePath); + const blob = this.container.getBlockBlobClient(blobName); + const buffer = Buffer.from(data, 'utf-8'); + + try { + await blob.upload(buffer, buffer.length, { + blobHTTPHeaders: { blobContentType: 'text/plain; charset=utf-8' }, + conditions: { ifNoneMatch: '*' }, + }); + } catch (err: any) { + if (err.statusCode === 409 || err.statusCode === 412) { + throw new StateKeyConflictError(filePath); + } + // Any other outcome is unknown — the upload may or may not have landed. + throw new StateBackendUncertaintyError( + 'azure-blob:createIfAbsent', + `conditional upload failed for "${filePath}": ${err?.statusCode ?? 'UNKNOWN'}`, + ); + } + } + async exists(filePath: string): Promise { const blobName = this.normalizePath(filePath); const blob = this.container.getBlobClient(blobName); diff --git a/templates/scribe-charter.md b/templates/scribe-charter.md index 0300782c3..a6ff283cb 100644 --- a/templates/scribe-charter.md +++ b/templates/scribe-charter.md @@ -77,7 +77,14 @@ perform these checks, **stop and report** rather than proceeding with an unverif **Worktree awareness:** Use the `TEAM ROOT` provided in the spawn prompt to resolve all `.squad/` paths. If no TEAM ROOT is given, run `git rev-parse --show-toplevel` as fallback. Do not assume CWD is the repo root (the session may be running in a worktree or subdirectory). -**State backend awareness:** Check `STATE_BACKEND` from the spawn prompt. Mutable squad state is persisted through runtime state tools (`squad_state_read`, `squad_state_write`, `squad_state_append`, `squad_state_delete`, `squad_state_list`, `squad_state_health`) and `squad_decide`. Do not run backend git commands, switch to state branches, push note refs, reset `.squad/`, or commit mutable state by hand. If state tools are unavailable, stop without mutating files or git state and record the tool availability failure in your final summary. +**State backend awareness:** Check `STATE_BACKEND` from the spawn prompt. Mutable squad state is persisted through runtime state tools (`squad_state_read`, `squad_state_write`, `squad_state_append`, `squad_state_create_if_absent`, `squad_state_delete`, `squad_state_list`, `squad_state_health`) and `squad_decide`. Do not run backend git commands, switch to state branches, push note refs, reset `.squad/`, or commit mutable state by hand. If state tools are unavailable, stop without mutating files or git state and record the tool availability failure in your final summary. + +**Exclusive canonical artifacts:** When exactly one canonical artifact must exist — a retrospective, a session log, a claim marker — create it with `squad_state_create_if_absent`, never `squad_state_write`. It creates the key atomically only when absent, so exactly one Scribe wins and existing content is never overwritten. Handle its two failure shapes explicitly: + +- `error: "conflict"` — another Scribe already created the canonical artifact. Do **not** retry as a create and do **not** overwrite. Read the winner's content with `squad_state_read` and append to it if you have something to add. +- `error: "uncertainty"` — the outcome is unknown. Do **not** assume success and do **not** write over the key. Stop and report the uncertainty in your final summary. + +Never emulate this with `squad_state_read` followed by `squad_state_write`; that check-then-write pattern is racy and silently destroys a concurrent Scribe's canonical artifact. After every substantial work session: diff --git a/templates/spawn-reference.md b/templates/spawn-reference.md index f036d1216..56c362a7a 100644 --- a/templates/spawn-reference.md +++ b/templates/spawn-reference.md @@ -123,6 +123,11 @@ prompt: | whenever they are available: - `squad_state_read` / `squad_state_list` for decisions, history, logs, and inbox entries - `squad_state_write` / `squad_state_append` for durable updates + - `squad_state_create_if_absent` when exactly one canonical artifact must exist + (retrospectives, session logs, claim markers). It creates the key atomically + only when absent and never overwrites; on `error: "conflict"` another agent + already won, and on `error: "uncertainty"` the outcome is unknown — do not + assume success and do not overwrite in either case. - `squad_state_delete` after Scribe merges inbox entries - `squad_state_health` when diagnosing backend availability - `squad_decide` for team-relevant decisions diff --git a/test/cli/state-mcp.test.ts b/test/cli/state-mcp.test.ts index 84db6868d..2a1a7ebfc 100644 --- a/test/cli/state-mcp.test.ts +++ b/test/cli/state-mcp.test.ts @@ -59,9 +59,56 @@ describe('state-mcp bridge', () => { expect(names).toContain('squad_decide'); expect(names).toContain('squad_state_write'); expect(names).toContain('squad_state_append'); + expect(names).toContain('squad_state_create_if_absent'); expect(tools.find(tool => tool.name === 'squad_state_write')?.inputSchema.required).toEqual(['key', 'content']); + expect(tools.find(tool => tool.name === 'squad_state_create_if_absent')?.inputSchema.required) + .toEqual(['key', 'content']); }); + it.each(['orphan', 'two-layer'] as const)( + 'exposes squad_state_create_if_absent so exactly one caller creates a canonical key through the %s backend', + async (stateBackend) => { + initSquad(stateBackend); + const messages: JsonRpcMessage[] = []; + const session = createStateMcpSession(TMP, message => messages.push(message as JsonRpcMessage)); + const key = 'log/2026-08-29T00-00-00Z-retrospective.md'; + + async function createIfAbsent(id: string, content: string): Promise> { + const index = messages.length; + await session.handleRequest({ + jsonrpc: '2.0', + id, + method: 'tools/call', + params: { name: 'squad_state_create_if_absent', arguments: { key, content } }, + }); + return resultAsRecord(messages[index]!); + } + + const first = await createIfAbsent('create-1', '# Canonical retro\n'); + const second = await createIfAbsent('create-2', '# Duplicate retro\n'); + + // Exactly one creator wins; the loser is surfaced as an MCP error result. + expect(first['isError']).not.toBe(true); + expect(second['isError']).toBe(true); + const loserText = (second['content'] as Array<{ text: string }>)[0]!.text; + expect(loserText).toMatch(/already exists/i); + + // The winner's content is preserved verbatim — never overwritten. + const readIndex = messages.length; + await session.handleRequest({ + jsonrpc: '2.0', + id: 'read-canonical', + method: 'tools/call', + params: { name: 'squad_state_read', arguments: { key } }, + }); + expect(resultAsRecord(messages[readIndex]!)['content']) + .toEqual([{ type: 'text', text: '# Canonical retro\n' }]); + + // Mutable state never leaks into the worktree for git-native backends. + expect(existsSync(join(TMP, '.squad', 'log', '2026-08-29T00-00-00Z-retrospective.md'))).toBe(false); + }, + ); + it('writes and reads two-layer state without mutating the worktree .squad files', async () => { initSquad('two-layer'); const messages: JsonRpcMessage[] = []; diff --git a/test/state-backend-create-if-absent.test.ts b/test/state-backend-create-if-absent.test.ts new file mode 100644 index 000000000..05c348c7f --- /dev/null +++ b/test/state-backend-create-if-absent.test.ts @@ -0,0 +1,555 @@ +/** + * Deterministic tests for createIfAbsent across all state backends. + * + * Requirements verified: + * - Exactly one concurrent creator succeeds; all others receive StateKeyConflictError. + * - Winner content is preserved unchanged. + * - No partial or duplicate state results. + * - StateKeyConflictError on existing key. + * - Repository isolation: operations in different repos/dirs do not conflict. + * - StateBackendUncertaintyError surfaces for two-layer disagreement and backend failure. + * - ToolRegistry exposes squad_state_create_if_absent with correct failure shapes. + * - FSStorageProvider and InMemoryStorageProvider behave correctly. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { mkdirSync, rmSync, writeFileSync, existsSync } from 'node:fs'; +import { join } from 'node:path'; +import { execSync } from 'node:child_process'; +import { randomBytes } from 'node:crypto'; +import { + WorktreeBackend, + GitNotesBackend, + OrphanBranchBackend, + TwoLayerBackend, + StateBackendStorageAdapter, +} from '../packages/squad-sdk/src/state-backend.js'; +import { StateKeyConflictError, StateBackendUncertaintyError } from '../packages/squad-sdk/src/storage/storage-error.js'; +import { FSStorageProvider } from '../packages/squad-sdk/src/storage/fs-storage-provider.js'; +import { InMemoryStorageProvider } from '../packages/squad-sdk/src/storage/in-memory-storage-provider.js'; +import { SQLiteStorageProvider } from '../packages/squad-sdk/src/storage/sqlite-storage-provider.js'; +import { ToolRegistry } from '../packages/squad-sdk/src/tools/index.js'; +import { clearResolveSquadCache } from '../packages/squad-sdk/src/resolution.js'; + +// ── Helpers ───────────────────────────────────────────────────────────────── + +const TMP = join(process.cwd(), `.test-cia-${randomBytes(4).toString('hex')}`); +const TMP2 = join(process.cwd(), `.test-cia2-${randomBytes(4).toString('hex')}`); + +function git(args: string, cwd: string): string { + return execSync(`git ${args}`, { cwd, encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] }).trim(); +} + +function initRepo(dir: string): void { + mkdirSync(dir, { recursive: true }); + git('init', dir); + git('config user.email "test@test.com"', dir); + git('config user.name "Test"', dir); + writeFileSync(join(dir, 'README.md'), '# test\n'); + git('add .', dir); + git('commit -m "init"', dir); +} + +function cleanup(...dirs: string[]): void { + for (const d of dirs) { + if (existsSync(d)) rmSync(d, { recursive: true, force: true }); + } +} + +// ── WorktreeBackend ────────────────────────────────────────────────────────── + +describe('WorktreeBackend.createIfAbsent', () => { + const squadDir = () => join(TMP, '.squad'); + + beforeEach(() => { + cleanup(TMP); + mkdirSync(squadDir(), { recursive: true }); + }); + afterEach(() => { clearResolveSquadCache(); cleanup(TMP); }); + + it('creates the key when absent', () => { + const b = new WorktreeBackend(squadDir()); + b.createIfAbsent('sessions/alpha.md', '# Alpha\n'); + expect(b.read('sessions/alpha.md')).toBe('# Alpha\n'); + }); + + it('throws StateKeyConflictError when key already exists', () => { + const b = new WorktreeBackend(squadDir()); + b.write('sessions/alpha.md', '# Original\n'); + expect(() => b.createIfAbsent('sessions/alpha.md', '# New\n')) + .toThrow(StateKeyConflictError); + // original content preserved + expect(b.read('sessions/alpha.md')).toBe('# Original\n'); + }); + + it('exactly one of two independent instances succeeds (concurrent create)', () => { + const b1 = new WorktreeBackend(squadDir()); + const b2 = new WorktreeBackend(squadDir()); + let successes = 0; + let conflicts = 0; + for (const b of [b1, b2]) { + try { + b.createIfAbsent('sessions/race.md', `writer-${successes + conflicts}\n`); + successes++; + } catch (e) { + if (e instanceof StateKeyConflictError) conflicts++; + else throw e; + } + } + expect(successes).toBe(1); + expect(conflicts).toBe(1); + // key exists with exactly the winner's content + const content = b1.read('sessions/race.md'); + expect(typeof content).toBe('string'); + expect(content).toMatch(/^writer-/); + }); + + it('repository isolation: different squad dirs do not conflict', () => { + const dir2 = join(TMP, '.squad2'); + mkdirSync(dir2, { recursive: true }); + const b1 = new WorktreeBackend(squadDir()); + const b2 = new WorktreeBackend(dir2); + b1.createIfAbsent('sessions/shared.md', 'repo1\n'); + // should not throw in b2 + expect(() => b2.createIfAbsent('sessions/shared.md', 'repo2\n')).not.toThrow(); + expect(b1.read('sessions/shared.md')).toBe('repo1\n'); + expect(b2.read('sessions/shared.md')).toBe('repo2\n'); + }); +}); + +// ── GitNotesBackend ────────────────────────────────────────────────────────── + +describe('GitNotesBackend.createIfAbsent', { timeout: 30_000 }, () => { + beforeEach(() => { cleanup(TMP); initRepo(TMP); }); + afterEach(() => { clearResolveSquadCache(); cleanup(TMP); }); + + it('creates a key when absent', () => { + const b = new GitNotesBackend(TMP); + b.createIfAbsent('sessions/beta.md', '# Beta\n'); + expect(b.read('sessions/beta.md')).toBe('# Beta\n'); + }); + + it('throws StateKeyConflictError when key already exists', () => { + const b = new GitNotesBackend(TMP); + b.write('sessions/beta.md', '# Original\n'); + expect(() => b.createIfAbsent('sessions/beta.md', '# New\n')) + .toThrow(StateKeyConflictError); + expect(b.read('sessions/beta.md')).toBe('# Original\n'); + }); + + it('exactly one of two sequential instances succeeds (concurrent create)', () => { + const b1 = new GitNotesBackend(TMP); + const b2 = new GitNotesBackend(TMP); + let successes = 0; + let conflicts = 0; + for (const b of [b1, b2]) { + try { + b.createIfAbsent('sessions/notes-race.md', `writer-${successes + conflicts}\n`); + successes++; + } catch (e) { + if (e instanceof StateKeyConflictError) conflicts++; + else throw e; + } + } + expect(successes).toBe(1); + expect(conflicts).toBe(1); + const content = b1.read('sessions/notes-race.md'); + expect(typeof content).toBe('string'); + }); + + it('repository isolation: different repos do not conflict', () => { + cleanup(TMP2); + initRepo(TMP2); + const b1 = new GitNotesBackend(TMP); + const b2 = new GitNotesBackend(TMP2); + b1.createIfAbsent('sessions/shared.md', 'repo1\n'); + expect(() => b2.createIfAbsent('sessions/shared.md', 'repo2\n')).not.toThrow(); + expect(b1.read('sessions/shared.md')).toBe('repo1\n'); + expect(b2.read('sessions/shared.md')).toBe('repo2\n'); + cleanup(TMP2); + }); +}); + +// ── OrphanBranchBackend ────────────────────────────────────────────────────── + +describe('OrphanBranchBackend.createIfAbsent', { timeout: 30_000 }, () => { + beforeEach(() => { cleanup(TMP); initRepo(TMP); }); + afterEach(() => { clearResolveSquadCache(); cleanup(TMP); }); + + it('creates a key when absent', () => { + const b = new OrphanBranchBackend(TMP); + b.createIfAbsent('sessions/gamma.md', '# Gamma\n'); + expect(b.read('sessions/gamma.md')).toBe('# Gamma\n'); + }); + + it('throws StateKeyConflictError when key already exists', () => { + const b = new OrphanBranchBackend(TMP); + b.write('sessions/gamma.md', '# Original\n'); + expect(() => b.createIfAbsent('sessions/gamma.md', '# New\n')) + .toThrow(StateKeyConflictError); + expect(b.read('sessions/gamma.md')).toBe('# Original\n'); + }); + + it('exactly one of two sequential instances succeeds (concurrent create)', () => { + const b1 = new OrphanBranchBackend(TMP); + const b2 = new OrphanBranchBackend(TMP); + let successes = 0; + let conflicts = 0; + for (const b of [b1, b2]) { + try { + b.createIfAbsent('sessions/orphan-race.md', `writer-${successes + conflicts}\n`); + successes++; + } catch (e) { + if (e instanceof StateKeyConflictError) conflicts++; + else throw e; + } + } + expect(successes).toBe(1); + expect(conflicts).toBe(1); + const content = b1.read('sessions/orphan-race.md'); + expect(typeof content).toBe('string'); + }); + + it('repository isolation: different repos do not conflict', () => { + cleanup(TMP2); + initRepo(TMP2); + const b1 = new OrphanBranchBackend(TMP); + const b2 = new OrphanBranchBackend(TMP2); + b1.createIfAbsent('sessions/shared.md', 'repo1\n'); + expect(() => b2.createIfAbsent('sessions/shared.md', 'repo2\n')).not.toThrow(); + expect(b1.read('sessions/shared.md')).toBe('repo1\n'); + expect(b2.read('sessions/shared.md')).toBe('repo2\n'); + cleanup(TMP2); + }); + + it('repeated conflicts stay typed and do not trip the circuit breaker', () => { + const b = new OrphanBranchBackend(TMP); + b.createIfAbsent('sessions/hot-key.md', '# Winner\n'); + // CIRCUIT_BREAKER_THRESHOLD is 5; drive well past it with legitimate conflicts. + for (let i = 0; i < 8; i++) { + expect(() => b.createIfAbsent('sessions/hot-key.md', `# Loser ${i}\n`)) + .toThrow(StateKeyConflictError); + } + // The backend is still healthy: an unrelated create still succeeds. + expect(() => b.createIfAbsent('sessions/still-healthy.md', '# Fine\n')).not.toThrow(); + // And the original winner's content was never overwritten. + expect(b.read('sessions/hot-key.md')).toBe('# Winner\n'); + }); +}); + +// ── TwoLayerBackend ────────────────────────────────────────────────────────── + +describe('TwoLayerBackend.createIfAbsent', { timeout: 30_000 }, () => { + beforeEach(() => { cleanup(TMP); initRepo(TMP); }); + afterEach(() => { clearResolveSquadCache(); cleanup(TMP); }); + + it('creates a key when absent (both layers)', () => { + const b = new TwoLayerBackend(TMP); + b.createIfAbsent('sessions/delta.md', '# Delta\n'); + expect(b.read('sessions/delta.md')).toBe('# Delta\n'); + }); + + it('throws StateKeyConflictError when key already exists in orphan layer', () => { + const b = new TwoLayerBackend(TMP); + b.write('sessions/delta.md', '# Original\n'); + expect(() => b.createIfAbsent('sessions/delta.md', '# New\n')) + .toThrow(StateKeyConflictError); + expect(b.read('sessions/delta.md')).toBe('# Original\n'); + }); + + it('exactly one of two sequential instances succeeds (concurrent create)', () => { + const b1 = new TwoLayerBackend(TMP); + const b2 = new TwoLayerBackend(TMP); + let successes = 0; + let conflicts = 0; + for (const b of [b1, b2]) { + try { + b.createIfAbsent('sessions/two-layer-race.md', `writer-${successes + conflicts}\n`); + successes++; + } catch (e) { + if (e instanceof StateKeyConflictError) conflicts++; + else throw e; + } + } + expect(successes).toBe(1); + expect(conflicts).toBe(1); + }); + + it('fail-closed: throws StateBackendUncertaintyError when notes already has the key but orphan does not', () => { + // Plant the key in notes only (no write through TwoLayerBackend.write, which would also set orphan). + const notes = new GitNotesBackend(TMP); + notes.write('sessions/disagreement.md', '# Notes only\n'); + + // OrphanBranchBackend doesn't have it — createIfAbsent on TwoLayerBackend: + // orphan succeeds, notes sees conflict → uncertainty. + const b = new TwoLayerBackend(TMP); + expect(() => b.createIfAbsent('sessions/disagreement.md', '# Two-layer\n')) + .toThrow(StateBackendUncertaintyError); + }); + + it('fail-closed: notes-layer failure (not conflict) also surfaces as StateBackendUncertaintyError', () => { + const b = new TwoLayerBackend(TMP); + // Force the notes layer to fail for a reason other than an existing key. + const boom = new Error('simulated notes backend outage'); + b.notes.createIfAbsent = () => { throw boom; }; + + expect(() => b.createIfAbsent('sessions/notes-outage.md', '# Payload\n')) + .toThrow(StateBackendUncertaintyError); + // The uncertainty must name the disagreement, not masquerade as a conflict. + try { + b.notes.createIfAbsent = () => { throw boom; }; + b.createIfAbsent('sessions/notes-outage-2.md', '# Payload\n'); + throw new Error('expected StateBackendUncertaintyError'); + } catch (e) { + expect(e).toBeInstanceOf(StateBackendUncertaintyError); + expect((e as StateBackendUncertaintyError).message).toMatch(/orphan succeeded but notes layer failed/); + } + }); + + it('two-layer conflict on an existing key is a conflict, never success-shaped', () => { + const b = new TwoLayerBackend(TMP); + b.createIfAbsent('sessions/once.md', '# Winner\n'); + expect(() => b.createIfAbsent('sessions/once.md', '# Loser\n')).toThrow(StateKeyConflictError); + expect(b.read('sessions/once.md')).toBe('# Winner\n'); + }); +}); + +// ── Backend uncertainty (fail-closed on repository identity) ───────────────── + +describe('createIfAbsent fails closed on uncertain repository identity', { timeout: 30_000 }, () => { + beforeEach(() => { cleanup(TMP); mkdirSync(TMP, { recursive: true }); }); + afterEach(() => { clearResolveSquadCache(); cleanup(TMP); }); + + it('GitNotesBackend throws StateBackendUncertaintyError outside a git repository', () => { + const notARepo = join(TMP, 'plain-dir'); + mkdirSync(notARepo, { recursive: true }); + const b = new GitNotesBackend(notARepo); + expect(() => b.createIfAbsent('sessions/nope.md', 'x\n')) + .toThrow(StateBackendUncertaintyError); + }); + + it('OrphanBranchBackend throws StateBackendUncertaintyError outside a git repository', () => { + const notARepo = join(TMP, 'plain-dir-2'); + mkdirSync(notARepo, { recursive: true }); + const b = new OrphanBranchBackend(notARepo); + expect(() => b.createIfAbsent('sessions/nope.md', 'x\n')) + .toThrow(StateBackendUncertaintyError); + }); + + it('never creates state when repository identity is uncertain', () => { + const notARepo = join(TMP, 'plain-dir-3'); + mkdirSync(notARepo, { recursive: true }); + const b = new OrphanBranchBackend(notARepo); + try { b.createIfAbsent('sessions/nope.md', 'x\n'); } catch { /* expected */ } + expect(existsSync(join(notARepo, 'sessions'))).toBe(false); + expect(existsSync(join(notARepo, '.git'))).toBe(false); + }); + + it('WorktreeBackend throws StateBackendUncertaintyError when the parent path is not a directory', () => { + const squadDir = join(TMP, '.squad'); + mkdirSync(squadDir, { recursive: true }); + // "sessions" is a FILE, so mkdir of the parent directory cannot succeed. + writeFileSync(join(squadDir, 'sessions'), 'not a directory\n'); + const b = new WorktreeBackend(squadDir); + expect(() => b.createIfAbsent('sessions/blocked.md', 'x\n')) + .toThrow(StateBackendUncertaintyError); + }); +}); + +// ── FSStorageProvider ──────────────────────────────────────────────────────── + +describe('FSStorageProvider.createIfAbsent', () => { + beforeEach(() => { cleanup(TMP); mkdirSync(TMP, { recursive: true }); }); + afterEach(() => { cleanup(TMP); }); + + it('creates a file when absent', async () => { + const fs = new FSStorageProvider(TMP); + await fs.createIfAbsent('sessions/new.md', '# New\n'); + expect(await fs.read('sessions/new.md')).toBe('# New\n'); + }); + + it('throws StateKeyConflictError when file already exists', async () => { + const fs = new FSStorageProvider(TMP); + await fs.write('sessions/exists.md', '# Original\n'); + await expect(fs.createIfAbsent('sessions/exists.md', '# New\n')) + .rejects.toThrow(StateKeyConflictError); + expect(await fs.read('sessions/exists.md')).toBe('# Original\n'); + }); + + it('exactly one of two concurrent creates succeeds', async () => { + const fs1 = new FSStorageProvider(TMP); + const fs2 = new FSStorageProvider(TMP); + mkdirSync(join(TMP, 'sessions'), { recursive: true }); + const results = await Promise.allSettled([ + fs1.createIfAbsent('sessions/concurrent.md', 'writer-1\n'), + fs2.createIfAbsent('sessions/concurrent.md', 'writer-2\n'), + ]); + const fulfilled = results.filter((r) => r.status === 'fulfilled'); + const rejected = results.filter( + (r) => r.status === 'rejected' && r.reason instanceof StateKeyConflictError, + ); + expect(fulfilled).toHaveLength(1); + expect(rejected).toHaveLength(1); + const content = await fs1.read('sessions/concurrent.md'); + expect(['writer-1\n', 'writer-2\n']).toContain(content); + }); +}); + +// ── InMemoryStorageProvider ────────────────────────────────────────────────── + +describe('InMemoryStorageProvider.createIfAbsent', () => { + it('creates a key when absent', async () => { + const mem = new InMemoryStorageProvider(); + await mem.createIfAbsent('sessions/mem.md', '# Mem\n'); + expect(await mem.read('sessions/mem.md')).toBe('# Mem\n'); + }); + + it('throws StateKeyConflictError when key already exists', async () => { + const mem = new InMemoryStorageProvider(); + await mem.write('sessions/mem.md', '# Original\n'); + await expect(mem.createIfAbsent('sessions/mem.md', '# New\n')) + .rejects.toThrow(StateKeyConflictError); + expect(await mem.read('sessions/mem.md')).toBe('# Original\n'); + }); + + it('exactly one of many concurrent creators wins and its content is preserved', async () => { + const mem = new InMemoryStorageProvider(); + const results = await Promise.allSettled( + Array.from({ length: 8 }, (_, i) => mem.createIfAbsent('sessions/many.md', `writer-${i}\n`)), + ); + expect(results.filter(r => r.status === 'fulfilled')).toHaveLength(1); + expect(results.filter( + r => r.status === 'rejected' && r.reason instanceof StateKeyConflictError, + )).toHaveLength(7); + + const winnerIndex = results.findIndex(r => r.status === 'fulfilled'); + expect(await mem.read('sessions/many.md')).toBe(`writer-${winnerIndex}\n`); + }); + + it('repository isolation: separate provider instances do not share keys', async () => { + const repo1 = new InMemoryStorageProvider(); + const repo2 = new InMemoryStorageProvider(); + await repo1.createIfAbsent('sessions/shared.md', 'repo1\n'); + await expect(repo2.createIfAbsent('sessions/shared.md', 'repo2\n')).resolves.toBeUndefined(); + expect(await repo1.read('sessions/shared.md')).toBe('repo1\n'); + expect(await repo2.read('sessions/shared.md')).toBe('repo2\n'); + }); +}); + +// ── SQLiteStorageProvider ──────────────────────────────────────────────────── + +describe('SQLiteStorageProvider.createIfAbsent', { timeout: 30_000 }, () => { + beforeEach(() => { cleanup(TMP); mkdirSync(TMP, { recursive: true }); }); + afterEach(() => { cleanup(TMP); }); + + it('creates when absent and conflicts on an existing key without overwriting', async () => { + const db = new SQLiteStorageProvider(join(TMP, 'state.db')); + await db.createIfAbsent('sessions/sqlite.md', '# Winner\n'); + expect(await db.read('sessions/sqlite.md')).toBe('# Winner\n'); + + await expect(db.createIfAbsent('sessions/sqlite.md', '# Loser\n')) + .rejects.toThrow(StateKeyConflictError); + expect(await db.read('sessions/sqlite.md')).toBe('# Winner\n'); + }); + + it('exactly one of two concurrent creators succeeds', async () => { + const db = new SQLiteStorageProvider(join(TMP, 'state2.db')); + const results = await Promise.allSettled([ + db.createIfAbsent('sessions/race.md', 'writer-1\n'), + db.createIfAbsent('sessions/race.md', 'writer-2\n'), + ]); + expect(results.filter(r => r.status === 'fulfilled')).toHaveLength(1); + expect(results.filter( + r => r.status === 'rejected' && r.reason instanceof StateKeyConflictError, + )).toHaveLength(1); + expect(['writer-1\n', 'writer-2\n']).toContain(await db.read('sessions/race.md')); + }); +}); + +// ── StateBackendStorageAdapter ─────────────────────────────────────────────── + +describe('StateBackendStorageAdapter.createIfAbsent', { timeout: 30_000 }, () => { + const squadDir = () => join(TMP, '.squad'); + + beforeEach(() => { cleanup(TMP); initRepo(TMP); mkdirSync(squadDir(), { recursive: true }); }); + afterEach(() => { clearResolveSquadCache(); cleanup(TMP); }); + + it('forwards create/conflict semantics from the underlying backend', async () => { + const backend = new OrphanBranchBackend(TMP); + const adapter = new StateBackendStorageAdapter(backend, squadDir()); + + await adapter.createIfAbsent(join(squadDir(), 'sessions/adapter.md'), '# Winner\n'); + expect(backend.read('sessions/adapter.md')).toBe('# Winner\n'); + + await expect(adapter.createIfAbsent(join(squadDir(), 'sessions/adapter.md'), '# Loser\n')) + .rejects.toThrow(StateKeyConflictError); + // Winning content preserved through the adapter. + expect(backend.read('sessions/adapter.md')).toBe('# Winner\n'); + }); + + it('forwards uncertainty from the underlying backend', async () => { + const backend = new OrphanBranchBackend(TMP); + backend.createIfAbsent = () => { + throw new StateBackendUncertaintyError('orphan:createIfAbsent(x)', 'simulated'); + }; + const adapter = new StateBackendStorageAdapter(backend, squadDir()); + await expect(adapter.createIfAbsent(join(squadDir(), 'sessions/x.md'), 'x\n')) + .rejects.toThrow(StateBackendUncertaintyError); + }); +}); + +// ── ToolRegistry: squad_state_create_if_absent ─────────────────────────────── + +describe('ToolRegistry squad_state_create_if_absent', { timeout: 30_000 }, () => { + const squadDir = () => join(TMP, '.squad'); + + beforeEach(() => { cleanup(TMP); initRepo(TMP); mkdirSync(squadDir(), { recursive: true }); }); + afterEach(() => { clearResolveSquadCache(); cleanup(TMP); }); + + function makeRegistry() { + const backend = new OrphanBranchBackend(TMP); + const adapter = new StateBackendStorageAdapter(backend, squadDir()); + return { registry: new ToolRegistry(squadDir(), undefined, adapter), backend }; + } + + it('appears in registered tools list', () => { + const { registry } = makeRegistry(); + expect(registry.getTool('squad_state_create_if_absent')).toBeDefined(); + }); + + it('returns success and creates the key when absent', async () => { + const { registry, backend } = makeRegistry(); + const tool = registry.getTool('squad_state_create_if_absent')!; + const result = await tool.handler({ key: 'sessions/tool-new.md', content: '# Created\n' }); + expect(result.resultType).toBe('success'); + expect(backend.read('sessions/tool-new.md')).toBe('# Created\n'); + }); + + it('returns failure with error="conflict" when key already exists', async () => { + const { registry, backend } = makeRegistry(); + backend.write('sessions/existing.md', '# Original\n'); + const tool = registry.getTool('squad_state_create_if_absent')!; + const result = await tool.handler({ key: 'sessions/existing.md', content: '# New\n' }); + expect(result.resultType).toBe('failure'); + expect((result as { error?: unknown }).error).toBe('conflict'); + // original content unchanged + expect(backend.read('sessions/existing.md')).toBe('# Original\n'); + }); + + it('returns failure for invalid/protected key', async () => { + const { registry } = makeRegistry(); + const tool = registry.getTool('squad_state_create_if_absent')!; + const result = await tool.handler({ key: 'team.md', content: '# Bad\n' }); + expect(result.resultType).toBe('failure'); + }); + + it('returns failure with error="conflict" — not success — on conflict (no success-shaped conflict)', async () => { + const { registry, backend } = makeRegistry(); + backend.write('sessions/guard.md', '# Guard\n'); + const tool = registry.getTool('squad_state_create_if_absent')!; + const result = await tool.handler({ key: 'sessions/guard.md', content: '# Attempt\n' }); + // Must NOT be success-shaped + expect(result.resultType).not.toBe('success'); + expect((result as { error?: unknown }).error).toBe('conflict'); + }); +});