From 1949ac6788586fc8cf3c718290f0d824ada61507 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 11 Nov 2026 20:06:36 -0600 Subject: [PATCH] test(chaos): fail the directory-lock crash point with the lock's own busy error (#20988) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The restart chaos harness can fail one recovery step on purpose (`--fail-recovery-step`), through a test-only crash point in the server > - The `directory-lock` step fails `agent_directory.operation_lock.before_acquire`, the per-agent Postgres lock that a volume-free server takes around remote agent-directory work > - The real lock path throws `AgentDirectoryOperationBusyError` when another run holds the lock. Preparation turns that error into a deferral and retries later > - The crash point threw a plain 409 `conflict()` with the same message, so the deferral never matched and the injected fault failed the run. A volume-free wave on 2026-11-12 showed this: the run failed with `native_continuation_requires_reconciliation` and needed Retry, where real contention would have been deferred > - This pull request makes the crash point throw the lock's own error class > - The benefit is that the `directory-lock` chaos cell tests the product's real handling of a busy agent directory ## Linked Issues or Issue Description **What is the problem?** The test-only crash point `agent_directory.operation_lock.before_acquire` throws a different error class from the lock it stands in for. **What is the expected behavior?** A run that meets the injected failure takes the same path as a run that meets real lock contention. **Steps to reproduce** Run a volume-free Daytona wave with `--fail-recovery-step directory-lock:1`, workload W2 and a K2 restart. The recovering run fails with `native_continuation_requires_reconciliation`; the stack shows the crash point inside `prepareCopy`, under `prepareWithAgentDirectoryDeferral`, which only defers `AgentDirectoryOperationBusyError`. ## What Changed - New test `server/src/__tests__/agent-directory-lock-crash-point.test.ts`. - `server/src/services/agent-directory-working-copies.ts`: the crash point's `fail` returns `new AgentDirectoryOperationBusyError()` (same 409 status and message) instead of a plain `conflict()`. ## Verification - `server/src/__tests__/agent-directory-lock-crash-point.test.ts` arms the point in fail mode through a control file (as restart chaos does) and checks that the injected failure is an `AgentDirectoryOperationBusyError` that `prepareWithAgentDirectoryDeferral` turns into a deferral. It fails on master (the injected error is a plain `HttpError`) and passes with this change. - Server typecheck passes. - The crash point is disarmed unless a chaos wave arms it, so production behavior does not change. - Live check: the next volume-free `directory-lock:1` chaos wave (W2, K2, Codex) shows whether the recovering run is deferred instead of failing. I will add the result to this PR. ## Risks - Low risk. The change is in a test-only crash point that is disarmed in production. The README step table already documents the error as the 409 "Agent directory operation is already active". ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use and code execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 --- .../agent-directory-lock-crash-point.test.ts | 31 +++++++++++++++++++ .../agent-directory-working-copies.ts | 2 +- 2 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 server/src/__tests__/agent-directory-lock-crash-point.test.ts diff --git a/server/src/__tests__/agent-directory-lock-crash-point.test.ts b/server/src/__tests__/agent-directory-lock-crash-point.test.ts new file mode 100644 index 00000000000..85f87a86fb8 --- /dev/null +++ b/server/src/__tests__/agent-directory-lock-crash-point.test.ts @@ -0,0 +1,31 @@ +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +describe("directory-lock crash point", () => { + let root: string | null = null; + + afterEach(async () => { + vi.unstubAllEnvs(); + vi.resetModules(); + if (root) await rm(root, { recursive: true, force: true }); + root = null; + }); + + it("fails with the lock's own busy error, so preparation defers it like real contention", async () => { + root = await mkdtemp(path.join(os.tmpdir(), "directory-lock-crash-point-")); + const control = path.join(root, "crash-point.control"); + await writeFile(control, "t1 agent_directory.operation_lock.before_acquire:1 fail\n"); + vi.stubEnv("PAPERCLIP_CRASH_POINT_CONTROL", control); + vi.stubEnv("PAPERCLIP_EPHEMERAL_SERVER", "true"); + vi.resetModules(); + const { agentDirectoryWorkingCopyService, AgentDirectoryOperationBusyError } = await import("../services/agent-directory-working-copies.js"); + const { prepareWithAgentDirectoryDeferral, AgentDirectoryPreparationDeferral } = await import("../services/heartbeat/agent-directory-busy.js"); + const service = agentDirectoryWorkingCopyService({} as never, async () => null, async row => row); + const row = { companyId: "company-1", runId: "run-1", agentId: "agent-1", location: "remote:sandbox-1", receipt: {} }; + const failure = service.collectStopped(row as never); + await expect(failure).rejects.toBeInstanceOf(AgentDirectoryOperationBusyError); + await expect(prepareWithAgentDirectoryDeferral(() => failure)).rejects.toBeInstanceOf(AgentDirectoryPreparationDeferral); + }); +}); diff --git a/server/src/services/agent-directory-working-copies.ts b/server/src/services/agent-directory-working-copies.ts index 8109e1ef45b..98ec9ab4ec8 100644 --- a/server/src/services/agent-directory-working-copies.ts +++ b/server/src/services/agent-directory-working-copies.ts @@ -659,7 +659,7 @@ export function agentDirectoryWorkingCopyService(db: Db, get: (companyId: string // Different controllers have different cache roots. Serialize their active // collection/cleanup operations in Postgres, without holding row locks over I/O. // Test instrumentation only (disarmed in production): restart chaos fails the Nth attempt with the lock's own conflict. - crashPoint("agent_directory.operation_lock.before_acquire", { fail: () => conflict("Agent directory operation is already active") }); + crashPoint("agent_directory.operation_lock.before_acquire", { fail: () => new AgentDirectoryOperationBusyError() }); return withDedicatedDbConnection(db, dedicated => dedicated.transaction(async tx => { const [lock] = await tx.execute(sql`select pg_try_advisory_xact_lock(hashtextextended(${`agent-directory:${row.companyId}:${row.agentId}`}, 0)) as acquired, (select setting::bigint from pg_settings where name = 'idle_in_transaction_session_timeout') as idle_timeout_ms`);