diff --git a/doc/connections/AI-CONNECTIONS.md b/doc/connections/AI-CONNECTIONS.md index 980b07463f..baf936fe6b 100644 --- a/doc/connections/AI-CONNECTIONS.md +++ b/doc/connections/AI-CONNECTIONS.md @@ -93,6 +93,9 @@ subsequent agent creation fails or is cancelled. ## Runtime isolation `prepareManagedAiRuntime` is shared by runs, environment tests, and adoption. +Claude ACP validates working directories on the selected execution target. A +sandbox directory does not need to exist on the Paperclip server. When the agent +has no configured directory, the test uses the remote target's working directory. It checks responsible identity, membership, compatibility, connection health, human audience and agent installation before reading credentials. Missing credentials produce an actionable configuration failure; responsible-user diff --git a/packages/adapters/claude-local/src/server/acp.test.ts b/packages/adapters/claude-local/src/server/acp.test.ts index a0441be380..ffb0747364 100644 --- a/packages/adapters/claude-local/src/server/acp.test.ts +++ b/packages/adapters/claude-local/src/server/acp.test.ts @@ -449,6 +449,28 @@ describe("claude_local ACP lane", () => { }); }); + it.each([undefined, "/sandbox/configured-workspace"])("checks sandbox directories on the sandbox (configured cwd=%s)", async (configuredCwd) => { + const remoteCwd = "/sandbox/workspace"; + const mkdir = vi.spyOn(fs, "mkdir").mockRejectedValue(new Error("Host filesystem must not be used")); + const execute = vi.fn(async () => ({ + exitCode: 0, signal: null, timedOut: false, stdout: "", stderr: "", + pid: null, startedAt: new Date().toISOString(), + })); + try { + const result = await testClaudeAcpEnvironment({ + companyId: "company-1", adapterType: "claude_local", + config: { cwd: configuredCwd, agentCommand: "claude-agent-acp", env: { ANTHROPIC_API_KEY: "fixture" } }, + executionTarget: { kind: "remote", transport: "sandbox", remoteCwd, runner: { execute } }, + }); + expect(result.status, JSON.stringify(result.checks)).toBe("pass"); + expect(result.checks).toContainEqual(expect.objectContaining({ + code: "claude_acp_cwd_valid", message: `Working directory is valid: ${configuredCwd ?? remoteCwd}`, + })); + expect(mkdir).not.toHaveBeenCalled(); + expect(JSON.stringify(execute.mock.calls)).toContain(`mkdir -p '${configuredCwd ?? remoteCwd}'`); + } finally { mkdir.mockRestore(); } + }); + it("reports ACP prerequisites for the ACP lane", async () => { const root = await makeTempRoot("paperclip-claude-acp-env-"); const commandPath = path.join(root, "bin", "claude-agent-acp"); diff --git a/packages/adapters/claude-local/src/server/acp.ts b/packages/adapters/claude-local/src/server/acp.ts index cc7b4b207d..27b253694d 100644 --- a/packages/adapters/claude-local/src/server/acp.ts +++ b/packages/adapters/claude-local/src/server/acp.ts @@ -15,6 +15,7 @@ import { } from "@paperclipai/adapter-utils/local-process-sandbox"; import { ensureAdapterExecutionTargetCommandResolvable, + ensureAdapterExecutionTargetDirectory, readAdapterExecutionTarget, resolveAdapterExecutionTargetCwd, runAdapterExecutionTargetProcess, @@ -718,9 +719,13 @@ export async function testClaudeAcpEnvironment( }); } - const cwd = asString(config.cwd, process.cwd()); + const cwd = resolveAdapterExecutionTargetCwd(target, asString(config.cwd, ""), process.cwd()); try { - await fs.mkdir(cwd, { recursive: true }); + await ensureAdapterExecutionTargetDirectory(`claude-acp-envtest-${Date.now()}`, target, cwd, { + cwd, + env: {}, + createIfMissing: true, + }); checks.push({ code: "claude_acp_cwd_valid", level: "info", diff --git a/server/src/__tests__/recovery-stale-issue-lock-sweep.test.ts b/server/src/__tests__/recovery-stale-issue-lock-sweep.test.ts index 2cc53380fb..aef5c24ae6 100644 --- a/server/src/__tests__/recovery-stale-issue-lock-sweep.test.ts +++ b/server/src/__tests__/recovery-stale-issue-lock-sweep.test.ts @@ -333,6 +333,54 @@ describeEmbeddedPostgres("recovery sweepStaleIssueLocks", () => { ); }); + it.each(["in_progress", "done"])("preserves a live legacy controller lease when the issue is %s", async (status) => { + const { companyId, agentId, runningRunId } = await seed(); + await db.update(heartbeatRuns).set({ + runtimeMode: "legacy", processPid: 2_000_000_000, + controllerBootId: randomUUID(), + controllerLeaseExpiresAt: new Date(Date.now() + 60_000), + }).where(eq(heartbeatRuns.id, runningRunId)); + const issueId = randomUUID(); + await db.insert(issues).values({ + id: issueId, companyId, title: "Remote controller still owns the run", status, + assigneeAgentId: agentId, executionRunId: runningRunId, checkoutRunId: runningRunId, + }); + const result = await recoveryService(db, { enqueueWakeup: vi.fn() }).sweepStaleIssueLocks(); + expect(result).toEqual({ cleared: 0, issueIds: [], terminalizedRunIds: [] }); + expect(await db.select({ status: heartbeatRuns.status }).from(heartbeatRuns) + .where(eq(heartbeatRuns.id, runningRunId))).toEqual([{ status: "running" }]); + expect(await db.select({ executionRunId: issues.executionRunId }).from(issues) + .where(eq(issues.id, issueId))).toEqual([{ executionRunId: runningRunId }]); + }); + + it.each(["renew", "replace", "claim"])("fences a legacy controller %s between the orphan check and terminal write", async (change) => { + const { companyId, agentId, runningRunId } = await seed(); + const bootId = randomUUID(); + await db.update(heartbeatRuns).set({ + runtimeMode: "legacy", processPid: 2_000_000_000, + controllerBootId: change === "claim" ? null : bootId, + controllerLeaseExpiresAt: new Date(Date.now() - 60_000), + }).where(eq(heartbeatRuns.id, runningRunId)); + await db.insert(issues).values({ + id: randomUUID(), companyId, title: "Controller changed during sweep", status: "in_progress", + assigneeAgentId: agentId, executionRunId: runningRunId, checkoutRunId: runningRunId, + }); + const result = await recoveryService(db, { + enqueueWakeup: vi.fn(), + beforeOrphanedRunTerminalWrite: async () => { + await db.update(heartbeatRuns).set({ + controllerBootId: change === "renew" ? bootId : randomUUID(), + // A replacement invalidates the old snapshot even if its lease expires. + controllerLeaseExpiresAt: new Date(Date.now() + (change === "replace" ? -30_000 : 60_000)), + }).where(eq(heartbeatRuns.id, runningRunId)); + }, + }).sweepStaleIssueLocks(); + expect(result).toEqual({ cleared: 0, issueIds: [], terminalizedRunIds: [] }); + expect(await db.select({ status: heartbeatRuns.status }).from(heartbeatRuns) + .where(eq(heartbeatRuns.id, runningRunId))).toEqual([{ status: "running" }]); + expect(mockTelemetryClient.track).not.toHaveBeenCalled(); + }); + it("preserves a process-less native run while same-run resumption owns its retry", async () => { const { companyId, agentId, runningRunId } = await seed(); const issueId = randomUUID(); diff --git a/server/src/services/recovery/service.ts b/server/src/services/recovery/service.ts index 648a96761c..f799023e2e 100644 --- a/server/src/services/recovery/service.ts +++ b/server/src/services/recovery/service.ts @@ -1,3 +1,4 @@ +import { hasLiveLegacyController } from "../legacy-controller-lease.js"; import { instanceSettingsService } from "../instance-settings.js"; import { isWaitingConversation, settleConversationTurn, deliverConversationComments } from "../agent-conversations.js"; import { @@ -5474,7 +5475,9 @@ export function recoveryService( // state is auditable. It never overwrites a status that another path already // made terminal. // - // Two independent authorities terminalize the run. Either one is enough: + // A live controller lease owns execution and finalization across server + // processes. Only after that ownership ends can either authority below + // terminalize the run: // // - Issue-terminal authority: the run's issue already reached a terminal // status (done or cancelled), but the run row is still "running". A healthy @@ -5513,6 +5516,12 @@ export function recoveryService( if (isNativeRunnerOwnershipHeld(run)) return { terminalized: false, status: run.status }; + // Another controller may own a sandbox run whose PID has no meaning on + // this host. Its live lease owns both execution and finalization, even if + // the issue is already terminal or this process has no in-memory handle. + if (await hasLiveLegacyController(db, run)) + return { terminalized: false, status: run.status }; + const pid = run.processPid ?? null; const processGroupId = run.processGroupId ?? null; @@ -5624,7 +5633,22 @@ export function recoveryService( and( eq(heartbeatRuns.id, run.id), eq(heartbeatRuns.status, "running"), + eq(heartbeatRuns.runtimeMode, run.runtimeMode), nativeRunnerOwnershipNotHeldCondition(), + // Recheck ownership in the write: a controller can renew or claim + // the run after the liveness read. An old snapshot cannot end a new + // controller's run, even if that controller's lease later expires. + run.runtimeMode === "legacy" + ? and( + run.controllerBootId + ? eq(heartbeatRuns.controllerBootId, run.controllerBootId) + : isNull(heartbeatRuns.controllerBootId), + or( + isNull(heartbeatRuns.controllerBootId), + sql`${heartbeatRuns.controllerLeaseExpiresAt} <= clock_timestamp()`, + ), + ) + : undefined, ), ) .returning()