diff --git a/.changeset/quiet-locks-follow-state.md b/.changeset/quiet-locks-follow-state.md new file mode 100644 index 000000000..4c4df8bb5 --- /dev/null +++ b/.changeset/quiet-locks-follow-state.md @@ -0,0 +1,5 @@ +--- +'@bradygaster/squad-sdk': patch +--- + +Coordinate memory index locks with the configured state backend. diff --git a/packages/squad-sdk/src/memory/index.ts b/packages/squad-sdk/src/memory/index.ts index b9e065d7e..d63f07597 100644 --- a/packages/squad-sdk/src/memory/index.ts +++ b/packages/squad-sdk/src/memory/index.ts @@ -536,6 +536,11 @@ export const REAL_COPILOT_UNAVAILABLE_REASON = const HOST_INJECTED_CLIENT_MISSING_REASON = 'hostInjectedCopilotAdapter is enabled, but no host-injected Copilot memory client was supplied. This is not real provider=copilot persistence.'; +type MemoryIndexLockStore = { lockAsync?: (key: string) => Promise<() => void> }; +type MemoryIndexLockProvider = { + createLockStore?: (lockAnchor: string) => MemoryIndexLockStore | Promise; +}; + function isRealCopilotProviderSelected(config: MemoryGovernanceConfig): boolean { return config.defaultProvider === 'copilot'; } @@ -650,18 +655,22 @@ export class LocalMemoryStore { * * All callers that do readIndex() → mutate → writeIndex() must go through * this method so concurrent writers cannot interleave and lose entries. - * The file lock coordinates callers across store instances and processes - * sharing the same .squad directory. + * Backend adapters coordinate on the shared state ref; filesystem storage + * keeps its lock under the resolved .squad directory. */ private async withIndexLock(fn: () => Promise): Promise { - const resolvedSquadDir = path.resolve(this.squadDir); - let lockRoot = resolvedSquadDir; - try { - lockRoot = await realpath(resolvedSquadDir); - } catch (error) { - if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + const lockProvider = this.storage as StorageProvider & MemoryIndexLockProvider; + let fileLock = await lockProvider.createLockStore?.(this.squadDir); + if (!fileLock) { + const resolvedSquadDir = path.resolve(this.squadDir); + let lockRoot = resolvedSquadDir; + try { + lockRoot = await realpath(resolvedSquadDir); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + } + fileLock = createFileStateStore(lockRoot); } - const fileLock = createFileStateStore(lockRoot); const releaseFileLock = await fileLock.lockAsync!('backend/locks/memory-index'); try { return await fn(); diff --git a/packages/squad-sdk/src/state-backend.ts b/packages/squad-sdk/src/state-backend.ts index 246ea6d6c..c5a35d0cb 100644 --- a/packages/squad-sdk/src/state-backend.ts +++ b/packages/squad-sdk/src/state-backend.ts @@ -14,6 +14,7 @@ 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'; +import type { BackendStateStore } from './backend/types.js'; const storage = new FSStorageProvider(); @@ -1099,6 +1100,11 @@ export class OrphanBranchBackend implements StateBackend { export class StateBackendStorageAdapter implements StorageProvider { constructor(private backend: StateBackend, private squadDir: string) {} + async createLockStore(lockAnchor: string): Promise { + const { createStateBackendStore } = await import('./backend/state-backend-store.js'); + return createStateBackendStore(this.backend, { lockAnchor }); + } + // ── Async operations ───────────────────────────────────────────────────── async createIfAbsent(filePath: string, data: string): Promise { this.backend.createIfAbsent(this.toRelative(filePath, { operation: 'createIfAbsent' }), data); diff --git a/test/backend.test.ts b/test/backend.test.ts index 9bdf500ad..e65dd2979 100644 --- a/test/backend.test.ts +++ b/test/backend.test.ts @@ -1730,7 +1730,7 @@ describe('PR #2253 Copilot review thread regressions', () => { const fs = await import('node:fs'); const os = await import('node:os'); const path = await import('node:path'); - const { OrphanBranchBackend, TwoLayerBackend } = await import('../packages/squad-sdk/src/state-backend.js'); + const { OrphanBranchBackend, TwoLayerBackend, StateBackendStorageAdapter } = await import('../packages/squad-sdk/src/state-backend.js'); const base = fs.mkdtempSync(path.join(os.tmpdir(), 'sq-wt-lock-')); try { const repoMain = path.join(base, 'main-repo'); @@ -1749,6 +1749,10 @@ describe('PR #2253 Copilot review thread regressions', () => { const storeWt = createStateBackendStore(new OrphanBranchBackend(repoWt), opts); const storeTwoLayerWt = createStateBackendStore(new TwoLayerBackend(repoWt), { ...opts, lockAnchor: path.join(repoWt, '.squad') }); const storeAlias = createStateBackendStore(new OrphanBranchBackend(repoAlias), opts); + const adapterStoreWt = new StateBackendStorageAdapter( + new OrphanBranchBackend(repoWt), path.join(repoWt, '.squad'), + ); + const adapterLockStoreWt = await adapterStoreWt.createLockStore(path.join(repoWt, '.squad')); const release = storeMain.lock!('backend/locks/shared-conv'); try { @@ -1756,6 +1760,7 @@ describe('PR #2253 Copilot review thread regressions', () => { expect(() => storeWt.lock!('backend/locks/shared-conv')).toThrow(/Timed out/); expect(() => storeTwoLayerWt.lock!('backend/locks/shared-conv')).toThrow(/Timed out/); expect(() => storeAlias.lock!('backend/locks/shared-conv')).toThrow(/Timed out/); + expect(() => adapterLockStoreWt.lock!('backend/locks/shared-conv')).toThrow(/Timed out/); // A different orphan branch ref has its own independent lock const storeOtherBranch = createStateBackendStore(new OrphanBranchBackend(repoWt, 'other-state'), opts); diff --git a/test/state-backend.test.ts b/test/state-backend.test.ts index 6f08078c7..94c9347e9 100644 --- a/test/state-backend.test.ts +++ b/test/state-backend.test.ts @@ -8,6 +8,7 @@ import { WorktreeBackend, GitNotesBackend, OrphanBranchBackend, TwoLayerBackend, import type { StateBackend, StateBackendType } from '../packages/squad-sdk/src/state-backend.js'; import { resolveSquadState, clearResolveSquadCache } from '../packages/squad-sdk/src/resolution.js'; import { ToolRegistry } from '../packages/squad-sdk/src/tools/index.js'; +import { LocalMemoryStore } from '../packages/squad-sdk/src/memory/index.js'; const TMP = join(process.cwd(), `.test-state-backend-${randomBytes(4).toString('hex')}`); function git(args: string, cwd = TMP): string { @@ -787,6 +788,25 @@ describe('ToolRegistry state tools with git-native backend', () => { expect(git('status --porcelain')).toBe(''); }); + it('uses the backend-aware lock store for memory index writes', { timeout: 30_000 }, async () => { + const backend = new OrphanBranchBackend(TMP); + const adapter = new StateBackendStorageAdapter(backend, squadDir()); + const createLockStore = vi.spyOn(adapter, 'createLockStore'); + const memory = new LocalMemoryStore(adapter, squadDir(), { rootKind: 'squad' }); + + const result = await memory.write({ + content: 'Memory index locks must follow the configured state backend.', + title: 'Backend-aware memory lock', + requestedClass: 'LOCAL', + }); + + expect(result.stored).toBe(true); + expect(createLockStore).toHaveBeenCalledWith(squadDir()); + expect(backend.read('memory/index.json')).toContain('Backend-aware memory lock'); + expect(existsSync(join(squadDir(), 'backend', 'locks'))).toBe(false); + expect(git('status --porcelain')).toBe(''); + }); + it('routes existing squad_decide writes through configured backend storage', { timeout: 20_000 }, async () => { const backend = new OrphanBranchBackend(TMP); const adapter = new StateBackendStorageAdapter(backend, squadDir());