diff --git a/server/src/__tests__/agent-instructions-service.test.ts b/server/src/__tests__/agent-instructions-service.test.ts index 653f0747437..beb7fafe75d 100644 --- a/server/src/__tests__/agent-instructions-service.test.ts +++ b/server/src/__tests__/agent-instructions-service.test.ts @@ -531,6 +531,63 @@ describe("agent instructions service", () => { expect(Array.from({ length: 8 }, (_, index) => `# Stock ${index}\n`)).toContain(live); }); + it("publishes owner metadata atomically with the materialization lock", async () => { + const paperclipHome = await makeTempDir("paperclip-agent-instructions-lock-publish-"); + cleanupDirs.add(paperclipHome); + process.env.PAPERCLIP_HOME = paperclipHome; + process.env.PAPERCLIP_INSTANCE_ID = "test-instance"; + const agent = makeAgent({}); + const managedRoot = path.join( + paperclipHome, + "instances", + "test-instance", + "companies", + "company-1", + "agents", + "agent-1", + "instructions", + ); + const lockPath = `${managedRoot}.paperclip-materialize.lock`; + const originalRename = fs.rename.bind(fs); + let pendingLockPath: string | null = null; + let publish: (() => void) | null = null; + const publishAllowed = new Promise((resolve) => { + publish = resolve; + }); + let pendingReady: (() => void) | null = null; + const pendingPrepared = new Promise((resolve) => { + pendingReady = resolve; + }); + const renameSpy = vi.spyOn(fs, "rename").mockImplementation(async (source, destination) => { + if (String(source).startsWith(`${lockPath}.pending-`) && String(destination) === lockPath) { + pendingLockPath = String(source); + pendingReady?.(); + await publishAllowed; + } + return originalRename(source, destination); + }); + const materialization = agentInstructionsService().materializeManagedBundle( + agent, + { "AGENTS.md": "# Stock\n" }, + ); + + try { + await pendingPrepared; + await expect(fs.stat(lockPath)).rejects.toThrow(); + expect(pendingLockPath).not.toBeNull(); + await expect(fs.readFile(path.join(pendingLockPath!, "owner.json"), "utf8")).resolves.toContain( + `"pid":${process.pid}`, + ); + const pendingFiles = await fs.readdir(pendingLockPath!); + expect(pendingFiles.some((file) => file.startsWith("heartbeat-"))).toBe(true); + } finally { + publish?.(); + } + + await expect(materialization).resolves.toMatchObject({ materialization: { action: "added" } }); + renameSpy.mockRestore(); + }); + it("does not evict an old materialization lock owned by a live process", async () => { const paperclipHome = await makeTempDir("paperclip-agent-instructions-live-lock-"); cleanupDirs.add(paperclipHome); diff --git a/server/src/services/agent-instructions.ts b/server/src/services/agent-instructions.ts index 1bd02864703..4fded35503e 100644 --- a/server/src/services/agent-instructions.ts +++ b/server/src/services/agent-instructions.ts @@ -338,37 +338,39 @@ async function withManagedBundleMaterializationLock( const ownerPath = path.join(lockPath, "owner.json"); const token = randomUUID(); const heartbeatPath = path.join(lockPath, `heartbeat-${token}`); + const pendingLockPath = `${lockPath}.pending-${token}`; const ownerProcessStartMarker = await processStartMarker(process.pid); const deadline = Date.now() + MANAGED_BUNDLE_LOCK_TIMEOUT_MS; await fs.mkdir(path.dirname(rootPath), { recursive: true }); - while (true) { - try { - await fs.mkdir(lockPath); + try { + await fs.mkdir(pendingLockPath); + await fs.writeFile( + path.join(pendingLockPath, "owner.json"), + `${JSON.stringify({ + pid: process.pid, + token, + processStartMarker: ownerProcessStartMarker, + createdAt: new Date().toISOString(), + })}\n`, + "utf8", + ); + await fs.writeFile(path.join(pendingLockPath, `heartbeat-${token}`), "", "utf8"); + while (true) { try { - await fs.writeFile( - ownerPath, - `${JSON.stringify({ - pid: process.pid, - token, - processStartMarker: ownerProcessStartMarker, - createdAt: new Date().toISOString(), - })}\n`, - "utf8", - ); - await fs.writeFile(heartbeatPath, "", "utf8"); + await fs.rename(pendingLockPath, lockPath); + break; } catch (error) { - await fs.rm(lockPath, { recursive: true, force: true }).catch(() => undefined); - throw error; + if (!(await statIfExists(lockPath))) throw error; + if (await removeStaleManagedBundleLock(lockPath)) continue; + if (Date.now() >= deadline) { + throw new Error(`Timed out waiting for managed instructions lock at ${lockPath}`); + } + await new Promise((resolve) => setTimeout(resolve, 25)); } - break; - } catch (error) { - if ((error as NodeJS.ErrnoException).code !== "EEXIST") throw error; - if (await removeStaleManagedBundleLock(lockPath)) continue; - if (Date.now() >= deadline) { - throw new Error(`Timed out waiting for managed instructions lock at ${lockPath}`); - } - await new Promise((resolve) => setTimeout(resolve, 25)); } + } catch (error) { + await fs.rm(pendingLockPath, { recursive: true, force: true }).catch(() => undefined); + throw error; } const heartbeat = setInterval(() => {