diff --git a/packages/paperclip-runner/src/configured-environment.ts b/packages/paperclip-runner/src/configured-environment.ts index 475e62c3cc9..5cd30c0e765 100644 --- a/packages/paperclip-runner/src/configured-environment.ts +++ b/packages/paperclip-runner/src/configured-environment.ts @@ -25,12 +25,16 @@ export const GENERATED_RUNTIME_ENVIRONMENT_KEYS: ReadonlySet = new Set([ "PAPERCLIP_GITHUB_BROKER_TOKEN", "PAPERCLIP_GITHUB_BROKER_URL", "PAPERCLIP_GITHUB_BRIDGE_TOKEN", "PAPERCLIP_GITHUB_LAUNCHER_DIR", ]); -const RESERVED_KEYS = new Set([ - "PATH", "HOME", "USERPROFILE", "CODEX_HOME", "AGENT_HOME", "SHELL", "NODE_OPTIONS", "NODE_PATH", - "PYTHONPATH", "PYTHONHOME", "RUBYOPT", "RUBYLIB", "PERL5OPT", "PERL5LIB", "BASH_ENV", "ENV", "ZDOTDIR", "GH_CONFIG_DIR", +export const PROVIDER_CREDENTIAL_ENVIRONMENT_KEYS: readonly string[] = [ "OPENAI_API_KEY", "CODEX_API_KEY", "ANTHROPIC_API_KEY", "ANTHROPIC_AUTH_TOKEN", "CLAUDE_CODE_OAUTH_TOKEN", "AWS_BEARER_TOKEN_BEDROCK", "OPENROUTER_API_KEY", "XAI_API_KEY", "CURSOR_API_KEY", "CURSOR_AUTH_TOKEN", "COPILOT_GITHUB_TOKEN", "KIMI_API_KEY", "GH_TOKEN", "GITHUB_TOKEN", "GH_ENTERPRISE_TOKEN", "GITHUB_ENTERPRISE_TOKEN", "PAPERCLIP_GIT_TOKEN", +]; + +const RESERVED_KEYS = new Set([ + "PATH", "HOME", "USERPROFILE", "CODEX_HOME", "AGENT_HOME", "SHELL", "NODE_OPTIONS", "NODE_PATH", + "PYTHONPATH", "PYTHONHOME", "RUBYOPT", "RUBYLIB", "PERL5OPT", "PERL5LIB", "BASH_ENV", "ENV", "ZDOTDIR", "GH_CONFIG_DIR", + ...PROVIDER_CREDENTIAL_ENVIRONMENT_KEYS, ]); function eligibleKey(key: string): boolean { diff --git a/server/src/__tests__/environment-retained-orphan-sandbox.test.ts b/server/src/__tests__/environment-retained-orphan-sandbox.test.ts new file mode 100644 index 00000000000..66e4d2926d2 --- /dev/null +++ b/server/src/__tests__/environment-retained-orphan-sandbox.test.ts @@ -0,0 +1,272 @@ +import { spawn } from "node:child_process"; +import { randomUUID } from "node:crypto"; +import { eq } from "drizzle-orm"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; +import { + agents, + companies, + createDb, + environmentLeases, + environments, + heartbeatRuns, + issueRecoveryActions, + issues, +} from "@paperclipai/db"; +import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; +import { currentProcessSpaceHash } from "../services/controller-process-identity.js"; +import { nextEnvironmentCleanupAt } from "../services/environment-cleanup-state.js"; +import { ENVIRONMENT_LEASE_CONTROLLER_KEY } from "../services/environment-lease-controller.js"; +import { hasPendingRunCleanup } from "../services/execution-blocker.js"; +import { heartbeatService, type HeartbeatEnvironmentRuntime } from "../services/heartbeat.js"; +import { hasRecoveredCleanupStopReceipt } from "../services/remote-execution-termination.js"; +import { taskChatRetryExecutionStopped } from "../services/chat-channels/task-retry-authority.js"; +import { createOwnershipReportState, runOwnershipReconciliationPass } from "../services/recovery/ownership-invariant-report.js"; +import { ORPHANED_SANDBOX_RECOVERY_KEY } from "../services/sandbox-stop-and-retain.js"; + +const support = await getEmbeddedPostgresTestSupport(); +const suite = support.supported ? describe : describe.skip; +const KIND = "environment_retained_orphan_sandbox"; + +async function exitedPid(): Promise { + const child = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { stdio: "ignore" }); + const pid = child.pid!; + await new Promise(resolve => { child.once("exit", () => resolve()); child.kill("SIGKILL"); }); + return pid; +} + +suite("retained orphan sandbox on the shared ownership path", () => { + let db!: ReturnType; + let temporary: Awaited> | null = null; + let companyId = ""; + let agentId = ""; + let environmentId = ""; + let issueId = ""; + + beforeAll(async () => { + temporary = await startEmbeddedPostgresTestDatabase("paperclip-retained-orphan-sandbox-"); + db = createDb(temporary.connectionString); + companyId = randomUUID(); agentId = randomUUID(); environmentId = randomUUID(); issueId = randomUUID(); + await db.insert(companies).values({ id: companyId, name: "Paperclip", issuePrefix: "ROS", + requireBoardApprovalForNewAgents: false }); + await db.insert(agents).values({ id: agentId, companyId, name: "Coder", role: "engineer", status: "idle", + adapterType: "codex_local", adapterConfig: {}, runtimeConfig: {}, permissions: {} }); + await db.insert(environments).values({ id: environmentId, name: "Fake Sandbox", driver: "sandbox", + status: "active", config: { provider: "fake", image: "ubuntu:24.04" } }); + await db.insert(issues).values({ id: issueId, companyId, title: "Orphaned work" }); + }, 30_000); + + afterEach(async () => { + await db.delete(issueRecoveryActions); + await db.delete(environmentLeases); + await db.delete(heartbeatRuns); + }); + + afterAll(async () => { + await temporary?.cleanup(); + }); + + async function run(status: string) { + const id = randomUUID(); + await db.insert(heartbeatRuns).values({ id, companyId, agentId, issueId, invocationSource: "on_demand", status, nextEventSeq: 1 }); + return id; + } + + async function row(id: string) { + const [found] = await db.select().from(environmentLeases).where(eq(environmentLeases.id, id)); + return found!; + } + + /** The end state of an orphaned run lease: the ownership settle stopped the + * sandbox and kept it, and the stop was confirmed. */ + async function retainedOrphan(runId: string, extra: { expiresAt?: Date; metadata?: Record } = {}) { + const id = randomUUID(), providerLeaseId = `sandbox://fake/${id}`, now = new Date(); + await db.insert(environmentLeases).values({ id, companyId, environmentId, issueId, heartbeatRunId: runId, + status: "released", cleanupStatus: "success", leasePolicy: "ephemeral", provider: "fake", providerLeaseId, + acquiredAt: new Date(now.getTime() - 60_000), releasedAt: now, expiresAt: extra.expiresAt ?? null, + metadata: { + [ORPHANED_SANDBOX_RECOVERY_KEY]: { recoveredOrphanedRunLease: true, ownerProof: "pid_dead", runId }, + remoteExecutionTermination: { schema: "paperclip.remote-termination.v1", companyId, runId, leaseId: id, + provider: "fake", providerLeaseId, state: "stopped", confirmedAt: now.toISOString() }, + sandboxStopAndRetainReceipt: { schema: "paperclip.sandbox-stop-and-retain-receipt.v1", requestId: randomUUID(), + companyId, runId, leaseId: id, provider: "fake", providerLeaseId, builtinProvider: "fake", + method: "builtin.stopLease", confirmedAt: now.toISOString() }, + ...extra.metadata, + } }); + return id; + } + + async function pass(record = true) { + const report = await runOwnershipReconciliationPass(db, { state: createOwnershipReportState(), record }); + return { report, kind: report.kinds.find(entry => entry.kind === KIND)! }; + } + + it("marks the lease when the orphaned-run-lease settle stops and keeps its sandbox", async () => { + const runId = await run("failed"); + const id = randomUUID(); + await db.insert(environmentLeases).values({ id, companyId, environmentId, heartbeatRunId: runId, status: "active", + provider: "fake", providerLeaseId: `sandbox://fake/${id}`, metadata: { [ENVIRONMENT_LEASE_CONTROLLER_KEY]: { + bootId: "earlier-boot", pid: await exitedPid(), hostHash: currentProcessSpaceHash(), processStartedAt: null } } }); + await pass(); + expect((await row(id)).metadata?.[ORPHANED_SANDBOX_RECOVERY_KEY]).toMatchObject({ + recoveredOrphanedRunLease: true, ownerProof: "pid_dead", runId }); + }); + + it("destroys a retained orphan that nothing references, through the cleanup sweep", async () => { + const leaseId = await retainedOrphan(await run("failed")); + const { kind } = await pass(); + expect(kind).toMatchObject({ inProgress: 1, settled: 1 }); + const released = await row(leaseId); + expect(released).toMatchObject({ status: "pending_cleanup", leasePolicy: "ephemeral", + failureReason: "retained_orphan_sandbox_released" }); + expect(released.metadata?.remoteExecutionTermination).toBeUndefined(); + expect(released.metadata?.workspaceRecoveryCleanup).toMatchObject({ runId: released.heartbeatRunId, + reason: "retained_orphan_sandbox_released", ownerProof: "owner_run_terminal", stopReceipt: { state: "stopped" } }); + // Admission and Retry still see a stopped sandbox while the destroy waits. + expect(hasRecoveredCleanupStopReceipt(released)).toBe(true); + expect(await hasPendingRunCleanup(db, companyId, released.heartbeatRunId!)).toBe(false); + const [failedRun] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, released.heartbeatRunId!)); + expect(await taskChatRetryExecutionStopped(db, failedRun!)).toBe(true); + // Due now, and the sweep destroys it from the recorded lease. + expect(await nextEnvironmentCleanupAt(db)).toBeLessThanOrEqual(Date.now()); + const teardown = vi.fn(async ({ lease }: { lease: { providerLeaseId: string | null } }) => + ({ providerLeaseId: lease.providerLeaseId, state: "destroyed" })); + await heartbeatService(db, { environmentRuntime: { retryPendingSandboxTeardown: teardown } as unknown as HeartbeatEnvironmentRuntime }) + .sweepPendingCleanupLeases({ backoffMs: 5 * 60_000 }); + expect(teardown).toHaveBeenCalledOnce(); + expect((await row(leaseId)).status).toBe("expired"); + }); + + it.each([ + ["an open recovery action on the issue", async (_runId: string) => { + await db.insert(issueRecoveryActions).values({ companyId, sourceIssueId: issueId, kind: "active_run_watchdog", + cause: "fixture", fingerprint: randomUUID(), nextAction: "Recover the workspace." }); + return {}; + }], + ["a queued run on the issue", async (_runId: string) => { await run("queued"); return {}; }], + ["a scheduled retry on the issue", async (_runId: string) => { await run("scheduled_retry"); return {}; }], + ["a stop-and-retain intent", async () => ({ metadata: { sandboxStopAndRetain: { requestId: "pending" } } })], + ["a native export resume", async () => ({ metadata: { nativeWorkspaceExportResume: { requestId: "pending" } } })], + ["a workspace restore recovery", async () => ({ metadata: { workspaceRestoreRecovery: { runId: "fixture" } } })], + ["an in-flight cleanup claim", async () => ({ metadata: { pendingCleanupInFlight: true, pendingCleanupAttemptId: "a" } })], + ["a pending legacy shutdown fence", async () => ({ metadata: { legacyShutdownWorkspaceSettlement: { state: "pending" } } })], + ] as const)("never destroys a sandbox recovery still wants: %s", async (_label, arrange) => { + const runId = await run("failed"); + const extra = await arrange(runId); + const leaseId = await retainedOrphan(runId, extra); + const before = await row(leaseId); + const { kind } = await pass(); + expect(kind).toMatchObject({ settled: 0 }); + expect(await row(leaseId)).toEqual(before); + }); + + it("never destroys a sandbox another live lease holds, or one whose run is still active", async () => { + const live = await retainedOrphan(await run("running")); + const sharedRun = await run("failed"); + const shared = await retainedOrphan(sharedRun); + const sharedRow = await row(shared); + await db.insert(environmentLeases).values({ companyId, environmentId, heartbeatRunId: await run("running"), + status: "active", provider: "fake", providerLeaseId: sharedRow.providerLeaseId }); + await pass(); + expect((await row(live)).status).toBe("released"); + expect((await row(shared)).status).toBe("released"); + }); + + it("keeps a softly referenced orphan only until its retention window ends, and wakes then", async () => { + const runId = await run("failed"); + await db.insert(issueRecoveryActions).values({ companyId, sourceIssueId: issueId, kind: "active_run_watchdog", + cause: "fixture", fingerprint: randomUUID(), nextAction: "Recover the workspace." }); + const expiresAt = new Date(Date.now() + 60 * 60_000); + const leaseId = await retainedOrphan(runId, { expiresAt }); + const first = await pass(); + expect(first.kind).toMatchObject({ inProgress: 0 }); + expect(first.report.nextDueAt?.getTime()).toBeLessThanOrEqual(expiresAt.getTime()); + expect((await row(leaseId)).status).toBe("released"); + // Past the window no recovery path accepts the stopped sandbox as a source. + await db.update(environmentLeases).set({ expiresAt: new Date(Date.now() - 1_000) }).where(eq(environmentLeases.id, leaseId)); + const second = await pass(); + expect(second.kind).toMatchObject({ settled: 1 }); + expect((await row(leaseId)).status).toBe("pending_cleanup"); + }); + + it("never changes anything on a read-only pass", async () => { + const leaseId = await retainedOrphan(await run("failed")); + const before = await row(leaseId); + const { kind } = await pass(false); + expect(kind).toMatchObject({ inProgress: 1, settled: 0, verdicts: { gone: 1 } }); + expect(await row(leaseId)).toEqual(before); + }); + + it("leaves a reusable sandbox to warm reuse", async () => { + const leaseId = await retainedOrphan(await run("failed")); + await db.update(environmentLeases).set({ leasePolicy: "reuse_by_environment" }).where(eq(environmentLeases.id, leaseId)); + const before = await row(leaseId); + const { kind } = await pass(); + expect(kind).toMatchObject({ inProgress: 0 }); + expect(await row(leaseId)).toEqual(before); + }); + + it("keeps a sandbox with a pending native workspace export even past its retention window", async () => { + const runId = await run("failed"); + const leaseId = await retainedOrphan(runId, { expiresAt: new Date(Date.now() - 1_000) }); + const lease = await row(leaseId); + await db.update(heartbeatRuns).set({ runtimeMode: "native", runnerProfileJson: { nativeWorkspaceSync: { + schema: "paperclip.native-workspace-sync/v3", state: "prepared", descriptorSha256: "a".repeat(64), + baselineSha256: "b".repeat(64), finalHostSha256: null, workspaceId: randomUUID(), leaseId, + providerLeaseId: lease.providerLeaseId, remoteCwd: "/workspace", resourceDisposition: "destroy" } } }) + .where(eq(heartbeatRuns.id, runId)); + const { kind } = await pass(); + // Not listed, so the settle watch does not poll it while the export waits. + expect(kind).toMatchObject({ inProgress: 0 }); + expect(await row(leaseId)).toEqual(lease); + }); + + it("releases a workspace-less native run's orphan whose descriptor stays prepared", async () => { + const runId = await run("failed"); + const leaseId = await retainedOrphan(runId); + const lease = await row(leaseId); + await db.update(heartbeatRuns).set({ runtimeMode: "native", runnerProfileJson: { + nativeExecutionInput: { workspace: { access: "none", cwd: null } }, + nativeWorkspaceSync: { schema: "paperclip.native-workspace-sync/v3", state: "prepared", descriptorSha256: "a".repeat(64), + baselineSha256: "b".repeat(64), finalHostSha256: null, workspaceId: randomUUID(), leaseId, + providerLeaseId: lease.providerLeaseId, remoteCwd: "/workspace", resourceDisposition: "destroy" } } }) + .where(eq(heartbeatRuns.id, runId)); + // Listed (no export obligation is visible to the listing); the settle + // decides with the exact export check. + const { kind } = await pass(false); + expect(kind).toMatchObject({ inProgress: 1 }); + }); + + it("releases an orphan whose run's prepared descriptor names another sandbox", async () => { + const runId = await run("failed"); + const leaseId = await retainedOrphan(runId); + await db.update(heartbeatRuns).set({ runtimeMode: "native", runnerProfileJson: { + nativeWorkspaceSync: { schema: "paperclip.native-workspace-sync/v3", state: "prepared", descriptorSha256: "a".repeat(64), + baselineSha256: "b".repeat(64), finalHostSha256: null, workspaceId: randomUUID(), leaseId: randomUUID(), + providerLeaseId: "sandbox://fake/other", remoteCwd: "/workspace", resourceDisposition: "destroy" } } }) + .where(eq(heartbeatRuns.id, runId)); + const { kind } = await pass(); + expect(kind).toMatchObject({ settled: 1 }); + expect((await row(leaseId)).status).toBe("pending_cleanup"); + }); + + it("does not hide an orphan behind a prepared descriptor the settle would reject", async () => { + const runId = await run("failed"); + const leaseId = await retainedOrphan(runId); + const lease = await row(leaseId); + await db.update(heartbeatRuns).set({ runtimeMode: "native", runnerProfileJson: { + nativeWorkspaceSync: { state: "prepared", leaseId, providerLeaseId: lease.providerLeaseId } } }) + .where(eq(heartbeatRuns.id, runId)); + const { kind } = await pass(); + expect(kind).toMatchObject({ settled: 1 }); + expect((await row(leaseId)).status).toBe("pending_cleanup"); + }); + + it("leaves an ordinary retained sandbox (no orphan marker) alone", async () => { + const leaseId = await retainedOrphan(await run("failed")); + const { [ORPHANED_SANDBOX_RECOVERY_KEY]: _marker, ...metadata } = (await row(leaseId)).metadata!; + await db.update(environmentLeases).set({ metadata }).where(eq(environmentLeases.id, leaseId)); + const { kind } = await pass(); + expect(kind).toMatchObject({ inProgress: 0 }); + expect((await row(leaseId)).status).toBe("released"); + }); +}); diff --git a/server/src/__tests__/environment-run-lease-ownership.test.ts b/server/src/__tests__/environment-run-lease-ownership.test.ts index a8a666a0ebc..669293cf660 100644 --- a/server/src/__tests__/environment-run-lease-ownership.test.ts +++ b/server/src/__tests__/environment-run-lease-ownership.test.ts @@ -1,6 +1,6 @@ import { spawn } from "node:child_process"; import { randomUUID } from "node:crypto"; -import { eq } from "drizzle-orm"; +import { eq, sql } from "drizzle-orm"; import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import { agents, @@ -257,6 +257,17 @@ suite("environment_run_lease_active on the shared ownership path", () => { }); describe("leases saved before holders were stamped", () => { + it("dates the first cleanup attempt as a top-level key even for a scalar metadata root", async () => { + const runId = await run("failed"); + const id = randomUUID(); + await db.insert(environmentLeases).values({ id, companyId, heartbeatRunId: runId, status: "active", + metadata: sql`'["provider-wrote-an-array"]'::jsonb` }); + await runOwnershipReconciliationPass(db, { state: createOwnershipReportState() }); + const settled = await row(id); + expect(settled.status).toBe("pending_cleanup"); + expect(typeof settled.metadata?.pendingCleanupRecoveredAtMs).toBe("number"); + }); + it("keeps the run fact alone for one release on a single-host server", async () => { const runId = await run("failed"); const leaseId = await lease(runId, null); diff --git a/server/src/__tests__/heartbeat-held-comment-wake.test.ts b/server/src/__tests__/heartbeat-held-comment-wake.test.ts index 2ee971fd4fb..24856d54ef2 100644 --- a/server/src/__tests__/heartbeat-held-comment-wake.test.ts +++ b/server/src/__tests__/heartbeat-held-comment-wake.test.ts @@ -4,7 +4,7 @@ import { tmpdir } from "node:os"; import path from "node:path"; import { and, eq, sql } from "drizzle-orm"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; -import { agents, agentWakeupRequests, authUsers, budgetPolicies, companies, companyMemberships, costEvents, createDb, heartbeatRuns, +import { activityLog, agents, agentWakeupRequests, authUsers, budgetPolicies, companies, companyMemberships, costEvents, createDb, heartbeatRuns, issueComments, issues } from "@paperclipai/db"; import { accountRunCost } from "../services/run-cost-accounting.js"; import { DELIVERY_QUEUES, subscribeDeliveryWork } from "../services/delivery-work-notifications.js"; @@ -12,6 +12,8 @@ import { requestTaskRetry } from "../services/task-retry.js"; import { queueIssueAssignmentWakeup } from "../services/issue-assignment-wakeup.js"; import { registerServerAdapter, unregisterServerAdapter } from "../adapters/index.js"; import { heartbeatService } from "../services/heartbeat.js"; +import { createOwnershipReportState, runOwnershipReconciliationPass } from "../services/recovery/ownership-invariant-report.js"; +import { applyOwnershipReportBackstops } from "../services/native-runtime/native-finalization-reconciler.js"; import { promoteDeferredWorkIntents, TEMPORARY_REFUSAL_TRIGGERS, WORK_INTENT_TERMINAL_OUTCOMES } from "../services/work-intent-outbox.js"; import { recoveryPostgresSupport, startRecoveryPostgres } from "./helpers/recovery-postgres.js"; @@ -116,6 +118,12 @@ describePostgres("saved intents while another run's accounting is pending", () = return woken; } + /** The agent's run closed the task, as a native status decision records it. */ + const closeByAgent = (f: { companyId: string; issueId: string; agentId: string }, actorType = "agent") => + db.insert(activityLog).values({ companyId: f.companyId, actorType, actorId: actorType === "agent" ? f.agentId : "person", + action: "issue.updated", entityType: "issue", entityId: f.issueId, + details: { source: "native_status_decision", fromStatus: "in_progress", toStatus: "done" } }); + async function settle(f: { companyId: string }) { await heartbeat.resumeQueuedRuns(); await heartbeat.reconcileStrandedAssignedIssues({ companyId: f.companyId }); @@ -394,10 +402,111 @@ describePostgres("saved intents while another run's accounting is pending", () = expect(started).toHaveLength(1); }, 180_000); + // Chaos s9401/t69: a restart landed between the comment commit (which + // reopened the finished task to todo) and its wake write. Reconciler rule: + // an undelivered user comment newer than the task's last run gets a wake. + it("delivers once a user comment whose wake a restart lost after a succeeded run", async () => { + const f = await seedHeldComment(); + await db.update(heartbeatRuns).set({ costAccountingPending: false }).where(eq(heartbeatRuns.id, f.pendingRunId)); + // The run that finished the task was native, as in the wave, and the + // comment came after it; the comment's wake was never written. + await db.update(heartbeatRuns).set({ runtimeMode: "native", startedAt: new Date(Date.now() - 120_000), + createdAt: new Date(Date.now() - 120_000) }).where(eq(heartbeatRuns.id, f.sourceRunId)); + await db.delete(agentWakeupRequests).where(eq(agentWakeupRequests.id, f.wakeId)); + await db.update(issueComments).set({ createdAt: new Date(Date.now() - 60_000), updatedAt: new Date(Date.now() - 60_000) }) + .where(eq(issueComments.id, f.commentId)); + await db.update(issues).set({ status: "todo" }).where(eq(issues.id, f.issueId)); + const report = await runOwnershipReconciliationPass(db, { state: createOwnershipReportState(), record: false }); + expect(report.violations.filter(row => row.reason === "user_comment_without_wake").map(row => row.recordId)) + .toEqual([f.commentId]); + await applyOwnershipReportBackstops(db, report); + await db.update(agentWakeupRequests).set({ updatedAt: new Date(Date.now() - 60_000) }) + .where(eq(agentWakeupRequests.companyId, f.companyId)); + await settle(f); await settle(f); + expect(deliveries(f.commentId)).toHaveLength(1); + // Delivered: the rule no longer fires, and nothing delivers it again. + const again = await runOwnershipReconciliationPass(db, { state: createOwnershipReportState(), record: false }); + expect(again.violations.filter(row => row.reason === "user_comment_without_wake")).toEqual([]); + await applyOwnershipReportBackstops(db, again); + await settle(f); + expect(deliveries(f.commentId)).toHaveLength(1); + }, 180_000); + + // Chaos s9401/t72: a message queued its own run while the first run was + // finishing. The first run completed the task, so the queued run was + // cancelled as a stale continuation and the unread message was dropped. + it("reopens a finished task for a message whose queued run turned stale, and delivers it once", async () => { + const f = await seedHeldComment(); + await db.update(heartbeatRuns).set({ costAccountingPending: false }).where(eq(heartbeatRuns.id, f.pendingRunId)); + await db.delete(agentWakeupRequests).where(eq(agentWakeupRequests.id, f.wakeId)); + // The message's own queued run; the first run then completed the task. + const [wake] = await db.insert(agentWakeupRequests).values({ companyId: f.companyId, agentId: f.agentId, source: "automation", + triggerDetail: "system", reason: "issue_commented", status: "queued", requestedByActorType: "user", requestedByActorId: f.userId, + payload: { issueId: f.issueId, commentId: f.commentId, mutation: "comment" } }).returning(); + const [stale] = await db.insert(heartbeatRuns).values({ companyId: f.companyId, agentId: f.agentId, invocationSource: "automation", + triggerDetail: "system", status: "queued", wakeupRequestId: wake.id, + contextSnapshot: { issueId: f.issueId, taskId: f.issueId, commentId: f.commentId, wakeCommentId: f.commentId, + wakeCommentIds: [f.commentId], wakeReason: "issue_commented", source: "issue.comment" } }).returning(); + await db.update(agentWakeupRequests).set({ runId: stale.id }).where(eq(agentWakeupRequests.id, wake.id)); + // The first run, which started before the message, completed the task after it. + await db.update(heartbeatRuns).set({ finishedAt: new Date(Date.now() + 1_000) }).where(eq(heartbeatRuns.id, f.sourceRunId)); + await db.update(issues).set({ status: "done", completedAt: new Date() }).where(eq(issues.id, f.issueId)); + await closeByAgent(f); + await settle(f); await settle(f); + expect((await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, stale.id)))[0]).toMatchObject({ + status: "cancelled", errorCode: "continuation_task_ownership_changed" }); + await db.update(agentWakeupRequests).set({ updatedAt: new Date(Date.now() - 60_000) }) + .where(and(eq(agentWakeupRequests.companyId, f.companyId), eq(agentWakeupRequests.status, "deferred_issue_execution"))); + await settle(f); await settle(f); + expect(deliveries(f.commentId)).toHaveLength(1); + await settle(f); + expect(deliveries(f.commentId)).toHaveLength(1); + }, 180_000); + + // The durable half: the stale check ran before the closing run settled (or + // its save failed), so only the cancelled stale wake carries the message. + // Once the closing run settles, the reconciler saves it and the drain + // reopens the task and delivers it once. + it("recovers a message a stale continuation dropped once the closing run settles", async () => { + const f = await seedHeldComment(); + await db.update(heartbeatRuns).set({ costAccountingPending: false }).where(eq(heartbeatRuns.id, f.pendingRunId)); + await db.update(agentWakeupRequests).set({ status: "cancelled", finishedAt: new Date(), + error: "continuation_task_ownership_changed" }).where(eq(agentWakeupRequests.id, f.wakeId)); + await db.update(heartbeatRuns).set({ finishedAt: new Date(Date.now() + 1_000) }).where(eq(heartbeatRuns.id, f.sourceRunId)); + await db.update(issues).set({ status: "done", completedAt: new Date() }).where(eq(issues.id, f.issueId)); + // A person closed it: that decision stands. + await closeByAgent(f, "user"); + const byPerson = await runOwnershipReconciliationPass(db, { state: createOwnershipReportState(), record: false }); + expect(byPerson.violations.filter(row => row.reason === "user_comment_dropped_by_stale_continuation")).toEqual([]); + // The agent's run closed it later without reading the message. + await closeByAgent(f); + // Another message's own turn succeeded later; it did not receive this one. + await db.insert(heartbeatRuns).values({ companyId: f.companyId, agentId: f.agentId, invocationSource: "automation", + triggerDetail: "system", status: "succeeded", startedAt: new Date(Date.now() + 2_000), finishedAt: new Date(Date.now() + 3_000), + contextSnapshot: { issueId: f.issueId, wakeReason: "issue_commented", wakeCommentIds: [randomUUID()], + executionContinuation: { messages: [{ id: f.commentId }] } } }); + const report = await runOwnershipReconciliationPass(db, { state: createOwnershipReportState(), record: false }); + expect(report.violations.filter(row => row.reason === "user_comment_dropped_by_stale_continuation") + .map(row => row.recordId)).toEqual([f.commentId]); + expect(await applyOwnershipReportBackstops(db, report)).toEqual({ retryNeeded: false }); + await db.update(agentWakeupRequests).set({ updatedAt: new Date(Date.now() - 60_000) }) + .where(and(eq(agentWakeupRequests.companyId, f.companyId), eq(agentWakeupRequests.status, "deferred_issue_execution"))); + await settle(f); await settle(f); + expect(deliveries(f.commentId)).toHaveLength(1); + const again = await runOwnershipReconciliationPass(db, { state: createOwnershipReportState(), record: false }); + expect(again.violations.filter(row => row.recordId === f.commentId)).toEqual([]); + }, 180_000); + + it("keeps a reconciler pass due when the backstop cannot save a message", async () => { + const report = { violations: [{ rule: "I4_exactly_once_intent", kind: "issue_comment", recordId: randomUUID(), + companyId: "not-a-company-id", runId: null, issueId: "not-an-issue-id", reason: "user_comment_without_wake" }] }; + expect(await applyOwnershipReportBackstops(db, report as never)).toEqual({ retryNeeded: true }); + }); + it("keeps the terminal outcomes closed and every temporary refusal mapped to a release trigger", () => { expect([...WORK_INTENT_TERMINAL_OUTCOMES]).toEqual(["delivered", "consumed", "task_cancelled", "hard_stop"]); for (const refusal of ["budget.blocked", "company.inactive", "agent.not_invokable", "issue_tree_hold_active", - "heartbeat.scheduling_suppressed"]) expect(TEMPORARY_REFUSAL_TRIGGERS[refusal]).toBeTruthy(); + "heartbeat.scheduling_suppressed", "wake_lost"]) expect(TEMPORARY_REFUSAL_TRIGGERS[refusal]).toBeTruthy(); }); it.each([ diff --git a/server/src/__tests__/issue-comment-cancel-routes.test.ts b/server/src/__tests__/issue-comment-cancel-routes.test.ts index 4d40e68f317..410f697e7b5 100644 --- a/server/src/__tests__/issue-comment-cancel-routes.test.ts +++ b/server/src/__tests__/issue-comment-cancel-routes.test.ts @@ -181,6 +181,9 @@ function makeIssue() { }; } +const finalDenial = "Only the author of a queued comment can discard it, from that queued message's menu on [PAP-1353](/PAP/issues/PAP-1353). " + + "This denial is final: no board, admin or agent override exists, so do not try other routes. Tell the author to discard it there."; + function makeComment(overrides: Record = {}) { return { id: "comment-1", @@ -225,6 +228,9 @@ describe("issue comment cancel routes", () => { from: vi.fn(() => ({ where: vi.fn(() => ({ orderBy: vi.fn(async () => mockAuthoritativeQueueWakes), + // Agent mutation guards read a single row (no task-watchdog run here). + then: (resolve: (rows: unknown[]) => unknown, reject?: (error: unknown) => unknown) => + Promise.resolve([]).then(resolve, reject), })), })), })), @@ -260,6 +266,9 @@ describe("issue comment cancel routes", () => { await options?.afterTombstone?.(deleted, "tx"); return deleted; }); + mockAccessService.decide.mockImplementation(async ({ action }: { action: string }) => ({ + action, allowed: action === "issue:read", reason: "allow_default", explanation: "Fixture task is readable", + })); mockAccessService.canUser.mockResolvedValue(false); mockAccessService.hasPermission.mockResolvedValue(false); mockFeedbackService.listIssueVotesForUser.mockResolvedValue([]); @@ -366,10 +375,86 @@ describe("issue comment cancel routes", () => { .delete("/api/issues/11111111-1111-4111-8111-111111111111/comments/comment-1?mode=cancel"); expect(res.status).toBe(403); - expect(res.body.error).toBe("Only the comment author can cancel queued comments"); + expect(res.body.error).toBe(finalDenial); expect(mockIssueService.removeComment).not.toHaveBeenCalled(); }); + it("tells an agent that discarding a person's queued comment is author-only and final, with the task link", async () => { + const agent = { type: "agent", agentId: "22222222-2222-4222-8222-222222222222", companyId: "company-1", runId: "run-1", source: "agent_key" }; + const res = await request(await installActor(createApp(), agent)) + .delete("/api/issues/11111111-1111-4111-8111-111111111111/queued-comments/comment-1") + .send({ queueId: "queue-1", revision: "revision-1" }); + + expect(res.status).toBe(403); + expect(res.body.error).toBe(finalDenial); + expect(res.body.error).not.toBe("Board access required"); + expect(mockIssueService.removeComment).not.toHaveBeenCalled(); + }); + + it("returns 404 without task details when the agent cannot read the private task", async () => { + mockAccessService.decide.mockImplementation(async ({ action }: { action: string }) => ({ + action, allowed: false, reason: "deny_issue_private", explanation: "Private task", + })); + const agent = { type: "agent", agentId: "33333333-3333-4333-8333-333333333333", companyId: "company-1", runId: "run-9", source: "agent_key" }; + const res = await request(await installActor(createApp(), agent)) + .delete("/api/issues/11111111-1111-4111-8111-111111111111/queued-comments/comment-1") + .send({ queueId: "queue-1", revision: "revision-1" }); + + expect(res.status).toBe(404); + expect(JSON.stringify(res.body)).not.toContain("PAP-1353"); + }); + + it("points an author agent at the cancel route it can use instead of claiming no route exists", async () => { + mockIssueService.getComment.mockResolvedValue(makeComment({ authorUserId: null, authorAgentId: "33333333-3333-4333-8333-333333333333" })); + const agent = { type: "agent", agentId: "33333333-3333-4333-8333-333333333333", companyId: "company-1", runId: "run-9", source: "agent_key" }; + const res = await request(await installActor(createApp(), agent)) + .delete("/api/issues/11111111-1111-4111-8111-111111111111/queued-comments/comment-1") + .send({ queueId: "queue-1", revision: "revision-1" }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("/api/issues/11111111-1111-4111-8111-111111111111/comments/comment-1?mode=cancel"); + expect(res.body.error).not.toContain("final"); + }); + + it("lets an author agent discard its own queued comment through the cancel route", async () => { + // Another agent queued this comment behind the assignee's active run; it can read and write the task. + mockAccessService.decide.mockImplementation(async ({ action }: { action: string }) => ({ + action, allowed: true, reason: "allow_default", explanation: "Assignee task", + })); + mockIssueService.getComment.mockResolvedValue(makeComment({ authorUserId: null, authorAgentId: "33333333-3333-4333-8333-333333333333" })); + mockIssueService.removeComment.mockResolvedValue(makeComment({ authorUserId: null, authorAgentId: "33333333-3333-4333-8333-333333333333" })); + const agent = { type: "agent", agentId: "33333333-3333-4333-8333-333333333333", companyId: "company-1", runId: "run-9", source: "agent_key" }; + const res = await request(await installActor(createApp(), agent)) + .delete("/api/issues/11111111-1111-4111-8111-111111111111/comments/comment-1?mode=cancel"); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(mockIssueService.removeComment).toHaveBeenCalledWith("comment-1"); + }); + + it("returns 404 instead of the author-only denial when a non-author cannot read the task", async () => { + mockIssueService.getComment.mockResolvedValue(makeComment({ authorUserId: "someone-else" })); + mockAccessService.decide.mockImplementation(async ({ action }: { action: string }) => ({ + action, allowed: false, reason: "deny_issue_private", explanation: "Private task", + })); + const res = await request(await installActor(createApp())) + .delete("/api/issues/11111111-1111-4111-8111-111111111111/comments/comment-1?mode=cancel"); + + expect(res.status).not.toBe(403); + expect(JSON.stringify(res.body)).not.toContain("PAP-1353"); + expect(mockIssueService.removeComment).not.toHaveBeenCalled(); + }); + + it("does not reveal another company's task through the agent queued-comment denial", async () => { + mockIssueService.getById.mockResolvedValue({ ...makeIssue(), companyId: "company-2" }); + const agent = { type: "agent", agentId: "22222222-2222-4222-8222-222222222222", companyId: "company-1", runId: "run-1", source: "agent_key" }; + const res = await request(await installActor(createApp(), agent)) + .delete("/api/issues/11111111-1111-4111-8111-111111111111/queued-comments/comment-1") + .send({ queueId: "queue-1", revision: "revision-1" }); + + expect(res.status).toBe(404); + expect(JSON.stringify(res.body)).not.toContain("PAP-1353"); + }); + it("deletes a normal authored comment as a tombstone without returning the original body", async () => { mockIssueService.getComment.mockResolvedValue( makeComment({ diff --git a/server/src/__tests__/ownership-reconciliation-report.test.ts b/server/src/__tests__/ownership-reconciliation-report.test.ts index e303af8eaef..d584a990654 100644 --- a/server/src/__tests__/ownership-reconciliation-report.test.ts +++ b/server/src/__tests__/ownership-reconciliation-report.test.ts @@ -13,6 +13,7 @@ import { createDb, environmentLeases, heartbeatRuns, + issueComments, issues, nativeRunFinalizations, sandboxCreationOperations, @@ -142,8 +143,9 @@ describe("report-only ownership reconciliation", () => { expect(new Set(names).size).toBe(names.length); expect(names.length).toBeGreaterThanOrEqual(40); expect(OWNABLE_KINDS.filter(kind => kind.settle !== null).map(kind => kind.id)) - .toEqual(["native_accepted_result_lease", "native_finalizer_lease", "native_cleanup_lease", "environment_run_lease_active", - "environment_teardown_claim", "native_workspace_finalize_attempt", "wake_request_claimed", "legacy_controller_lease"]); + .toEqual(["native_accepted_result_lease", "native_finalizer_lease", "native_cleanup_lease", "sandbox_creation_intent", + "environment_run_lease_active", "environment_teardown_claim", "environment_retained_orphan_sandbox", + "native_workspace_finalize_attempt", "wake_request_claimed", "legacy_controller_lease"]); expect(OWNABLE_KINDS.filter(kind => !kind.listInProgress).every(kind => kind.coverageGap)).toBe(true); }); @@ -405,6 +407,35 @@ describe("report-only ownership reconciliation", () => { expect(report.violations.filter(violation => violation.rule === "I4_exactly_once_intent")).toEqual([]); }); + // Reconciler rule (chaos s9401/t69): an undelivered user comment newer than + // the task's last run gets a wake. A carried, a too-young, or an + // already-read comment does not fire it. + it("finds a user comment newer than the task's last run that no wake carries", async () => { + const issueId = randomUUID(); + await db.insert(issues).values({ id: issueId, companyId, title: "Reopened", status: "todo", assigneeAgentId: agentId }); + await db.insert(heartbeatRuns).values({ companyId, agentId, status: "succeeded", invocationSource: "manual", + runtimeMode: "native", contextSnapshot: { issueId }, startedAt: minutesAgo(5), createdAt: minutesAgo(5), finishedAt: minutesAgo(4) }); + const [lost, carried, young] = await db.insert(issueComments).values([ + { companyId, issueId, authorUserId: "user-1", body: "lost wake", createdAt: minutesAgo(2) }, + { companyId, issueId, authorUserId: "user-1", body: "carried", createdAt: minutesAgo(2) }, + { companyId, issueId, authorUserId: "user-1", body: "young", createdAt: new Date(Date.now() - 5_000) }, + ]).returning(); + // An agent's own comment written under its user (local CLI) is not user intent. + await db.insert(issueComments).values({ companyId, issueId, authorUserId: "user-1", authorAgentId: agentId, + body: "self", createdAt: minutesAgo(2) }); + await db.insert(agentWakeupRequests).values({ companyId, agentId, source: "automation", status: "queued", + payload: { issueId, commentId: carried.id } }); + const report = await runOwnershipReconciliationPass(db, { state: createOwnershipReportState(), record: false }); + expect(report.violations.filter(row => row.reason === "user_comment_without_wake").map(row => row.recordId)).toEqual([lost.id]); + // The young comment may still get its normal wake: a deadline, not a report. + expect(report.nextDueAt?.getTime()).toBe(young.createdAt.getTime() + 30_000); + // A run that started after the comment read it. + await db.insert(heartbeatRuns).values({ companyId, agentId, status: "succeeded", invocationSource: "manual", + contextSnapshot: { issueId }, startedAt: minutesAgo(1), createdAt: minutesAgo(1), finishedAt: new Date() }); + const later = await runOwnershipReconciliationPass(db, { state: createOwnershipReportState(), record: false }); + expect(later.violations.filter(row => row.reason === "user_comment_without_wake")).toEqual([]); + }); + it("keeps a backstop deadline for a lifted pause still inside its grace window", async () => { const updatedAt = minutesAgo(1); await db.insert(agentWakeupRequests).values({ companyId, agentId, source: "automation", status: "deferred_issue_execution", diff --git a/server/src/__tests__/sandbox-creation-intent-ownership.test.ts b/server/src/__tests__/sandbox-creation-intent-ownership.test.ts new file mode 100644 index 00000000000..55f6137a670 --- /dev/null +++ b/server/src/__tests__/sandbox-creation-intent-ownership.test.ts @@ -0,0 +1,135 @@ +import { spawn } from "node:child_process"; +import { randomUUID } from "node:crypto"; +import { eq } from "drizzle-orm"; +import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { companies, createDb, sandboxCreationOperations } from "@paperclipai/db"; +import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; +import { currentControllerProcessStamp, currentProcessSpaceHash } from "../services/controller-process-identity.js"; +import { nextEnvironmentCleanupAt } from "../services/environment-cleanup-state.js"; +import { createOwnershipReportState, runOwnershipReconciliationPass } from "../services/recovery/ownership-invariant-report.js"; +import { settleSandboxCreationIntent } from "../services/sandbox-creation-intent-settlement.js"; + +const support = await getEmbeddedPostgresTestSupport(); +const suite = support.supported ? describe : describe.skip; +const KIND = "sandbox_creation_intent"; + +async function exitedPid(): Promise { + const child = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { stdio: "ignore" }); + const pid = child.pid!; + await new Promise(resolve => { child.once("exit", () => resolve()); child.kill("SIGKILL"); }); + return pid; +} + +suite("sandbox_creation_intent on the shared ownership path", () => { + let db!: ReturnType; + let temporary: Awaited> | null = null; + let companyId = ""; + + beforeAll(async () => { + temporary = await startEmbeddedPostgresTestDatabase("paperclip-sandbox-creation-intent-ownership-"); + db = createDb(temporary.connectionString); + companyId = randomUUID(); + await db.insert(companies).values({ id: companyId, name: "Paperclip", issuePrefix: "SCI", + requireBoardApprovalForNewAgents: false }); + }, 30_000); + + afterEach(async () => { + await db.delete(sandboxCreationOperations); + }); + + afterAll(async () => { + await temporary?.cleanup(); + }); + + /** The state a crash between the intent write and the provider reply leaves: + * a pending intent waiting for its 30-minute settle deadline. */ + async function intent(creationController: Record) { + const id = randomUUID(); + const settleAt = new Date(Date.now() + 30 * 60_000); + await db.insert(sandboxCreationOperations).values({ id, companyId, originalEnvironmentId: randomUUID(), + runId: randomUUID(), requestKey: "run", provider: "fixture", driverKey: "fixture", state: "pending", config: {}, + nextAttemptAt: settleAt, metadata: { creationController, creationSettleAt: settleAt.toISOString() } }); + return id; + } + + async function row(id: string) { + const [found] = await db.select().from(sandboxCreationOperations).where(eq(sandboxCreationOperations.id, id)); + return found!; + } + + async function pass(record = true) { + const report = await runOwnershipReconciliationPass(db, { state: createOwnershipReportState(), record }); + return report.kinds.find(entry => entry.kind === KIND)!; + } + + it("makes a dead creator's intent due on the next pass, keeping the provider absence deadline", async () => { + const id = await intent({ bootId: "earlier-boot", pid: await exitedPid(), hostHash: currentProcessSpaceHash(), + processStartedAt: null }); + expect(await pass()).toMatchObject({ settled: 1, reasons: { pid_dead: 1 } }); + const settled = await row(id); + expect(settled.state).toBe("pending"); + expect(settled.nextAttemptAt.getTime()).toBeLessThanOrEqual(Date.now()); + expect(settled.metadata).toMatchObject({ creationControllerLost: true, creationControllerLostProof: "pid_dead" }); + // Creation recovery is due now; the saved settle deadline still guards absence. + expect(await nextEnvironmentCleanupAt(db)).toBeLessThanOrEqual(Date.now()); + expect(Date.parse(String(settled.metadata.creationSettleAt))).toBeGreaterThan(Date.now() + 25 * 60_000); + // A lost creator's intent belongs to creation recovery now. + expect(await pass()).toMatchObject({ inProgress: 0 }); + }); + + it("keeps the deadline of a live creator, a creator on another host, and an intent with no creator", async () => { + const ids = [ + await intent(currentControllerProcessStamp()), + await intent({ bootId: "other", pid: await exitedPid(), hostHash: "f".repeat(32), processStartedAt: null }), + await intent({}), + ]; + const before = await Promise.all(ids.map(row)); + expect(await pass()).toMatchObject({ settled: 0, + reasons: { current_process: 1, different_process_space: 1, no_owner_stamp: 1 } }); + expect(await Promise.all(ids.map(row))).toEqual(before); + }); + + describe("the settle compares and swaps on the exact creator it was given", () => { + async function record(id: string, creator: Record) { + return { recordId: id, companyId, runId: null, issueId: null, updatedAt: new Date(), + owner: { kind: "process" as const, stamp: { kind: "local" as const, scope: KIND, generation: null, host: null, + processSpace: String(creator.hostHash), bootId: String(creator.bootId), pid: Number(creator.pid), + processStartedAt: null, processGroupId: null } } }; + } + const proof = { verdict: "gone" as const, reason: "pid_dead" as const }; + + it.each([ + ["a creator that changed since the proof", "owner_changed_since_proof"], + ["an intent already due", "intent_already_due"], + ["an intent no longer pending", "intent_no_longer_pending"], + ["an intent whose creator is already lost", "creator_already_lost"], + ] as const)("leaves %s unchanged", async (label, reason) => { + const creator = { bootId: "earlier-boot", pid: await exitedPid(), hostHash: currentProcessSpaceHash(), processStartedAt: null }; + const id = await intent(creator); + if (label.startsWith("a creator")) { + await db.update(sandboxCreationOperations).set({ metadata: { creationController: { ...creator, bootId: "newer-boot" } } }) + .where(eq(sandboxCreationOperations.id, id)); + } else if (label.includes("already due")) { + await db.update(sandboxCreationOperations).set({ nextAttemptAt: new Date(Date.now() - 1_000) }) + .where(eq(sandboxCreationOperations.id, id)); + } else if (label.includes("no longer pending")) { + await db.update(sandboxCreationOperations).set({ state: "cleanup_pending" }).where(eq(sandboxCreationOperations.id, id)); + } else { + await db.update(sandboxCreationOperations).set({ metadata: { creationController: creator, creationControllerLost: true } }) + .where(eq(sandboxCreationOperations.id, id)); + } + const before = await row(id); + expect(await settleSandboxCreationIntent(db, await record(id, creator), { now: new Date(), proof })) + .toEqual({ settled: false, reason }); + expect(await row(id)).toEqual(before); + }); + }); + + it("never changes an intent on a read-only pass", async () => { + const id = await intent({ bootId: "earlier-boot", pid: await exitedPid(), hostHash: currentProcessSpaceHash(), + processStartedAt: null }); + const before = await row(id); + expect(await pass(false)).toMatchObject({ settled: 0, verdicts: { gone: 1 } }); + expect(await row(id)).toEqual(before); + }); +}); diff --git a/server/src/__tests__/sandbox-creation-operations.test.ts b/server/src/__tests__/sandbox-creation-operations.test.ts index d66102567f0..784f7983a7a 100644 --- a/server/src/__tests__/sandbox-creation-operations.test.ts +++ b/server/src/__tests__/sandbox-creation-operations.test.ts @@ -367,12 +367,12 @@ describe("durable sandbox creation through the runtime entry point", () => { await runtime().retryPendingSandboxTeardown({ environment: null, lease: (await environmentService(db).getLeaseById(lease!.id))! }); expect(resources.size).toBe(0); expect(destroys).toBe(1); expect(creates).toBe(1); }); - it("pages past more unprovable creations than one page to find an abandoned one", async () => { + it("pages past more unprovable creations than one settle page (200) to find an abandoned one", async () => { const pid = await exitedPid(); const operation = await interruptedCreation(async current => ({ ...current, bootId: randomUUID(), pid })); const { id: _id, runId: _runId, ...row } = operation; // Sorted ahead of the abandoned intent: creators on another host cannot be judged. - await db.insert(sandboxCreationOperations).values(Array.from({ length: 105 }, (_, index) => ({ ...row, + await db.insert(sandboxCreationOperations).values(Array.from({ length: 205 }, (_, index) => ({ ...row, id: `00000000-0000-4000-8000-${String(index).padStart(12, "0")}`, runId: randomUUID(), metadata: { ...row.metadata, creationController: { ...(row.metadata.creationController as object), bootId: randomUUID(), pid, hostHash: "f".repeat(32) } } }))); await runtime().flushDeferredOrphanCleanups(); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index be8104f0189..7a3c6542b1d 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -415,6 +415,20 @@ const MAX_ISSUE_COMMENT_LIMIT = 500; const updateIssueRouteSchema = updateIssueSchema.extend({ interrupt: z.boolean().optional(), }); +/** + * Sent only to a caller that did not write the queued comment: its author is the + * only one who can discard it, and no board, admin or agent override exists. Say so with the task link, so an agent that tried can + * relay the human route at once instead of searching for another endpoint. + */ +function queuedCommentAuthorOnlyDenial(issue: { identifier?: string | null }) { + const identifier = issue.identifier ?? null; + const prefix = identifier && /^[A-Z][A-Z0-9]*-\d+$/.test(identifier) ? identifier.split("-")[0] : null; + const where = identifier && prefix ? `[${identifier}](/${prefix}/issues/${identifier})` : "its task"; + return `Only the author of a queued comment can discard it, from that queued message's menu on ${where}. ` + + "This denial is final: no board, admin or agent override exists, so do not try other routes. " + + `Tell the author to discard it there.`; +} + const queuedCommentMutationTargetSchema = z.object({ queueId: z.string().min(1), revision: z.string().min(1), @@ -17564,6 +17578,25 @@ export function issueRoutes( "/issues/:id/queued-comments/:commentId", validate(queuedCommentMutationTargetSchema), async (req, res) => { + if (req.actor.type === "agent") { + // Agents never use this board route. Check visibility first so a denial + // never confirms a task the agent cannot read. + const issue = await getAccessibleResource(req, res, svc.getById(req.params.id as string), "Issue not found"); + if (!issue) return; + if (!(await assertIssueReadAllowed(req, res, issue))) return; + const comment = await svc.getComment(req.params.commentId as string); + if (!comment || comment.issueId !== issue.id) { + res.status(404).json({ error: "Comment not found" }); + return; + } + if (comment.authorAgentId && comment.authorAgentId === req.actor.agentId) { + throw forbidden( + "Agents discard their own queued comments with " + + `DELETE /api/issues/${issue.id}/comments/${comment.id}?mode=cancel, not this board route.`, + ); + } + throw forbidden(queuedCommentAuthorOnlyDenial(issue)); + } assertBoard(req); if (!req.actor.userId) throw forbidden("Board user context required"); const id = req.params.id as string; @@ -18845,8 +18878,9 @@ export function issueRoutes( : false; if (deleteMode === "cancel" || pendingQueueWake || isLegacyQueuedComment) { if (!actorOwnsComment) { + if (!(await assertIssueReadAllowed(req, res, issue))) return; res.status(403).json({ - error: "Only the comment author can cancel queued comments", + error: queuedCommentAuthorOnlyDenial(issue), }); return; } diff --git a/server/src/services/chat-channels/task-retry-authority.ts b/server/src/services/chat-channels/task-retry-authority.ts index d17bcc4fc04..c2018c6e5ff 100644 --- a/server/src/services/chat-channels/task-retry-authority.ts +++ b/server/src/services/chat-channels/task-retry-authority.ts @@ -3,7 +3,7 @@ import { agentWakeupRequests, environmentLeases, heartbeatRuns, issueRecoveryAct import { buildExecutionContinuation } from "../execution-continuation.js"; import { readTaskRetryIntent } from "../task-retry.js"; import { hasNativeLocalProcessStop, hasHistoricalSuspendedNativeSession, recordedLocalExecutionStopped } from "../native-local-process-stop.js"; -import { hasRemoteTerminationReceipt } from "../remote-execution-termination.js"; +import { hasRecoveredCleanupStopReceipt, hasRemoteTerminationReceipt } from "../remote-execution-termination.js"; import { adapterExecutionControls } from "../adapter-execution-control.js"; import { getExecutionBlocker, hasPendingRunCleanup } from "../execution-blocker.js"; import { readSystemRecovery } from "../system-recovery-state.js"; @@ -74,7 +74,11 @@ async function taskChatRetryExecutionContained(db: Db, run: typeof heartbeatRuns const leases = await db.select().from(environmentLeases).where(and( eq(environmentLeases.companyId, run.companyId), eq(environmentLeases.heartbeatRunId, run.id), )).for("share", { noWait: true }); - if (leases.some(lease => lease.provider !== "local")) return leases.every(hasRemoteTerminationReceipt); + // A verified stop counts, including one whose destroy is queued as stopped + // cleanup (a recovered or retained orphan sandbox awaiting teardown). + if (leases.some(lease => lease.provider !== "local")) { + return leases.every(lease => hasRemoteTerminationReceipt(lease) || hasRecoveredCleanupStopReceipt(lease)); + } return await recordedLocalExecutionStopped(db, run) || await hasNativeLocalProcessStop(db, run.companyId, run.id) || await hasHistoricalSuspendedNativeSession(db, run); diff --git a/server/src/services/controller-process-identity.test.ts b/server/src/services/controller-process-identity.test.ts index e029bf52589..2fd8b9a465f 100644 --- a/server/src/services/controller-process-identity.test.ts +++ b/server/src/services/controller-process-identity.test.ts @@ -4,7 +4,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { processExists, processStartsDefinitelyDiffer } from "./controller-process-identity.js"; import { isPidAlive, isProcessGroupAlive } from "./local-service-supervisor.js"; import { readProcessStartedAt } from "./hot-restart.js"; -import { verifiedRecoveryProcessIsAlive } from "./native-runtime/native-session-executor.js"; +import { recoveryProcessMayBeAlive } from "./native-runtime/native-session-executor.js"; describe("shared process identity proof", () => { afterEach(() => vi.restoreAllMocks()); @@ -41,9 +41,14 @@ describe("shared process identity proof", () => { try { const observed = new Date((await readProcessStartedAt(live.pid!))!); const startedAt = new Date(observed.getTime() + 500).toISOString(); - await expect(verifiedRecoveryProcessIsAlive({ pid: live.pid!, startedAt })).resolves.toBe(true); - const recycled = new Date(observed.getTime() + 5_000).toISOString(); - await expect(verifiedRecoveryProcessIsAlive({ pid: live.pid!, startedAt: recycled })).resolves.toBe(false); + await expect(recoveryProcessMayBeAlive({ pid: live.pid!, startedAt })).resolves.toBe(true); + // The live PID started definitely after the recorded runner: reused, so gone. + const recycled = new Date(observed.getTime() - 5_000).toISOString(); + await expect(recoveryProcessMayBeAlive({ pid: live.pid!, startedAt: recycled })).resolves.toBe(false); + // A live PID that started before the recorded runner is an inconsistent + // stamp (shared prover), never exit evidence. + const inconsistent = new Date(observed.getTime() + 5_000).toISOString(); + await expect(recoveryProcessMayBeAlive({ pid: live.pid!, startedAt: inconsistent })).resolves.toBe(true); } finally { const exited = once(live, "exit"); live.kill("SIGKILL"); diff --git a/server/src/services/environment-cleanup-state.ts b/server/src/services/environment-cleanup-state.ts index c281e19360c..a2c9e86dd7d 100644 --- a/server/src/services/environment-cleanup-state.ts +++ b/server/src/services/environment-cleanup-state.ts @@ -55,12 +55,13 @@ export function recoverableSandboxCreationCondition() { )); } -/** A run lease the ownership settle moved to cleanup, before its first +/** A run lease the ownership settle moved to cleanup (an orphaned active + * lease, or a retained orphan sandbox nothing references any more), before its first * cleanup attempt. Its holder is proven gone, so the first stop or teardown is * due now regardless of how recently the lease was used; a failed attempt * counts and the normal retry backoff applies from then on. */ export function recoveredOrphanFirstAttemptSql() { - return sql`(${environmentLeases.failureReason} = 'orphaned_active_lease_recovered' + return sql`(${environmentLeases.failureReason} in ('orphaned_active_lease_recovered', 'retained_orphan_sandbox_released') or ${environmentLeases.metadata}->'sandboxStopAndRetain'->>'recoveredOrphanedRunLease' = 'true') and coalesce(${environmentLeases.metadata}->>'pendingCleanupRetryAttempts', '0') = '0' and coalesce(${environmentLeases.metadata}->>'pendingCleanupInFlight', 'false') <> 'true'`; diff --git a/server/src/services/environment-retained-orphan-settlement.ts b/server/src/services/environment-retained-orphan-settlement.ts new file mode 100644 index 00000000000..285952b4c68 --- /dev/null +++ b/server/src/services/environment-retained-orphan-settlement.ts @@ -0,0 +1,197 @@ +import { and, eq, inArray, ne, notInArray, sql } from "drizzle-orm"; +import { environmentLeases, heartbeatRuns, issueRecoveryActions, type Db } from "@paperclipai/db"; +import { writeEnvironmentCleanup } from "./environment-cleanup-notifications.js"; +import { nativeWorkspaceExportPendingForLease } from "./native-runtime/native-workspace-finalization-state.js"; +import type { OwnerProof } from "./owner-proof.js"; +import type { InProgressRecord, SettleOutcome } from "./recovery/ownership-registry.js"; +import { hasRemoteTerminationReceipt } from "./remote-execution-termination.js"; +import { ORPHANED_SANDBOX_RECOVERY_KEY } from "./sandbox-stop-and-retain.js"; + +/** Failure reason on a retained orphan sandbox released for destruction. */ +export const RETAINED_ORPHAN_RELEASED_REASON = "retained_orphan_sandbox_released"; + +const TERMINAL_RUN_STATUSES = ["succeeded", "failed", "cancelled", "timed_out", "interrupted"]; +/** Runs on the same issue that may still continue from this sandbox. */ +const OPEN_RUN_STATUSES = ["queued", "scheduled_retry", "running"]; + +type LeaseRow = typeof environmentLeases.$inferSelect; + +/** A retained orphan: the ownership settle stopped and kept its sandbox, and + * the stop is confirmed. */ +export function retainedOrphanSandboxCondition() { + return and(eq(environmentLeases.status, "released"), eq(environmentLeases.cleanupStatus, "success"), + // An ephemeral lease only: a reusable lease stays for warm reuse, and a + // retain-on-failure lease belongs to workspace restore recovery. + eq(environmentLeases.leasePolicy, "ephemeral"), + sql`${environmentLeases.heartbeatRunId} is not null`, sql`${environmentLeases.provider} is not null`, + sql`${environmentLeases.providerLeaseId} is not null`, + sql`${environmentLeases.metadata} ? ${ORPHANED_SANDBOX_RECOVERY_KEY}`, + sql`${environmentLeases.metadata} ? 'sandboxStopAndRetainReceipt'`); +} + +/** SQL mirror of the "held only while the stop is a usable source" checks in + * `heldBy`, so a held orphan is not listed (and not polled) until its last + * reference clears or its retention window ends. The settle re-checks all of + * them, plus the pending native export, under the row lock. */ +function softlyHeldSql(now?: Date) { + const at = now ? sql`${now.toISOString()}::timestamptz` : sql`now()`; + const issue = sql`coalesce(run.native_issue_id, run.issue_id, ${environmentLeases.issueId})`; + return sql`(${environmentLeases.expiresAt} is null or ${environmentLeases.expiresAt} > ${at}) and ( + exists (select 1 from issue_recovery_actions action, heartbeat_runs run + where run.id = ${environmentLeases.heartbeatRunId} and action.company_id = ${environmentLeases.companyId} + and action.source_issue_id = ${issue} and action.status not in ('resolved', 'cancelled')) + or exists (select 1 from heartbeat_runs run, heartbeat_runs other + where run.id = ${environmentLeases.heartbeatRunId} and other.company_id = ${environmentLeases.companyId} + and other.id <> run.id and other.status in ('queued', 'scheduled_retry', 'running') + and (other.issue_id = ${issue} or other.native_issue_id = ${issue})))`; +} + +function alwaysHeldSql() { + return sql`(${environmentLeases.metadata} ?| array['nativeWorkspaceExportResume', 'sandboxStopAndRetain', + 'workspaceRestoreRecovery', 'workspaceRecoveryCleanup', 'runnerTaskSourceConsumer'] + or coalesce(${environmentLeases.metadata}->>'pendingCleanupInFlight', 'false') = 'true' + or coalesce(${environmentLeases.metadata}#>>'{legacyShutdownWorkspaceSettlement,state}' = 'pending', false) + or exists (select 1 from heartbeat_runs run where run.id = ${environmentLeases.heartbeatRunId} + and run.status not in ('succeeded', 'failed', 'cancelled', 'timed_out', 'interrupted')) + -- A prepared native workspace bound to this exact sandbox, for a run with + -- workspace access. This is deliberately no wider than the settle's own + -- export check: an orphan the settle would release must never be hidden + -- here. Narrower cases (a prepared remote workspace, an observed failure) + -- are listed and held by the settle until their export settles. + -- The descriptor must also be one the settle's reader accepts + -- (readNativeWorkspaceSyncReference); an invalid one holds nothing. + or exists (select 1 from heartbeat_runs run where run.id = ${environmentLeases.heartbeatRunId} + and run.runtime_mode = 'native' + and run.runner_profile_json#>>'{nativeWorkspaceSync,schema}' in + ('paperclip.native-workspace-sync/v1', 'paperclip.native-workspace-sync/v2', 'paperclip.native-workspace-sync/v3') + and run.runner_profile_json#>>'{nativeWorkspaceSync,state}' = 'prepared' + and coalesce(run.runner_profile_json#>>'{nativeWorkspaceSync,descriptorSha256}' ~ '^[0-9a-f]{64}$', false) + and coalesce(run.runner_profile_json#>>'{nativeWorkspaceSync,baselineSha256}' ~ '^[0-9a-f]{64}$', false) + and coalesce(jsonb_typeof(run.runner_profile_json#>'{nativeWorkspaceSync,finalHostSha256}'), 'null') = 'null' + and jsonb_typeof(run.runner_profile_json#>'{nativeWorkspaceSync,workspaceId}') = 'string' + and jsonb_typeof(run.runner_profile_json#>'{nativeWorkspaceSync,remoteCwd}') = 'string' + and coalesce(run.runner_profile_json#>>'{nativeWorkspaceSync,resourceDisposition}', 'destroy') + in ('keep_running', 'stop_and_retain', 'destroy') + and run.runner_profile_json#>>'{nativeWorkspaceSync,leaseId}' = ${environmentLeases.id}::text + and run.runner_profile_json#>>'{nativeWorkspaceSync,providerLeaseId}' = ${environmentLeases.providerLeaseId} + and coalesce(run.runner_profile_json#>>'{nativeExecutionInput,workspace,access}', '') <> 'none') + or exists (select 1 from environment_leases other where other.id <> ${environmentLeases.id} + and other.provider = ${environmentLeases.provider} and other.provider_lease_id = ${environmentLeases.providerLeaseId} + and other.status in ('active', 'retained', 'pending_cleanup')))`; +} + +/** Retained orphans that nothing appears to reference now. */ +// The listing is built once, so it reads the database clock. +export function releasableRetainedOrphanCondition(now?: Date) { + return and(retainedOrphanSandboxCondition(), sql`not ${alwaysHeldSql()}`, sql`not (${softlyHeldSql(now)})`); +} + +/** When the next retained orphan's retention window ends while only soft + * references hold it; the reconciler wakes then to release it. */ +export async function nextRetainedOrphanDueAt(db: Db, now: Date): Promise { + const [row] = await db.select({ due: sql`min(${environmentLeases.expiresAt})` }).from(environmentLeases) + .where(and(retainedOrphanSandboxCondition(), sql`not ${alwaysHeldSql()}`, sql`${softlyHeldSql(now)}`, + sql`${environmentLeases.expiresAt} is not null`)); + return row?.due ? new Date(row.due) : null; +} + +/** + * Settle of the `environment_retained_orphan_sandbox` kind (plan Phase 2). + * + * The orphaned-run-lease settle stops a dead holder's sandbox and keeps it, + * because recovery may still need its files. This settle destroys that + * retained orphan once no obligation references it, so it is never kept + * longer than the environment's own retention allows. + * + * It decides from database facts only, inside one transaction that locks the + * lease row: + * - the run is terminal; + * - always held: an active operation on the sandbox (a native export resume, + * a stop-and-retain intent, an in-flight cleanup claim, a workspace restore + * recovery, a pending legacy shutdown fence, a bound source-recovery + * consumer) or another live lease on the same provider resource; + * - always held: a pending native workspace export or preservation fence; + * - held while the stop receipt is still a usable workspace source (the + * lease's provider-attested `expiresAt` has not passed): an open recovery + * action on the issue, or a queued, scheduled or running run on the issue. Past that + * retention window no recovery path accepts the stopped sandbox as a source + * (`hasConfirmedSandboxStopAndRetain`), so these no longer hold it. + * + * Then it hands the sandbox to the pending-cleanup sweep for destruction, + * with a compare-and-swap on the exact row it judged. It never calls a + * provider itself and never runs on the turn path. + */ +export async function settleRetainedOrphanSandbox(db: Db, record: InProgressRecord, + context: { now: Date; proof: OwnerProof }): Promise { + if (!record.runId) return { settled: false, reason: "lease_has_no_run" }; + const runId = record.runId; + return db.transaction(async transaction => { + const tx = transaction as unknown as Db; + const [lease] = await tx.select().from(environmentLeases).where(and(eq(environmentLeases.id, record.recordId), + eq(environmentLeases.heartbeatRunId, runId), eq(environmentLeases.status, "released"), + eq(environmentLeases.cleanupStatus, "success"))).for("update"); + if (!lease || !hasRemoteTerminationReceipt(lease) || lease.leasePolicy !== "ephemeral" || !lease.metadata?.[ORPHANED_SANDBOX_RECOVERY_KEY] + || !lease.provider || !lease.providerLeaseId) { + return { settled: false, reason: "no_longer_a_retained_orphan" }; + } + const held = await heldBy(tx, lease, runId, context.now); + if (held) return { settled: false, reason: held }; + const now = context.now; + // The destroy is queued through the recognized stopped-cleanup contract + // (`workspaceRecoveryCleanup` with the verified stop receipt), so admission + // and Retry keep treating the sandbox as stopped while it waits. + const released = await writeEnvironmentCleanup(tx, write => write.update(environmentLeases).set({ + status: "pending_cleanup", cleanupStatus: "failed", + failureReason: RETAINED_ORPHAN_RELEASED_REASON, updatedAt: now, + metadata: sql`(${environmentLeases.metadata} - 'remoteExecutionTermination') || ${JSON.stringify({ + workspaceRecoveryCleanup: { runId, stopReceipt: lease.metadata?.remoteExecutionTermination, + reason: RETAINED_ORPHAN_RELEASED_REASON, releasedAt: now.toISOString(), ownerProof: context.proof.reason }, + pendingCleanupInFlight: false, pendingCleanupRetryAfterMs: 0, pendingCleanupRetryAttempts: 0, + pendingCleanupLeaseExpiresAtMs: 0 })}::jsonb`, + }).where(and(eq(environmentLeases.id, lease.id), eq(environmentLeases.heartbeatRunId, runId), + eq(environmentLeases.status, "released"), eq(environmentLeases.cleanupStatus, "success"), + sql`${environmentLeases.metadata} = ${JSON.stringify(lease.metadata)}::jsonb`)) + .returning({ id: environmentLeases.id }), { companyId: lease.companyId, runId, leaseId: lease.id }); + return released.length ? { settled: true, reason: context.proof.reason } : { settled: false, reason: "lease_changed_since_proof" }; + }); +} + +async function heldBy(tx: Db, lease: LeaseRow, runId: string, now: Date): Promise { + const metadata = lease.metadata ?? {}; + const [run] = await tx.select({ status: heartbeatRuns.status, issueId: heartbeatRuns.issueId, + nativeIssueId: heartbeatRuns.nativeIssueId }).from(heartbeatRuns).where(eq(heartbeatRuns.id, runId)); + if (run && !TERMINAL_RUN_STATUSES.includes(run.status)) return "run_active"; + + // Always held: an operation on this sandbox is still in progress. + if (metadata.nativeWorkspaceExportResume !== undefined) return "native_export_resume"; + if (metadata.sandboxStopAndRetain !== undefined) return "stop_and_retain_intent"; + if (metadata.pendingCleanupInFlight === true) return "cleanup_claim_in_flight"; + if (metadata.workspaceRestoreRecovery !== undefined) return "workspace_restore_recovery"; + if (metadata.workspaceRecoveryCleanup !== undefined) return "workspace_recovery_cleanup"; + if (metadata.runnerTaskSourceConsumer !== undefined) return "source_recovery_consumer"; + const shutdown = metadata.legacyShutdownWorkspaceSettlement as Record | undefined; + if (shutdown?.state === "pending") return "legacy_shutdown_fence"; + const [other] = await tx.select({ id: environmentLeases.id }).from(environmentLeases).where(and( + ne(environmentLeases.id, lease.id), eq(environmentLeases.provider, lease.provider!), + eq(environmentLeases.providerLeaseId, lease.providerLeaseId!), + inArray(environmentLeases.status, ["active", "retained", "pending_cleanup"]))).limit(1); + if (other) return "provider_resource_shared"; + + // A pending native export is an active preservation operation; the cleanup + // sweep would defer the destroy anyway, so it always holds the sandbox. + if (await nativeWorkspaceExportPendingForLease(tx, lease)) return "workspace_preservation_pending"; + // Held only while the stopped sandbox is still a usable workspace source. + if (lease.expiresAt && lease.expiresAt <= now) return null; + const issueId = run?.nativeIssueId ?? run?.issueId ?? lease.issueId; + if (!issueId) return null; + const [action] = await tx.select({ id: issueRecoveryActions.id }).from(issueRecoveryActions).where(and( + eq(issueRecoveryActions.companyId, lease.companyId), eq(issueRecoveryActions.sourceIssueId, issueId), + notInArray(issueRecoveryActions.status, ["resolved", "cancelled"]))).limit(1); + if (action) return "recovery_action_open"; + const [next] = await tx.select({ id: heartbeatRuns.id }).from(heartbeatRuns).where(and( + eq(heartbeatRuns.companyId, lease.companyId), ne(heartbeatRuns.id, runId), + inArray(heartbeatRuns.status, OPEN_RUN_STATUSES), + sql`(${heartbeatRuns.issueId} = ${issueId} or ${heartbeatRuns.nativeIssueId} = ${issueId})`)).limit(1); + if (next) return "issue_run_pending"; + return null; +} diff --git a/server/src/services/environment-run-lease-settlement.ts b/server/src/services/environment-run-lease-settlement.ts index fb1c79a471e..f6020a1ed6f 100644 --- a/server/src/services/environment-run-lease-settlement.ts +++ b/server/src/services/environment-run-lease-settlement.ts @@ -2,6 +2,7 @@ import { and, eq, inArray, ne, notInArray, or, sql } from "drizzle-orm"; import { environmentLeases, environments, heartbeatRuns, nativeRunFinalizations, nativeRunResults, type Db } from "@paperclipai/db"; import { currentControllerBootId } from "./controller-process-identity.js"; import { writeEnvironmentCleanup } from "./environment-cleanup-notifications.js"; +import { pendingCleanupMetadataObjectSql } from "./environment-cleanup-state.js"; import { readEnvironmentLeaseController } from "./environment-lease-controller.js"; import { allowLegacyShutdownWorkspaceCleanup } from "./legacy-shutdown-workspace-settlement.js"; import { nativeWorkspaceExportPendingForLease } from "./native-runtime/native-workspace-finalization-state.js"; @@ -77,7 +78,8 @@ export async function settleEnvironmentRunLease(db: Db, record: InProgressRecord const flipped = await writeEnvironmentCleanup(tx, write => write.update(environmentLeases) .set({ status: "pending_cleanup", failureReason: ORPHANED_RUN_LEASE_FAILURE_REASON, // When the first cleanup attempt became due (the row keeps updatedAt). - metadata: sql`coalesce(${environmentLeases.metadata}, '{}'::jsonb) || ${JSON.stringify({ + // A scalar or array root reads as an empty object, so the key is top level. + metadata: sql`${pendingCleanupMetadataObjectSql()} || ${JSON.stringify({ pendingCleanupRecoveredAtMs: context.now.getTime() })}::jsonb` }) .where(and(eq(environmentLeases.id, lease.id), eq(environmentLeases.status, "active"), eq(environmentLeases.heartbeatRunId, runId), diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index ad1a131686d..785f697d1fe 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -320,6 +320,7 @@ import { import { buildHeartbeatRunStatusLiveEventPayload } from "./heartbeat-run-status-payload.js"; export { buildHeartbeatRunStatusLiveEventPayload } from "./heartbeat-run-status-payload.js"; import { StaleExecutionContinuationError } from "./execution-continuation.js"; +import { deferUndeliveredCommentsOfStaleRun } from "./heartbeat/stale-comment-continuation.js"; import { renderPaperclipWakePrompt } from "@paperclipai/adapter-utils/server-utils"; import { @@ -3796,6 +3797,12 @@ export function heartbeatService( // The queued continuation became obsolete before adapter dispatch. // Use cancellation settlement so wakeup, issue ownership, agent state, // and notifications agree; do not retry work for the previous owner. + // A user's message it carried that no run read is saved first, so the + // release reopens the finished task for it (work-intent outbox). If + // this fast path fails or the closing run has not settled yet, the + // ownership reconciler saves it once that run settles. + await deferUndeliveredCommentsOfStaleRun(db, run).catch(err => + logger.warn({ err, runId: run.id }, "could not save the messages of a stale continuation")); await cancelRunInternal(run.id, outerErr.code, { errorCode: outerErr.code, eventMessage: "stale execution continuation cancelled before dispatch", diff --git a/server/src/services/heartbeat/queue.ts b/server/src/services/heartbeat/queue.ts index 73dec05433f..0f0ebdf8f7d 100644 --- a/server/src/services/heartbeat/queue.ts +++ b/server/src/services/heartbeat/queue.ts @@ -1779,8 +1779,10 @@ export function createHeartbeatQueue(db: Db, dependencies: HeartbeatQueueDepende // which resumes through its own live authorization), and only pauses // this query can see lifted. A refusal by any other gate rewrites the // receipt's reason, so it never retries in a loop here. - sql`${agentWakeupRequests.payload}->'executionWait'->>'trigger' = 'pause_lifted'`, - sql`${agentWakeupRequests.payload}->'executionWait'->>'reason' in ('company.inactive', 'agent.not_invokable')`, + // A message the reconciler saved after its wake was lost waits on no + // hold, so the same explicit wake sends it. + sql`${agentWakeupRequests.payload}->'executionWait'->>'trigger' in ('pause_lifted', 'reconciler_backstop')`, + sql`${agentWakeupRequests.payload}->'executionWait'->>'reason' in ('company.inactive', 'agent.not_invokable', 'wake_lost')`, eq(agentWakeupRequests.requestedByActorType, "user"), sql`coalesce(${agentWakeupRequests.idempotencyKey}, '') not like 'chat-inbound:%'`, sql`exists (select 1 from agents pause_agent where pause_agent.id = ${agentWakeupRequests.agentId} diff --git a/server/src/services/heartbeat/stale-comment-continuation.ts b/server/src/services/heartbeat/stale-comment-continuation.ts new file mode 100644 index 00000000000..bedd51564e8 --- /dev/null +++ b/server/src/services/heartbeat/stale-comment-continuation.ts @@ -0,0 +1,46 @@ +import { agentWakeupRequests, heartbeatRuns, issueComments, type Db } from "@paperclipai/db"; +import { and, eq, inArray, sql } from "drizzle-orm"; +import { saveUndeliveredUserComments, taskClosedByAgentSql } from "../work-intent-outbox.js"; + +const UUID = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +/** + * Work-intent outbox (invariant 4). A user's message can wake its own queued + * run while an earlier run of the task is still finishing. When that earlier + * run completes the task first, the queued run reaches a done task and is + * cancelled as a stale continuation before dispatch, but the earlier run never + * read the message. Cancelling it alone would drop the message (chaos + * s9401/t72). + * + * Before that cancellation, the message's undelivered comments are saved as a + * deferred receipt of the same user. The run's cancellation releases the task, + * and the release drain reopens the done task for the user's message and + * delivers it once. Only a task that an agent run completed without reading + * the message qualifies: that run started before the comment and succeeded + * after it. A task a person marked done ends the message's intent. + * Returns the saved receipt id, or null when nothing is owed. + */ +export async function deferUndeliveredCommentsOfStaleRun(db: Db, run: typeof heartbeatRuns.$inferSelect): Promise { + const context = run.contextSnapshot && typeof run.contextSnapshot === "object" ? run.contextSnapshot as Record : {}; + const issueId = typeof context.issueId === "string" ? context.issueId : null; + const commentIds = Array.isArray(context.wakeCommentIds) + ? context.wakeCommentIds.filter((id): id is string => typeof id === "string" && UUID.test(id)) : []; + if (!issueId || !UUID.test(issueId) || commentIds.length === 0) return null; + const unread = await db.select({ id: issueComments.id }).from(issueComments).where(and( + eq(issueComments.companyId, run.companyId), eq(issueComments.issueId, issueId), inArray(issueComments.id, commentIds), + sql`exists (select 1 from ${heartbeatRuns} completing where completing.company_id = ${run.companyId} + and completing.agent_id = ${run.agentId} and completing.id <> ${run.id} and completing.status = 'succeeded' + and (completing.context_snapshot->>'issueId' = ${issueId} or completing.native_issue_id = ${issueId}) + and completing.started_at < ${issueComments.createdAt} and completing.finished_at >= ${issueComments.createdAt})`)); + const owed = commentIds.filter(id => unread.some(row => row.id === id)); + if (owed.length === 0) return null; + // An overlapping run is not proof the agent closed the task: a person can + // mark it done while that run finishes. Only an agent-made close reopens. + const [closed] = await db.execute<{ byAgent: boolean }>(sql`select ${taskClosedByAgentSql(run.companyId, issueId)} as "byAgent"`); + if (closed?.byAgent !== true) return null; + const [wake] = run.wakeupRequestId ? await db.select().from(agentWakeupRequests) + .where(and(eq(agentWakeupRequests.companyId, run.companyId), eq(agentWakeupRequests.id, run.wakeupRequestId))) : []; + return saveUndeliveredUserComments(db, { companyId: run.companyId, issueId, agentId: run.agentId, commentIds: owed, + allowedStatuses: ["done"], requestedByActorId: wake?.requestedByActorType === "user" ? wake.requestedByActorId : null, + source: wake?.source, triggerDetail: wake?.triggerDetail }); +} diff --git a/server/src/services/native-local-process-stop.test.ts b/server/src/services/native-local-process-stop.test.ts new file mode 100644 index 00000000000..1b649ba7c10 --- /dev/null +++ b/server/src/services/native-local-process-stop.test.ts @@ -0,0 +1,71 @@ +import { randomUUID } from "node:crypto"; +import { hostname } from "node:os"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { agents, companies, createDb, heartbeatRuns } from "@paperclipai/db"; +import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "../__tests__/helpers/embedded-postgres.js"; +import { currentProcessSpaceHash, currentProcessStartedAtMs } from "./controller-process-identity.js"; +import { appendHeartbeatRunEvent } from "./heartbeat-run-events.js"; +import { PROCESS_IDENTITY_RECORDED, recordedLocalExecutionStopped } from "./native-local-process-stop.js"; + +const support = await getEmbeddedPostgresTestSupport(); + +// Phase 2: a bound local execution is judged by the shared prover +// (proveOwnerGone), in the PID space the row recorded. +(support.supported ? describe : describe.skip)("recordedLocalExecutionStopped", () => { + let database: Awaited>; + let db: ReturnType; + let companyId: string; + let agentId: string; + beforeAll(async () => { + database = await startEmbeddedPostgresTestDatabase("native-local-stop-"); + db = createDb(database.connectionString, { maxConnections: 2 }); + companyId = randomUUID(); agentId = randomUUID(); + await db.insert(companies).values({ id: companyId, name: "Local stop", issuePrefix: `L${companyId.slice(0, 7)}` }); + await db.insert(agents).values({ id: agentId, companyId, name: "Agent", role: "general", adapterType: "codex_local", + status: "idle" }); + }, 60_000); + afterAll(async () => { await db?.$client.end({ timeout: 1 }); await database?.cleanup(); }); + + /** A failed run with its process identity bound by the launch event. */ + async function execution(input: { pid: number; startedAt: Date; processHostHash: string | null }) { + const [run] = await db.insert(heartbeatRuns).values({ companyId, agentId, status: "failed", runtimeMode: "native", + processPid: input.pid, processGroupId: input.pid, processStartedAt: input.startedAt, + processHostHash: input.processHostHash }).returning(); + await appendHeartbeatRunEvent(db, { companyId, runId: run!.id, agentId, eventType: PROCESS_IDENTITY_RECORDED, + payload: { targetKind: "local", hostname: hostname(), processPid: input.pid, processGroupId: input.pid, + processStartedAt: input.startedAt.toISOString() } }); + return run!; + } + const deadPid = 2_000_000_001; + const minuteAgo = () => new Date(Date.now() - 60_000); + const ownStart = async () => new Date((await currentProcessStartedAtMs())!); + + it("is stopped when its dead PID was recorded in this PID space", async () => { + expect(await recordedLocalExecutionStopped(db, await execution({ pid: deadPid, startedAt: minuteAgo(), + processHostHash: currentProcessSpaceHash() }))).toBe(true); + }); + + it("is never stopped by a dead PID recorded in another PID space", async () => { + expect(await recordedLocalExecutionStopped(db, await execution({ pid: deadPid, startedAt: minuteAgo(), + processHostHash: "another-pid-space" }))).toBe(false); + }); + + it("keeps the host-bound proof for a row saved before its PID space", async () => { + expect(await recordedLocalExecutionStopped(db, await execution({ pid: deadPid, startedAt: minuteAgo(), + processHostHash: null }))).toBe(true); + }); + + it("is not stopped while the same process is alive", async () => { + expect(await recordedLocalExecutionStopped(db, await execution({ pid: process.pid, startedAt: await ownStart(), + processHostHash: currentProcessSpaceHash() }))).toBe(false); + }); + + it("is stopped when a later process reused its PID and leads the group", async () => { + // This test process started definitely after the recorded execution. + const recorded = new Date((await ownStart()).getTime() - 5_000); + // A group with this id is either empty or led by this later process, so + // it cannot hold the recorded execution's children. + expect(await recordedLocalExecutionStopped(db, await execution({ pid: process.pid, startedAt: recorded, + processHostHash: currentProcessSpaceHash() }))).toBe(true); + }); +}); diff --git a/server/src/services/native-local-process-stop.ts b/server/src/services/native-local-process-stop.ts index 5999ea523b6..47f4055bf11 100644 --- a/server/src/services/native-local-process-stop.ts +++ b/server/src/services/native-local-process-stop.ts @@ -7,6 +7,8 @@ import { DELIVERY_QUEUES, notifyDeliveryWork } from "./delivery-work-notificatio import { nativeSessionProviderEvidence } from "./native-runtime/native-session-resume.js"; import { hasRemoteTerminationReceipt } from "./remote-execution-termination.js"; import os from "node:os"; +import { currentProcessSpaceHash } from "./controller-process-identity.js"; +import { proveOwnerGone } from "./owner-proof.js"; export const PROCESS_START_REQUESTED = "native.process_start_requested"; export const PROCESS_IDENTITY_RECORDED = "native.process_identity_recorded"; @@ -26,7 +28,7 @@ function absent(pid: number): boolean { /** A fresh workspace is not process containment. Bind an OS observation to * the original host, process birth, and launch event before accepting it. */ export async function recordedLocalExecutionStopped(db: Db, run: Pick) { + "id" | "companyId" | "processPid" | "processGroupId" | "processStartedAt"> & { processHostHash?: string | null }) { const [identity] = await db.select({ eventType: heartbeatRunEvents.eventType, payload: heartbeatRunEvents.payload }).from(heartbeatRunEvents).where(and( eq(heartbeatRunEvents.companyId, run.companyId), eq(heartbeatRunEvents.runId, run.id), inArray(heartbeatRunEvents.eventType, [PROCESS_START_REQUESTED, PROCESS_IDENTITY_RECORDED]), @@ -36,8 +38,15 @@ export async function recordedLocalExecutionStopped(db: Db, run: Pick { } }); + // Phase 2: run processes and controllers are judged by the shared prover. + it("never releases on a dead PID recorded in another PID space", async () => { + const { runId } = await nativeRun({ status: "failed", errorCode: "adapter_failed", + coordinator: { leaseOwner: `default:resume:${randomUUID()}`, leaseExpiresAt: new Date(0) } }); + await db.update(heartbeatRuns).set({ processPid: 2_000_000_001, processGroupId: null, + processStartedAt: new Date(Date.now() - 60_000), processHostHash: "another-pid-space" }) + .where(eq(heartbeatRuns.id, runId)); + await expect(releaseAbandonedNativeOwners(db)).resolves.toBe(0); + expect(await coordinator(runId)).toMatchObject({ phase: "observed" }); + await db.update(heartbeatRuns).set({ processHostHash: currentProcessSpaceHash() }).where(eq(heartbeatRuns.id, runId)); + await expect(releaseAbandonedNativeOwners(db)).resolves.toBe(1); + expect(await db.select().from(activityLog).where(and(eq(activityLog.runId, runId), + eq(activityLog.action, "legacy_host_unknown_dead_pid_proof")))).toEqual([]); + }); + + it("keeps the one-release fallback for a run process saved before its PID space, and records it", async () => { + const { runId } = await nativeRun({ status: "failed", errorCode: "adapter_failed", + coordinator: { leaseOwner: `default:resume:${randomUUID()}`, leaseExpiresAt: new Date(0) } }); + await db.update(heartbeatRuns).set({ processPid: 2_000_000_001, processGroupId: null, + processStartedAt: new Date(Date.now() - 60_000), processHostHash: null }).where(eq(heartbeatRuns.id, runId)); + await expect(releaseAbandonedNativeOwners(db)).resolves.toBe(1); + const proofs = await db.select().from(activityLog).where(and(eq(activityLog.runId, runId), + eq(activityLog.action, "legacy_host_unknown_dead_pid_proof"))); + expect(proofs.map(proof => proof.details)).toEqual([{ reason: "pid_dead", source: "native_failed_run_process" }]); + }); + // `ps -o lstart` on macOS reports whole seconds, while a recorded start can // carry milliseconds. A sub-second gap must not be read as a recycled PID. it.each(["controller", "run process"])("keeps the owner when the live %s start time differs by under a second", async (holder) => { diff --git a/server/src/services/native-runtime/native-failed-run-owner-release.ts b/server/src/services/native-runtime/native-failed-run-owner-release.ts index 3ea34cda26b..ef7ddff6456 100644 --- a/server/src/services/native-runtime/native-failed-run-owner-release.ts +++ b/server/src/services/native-runtime/native-failed-run-owner-release.ts @@ -2,15 +2,15 @@ import { and, asc, eq, gt, inArray, isNotNull, isNull, lte, or, sql } from "driz import { environmentLeases, heartbeatRuns, issues, nativeRunFinalizations, nativeRunResults, type Db } from "@paperclipai/db"; import { logger } from "../../middleware/logger.js"; import { adapterExecutionControls } from "../adapter-execution-control.js"; -import { processExists, processStartsDefinitelyDiffer } from "../controller-process-identity.js"; +import { proveOwnerGone, type LocalOwnerStamp } from "../owner-proof.js"; import { logActivity, publishActivity, type ActivityPublication } from "../activity-log.js"; import { DELIVERY_QUEUES, notifyDeliveryWork } from "../delivery-work-notifications.js"; import { notifyExecutionRunWork } from "../execution-control-notifications.js"; -import { readProcessStartedAt } from "../hot-restart.js"; import { notifyNativeFinalizationWork } from "../native-finalization-notifications.js"; import { LEGACY_WORKSPACE_RECOVERY_KEY } from "../workspace-restore-recovery-state.js"; import { activeNativeExecutionRunIds } from "./native-active-runs.js"; -import { isCurrentNativeControllerProcess } from "./native-restart-recovery.js"; +import { isCurrentNativeControllerProcess, recordLegacyHostFallbackProof } from "./native-restart-recovery.js"; +import { nativeControllerOwnerStamp, runProcessOwnerStamp } from "./native-owner-stamps.js"; /** * A native run can fail outside the native session executor: environment lease @@ -85,51 +85,48 @@ export async function nextAbandonedNativeOwnerReleaseAt(db: Db): Promise { - if (!pid || processAbsent(pid)) return false; - if (!startedAt) return true; - const observed = await readProcessStartedAt(pid).catch(() => null); - return !processStartsDefinitelyDiffer(startedAt, observed); +/** True unless the shared prover proves the stamped owner gone. Only a `gone` + * verdict releases: a live, unprovable or foreign-space owner may be live. */ +async function ownerMayBeLive(stamp: LocalOwnerStamp, source: string, fallbacks: FallbackUse[]): Promise { + const proof = await proveOwnerGone(stamp); + if (proof.verdict !== "gone") return true; + if (proof.legacyHostFallback) fallbacks.push({ reason: proof.reason, source }); + return false; } /** * The run-level process evidence `agentExecutionsHaveStopped` requires. This - * process answers for itself from its in-memory execution registries. + * process answers for itself from its in-memory execution registries; a + * recorded adapter process (PID and group) is judged by the shared prover. */ -async function runProcessMayBeLive(run: Run, now: Date): Promise { +async function runProcessMayBeLive(run: Run, now: Date, fallbacks: FallbackUse[]): Promise { if (adapterExecutionControls.has(run.id) || activeNativeExecutionRunIds.has(run.id)) return true; if (run.controllerLeaseExpiresAt && run.controllerLeaseExpiresAt > now) return true; - // Same process identity rule as `agentExecutionsHaveStopped`: a recorded PID - // that now has another start time was recycled, and so was its own group. - let processAlive = Boolean(run.processPid && !processAbsent(run.processPid)); - let groupAlive = Boolean(run.processGroupId && !processAbsent(-run.processGroupId)); - if (processAlive && run.processStartedAt) { - const observed = await readProcessStartedAt(run.processPid!).catch(() => null); - if (processStartsDefinitelyDiffer(run.processStartedAt, observed)) { - processAlive = false; - if (run.processGroupId === run.processPid) groupAlive = false; - } - } - return processAlive || groupAlive; + if (!run.processPid && !run.processGroupId) return false; + return ownerMayBeLive(runProcessOwnerStamp(run, "native_failed_run_process", { legacyFallback: true }), + "native_failed_run_process", fallbacks); } /** An expired owner's recorded controller, unless this process is that controller. */ -async function controllerMayBeLive(coordinator: Coordinator): Promise { +async function controllerMayBeLive(coordinator: Coordinator, fallbacks: FallbackUse[]): Promise { if (isCurrentNativeControllerProcess(coordinator)) return false; // A previous boot of this process can share its PID, but cannot be its controller. if (coordinator.controllerPid === process.pid) return false; - return recordedProcessMayBeLive(coordinator.controllerPid, coordinator.controllerProcessStartedAt); + if (!coordinator.controllerPid) return false; + // Not this boot (checked above), so the prover must not treat its boot id as live. + return ownerMayBeLive({ ...nativeControllerOwnerStamp(coordinator, "native_failed_run_controller"), bootId: null }, + "native_failed_run_controller", fallbacks); } async function settleFailedRunOwner(db: Db, release: Release, now: Date): Promise { const publications: ActivityPublication[] = []; const settled = await db.transaction(async transaction => { const tx = transaction as unknown as Db; + const fallbacks: FallbackUse[] = []; await tx.execute(sql`select set_config('statement_timeout', '15000', true), set_config('lock_timeout', '1000', true)`); const [binding] = await tx.select({ companyId: nativeRunFinalizations.companyId, issueId: nativeRunFinalizations.issueId }) .from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, release.runId)); @@ -151,8 +148,9 @@ async function settleFailedRunOwner(db: Db, release: Release, now: Date): Promis || run.resultJson?.[LEGACY_WORKSPACE_RECOVERY_KEY] != null) return false; const [heldElsewhere] = await tx.select({ id: heartbeatRuns.id }).from(heartbeatRuns) .where(and(eq(heartbeatRuns.id, run.id), or(unsettledRunEnvironmentLease, pendingNativeStop))).limit(1); - if (heldElsewhere || await runProcessMayBeLive(run, now) || await controllerMayBeLive(coordinator)) return false; - } else if (await runProcessMayBeLive(run, now)) { + if (heldElsewhere || await runProcessMayBeLive(run, now, fallbacks) + || await controllerMayBeLive(coordinator, fallbacks)) return false; + } else if (await runProcessMayBeLive(run, now, fallbacks)) { // The claim never reached the executor, but the run may still own a local process. return false; } @@ -194,6 +192,9 @@ async function settleFailedRunOwner(db: Db, release: Release, now: Date): Promis if (!updated) return false; await tx.update(heartbeatRuns).set({ nativePhase: "terminal_failure", nativePhaseUpdatedAt: now, updatedAt: now }) .where(eq(heartbeatRuns.id, run.id)); + for (const use of fallbacks) { + await recordLegacyHostFallbackProof(tx, run, { allowed: true, reason: use.reason, legacyHostFallback: true }, use.source); + } await logActivity(tx, { companyId: run.companyId, actorType: "system", actorId: ACTOR_ID, action: "native.failed_run_owner_released", entityType: "heartbeat_run", entityId: run.id, diff --git a/server/src/services/native-runtime/native-finalization-reconciler.ts b/server/src/services/native-runtime/native-finalization-reconciler.ts index 45e15f3feb7..7e22e0c20bf 100644 --- a/server/src/services/native-runtime/native-finalization-reconciler.ts +++ b/server/src/services/native-runtime/native-finalization-reconciler.ts @@ -18,12 +18,14 @@ import type { createDeliveryWorkCoordinator } from "../delivery-work-coordinator import { createOwnershipReportState, DEFERRED_INTENT_NOT_PROMOTED, + USER_COMMENT_DROPPED_BY_STALE_RUN, + USER_COMMENT_WITHOUT_WAKE, ownershipReportFingerprint, recheckSettlingKinds, runOwnershipReconciliationPass, type OwnershipReport, } from "../recovery/ownership-invariant-report.js"; -import { promoteDeferredWorkIntents } from "../work-intent-outbox.js"; +import { deferredWorkIntentWait, promoteDeferredWorkIntents, saveUndeliveredUserComments } from "../work-intent-outbox.js"; import { createHash, randomUUID } from "node:crypto"; import { and, asc, desc, eq, gt, inArray, isNotNull, isNull, lt, lte, not, notInArray, or, sql, type SQL } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; @@ -824,13 +826,37 @@ export const OWNERSHIP_REPORT_INTERVAL_MS = 30_000; /** Section E backstop: a deferred intent whose hold cleared without a * release event is promoted through the one promotion function. Promotion * re-runs normal admission, so a wrong report never bypasses a gate. */ -async function promoteMissedDeferredIntents(db: Db, report: OwnershipReport) { +export async function applyOwnershipReportBackstops(db: Db, report: OwnershipReport): Promise<{ retryNeeded: boolean }> { + let retryNeeded = false; + // Rules: an undelivered user comment newer than the task's last run gets a + // wake, and a comment a stale continuation dropped reopens its done task. + // Save the missing outbox entry, then promote it. + for (const violation of report.violations.filter(row => + row.reason === USER_COMMENT_WITHOUT_WAKE || row.reason === USER_COMMENT_DROPPED_BY_STALE_RUN)) { + if (!violation.companyId || !violation.issueId) continue; + const lost = violation.reason === USER_COMMENT_WITHOUT_WAKE; + try { + const [issue] = await db.select({ assigneeAgentId: issues.assigneeAgentId }).from(issues) + .where(and(eq(issues.companyId, violation.companyId), eq(issues.id, violation.issueId))); + if (!issue?.assigneeAgentId) continue; + const saved = await saveUndeliveredUserComments(db, { companyId: violation.companyId, issueId: violation.issueId, + agentId: issue.assigneeAgentId, commentIds: [violation.recordId], allowedStatuses: lost ? ["todo", "in_progress"] : ["done"], + ...(lost ? { executionWait: deferredWorkIntentWait("wake_lost", "This message's wake was lost; it is sent now.") } : {}) }); + if (saved) await db.transaction(tx => promoteDeferredWorkIntents(tx, { companyId: violation.companyId!, + trigger: "reconciler_backstop", issueId: violation.issueId })); + } catch (error) { + // The message still has no saved receipt: keep a pass due. + retryNeeded = true; + logger.warn({ err: error, commentId: violation.recordId }, "could not save a user comment without a wake"); + } + } const companies = new Set(report.violations.filter(violation => violation.reason === DEFERRED_INTENT_NOT_PROMOTED) .flatMap(violation => violation.companyId ? [violation.companyId] : [])); for (const companyId of companies) { await db.transaction(tx => promoteDeferredWorkIntents(tx, { companyId, trigger: "reconciler_backstop" })) - .catch(error => logger.warn({ err: error, companyId }, "deferred intent backstop promotion failed")); + .catch(error => { retryNeeded = true; logger.warn({ err: error, companyId }, "deferred intent backstop promotion failed"); }); } + return { retryNeeded }; } export function registerOwnershipReconciliationWork( @@ -895,12 +921,13 @@ export function registerOwnershipReconciliationWork( lastPassAt = Date.now(); pendingWake = false; const report = await runOwnershipReconciliationPass(db, { state }); - await promoteMissedDeferredIntents(db, report); + const backstop = await applyOwnershipReportBackstops(db, report); const fingerprint = ownershipReportFingerprint(report); const changing = report.inProgress > 0 && fingerprint !== lastFingerprint; // A settling kind's live owner can die without writing a signal, so - // keep one bounded re-check due while such records exist. - const fullPassDue = changing || report.awaitingSecondProof > 0 || report.retryNeeded; + // keep one bounded re-check due while such records exist. A backstop + // that could not save or promote keeps one due as well. + const fullPassDue = changing || report.awaitingSecondProof > 0 || report.retryNeeded || backstop.retryNeeded; watchOnly = !fullPassDue && report.settleWatch > 0; const followUp = fullPassDue || report.settleWatch > 0 ? Date.now() + OWNERSHIP_REPORT_INTERVAL_MS : null; diff --git a/server/src/services/native-runtime/native-owner-stamps.ts b/server/src/services/native-runtime/native-owner-stamps.ts new file mode 100644 index 00000000000..8bd2bd9dfdc --- /dev/null +++ b/server/src/services/native-runtime/native-owner-stamps.ts @@ -0,0 +1,47 @@ +import type { heartbeatRuns, nativeRunFinalizations } from "@paperclipai/db"; +import type { LocalOwnerStamp } from "../owner-proof.js"; + +/** + * Owner stamps for the native records that `proveOwnerGone` judges (plan + * Phase 2). The ownership registry and the native recovery paths build their + * stamps here, so one record has one proof. + */ + +type ControllerRow = Pick & { + controllerHostHash?: string | null; + controllerGeneration?: number | null; +}; + +/** NRF records the controller's boot, PID, start time and, since #19868, its + * PID space (`controller_host_hash`). Rows saved before that keep the + * one-release local proof on single-host servers (`processSpaceUnrecorded`, + * recorded as `legacy_host_unknown_dead_pid_proof`, #19883). */ +export function nativeControllerOwnerStamp(row: ControllerRow, scope: string): LocalOwnerStamp { + return { + kind: "local", scope, generation: row.controllerGeneration ?? null, host: null, + processSpace: row.controllerHostHash ?? null, + processSpaceUnrecorded: row.controllerHostHash == null, + bootId: row.controllerBootId, pid: row.controllerPid, + processStartedAt: row.controllerProcessStartedAt?.toISOString() ?? null, processGroupId: null, + }; +} + +type RunProcessRow = Pick & { + processHostHash?: string | null; +}; + +/** heartbeat_runs records the adapter child's PID, group, start time and, + * since #19871, its PID space (`process_host_hash`, or "remote" for a sandbox + * PID, which a local probe never judges). With `legacyFallback`, a row saved + * before that column keeps the one-release local proof; without it, such a row + * is unprovable. */ +export function runProcessOwnerStamp(row: RunProcessRow, scope: string, + options: { legacyFallback?: boolean } = {}): LocalOwnerStamp { + return { + kind: "local", scope, generation: null, host: null, processSpace: row.processHostHash ?? null, + ...(options.legacyFallback ? { processSpaceUnrecorded: row.processHostHash == null } : {}), + bootId: null, pid: row.processPid, processStartedAt: row.processStartedAt?.toISOString() ?? null, + processGroupId: row.processGroupId, + }; +} diff --git a/server/src/services/native-runtime/native-restart-recovery.test.ts b/server/src/services/native-runtime/native-restart-recovery.test.ts index 2e3d55ea235..3f88219608a 100644 --- a/server/src/services/native-runtime/native-restart-recovery.test.ts +++ b/server/src/services/native-runtime/native-restart-recovery.test.ts @@ -270,6 +270,33 @@ describe("native controller takeover fencing", () => { }); }); + // Phase 2: the local branch is the shared prover (proveOwnerGone). + it("accepts a reused PID only when the live process started definitely later", async () => { + const live = { isProcessAlive: () => true }; + await expect(evaluateNativeControllerTakeover({ owner: owner(), now, ...live, + readProcessStartedAt: async () => new Date(recordedStart.getTime() + 5_000) })) + .resolves.toEqual({ allowed: true, reason: "controller_pid_recycled" }); + // An observed start before the recorded one is an inconsistent stamp, + // which the previous private proof accepted as recycled. + await expect(evaluateNativeControllerTakeover({ owner: owner(), now, ...live, + readProcessStartedAt: async () => new Date(recordedStart.getTime() - 5_000) })) + .resolves.toEqual({ allowed: false, reason: "controller_pid_start_inconsistent" }); + // Within the 1 s precision window it is the same live controller. + await expect(evaluateNativeControllerTakeover({ owner: owner(), now, ...live, + readProcessStartedAt: async () => new Date(recordedStart.getTime() + 500) })) + .resolves.toEqual({ allowed: false, reason: "controller_still_alive" }); + await expect(evaluateNativeControllerTakeover({ owner: owner(), now, ...live, + readProcessStartedAt: async () => null })) + .resolves.toEqual({ allowed: false, reason: "controller_still_alive" }); + }); + + it("judges a controller recorded in this PID space, and keeps the operator attestation", async () => { + await expect(evaluateNativeControllerTakeover({ owner: { ...owner(), controllerHostHash: currentProcessSpaceHash() }, + now, isProcessAlive: () => false })).resolves.toEqual({ allowed: true, reason: "controller_process_dead" }); + await expect(evaluateNativeControllerTakeover({ owner: { ...owner(), controllerHostHash: "operator_attested" }, + now, isProcessAlive: () => true })).resolves.toEqual({ allowed: true, reason: "operator_attested_controller_stopped" }); + }); + it.each([ ["another host", "another-host-pid-space"], ["an unreadable PID space", "unknown"], diff --git a/server/src/services/native-runtime/native-restart-recovery.ts b/server/src/services/native-runtime/native-restart-recovery.ts index b780294b4e2..a5c799ff64a 100644 --- a/server/src/services/native-runtime/native-restart-recovery.ts +++ b/server/src/services/native-runtime/native-restart-recovery.ts @@ -36,10 +36,10 @@ import { import { REMOTE_CONTROLLER_LEASE_MS } from "./native-runner-recovery-contract.js"; import { PROVIDER_ATTEMPT_CREDIT_GENERATION_LIMIT, providerAttemptChargesBudget, providerAttemptWindowStart, readProviderAttemptOutcome } from "./provider-attempt-outcome.js"; import { readProcessStartedAt } from "../hot-restart.js"; -import { ephemeralServerEnabled } from "../ephemeral-server-mode.js"; +import { currentProcessProbe, proveOwnerGone, type ProcessProbe } from "../owner-proof.js"; import { persistActivity } from "../activity-log.js"; import { OPERATOR_ATTESTED_CONTROLLER_HOST, requestNativeControllerStopConfirmation } from "./native-controller-host-recovery.js"; -import { currentControllerBootId, currentProcessSpaceHash, currentProcessStartedAtMs, processStartsDefinitelyDiffer, sameProcessSpace } from "../controller-process-identity.js"; +import { currentControllerBootId, currentProcessSpaceHash, currentProcessStartedAtMs, processStartsDefinitelyDiffer } from "../controller-process-identity.js"; import { getServerProcessStartedAt } from "../../server-info.js"; import { redactSensitiveText } from "../../redaction.js"; import { isUniqueViolation } from "../../db-errors.js"; @@ -418,8 +418,6 @@ export async function evaluateNativeControllerTakeover(input: { ? { allowed: false, reason: "remote_controller_lease_live" } : { allowed: true, reason: "remote_controller_lease_released_or_expired" }; } - const isAlive = input.isProcessAlive ?? processIsAlive; - const readStartedAt = input.readProcessStartedAt ?? observedProcessStart; if ((!owner.leaseOwner || !owner.leaseExpiresAt) && !input.requireStoppedController) { return { allowed: true, reason: "unowned" }; } @@ -440,21 +438,32 @@ export async function evaluateNativeControllerTakeover(input: { if (recordsHost && owner.controllerHostHash === OPERATOR_ATTESTED_CONTROLLER_HOST) { return { allowed: true, reason: "operator_attested_controller_stopped" }; } - // A PID can only be judged in the PID space that recorded it. A controller - // on another host or namespace (or an unreadable local space) is never - // proven gone by a local probe; its lease or fence decides instead. - if (owner.controllerHostHash && !sameProcessSpace(owner.controllerHostHash)) { - return { allowed: false, reason: "controller_host_unprovable" }; - } - // A row saved before the host column existed. An ephemeral (multi-host) - // server can never assume it was written here; a board operator decides. - if (recordsHost && owner.controllerHostHash == null && ephemeralServerEnabled()) { - return { allowed: false, reason: "controller_host_unrecorded" }; - } - // Single-host servers keep the previous local proof for one release, and - // every use is recorded so the release that removes it can check for rows. - const fallback = recordsHost && owner.controllerHostHash == null ? { legacyHostFallback: true as const } : {}; - if (!isAlive(priorPid)) { + // The local branch is the shared prover (plan Phase 2): a PID is judged + // only in the PID space that recorded it, a reused PID must have started + // definitely later, and EPERM is alive. A coordinator row saved before the + // host column keeps the one-release local proof on single-host servers + // (`processSpaceUnrecorded`; each use is recorded by the caller as + // LEGACY_HOST_FALLBACK_EVENT), and an ephemeral server never judges it. + // Owners built from receipts that never stored a PID space are judged in + // this PID space: their callers checked the recorded host first. + const base = currentProcessProbe(); + const probe: ProcessProbe = { + ...base, + // No platform-deployment shortcut: this stamp records no worker. + deployment: null, + pidExists: input.isProcessAlive ?? base.pidExists, + readStartedAt: input.readProcessStartedAt ?? base.readStartedAt, + }; + const proof = await proveOwnerGone({ + kind: "local", scope: "native_controller", generation: null, host: null, + processSpace: recordsHost ? owner.controllerHostHash ?? null : probe.processSpace, + processSpaceUnrecorded: recordsHost && owner.controllerHostHash == null, + bootId: null, pid: priorPid, processStartedAt: owner.controllerProcessStartedAt?.toISOString() ?? null, + processGroupId: null, + }, probe); + const fallback = proof.legacyHostFallback ? { legacyHostFallback: true as const } : {}; + if (proof.verdict === "gone") { + if (proof.reason === "pid_reused") return { allowed: true, reason: "controller_pid_recycled", ...fallback }; return { allowed: true, reason: @@ -464,18 +473,14 @@ export async function evaluateNativeControllerTakeover(input: { ...fallback, }; } - - const observedStartedAt = await readStartedAt(priorPid); - if ( - owner.controllerProcessStartedAt && - observedStartedAt && - definitivelyDifferentProcessStart( - owner.controllerProcessStartedAt, - observedStartedAt, - ) - ) { - return { allowed: true, reason: "controller_pid_recycled", ...fallback }; + if (proof.reason === "process_space_unrecorded") return { allowed: false, reason: "controller_host_unrecorded" }; + if (proof.reason === "different_process_space" || proof.reason === "unknown_process_space" + || proof.reason === "different_host") { + return { allowed: false, reason: "controller_host_unprovable" }; } + // A live PID observed to start before the recorded controller is an + // inconsistent stamp, never proof of reuse. + if (proof.reason === "pid_start_inconsistent") return { allowed: false, reason: "controller_pid_start_inconsistent" }; const coordinated = input.coordinatedPreviousController; if ( diff --git a/server/src/services/native-runtime/native-session-executor.test.ts b/server/src/services/native-runtime/native-session-executor.test.ts index c2fcfea2347..58eaaa7c44a 100644 --- a/server/src/services/native-runtime/native-session-executor.test.ts +++ b/server/src/services/native-runtime/native-session-executor.test.ts @@ -12938,6 +12938,98 @@ describe("native warm session supervision", () => { }, ); + it.each([ + { name: "unchanged configuration", reuse: true }, + { name: "unchanged provider key", firstEnvironment: { OPENAI_API_KEY: "fixture-key" }, secondEnvironment: { OPENAI_API_KEY: "fixture-key" }, reuse: true }, + { name: "rotated provider key", firstEnvironment: { OPENAI_API_KEY: "fixture-key" }, secondEnvironment: { OPENAI_API_KEY: "fixture-rotated-key" }, reuse: false }, + { name: "rotated Claude token", firstEnvironment: { CLAUDE_CODE_OAUTH_TOKEN: "fixture-token" }, secondEnvironment: { CLAUDE_CODE_OAUTH_TOKEN: "fixture-rotated-token" }, reuse: false }, + { name: "tool refresh", refreshTools: true, reuse: false }, + { name: "network access change", firstEnvironment: { PAPERCLIP_RUNNER_NETWORK_ACCESS: "disabled" }, secondEnvironment: { PAPERCLIP_RUNNER_NETWORK_ACCESS: "enabled" }, reuse: false }, + { name: "run-scoped environment broker", firstEnvironment: { PAPERCLIP_GITHUB_BROKER_TOKEN: "first-run-capability" }, secondEnvironment: { PAPERCLIP_GITHUB_BROKER_TOKEN: "second-run-capability" }, reuse: false }, + { name: "Core-hosted GitHub access", managedGitHub: true, reuse: true, adoptionReuse: false }, + ] as Array<{ name: string; firstEnvironment?: Record; secondEnvironment?: Record; + refreshTools?: boolean; managedGitHub?: boolean; reuse: boolean; adoptionReuse?: boolean }>)( + "decides warm and adopted process reuse from one key: $name", + async ({ name, firstEnvironment = {}, secondEnvironment = {}, refreshTools, managedGitHub = false, reuse, adoptionReuse = reuse }) => { + const decisions: Record = {}; + for (const path of ["warm", "adoption"] as const) { + githubAccess.create.mockReset().mockImplementation(async () => ({ env: {}, ready: true, duplexRequested: false, + activate: githubAccess.activate.mockReset().mockImplementation(() => vi.fn()), stop: githubAccess.stop.mockReset().mockResolvedValue(undefined) })); + const stateBase = await mkdtemp(join(tmpdir(), "paperclip-runnerd-reuse-key-")); + const previousStateDirectory = process.env.PAPERCLIP_RUNNER_STATE_DIR; + const previousPaperclipHome = process.env.PAPERCLIP_HOME; + process.env.PAPERCLIP_RUNNER_STATE_DIR = stateBase; + process.env.PAPERCLIP_HOME = stateBase; + const scope = `${path}-${name.replaceAll(" ", "-")}`; + const first = { + ...execution, + binding: { ...execution.binding, runId: `run-reuse-key-first-${scope}`, executionWorkspaceId: "workspace-reuse-key" }, + workspace: { cwd: "/tmp/runnerd-reuse-key", repoUrl: null, repoRef: null, branchName: null }, + session: { normalizedSessionId: `session-reuse-key-${scope}`, driverKind: "codex_app_server" as const, + protocolVersion: 1 as const, lifecyclePolicy: { mode: "warm" as const, idleTimeoutMs: 60_000 } }, + } as NativeExecutionInputV1; + const second = { ...first, binding: { ...first.binding, runId: `run-reuse-key-second-${scope}` } } as NativeExecutionInputV1; + const target = { kind: "remote" as const, transport: "sandbox" as const, environmentId: "environment-reuse-key", + remoteCwd: "/home/daytona/paperclip-workspace", runner: { execute: vi.fn() } } as never; + const firstSession = { close: vi.fn(async () => undefined) }; + const result = { result: { summary: "completed" }, terminal: { runTerminalState: "succeeded" }, turnId: "turn", + normalizedSessionId: first.session.normalizedSessionId, providerSessionId: "provider-reuse-key", driverKind: "test", + driverVersion: "1", nativeEventCount: 1, highestContiguousSourceSeq: 1, usage: null }; + state.execute.mockReset() + .mockImplementationOnce(async (options) => { + await options.onCheckpoint?.({ + identity: { runId: first.binding.runId, sessionId: first.session.normalizedSessionId, + companyId: first.binding.companyId, issueId: first.binding.issueId, agentId: first.binding.agentId }, + providerSessionId: "provider-reuse-key", activeTurnId: "provider-turn-reuse-key", + semanticResult: { summary: "first" }, terminal: { runTerminalState: "succeeded" }, + }); + options.onSession?.(firstSession); + return result; + }) + .mockImplementationOnce(async (options) => { + decisions[path] = options.existingSession === firstSession; + return result; + }); + const inspected = vi.spyOn(remoteWarmRecoveryRuntime, "inspectRemoteWarmSessionRecovery").mockImplementation(async request => { + decisions[path] = request.canReuseProcess(first); + throw new Error("fixture adoption decision recorded"); + }); + try { + await executePaperclipNativeSession({ db: leaseDb(first), execution: first, runnerInstanceId: "runner-reuse-key", + useRunnerd: true, managedGitHub, runnerExecutionTarget: target, + runnerEnvironment: { PAPERCLIP_GITHUB_AUTH_MODE: "managed", ...firstEnvironment } }); + const durableRoot = (await readdir(stateBase, { withFileTypes: true })) + .filter(entry => entry.isDirectory() && /^[a-f0-9]{64}$/.test(entry.name)).map(entry => join(stateBase, entry.name))[0]!; + const durableIdentity = { runId: first.binding.runId, normalizedSessionId: first.session.normalizedSessionId, + runnerInstanceId: "runner-reuse-key", environmentLeaseId: first.binding.executionWorkspaceId }; + await mkdir(join(durableRoot, "control-plane"), { recursive: true }); + await writeFile(join(durableRoot, "control-plane", "control-plane-state.json"), JSON.stringify(durableControlPlaneState(durableIdentity))); + await mkdir(join(durableRoot, "runner"), { recursive: true }); + await writeFile(join(durableRoot, "runner", "runner-state.json"), JSON.stringify(durableRunnerState(durableIdentity, "ready"))); + if (path === "adoption") expect(await closeIdleWarmNativeSessionsForRestart()).toEqual({ closed: 1, busy: 0, failed: 0 }); + const continuation = executePaperclipNativeSession({ + db: { ...leaseDb(second), select: () => ({ from: () => ({ where: () => ({ limit: () => + Promise.resolve([{ status: "succeeded", runnerProfileJson: { nativeExecutionInput: first } }]) }) }) }) } as unknown as Db, + execution: second, runnerInstanceId: "runner-reuse-key", useRunnerd: true, managedGitHub, refreshTools, + runnerExecutionTarget: target, runnerEnvironment: { PAPERCLIP_GITHUB_AUTH_MODE: "managed", ...secondEnvironment } }); + if (path === "adoption") await expect(continuation).rejects.toThrow("fixture adoption decision recorded"); + else await continuation; + expect(decisions[path]).toBe(path === "adoption" ? adoptionReuse : reuse); + if (path === "warm") expect(firstSession.close).toHaveBeenCalledTimes(reuse ? 0 : 1); + } finally { + inspected.mockRestore(); + await closeIdleWarmNativeSessionsForRestart(); + if (previousStateDirectory === undefined) delete process.env.PAPERCLIP_RUNNER_STATE_DIR; + else process.env.PAPERCLIP_RUNNER_STATE_DIR = previousStateDirectory; + if (previousPaperclipHome === undefined) delete process.env.PAPERCLIP_HOME; + else process.env.PAPERCLIP_HOME = previousPaperclipHome; + await rm(stateBase, { recursive: true, force: true }); + } + } + expect(decisions).toEqual({ warm: reuse, adoption: adoptionReuse }); + }, + ); + it("does not replace a different company's warm session with the same normalized id", async () => { const firstClose = vi.fn(async () => undefined); const secondClose = vi.fn(async () => undefined); diff --git a/server/src/services/native-runtime/native-session-executor.ts b/server/src/services/native-runtime/native-session-executor.ts index d7e3759d66f..f4aacfdb298 100644 --- a/server/src/services/native-runtime/native-session-executor.ts +++ b/server/src/services/native-runtime/native-session-executor.ts @@ -68,7 +68,7 @@ import { isKnownNativeSessionRootEntry, } from "./native-session-root-entries.js"; import { createRunPreparationTiming } from "../execution-preparation-timing.js"; -import { configuredEnvironment } from "../../vendor/paperclip-runner/index.js"; +import { configuredEnvironment, PROVIDER_CREDENTIAL_ENVIRONMENT_KEYS } from "../../vendor/paperclip-runner/index.js"; import { parseProviderPerformanceSpans } from "../../vendor/paperclip-runner/index.js"; import { apiKeyBillingType } from "@paperclipai/adapter-utils/api-billing-type"; import { agents } from "@paperclipai/db"; @@ -122,7 +122,7 @@ import { boundedExecutionCleanup, EXECUTION_CONTROL_DEADLINE_MS, } from "../execution-control-deadline.js"; -import { createHash, randomUUID } from "node:crypto"; +import { createHash, createHmac, randomUUID } from "node:crypto"; import { execFile, execFileSync } from "node:child_process"; import { chmodSync, @@ -241,7 +241,7 @@ import { assertNativeRetainedSandboxBinding, deferNativeControllerRecovery, Nati import { redactRunnerApiPayloads } from "./runner-api-log-redaction.js"; import { appendHeartbeatRunEvent } from "../heartbeat-run-events.js"; import { appendNativeCredentialUpdate } from "./native-credential-updates.js"; -import { nativeSha256 } from "./canonical.js"; +import { canonicalNativeJson, nativeSha256 } from "./canonical.js"; import { lockSourceRecoveryCoordinators } from "./native-source-recovery-ownership.js"; import { PaperclipRunnerToolAuthority } from "./paperclip-runner-tool-authority.js"; import { createAssignedMcpTools, getAssignedMcpGateway } from "./assigned-mcp-tools.js"; @@ -302,8 +302,8 @@ import { noteNativeRunnerEvidence } from "./native-runner-evidence-signal.js"; import { removeNativeHarnessBackup } from "./native-harness-backup-cleanup.js"; import { registerLiveRunnerGoalController } from "../runner-goal-control-broker.js"; import { applyRunnerGoalPrpEvent } from "../runner-goals.js"; -import { readProcessStartedAt } from "../hot-restart.js"; -import { processExists, processStartsDefinitelyDiffer } from "../controller-process-identity.js"; +import { currentProcessSpaceHash } from "../controller-process-identity.js"; +import { proveOwnerGone } from "../owner-proof.js"; import { NativeRunnerOwnershipUnverifiedError, isNativeRunnerOwnershipHeld, @@ -591,32 +591,33 @@ const nativeRuntimeRequestResolutions = new Map< NativeRuntimeRequestResolution >(); +/** The shared prover's verdict on a recovered local runner, recorded on this + * host by an earlier boot (no boot id, so this process is never assumed). */ +function proveRecoveryProcess(input: { pid: number; startedAt: string }) { + return proveOwnerGone({ kind: "local", scope: "native_recovery_runner", generation: null, host: null, + processSpace: currentProcessSpaceHash(), bootId: null, pid: input.pid, processStartedAt: input.startedAt, + processGroupId: null }); +} + /** - * True when the recorded runner still owns its PID. A start time within the - * shared precision window is the same process: `ps -o lstart` on macOS has - * whole-second precision, so an exact match would call a live runner gone. + * False only when the shared prover proves the recorded runner gone (its PID + * is dead or reused by a later process). The runner treats false as exit + * evidence, so an unreadable or inconsistent start time stays alive. */ -export async function verifiedRecoveryProcessIsAlive(input: { +export async function recoveryProcessMayBeAlive(input: { pid: number; startedAt: string; }): Promise { - if (!processExists(input.pid)) return false; - try { - const observed = await readProcessStartedAt(input.pid); - return ( - observed !== null && - !processStartsDefinitelyDiffer(input.startedAt, observed) - ); - } catch { - return false; - } + return (await proveRecoveryProcess(input)).verdict !== "gone"; } async function signalVerifiedRecoveryProcess( input: { pid: number; startedAt: string }, signal: NodeJS.Signals, ): Promise { - if (!(await verifiedRecoveryProcessIsAlive(input))) return false; + // Signal only the proven same process (start time within the shared + // precision window), never an ambiguous or reused PID. + if ((await proveRecoveryProcess(input)).verdict !== "alive") return false; try { return process.kill(input.pid, signal); } catch { @@ -668,22 +669,15 @@ type WarmNativeSession = { runId: string; issueId: string; instructionWorkingCopy?: NativeInstructionWorkingCopy; - // Keep the admitted physical root independent of the per-run collection - // capability, which instruction preparation adopts before final admission. - instructionRoot?: string; instructionPreparationRunId?: string; agentId: string; instructionCopy?: { runId: string; root?: string; targetIdentity: string; collectStopped: () => Promise }; preparingRunId?: string; - managedAiCredentialIdentity?: string; - credentialRunId?: string; githubAccess?: NativeGitHubAccess; - githubAuthenticationMode?: string; - networkAccess: boolean; session: NativeSession; ownerToken: symbol; configDigest: string; - configuredEnvironmentDigest: string; + processConfigDigest?: string; ownerScope: string; normalizedSessionId: string; companyId: string; @@ -5467,7 +5461,7 @@ const RETIREMENT_REASON_NAMES = { recoveryState: new Set(["blocked", "resuming_session", "awaiting_evidence", "awaiting_runner_reattach"]), controllerReason: new Set(["controller_still_alive", "coordinated_controller_still_alive", "remote_controller_lease_live", "expired_lease_without_controller_identity", "live_legacy_lease_without_controller_identity", - "controller_host_unprovable", "controller_host_unrecorded"]), + "controller_host_unprovable", "controller_host_unrecorded", "controller_pid_start_inconsistent"]), sandboxOutcome: new Set(["created", "resumed", "replacement"]), } as const; @@ -7604,17 +7598,29 @@ function needsCoreGitHubAccess(input: Pick[0]): string | undefined { - if (needsCoreGitHubAccess(input) || input.runnerEnvironment?.PAPERCLIP_GITHUB_BROKER_TOKEN || input.instructionWorkingCopy) return undefined; +function nativeProcessReuseKey(input: Parameters[0]): string | undefined { + const environment = input.runnerEnvironment; + if (!input.managedGitHub && environment?.PAPERCLIP_GITHUB_BROKER_TOKEN) return undefined; + const schema = "paperclip.native-process-reuse.v2"; + const providerCredentials = PROVIDER_CREDENTIAL_ENVIRONMENT_KEYS.flatMap(key => environment?.[key] ? [[key, environment[key]]] : []); return nativeSha256({ - environment: configuredEnvironment(input.runnerEnvironment), + schema, + environment: configuredEnvironment(environment), + providerCredentials: createHmac("sha256", `${schema}:${nativeSessionScopeKey(input.execution)}`) + .update(canonicalNativeJson(providerCredentials)).digest("hex"), managedAiCredentialIdentity: input.managedAiCredentialIdentity ?? null, observabilityCompatibilityKey: input.observability?.compatibilityKey ?? null, - githubAuthenticationMode: input.runnerEnvironment?.PAPERCLIP_GITHUB_AUTH_MODE ?? null, - networkAccess: input.runnerEnvironment?.PAPERCLIP_RUNNER_NETWORK_ACCESS === "enabled", + githubAuthenticationMode: environment?.PAPERCLIP_GITHUB_AUTH_MODE ?? null, + networkAccess: environment?.PAPERCLIP_RUNNER_NETWORK_ACCESS === "enabled", + coreGitHubAccess: needsCoreGitHubAccess(input), + instructionRoot: input.instructionWorkingCopy?.root ?? null, }); } +function canReuseNativeProcess(input: Parameters[0], processConfigDigest: unknown): boolean { + return input.refreshTools !== true && typeof processConfigDigest === "string" && processConfigDigest === nativeProcessReuseKey(input); +} + function loadWarmNativeCheckpoint( execution: NativeExecutionInput, configDigest: string, @@ -9630,13 +9636,9 @@ async function executePaperclipNativeSessionWithinScope( } return loadWarmNativeCheckpoint(previous, nativeSessionConfigDigest(previous, "remote"), "remote"); }, - canReuseProcess: previous => { - const expected = warmNativeProcessConfigDigest(input); - if (!expected) return false; - const envelope = record(JSON.parse(readBoundedNativeFile(nativeSessionCheckpointPath(previous), - NATIVE_WARM_CHECKPOINT_MAX_BYTES, "native_session_supervisor_checkpoint_too_large").toString("utf8"))); - return envelope.processConfigDigest === expected; - }, + canReuseProcess: previous => !needsCoreGitHubAccess(input) && canReuseNativeProcess(input, + record(JSON.parse(readBoundedNativeFile(nativeSessionCheckpointPath(previous), + NATIVE_WARM_CHECKPOINT_MAX_BYTES, "native_session_supervisor_checkpoint_too_large").toString("utf8"))).processConfigDigest), }); remoteWarmRecovery.releasedSource = releasedSource; } @@ -10191,7 +10193,7 @@ async function executePaperclipNativeSessionWithinScope( }; const controllerCompatibility = { configDigest: nativeSessionConfigDigest(input.execution, input.runnerExecutionTarget?.kind ?? "local"), - processConfigDigest: warmNativeProcessConfigDigest(input) ?? null, + processConfigDigest: nativeProcessReuseKey(input) ?? null, }; const controlPlane = new PaperclipControlPlanePort( input.db, @@ -10538,9 +10540,6 @@ async function executePaperclipNativeSessionWithinScope( lifecyclePolicy.mode === "warm" ? controllerCompatibility.configDigest : null; - const configuredEnvironmentDigest = createHash("sha256") - .update(JSON.stringify(configuredEnvironment(input.runnerEnvironment))) - .digest("hex"); const warmSessionOwnerToken = Symbol( `native-warm-session:${input.execution.binding.runId}`, ); @@ -10591,34 +10590,15 @@ async function executePaperclipNativeSessionWithinScope( } } if (entry) { - // Old run-scoped environments still require process replacement. A - // session-owned broker can change run authority without replacing it. - const hasBrokerCapability = Boolean( - !input.managedGitHub && input.runnerEnvironment?.PAPERCLIP_GITHUB_BROKER_TOKEN, - ); - const credentialRunChanged = - Boolean(entry.credentialRunId) !== hasBrokerCapability || - (hasBrokerCapability && - entry.credentialRunId !== input.execution.binding.runId); if ( (entry.pluginTaskId !== undefined && (input.runnerExecutionTarget?.kind !== "remote" || input.runnerExecutionTarget.transport !== "sandbox" || entry.pluginTaskId !== input.runnerExecutionTarget.sandboxLeaseAcquisition?.providerLeaseId)) || - input.refreshTools === true || entry.closeOnReleaseReason !== undefined || entry.configDigest !== warmConfigDigest || - entry.observabilityCompatibilityKey !== input.observability?.compatibilityKey || - entry.configuredEnvironmentDigest !== configuredEnvironmentDigest || - entry.instructionRoot !== input.instructionWorkingCopy?.root || - entry.managedAiCredentialIdentity !== input.managedAiCredentialIdentity || - credentialRunChanged || + !canReuseNativeProcess(input, entry.processConfigDigest) || Boolean(entry.githubAccess) !== needsCoreGitHubAccess(input) || entry.githubAccess?.ready === false || - (entry.githubAccess !== undefined && entry.githubAccess.duplexRequested !== adapterExecutionTargetEnablesSandboxDuplexBridge(input.runnerExecutionTarget)) || - entry.githubAuthenticationMode !== - input.runnerEnvironment?.PAPERCLIP_GITHUB_AUTH_MODE || - entry.networkAccess !== - (input.runnerEnvironment?.PAPERCLIP_RUNNER_NETWORK_ACCESS === - "enabled") + (entry.githubAccess !== undefined && entry.githubAccess.duplexRequested !== adapterExecutionTargetEnablesSandboxDuplexBridge(input.runnerExecutionTarget)) ) { await finishStartupContextForFallback(); pluginTaskDescription=undefined; @@ -11152,24 +11132,13 @@ async function executePaperclipNativeSessionWithinScope( runId: input.execution.binding.runId, issueId: input.execution.binding.issueId, instructionWorkingCopy: input.instructionWorkingCopy?.checkpointWarm ? undefined : input.instructionWorkingCopy, - instructionRoot: input.instructionWorkingCopy?.root, agentId: input.execution.binding.agentId, - managedAiCredentialIdentity: input.managedAiCredentialIdentity, - githubAuthenticationMode: - input.runnerEnvironment?.PAPERCLIP_GITHUB_AUTH_MODE, - networkAccess: - input.runnerEnvironment - ?.PAPERCLIP_RUNNER_NETWORK_ACCESS === "enabled", githubAccess, - credentialRunId: !input.managedGitHub && input.runnerEnvironment - ?.PAPERCLIP_GITHUB_BROKER_TOKEN - ? input.execution.binding.runId - : undefined, session, ownerToken: warmSessionOwnerToken, configDigest: warmConfigDigest, + processConfigDigest: controllerCompatibility.processConfigDigest ?? undefined, observabilityCompatibilityKey: input.observability?.compatibilityKey, - configuredEnvironmentDigest, ownerScope: nativeSessionOwnerScope( input.execution, input.runnerExecutionTarget?.environmentId ?? null, ), @@ -14664,7 +14633,7 @@ async function createRunnerdBackendWithinSessionClaim( target, binding: input.execution.binding, trace: input.trace, onLog: input.onLog, }).controlPlaneRegistration, adoptExistingRunner: remoteDot?.adoptExistingRunner ?? (recoveredProcess ? { ...recoveredProcess, - isAlive: () => verifiedRecoveryProcessIsAlive(recoveredProcess), + isAlive: () => recoveryProcessMayBeAlive(recoveredProcess), signal: (signal: NodeJS.Signals = "SIGTERM") => signalVerifiedRecoveryProcess(recoveredProcess, signal) } : undefined), }; const backendOptions = { @@ -16012,7 +15981,7 @@ async function createRunnerdBackendWithinSessionClaim( : target.kind === "local" && localRecoveryProcess ? { ...localRecoveryProcess, - isAlive: () => verifiedRecoveryProcessIsAlive(localRecoveryProcess), + isAlive: () => recoveryProcessMayBeAlive(localRecoveryProcess), signal: (signal: NodeJS.Signals) => signalVerifiedRecoveryProcess(localRecoveryProcess, signal), } diff --git a/server/src/services/native-runtime/plugin-runner-release-resume.integration.test.ts b/server/src/services/native-runtime/plugin-runner-release-resume.integration.test.ts index d94697eba8f..4da3feb7710 100644 --- a/server/src/services/native-runtime/plugin-runner-release-resume.integration.test.ts +++ b/server/src/services/native-runtime/plugin-runner-release-resume.integration.test.ts @@ -41,9 +41,12 @@ const parkId = (source: DurableRecoveryIdentity) => `park_${sha(JSON.stringify(O describeEmbeddedPostgres("plugin Runner release/resume through the Core execution entry", () => { const environment = nativeRestartFixture(); const nativeIt = process.platform === "win32" ? it.skip : realProcessIt; - nativeIt.for([...(["codex", "claude"] as const).flatMap(harness => (["cached", "adopted"] as const).map(mode => [harness, mode] as const)), ["codex", "adopted cancellation"] as const, ["codex", "adopted legacy cancellation"] as const])("completes %s runs through a retained %s process", { timeout: 60_000 }, async ([harness, mode], context) => { + nativeIt.for([...(["codex", "claude"] as const).flatMap(harness => (["cached", "adopted"] as const).map(mode => [harness, mode] as const)), ["codex", "adopted cancellation"] as const, ["codex", "adopted legacy cancellation"] as const, + ["codex", "cached managed GitHub"] as const, ["codex", "adopted managed GitHub"] as const])("completes %s runs through a retained %s process", { timeout: 60_000 }, async ([harness, mode], context) => { if (harness === "claude" && !existsSync(fakeAcpxSidecar)) context.skip(); const adopted = mode.startsWith("adopted"), cancellation = mode.includes("cancellation"), qualifiedCancellation = mode === "adopted cancellation"; + const managedGitHub = mode.endsWith("managed GitHub"), brokerUrl = "https://core.example.test/api", brokerToken = (runId: string) => `fixture-broker-${runId}`; + const preparedBrokers: Array<{ runId: string; token?: string }> = []; const first = await environment.seedRun(`RELEASE-ENTRY-${harness}-${mode}`); const { db, runId, issueId, native } = first; const { companyId, agentId, runtimeRoot } = environment.current; @@ -130,9 +133,12 @@ describeEmbeddedPostgres("plugin Runner release/resume through the Core executio const task = async (raw: unknown, authorityRunId: string): Promise => { const operation = environmentTaskOperationSchema.parse(raw); if (operation.kind === "describe") return { kind: "description", taskId, launchPreparationVersion: 1, - projectAuthCheckVersion: 1, warmTaskVersion: 1, releaseResumeVersion: 1, runner: artifact, ...(qualifiedCancellation ? { executionContextVersion: 1 } : {}) }; + projectAuthCheckVersion: 1, warmTaskVersion: 1, releaseResumeVersion: 1, runner: artifact, ...(qualifiedCancellation ? { executionContextVersion: 1 } : {}), + ...(managedGitHub ? { githubCredentialBrokerVersion: 1 } : {}) }; if (operation.kind === "prepare") { expect(operation.requirements.identity.runId).toBe(authorityRunId); + expect(operation.githubBroker).toEqual(managedGitHub ? { url: brokerUrl, token: brokerToken(authorityRunId) } : undefined); + preparedBrokers.push({ runId: authorityRunId, token: operation.githubBroker?.token }); expect(operation.accountIdentity).toBe("fixture-account"); if (operation.priorIdentity) expect(sameSource(operation.priorIdentity, await state())).toBe(true); expect(operation.requirements.settings.managedProfile).toBeUndefined(); @@ -204,7 +210,9 @@ describeEmbeddedPostgres("plugin Runner release/resume through the Core executio await db.update(heartbeatRuns).set({ runnerProfileJson: { nativeExecutionInput: current }, runnerInstanceId: native.runnerInstanceId, completionContractId: contract!.id, completionContractSha256: contract!.canonicalSha256, nativeSessionId: current.session.normalizedSessionId }) .where(eq(heartbeatRuns.id, current.binding.runId)); return executePaperclipNativeSession({ db, execution: current, runnerInstanceId: native.runnerInstanceId, useRunnerd: true, runnerIngressAuthorized: true, - runnerEnvironment: harness === "claude" ? { ANTHROPIC_API_KEY: "synthetic-fixture" } : {}, + runnerEnvironment: { ...(harness === "claude" ? { ANTHROPIC_API_KEY: "synthetic-fixture" } : {}), + ...(managedGitHub ? { PAPERCLIP_GITHUB_BROKER_URL: brokerUrl, PAPERCLIP_GITHUB_BROKER_TOKEN: brokerToken(current.binding.runId) } : {}) }, + managedGitHub, managedAiCredentialHome: home, managedAiCredentialIdentity: "fixture-revision", managedAiAccountIdentity: "fixture-account", turnTimeoutMs: 15_000, runnerExecutionTarget: { kind: "remote", transport: "sandbox", remoteCwd: workspace, environmentId, leaseId, providerKey: "plugin:fixture:tasks", runnerTask: operation => task(operation, current.binding.runId), sandboxLeaseAcquisition: { outcome: leaseId === leaseA ? "created" : "resumed", providerLeaseId: taskId } } }); @@ -283,6 +291,7 @@ describeEmbeddedPostgres("plugin Runner release/resume through the Core executio } expect(claims).toEqual(releases.slice(0, -1)); expect(releases[1]!.runId).toBe(runB); expect(releases[1]!.environmentLeaseId).toBe(leaseA); expect(handle!.child.pid).toBe(pid); expect(submissions).toBe(1); + expect([...new Set(preparedBrokers.map(prepared => prepared.runId))]).toEqual(releases.map(release => release.runId)); const methods = (await readFile(calls, "utf8")).trim().split(/\r?\n/); if (harness === "codex") { expect(methods.filter(method => method === "process-start")).toHaveLength(1); diff --git a/server/src/services/quarantined-task-retry.test.ts b/server/src/services/quarantined-task-retry.test.ts index f08669a6e84..f87d720da71 100644 --- a/server/src/services/quarantined-task-retry.test.ts +++ b/server/src/services/quarantined-task-retry.test.ts @@ -17,6 +17,7 @@ import { getExecutionBlocker } from "./execution-blocker.js"; import { readFreshWorkspaceReadiness } from "./fresh-workspace-continuation.js"; import { instanceSettingsService } from "./instance-settings.js"; import { PROCESS_IDENTITY_RECORDED, PROCESS_START_REQUESTED } from "./native-local-process-stop.js"; +import { currentProcessStartedAtMs } from "./controller-process-identity.js"; import { nativeSha256 } from "./native-runtime/canonical.js"; import { verifyStoppedNativeSessionForContinuation } from "./native-runtime/native-session-executor.js"; import { requestTaskRetry } from "./task-retry.js"; @@ -185,7 +186,10 @@ describe("task Retry after inherited native cleanup quarantine", () => { })); } if (mode === "worker_alive") { - await db.update(heartbeatRuns).set({ processPid: process.pid, processGroupId: process.pid }) + // This test process, with its real start time: the shared prover then + // sees the same live worker, not a reused PID. + const startedAt = new Date((await currentProcessStartedAtMs())!); + await db.update(heartbeatRuns).set({ processPid: process.pid, processGroupId: process.pid, processStartedAt: startedAt }) .where(eq(heartbeatRuns.companyId, f.companyId)); const [owner] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, f.ownerId)); await appendHeartbeatRunEvent(db, { companyId: f.companyId, runId: f.ownerId, agentId: f.agentId, diff --git a/server/src/services/recovery/ownership-invariant-report.ts b/server/src/services/recovery/ownership-invariant-report.ts index ff314c14c41..269bb7e153d 100644 --- a/server/src/services/recovery/ownership-invariant-report.ts +++ b/server/src/services/recovery/ownership-invariant-report.ts @@ -6,7 +6,8 @@ import { issues, type Db, } from "@paperclipai/db"; -import { TEMPORARY_REFUSAL_TRIGGERS } from "../work-intent-outbox.js"; +import { taskClosedByAgentSql, TEMPORARY_REFUSAL_TRIGGERS } from "../work-intent-outbox.js"; +import { deliveredQueuedCommentIds } from "../queued-comment-delivery.js"; import { logger } from "../../middleware/logger.js"; import { persistActivity } from "../activity-log.js"; import { proveOwnerGone, type OwnerProof, type ProcessProbe } from "../owner-proof.js"; @@ -366,6 +367,97 @@ async function checkDeferredIntentsPromoted(db: Db, now: Date, limit: number): P } export const DEFERRED_INTENT_NOT_PROMOTED = "deferred_after_hold_cleared"; +export const USER_COMMENT_WITHOUT_WAKE = "user_comment_without_wake"; +/** A user comment younger than this may still be getting its normal wake. */ +const COMMENT_WAKE_GRACE_MS = 30_000; + +export const USER_COMMENT_DROPPED_BY_STALE_RUN = "user_comment_dropped_by_stale_continuation"; + +/** Invariant 4, the durable half of the stale-continuation rule (chaos + * s9401/t72). A user comment on a done task whose only carrying wakes were + * cancelled as stale continuations, that a settled agent run which started + * before it never read (it finished after the comment), and that no later + * run received (the shared per-comment delivery check). The decision waits for the closing run to settle, so it + * also covers a stale check that ran before that run reached succeeded, or a + * save that failed. The worker saves it so the release drain reopens the + * task and delivers it once. */ +async function checkStaleDroppedComments(db: Db, now: Date, limit: number): Promise { + const since = new Date(now.getTime() - INTENT_LOOKBACK_MS).toISOString(); + const rows = await db.execute<{ id: string; company_id: string; issue_id: string; agent_id: string }>(sql` + select c.id, c.company_id, c.issue_id, i.assignee_agent_id as agent_id from issue_comments c + join issues i on i.id = c.issue_id and i.company_id = c.company_id + where c.author_user_id is not null and c.deleted_at is null + and c.author_agent_id is null and c.created_by_run_id is null and c.created_at >= ${since}::timestamptz + and i.status = 'done' and i.assignee_agent_id is not null and i.conversation_agent_id is null + and ${taskClosedByAgentSql(sql`c.company_id`, sql`c.issue_id`)} + and exists (select 1 from agent_wakeup_requests stale where stale.company_id = c.company_id + and stale.payload->>'issueId' = c.issue_id::text and stale.status = 'cancelled' + and stale.error = 'continuation_task_ownership_changed' + and (stale.payload->>'commentId' = c.id::text + or coalesce(stale.payload#>'{_paperclipWakeContext,wakeCommentIds}', '[]'::jsonb) ? c.id::text)) + and not exists (select 1 from agent_wakeup_requests open_wake where open_wake.company_id = c.company_id + and open_wake.payload->>'issueId' = c.issue_id::text + and open_wake.status not in ('cancelled', 'skipped', 'failed', 'coalesced') + and (open_wake.payload->>'commentId' = c.id::text + or coalesce(open_wake.payload#>'{_paperclipWakeContext,wakeCommentIds}', '[]'::jsonb) ? c.id::text)) + and exists (select 1 from heartbeat_runs closing where closing.company_id = c.company_id + and closing.agent_id = i.assignee_agent_id and closing.status = 'succeeded' + and (closing.context_snapshot->>'issueId' = c.issue_id::text or closing.native_issue_id = c.issue_id) + and closing.started_at < c.created_at and closing.finished_at >= c.created_at) + and not exists (select 1 from heartbeat_runs r where r.company_id = c.company_id + and (r.context_snapshot->>'issueId' = c.issue_id::text or r.native_issue_id = c.issue_id) + and r.status in ('queued', 'running', 'scheduled_retry')) + order by c.created_at limit ${Math.min(limit, MAX_RECORDS_PER_KIND)}`); + // Consumption is per comment: a later run counts only if it received this + // comment (its own wake input, or a retry's task history), never ordinary + // task history of another message's turn. + const owed: typeof rows[number][] = []; + for (const row of rows) { + const delivered = await deliveredQueuedCommentIds(db, { companyId: row.company_id, issueId: row.issue_id, + agentId: row.agent_id, commentIds: [row.id] }); + if (!delivered.has(row.id)) owed.push(row); + } + return owed.map(row => ({ rule: "I4_exactly_once_intent" as const, kind: "issue_comment", recordId: row.id, + companyId: row.company_id, runId: null, issueId: row.issue_id, reason: USER_COMMENT_DROPPED_BY_STALE_RUN })); +} + +/** Invariant 4, reconciler-owned rule: an undelivered user comment newer than + * the task's last run gets a wake. A comment commits before its wake is + * written, so a restart in between loses the wake (chaos s9401/t69). This + * finds a user comment on an assigned, non-conversation task with no live run + * or lock, newer than every run of the task, that no wake request carries. + * The reconciliation worker saves it as a deferred receipt and promotes it. */ +async function checkUserCommentsWithoutWake(db: Db, now: Date, limit: number): Promise<{ violations: InvariantViolation[]; nextDueAt: Date | null }> { + const since = new Date(now.getTime() - INTENT_LOOKBACK_MS).toISOString(); + const rows = await db.execute<{ id: string; company_id: string; issue_id: string; created_at: string }>(sql` + select c.id, c.company_id, c.issue_id, c.created_at::text as created_at from issue_comments c + join issues i on i.id = c.issue_id and i.company_id = c.company_id + where c.author_user_id is not null and c.deleted_at is null + and c.author_agent_id is null and c.created_by_run_id is null + and c.created_at >= ${since}::timestamptz + and i.assignee_agent_id is not null and i.conversation_agent_id is null + and i.status in ('todo', 'in_progress') and i.execution_run_id is null + and exists (select 1 from heartbeat_runs prior where prior.company_id = c.company_id + and (prior.context_snapshot->>'issueId' = c.issue_id::text or prior.native_issue_id = c.issue_id)) + and not exists (select 1 from heartbeat_runs r where r.company_id = c.company_id + and (r.context_snapshot->>'issueId' = c.issue_id::text or r.native_issue_id = c.issue_id) + and (r.status in ('queued', 'running', 'scheduled_retry') or coalesce(r.started_at, r.created_at) >= c.created_at)) + and not exists (select 1 from agent_wakeup_requests w where w.company_id = c.company_id + and w.payload->>'issueId' = c.issue_id::text + and (w.payload->>'commentId' = c.id::text + or coalesce(w.payload#>'{_paperclipWakeContext,wakeCommentIds}', '[]'::jsonb) ? c.id::text)) + order by c.created_at limit ${Math.min(limit, MAX_RECORDS_PER_KIND)}`); + // A comment inside the grace may still get its normal wake: check it again + // when the grace ends (a saved deadline), and report only older ones. + const graceEnd = now.getTime() - COMMENT_WAKE_GRACE_MS; + const young = [...rows].map(row => new Date(row.created_at).getTime()).filter(at => at >= graceEnd); + return { + violations: [...rows].filter(row => new Date(row.created_at).getTime() < graceEnd) + .map(row => ({ rule: "I4_exactly_once_intent" as const, kind: "issue_comment", recordId: row.id, + companyId: row.company_id, runId: null, issueId: row.issue_id, reason: USER_COMMENT_WITHOUT_WAKE })), + nextDueAt: young.length ? new Date(Math.min(...young) + COMMENT_WAKE_GRACE_MS) : null, + }; +} /** Invariant 4 with section E's closed outcomes: an accepted intent (a user's * request or a message) ended terminally on a refusal that @@ -536,6 +628,12 @@ export async function runOwnershipReconciliationPass(db: Db, options: { for (const check of [ () => checkAdmissionIndependence(db, state, now, limit), () => checkExactlyOnceIntents(db, state, now, limit), + () => checkStaleDroppedComments(db, now, limit), + async () => { + const result = await checkUserCommentsWithoutWake(db, now, limit); + if (result.nextDueAt && (!nextDueAt || result.nextDueAt < nextDueAt)) nextDueAt = result.nextDueAt; + return result.violations; + }, async () => { const result = await checkDeferredIntentsPromoted(db, now, limit); if (result.nextDueAt && (!nextDueAt || result.nextDueAt < nextDueAt)) nextDueAt = result.nextDueAt; @@ -554,6 +652,19 @@ export async function runOwnershipReconciliationPass(db: Db, options: { logger.warn({ err: error }, "ownership invariant check failed; the report continues"); } } + // A settling kind can become settleable at a known time without a signal + // (a retention window ending); wake then. + for (const kind of kinds) { + if (!settleRecords || !kind.settle || !kind.nextDueAt) continue; + try { + const due = await kind.nextDueAt(db, now); + const current = nextDueAt as Date | null; + if (due && (!current || due < current)) nextDueAt = due; + } catch (error) { + retryNeeded = true; + logger.warn({ err: error, kind: kind.id }, "ownership due-time read failed; a later pass retries"); + } + } // Only a first proof needs a second pass; a record already reported for I2 // is not a reason to keep scanning. diff --git a/server/src/services/recovery/ownership-registry.ts b/server/src/services/recovery/ownership-registry.ts index aee032246e4..848c0992fdf 100644 --- a/server/src/services/recovery/ownership-registry.ts +++ b/server/src/services/recovery/ownership-registry.ts @@ -33,6 +33,7 @@ import { } from "../native-runtime/native-accepted-result-lease-settlement.js"; import { settleClaimedWakeRequest } from "./wake-request-claimed-settlement.js"; import { settleAbandonedWorkspaceFinalizeAttempt } from "./workspace-finalize-attempt-settlement.js"; +import { nativeControllerOwnerStamp, runProcessOwnerStamp } from "../native-runtime/native-owner-stamps.js"; import { legacyControllerOwnerStamp, settleLegacyControllerLease } from "../legacy-controller-lease.js"; import { nativeLeaseHolderStamp, @@ -49,6 +50,12 @@ import { } from "../environment-cleanup-state.js"; import { settleEnvironmentRunLease } from "../environment-run-lease-settlement.js"; import { settleEnvironmentTeardownClaim } from "../environment-teardown-claim-settlement.js"; +import { settleSandboxCreationIntent } from "../sandbox-creation-intent-settlement.js"; +import { + nextRetainedOrphanDueAt, + releasableRetainedOrphanCondition, + settleRetainedOrphanSandbox, +} from "../environment-retained-orphan-settlement.js"; /** * Registry of ownable record kinds (plan section C). @@ -122,6 +129,9 @@ export type OwnableKind = { settle: SettleRecord | null; /** Optional operator action for an unprovable owner of a settling kind. */ onUnprovable?: UnprovableAction; + /** When a record of this kind next becomes settleable without a signal + * (a retention window ending). The reconciler wakes then. */ + nextDueAt?: (db: Db, now: Date) => Promise; /** Plan invariants this kind is checked against (1–8). */ invariantChecks: readonly number[]; /** Why the kind cannot be listed, when `listInProgress` is null. */ @@ -154,20 +164,8 @@ const runOwner = (runId: string | null | undefined): RecordOwner => runId ? { ki type NrfRow = typeof nativeRunFinalizations.$inferSelect; -/** NRF records the controller's boot, PID, start time and, since #19868, its - * PID space (`controller_host_hash`). Rows written before that have no space - * and stay unprovable for a foreign boot (inventory finding 2). */ -function nrfStamp(row: NrfRow, scope: string): LocalOwnerStamp { - return { - kind: "local", scope, generation: row.controllerGeneration ?? null, host: null, - processSpace: row.controllerHostHash ?? null, - // Rows saved before #19868 keep the one-release local proof on single-host - // servers (`legacy_host_unknown_dead_pid_proof`, #19883). - processSpaceUnrecorded: row.controllerHostHash == null, - bootId: row.controllerBootId, pid: row.controllerPid, - processStartedAt: row.controllerProcessStartedAt?.toISOString() ?? null, processGroupId: null, - }; -} +/** The NRF controller stamp, shared with native recovery (native-owner-stamps.ts). */ +const nrfStamp = (row: NrfRow, scope: string): LocalOwnerStamp => nativeControllerOwnerStamp(row, scope); function nrfKind(id: string, title: string, obligation: Obligation, where: SQL | undefined, owner: (row: NrfRow) => RecordOwner = row => ({ kind: "process", stamp: nrfStamp(row, id) }), @@ -247,16 +245,9 @@ function runKind(id: string, title: string, obligation: Obligation, where: SQL | }; } -/** heartbeat_runs records the adapter child's PID, start time and, since - * #19871, its PID space (`process_host_hash`). Older rows have no space and - * stay unprovable. */ -function runProcessStamp(row: RunRow, scope: string): LocalOwnerStamp { - return { - kind: "local", scope, generation: null, host: null, processSpace: row.processHostHash ?? null, bootId: null, - pid: row.processPid, processStartedAt: row.processStartedAt?.toISOString() ?? null, - processGroupId: row.processGroupId, - }; -} +/** The adapter child's stamp (native-owner-stamps.ts). Rows saved before + * `process_host_hash` stay unprovable here. */ +const runProcessStamp = (row: RunRow, scope: string): LocalOwnerStamp => runProcessOwnerStamp(row, scope); const legacyControllerLeaseWhere = and(eq(heartbeatRuns.runtimeMode, "legacy"), eq(heartbeatRuns.status, "running"), isNotNull(heartbeatRuns.controllerBootId), sql`${heartbeatRuns.controllerLeaseExpiresAt} > now()`); @@ -633,13 +624,18 @@ export const OWNABLE_KINDS: readonly OwnableKind[] = [ // Environments and sandboxes { + // Shared path since Phase 2: a creator proven gone makes its intent due + // now; creation recovery then reconciles it with the provider. An intent + // whose creator is already marked lost belongs to that recovery deadline. id: "sandbox_creation_intent", title: "Sandbox creation intent", obligation: "ownership", storage: "database", - settle: null, invariantChecks: [1, 2, 3, 7], + settle: settleSandboxCreationIntent, invariantChecks: [1, 2, 3, 7], listInProgress: async (db, limit, offset) => { const rows = await db.select({ id: sandboxCreationOperations.id, companyId: sandboxCreationOperations.companyId, runId: sandboxCreationOperations.runId, issueId: sandboxCreationOperations.issueId, metadata: sandboxCreationOperations.metadata, updatedAt: sandboxCreationOperations.updatedAt }) - .from(sandboxCreationOperations).where(eq(sandboxCreationOperations.state, "pending")).orderBy(asc(sandboxCreationOperations.id)).limit(limit + 1).offset(offset); + .from(sandboxCreationOperations).where(and(eq(sandboxCreationOperations.state, "pending"), + sql`coalesce(${sandboxCreationOperations.metadata}->>'creationControllerLost', '') <> 'true'`)) + .orderBy(asc(sandboxCreationOperations.id)).limit(limit + 1).offset(offset); const result = page(rows, limit); return { truncated: result.truncated, records: result.rows.map(row => ({ recordId: row.id, companyId: row.companyId, runId: row.runId, issueId: row.issueId, @@ -666,6 +662,16 @@ export const OWNABLE_KINDS: readonly OwnableKind[] = [ row => runOwner(row.heartbeatRunId)), leaseKind("environment_retained_hold", "Retained / reusable hold", "workspace_preservation", eq(environmentLeases.status, "retained"), () => ({ kind: "none" }), [1, 7]), + { + // Shared path since Phase 2: a sandbox the orphaned-run-lease settle + // stopped and kept is destroyed once nothing references it. Only + // releasable orphans are listed, so a held one is never polled; its + // retention window end is a due time. + ...leaseKind("environment_retained_orphan_sandbox", "Retained orphan sandbox", "workspace_preservation", + releasableRetainedOrphanCondition(), row => runOwner(row.heartbeatRunId), [1]), + settle: settleRetainedOrphanSandbox, + nextDueAt: nextRetainedOrphanDueAt, + }, spoolKind("sandbox_orphan_cleanup_spool", "Orphan cleanup buffer/spool", "ownership", idleOrphanSpoolPath), leaseKind("environment_login_probe_lease", "Login/probe lease", "ownership", and(eq(environmentLeases.status, "active"), isNull(environmentLeases.heartbeatRunId)), diff --git a/server/src/services/sandbox-creation-intent-settlement.ts b/server/src/services/sandbox-creation-intent-settlement.ts new file mode 100644 index 00000000000..766aa2d30d9 --- /dev/null +++ b/server/src/services/sandbox-creation-intent-settlement.ts @@ -0,0 +1,55 @@ +import { and, eq, gt, sql } from "drizzle-orm"; +import { sandboxCreationOperations, type Db } from "@paperclipai/db"; +import { readControllerProcess } from "./controller-process-identity.js"; +import { notifyEnvironmentCleanup } from "./environment-cleanup-notifications.js"; +import type { OwnerProof } from "./owner-proof.js"; +import type { InProgressRecord, SettleOutcome } from "./recovery/ownership-registry.js"; + +/** + * Settle of the `sandbox_creation_intent` ownership kind (plan Phase 2). + * + * A creation intent is written before the provider call and names the server + * process that makes it. While that process lives, the intent waits for its + * settle deadline. Once the shared prover proves the creator gone, it cannot + * still be waiting for the provider, so this settle makes the intent due now + * and marks the creator lost; the creation recovery worker then reconciles it. + * + * It settles ownership only. Whether the sandbox exists is still decided by + * the provider: absence stays inconclusive until the creation settle deadline + * (`creationAbsenceInconclusive`), and an unidentifiable creation becomes an + * operator reconciliation. A creator on another host is never proven gone by + * a process proof; its intent keeps the deadline, which is the handoff. + * + * The update compares and swaps on the exact creator the proof judged and + * only moves a deadline that is still in the future. + */ +export async function settleSandboxCreationIntent(db: Db, record: InProgressRecord, + context: { now: Date; proof: OwnerProof }): Promise { + if (record.owner.kind !== "process") return { settled: false, reason: "owner_not_a_process" }; + const stamp = record.owner.stamp; + const now = context.now; + return db.transaction(async transaction => { + const tx = transaction as unknown as Db; + const [operation] = await tx.select().from(sandboxCreationOperations) + .where(and(eq(sandboxCreationOperations.id, record.recordId), eq(sandboxCreationOperations.state, "pending"))) + .for("update"); + if (!operation) return { settled: false, reason: "intent_no_longer_pending" }; + if (operation.metadata.creationControllerLost === true) return { settled: false, reason: "creator_already_lost" }; + const creator = readControllerProcess(operation.metadata.creationController); + if (!creator || creator.bootId !== stamp.bootId || creator.pid !== stamp.pid || creator.hostHash !== stamp.processSpace) { + return { settled: false, reason: "owner_changed_since_proof" }; + } + if (operation.nextAttemptAt <= now) return { settled: false, reason: "intent_already_due" }; + // Wake creation recovery before the write it announces. + await notifyEnvironmentCleanup(tx, { companyId: operation.companyId, runId: operation.runId }); + const updated = await tx.update(sandboxCreationOperations).set({ nextAttemptAt: now, updatedAt: now, + metadata: sql`coalesce(${sandboxCreationOperations.metadata}, '{}'::jsonb) || ${JSON.stringify({ + creationControllerLost: true, creationControllerLostProof: context.proof.reason })}::jsonb` }) + .where(and(eq(sandboxCreationOperations.id, operation.id), eq(sandboxCreationOperations.state, "pending"), + gt(sandboxCreationOperations.nextAttemptAt, now), + sql`coalesce(${sandboxCreationOperations.metadata}->>'creationControllerLost', '') <> 'true'`, + sql`${sandboxCreationOperations.metadata}->'creationController' = ${JSON.stringify(operation.metadata.creationController)}::jsonb`)) + .returning({ id: sandboxCreationOperations.id }); + return updated.length ? { settled: true, reason: context.proof.reason } : { settled: false, reason: "intent_no_longer_pending" }; + }); +} diff --git a/server/src/services/sandbox-creation-operations.ts b/server/src/services/sandbox-creation-operations.ts index 8e11ce48c37..bbc8369962e 100644 --- a/server/src/services/sandbox-creation-operations.ts +++ b/server/src/services/sandbox-creation-operations.ts @@ -1,7 +1,7 @@ import { collectSecretRefPaths, isUuidSecretRef, parseSecretRefBindingObject, readConfigValueAtPath } from "./json-schema-secret-refs.js"; import { createHash, randomUUID } from "node:crypto"; -import { and, asc, eq, gt, lte, or, sql } from "drizzle-orm"; -import { controllerProcessExited, currentControllerBootId, currentControllerProcess, readControllerProcess } from "./controller-process-identity.js"; +import { and, eq, lte, or, sql } from "drizzle-orm"; +import { currentControllerProcess } from "./controller-process-identity.js"; import { companies, environments, sandboxCreationOperations, type Db } from "@paperclipai/db"; import { readEnvironmentCreationCleanupError, type PluginEnvironmentLease, type PluginEnvironmentLeaseObservation } from "@paperclipai/plugin-sdk"; import { assertEnvironmentCompanyBinding, environmentService } from "./environments.js"; @@ -15,37 +15,6 @@ export type SandboxCreationOperation = typeof sandboxCreationOperations.$inferSe // A live creator may still be waiting for the provider, and a provider may still // materialize an aborted request. Absence is conclusive only after this delay. const CREATION_SETTLE_FLOOR_MS = 30 * 60_000; -const CREATION_PROMOTION_PAGE_SIZE = 100; - -/** After a restart, creation intents from an exited server cannot still be in - * flight there. Reconcile them now instead of at the settle deadline. Absence - * stays inconclusive until that deadline; see recoverSandboxCreations. */ -async function promoteAbandonedSandboxCreations(db: Db): Promise { - const now = new Date(); - // Page past live, foreign, and unidentified creators so they cannot hide an - // abandoned intent behind them. - let after: string | null = null; - for (;;) { - const waiting: Array> = await db - .select({ id: sandboxCreationOperations.id, metadata: sandboxCreationOperations.metadata }) - .from(sandboxCreationOperations).where(and( - eq(sandboxCreationOperations.state, "pending"), gt(sandboxCreationOperations.nextAttemptAt, now), - sql`${sandboxCreationOperations.metadata}->'creationController'->>'bootId' is distinct from ${currentControllerBootId()}`, - sql`coalesce(${sandboxCreationOperations.metadata}->>'creationControllerLost', '') <> 'true'`, - after === null ? undefined : gt(sandboxCreationOperations.id, after), - )).orderBy(asc(sandboxCreationOperations.id)).limit(CREATION_PROMOTION_PAGE_SIZE); - for (const operation of waiting) { - const controller = readControllerProcess(operation.metadata.creationController); - if (!controller || !await controllerProcessExited(controller)) continue; - await db.update(sandboxCreationOperations).set({ nextAttemptAt: now, updatedAt: now, - metadata: sql`coalesce(${sandboxCreationOperations.metadata}, '{}'::jsonb) || '{"creationControllerLost":true}'::jsonb` }) - .where(and(eq(sandboxCreationOperations.id, operation.id), eq(sandboxCreationOperations.state, "pending"), - gt(sandboxCreationOperations.nextAttemptAt, now))); - } - if (waiting.length < CREATION_PROMOTION_PAGE_SIZE) return; - after = waiting[waiting.length - 1]!.id; - } -} function creationAbsenceInconclusive(operation: SandboxCreationOperation): boolean { const settleAt = Date.parse(typeof operation.metadata.creationSettleAt === "string" ? operation.metadata.creationSettleAt : ""); @@ -199,7 +168,13 @@ export async function recoverSandboxCreations(db: Db, inspect: (operation: SandboxCreationOperation) => Promise, onTaskRecovered?: (operation: SandboxCreationOperation) => void | Promise, ): Promise { - await promoteAbandonedSandboxCreations(db); + // After a restart, an intent whose creator is proven gone cannot still be in + // flight there: the shared ownership path makes it due now. Absence stays + // inconclusive until the settle deadline (creationAbsenceInconclusive). + const { settleOwnableKind } = await import("./recovery/ownership-invariant-report.js"); + await settleOwnableKind(db, "sandbox_creation_intent").catch(() => { + logger.warn({ errorKind: "sandbox_creation_intent_settle_failed" }, "Sandbox creation intents keep their saved deadlines"); + }); const due = await db.select().from(sandboxCreationOperations).where(and( recoverableSandboxCreationCondition(), lte(sandboxCreationOperations.nextAttemptAt, new Date()), )).limit(100); diff --git a/server/src/services/sandbox-stop-and-retain.ts b/server/src/services/sandbox-stop-and-retain.ts index c17c6a10965..a1ccefc1d6d 100644 --- a/server/src/services/sandbox-stop-and-retain.ts +++ b/server/src/services/sandbox-stop-and-retain.ts @@ -11,6 +11,10 @@ import type { SandboxAbsenceProof } from "./sandbox-absence-proof.js"; type Lease = Pick; export const SANDBOX_STOP_AND_RETAIN_KEY = "sandboxStopAndRetain"; +/** Top-level marker an ownership settle leaves on a lease whose sandbox it + * stopped and kept. It outlives the stop intent, so the retained orphan can + * be destroyed once nothing references it. */ +export const ORPHANED_SANDBOX_RECOVERY_KEY = "orphanedRunLeaseRecovery"; /** A successful stop is not by itself proof that the allocation was retained. * Require the dedicated stop-only dispatch receipt and its independent stop @@ -75,8 +79,9 @@ export function readStopOnlyCleanup(lease: Lease) { /** Persist before provider dispatch so an older worker or a restart cannot turn * an explicit stop into ordinary destructive release. Invalid pins stay pending. - * `recovery` marks an ownership settle on the intent; the pending-cleanup - * sweep dispatches such a stop's first attempt without its retry backoff. */ + * `recovery` marks an ownership settle on the intent and on the lease + * (`ORPHANED_SANDBOX_RECOVERY_KEY`); the pending-cleanup sweep dispatches such + * a stop's first attempt without its retry backoff. */ export async function prepareSandboxStopAndRetain(db: Db, lease: Lease, recovery?: Record) { return prepareStopAndRetain(db, lease, "active", recovery); } @@ -158,6 +163,8 @@ async function prepareStopAndRetain(db: Db, lease: Lease, fromStatus: "active" | status: "pending_cleanup", cleanupStatus: "failed", failureReason: "sandbox_stop_pending", releasedAt: now, updatedAt: now, metadata: sql`(coalesce(${environmentLeases.metadata}, '{}'::jsonb) - 'remoteExecutionTermination' - 'sandboxStopAndRetainReceipt') || ${JSON.stringify({ [SANDBOX_STOP_AND_RETAIN_KEY]: intent, pendingCleanupAttemptId: requestId, + ...(fromStatus === "active" && Object.keys(recovery).length + ? { [ORPHANED_SANDBOX_RECOVERY_KEY]: { ...recovery, runId: lease.heartbeatRunId, recoveredAt: now.toISOString() } } : {}), pendingCleanupInFlight: false, pendingCleanupLeaseExpiresAtMs: 0, pendingCleanupRetryAfterMs: 0, pendingCleanupRetryAttempts: 0, pendingCleanupRetryCapWarned: false, })}::jsonb`, diff --git a/server/src/services/work-intent-outbox.ts b/server/src/services/work-intent-outbox.ts index 22c5b763635..89034210e03 100644 --- a/server/src/services/work-intent-outbox.ts +++ b/server/src/services/work-intent-outbox.ts @@ -1,6 +1,8 @@ -import { and, eq, sql } from "drizzle-orm"; -import { agentWakeupRequests, type Db } from "@paperclipai/db"; -import { notifyDeferredExecutionEligibility, notifyDeferredWakesForIssue } from "./deferred-execution-notifications.js"; +import { and, eq, inArray, isNotNull, isNull, sql } from "drizzle-orm"; +import { agentWakeupRequests, issueComments, issues, type Db } from "@paperclipai/db"; +import { notifyDeferredExecutionEligibility, notifyDeferredExecutionWork, notifyDeferredWakesForIssue } from "./deferred-execution-notifications.js"; +import { withQueuedCommentIdsInWakePayload } from "./issue-queued-comment-queue.js"; +import { deliveredQueuedCommentIds } from "./queued-comment-delivery.js"; import { notifyIssueRecovery } from "./issue-recovery-notifications.js"; import { notifyRunDispatch } from "./run-dispatch-notifications.js"; @@ -37,6 +39,8 @@ export const TEMPORARY_REFUSAL_TRIGGERS: Readonly>'issueId' = ${input.issueId}` : undefined, )); } + +const DEFERRED_WAKE_CONTEXT_KEY = "_paperclipWakeContext"; + +/** SQL: the latest recorded `done` transition of the task was not made by a + * person. A task a person closed (or with no recorded close) is never + * reopened for an earlier message. */ +export function taskClosedByAgentSql(companyId: unknown, issueId: unknown) { + return sql`coalesce((select closed.actor_type from activity_log closed + where closed.company_id = ${companyId} and closed.entity_type = 'issue' and closed.entity_id = ${issueId}::text + and closed.action = 'issue.updated' + and (closed.details->>'toStatus' = 'done' or closed.details->>'status' = 'done') + order by closed.created_at desc limit 1), 'user') <> 'user'`; +} + +/** + * Record the missing outbox entry for user messages whose wake was lost or + * ended without delivering them: one deferred receipt of the user that + * carries the undelivered comments. Only the task's assigned agent, and only + * live user comments that no completed run received, are saved. Statuses the + * caller allows (for example `done`, which the release drain reopens for a + * user's message) gate it. Normal admission and the release drain deliver + * the receipt once. Returns the receipt id, or null when nothing is owed. + */ +export async function saveUndeliveredUserComments(db: Db, input: { + companyId: string; issueId: string; agentId: string; commentIds: string[]; + allowedStatuses: readonly string[]; requestedByActorId?: string | null; + source?: string | null; triggerDetail?: string | null; + /** Names why the receipt waits and what promotes it. */ + executionWait?: Record; +}): Promise { + if (input.commentIds.length === 0) return null; + return db.transaction(async tx => { + const [issue] = await tx.select({ status: issues.status, assigneeAgentId: issues.assigneeAgentId, + conversationAgentId: issues.conversationAgentId }).from(issues) + .where(and(eq(issues.companyId, input.companyId), eq(issues.id, input.issueId))).for("update"); + // A reassigned or cancelled task ends the intent; a conversation keeps + // its own inbound authorization path. + if (!issue || issue.assigneeAgentId !== input.agentId || issue.conversationAgentId || + !input.allowedStatuses.includes(issue.status)) return null; + if (issue.status === "done") { + const [closed] = await tx.execute<{ byAgent: boolean }>(sql`select ${taskClosedByAgentSql(input.companyId, input.issueId)} as "byAgent"`); + if (closed?.byAgent !== true) return null; + } + const live = await tx.select({ id: issueComments.id, authorUserId: issueComments.authorUserId }).from(issueComments).where(and( + eq(issueComments.companyId, input.companyId), eq(issueComments.issueId, input.issueId), + inArray(issueComments.id, input.commentIds), isNotNull(issueComments.authorUserId), isNull(issueComments.deletedAt), + // A direct human comment: no agent author and no creating run. + isNull(issueComments.authorAgentId), isNull(issueComments.createdByRunId))); + const delivered = await deliveredQueuedCommentIds(tx as unknown as Db, { companyId: input.companyId, + issueId: input.issueId, agentId: input.agentId, commentIds: live.map(row => row.id) }); + const remaining = input.commentIds.filter(id => live.some(row => row.id === id) && !delivered.has(id)); + if (remaining.length === 0) return null; + const latest = remaining.at(-1)!; + await notifyDeferredExecutionWork(tx, { companyId: input.companyId }); + await notifyDeferredWakesForIssue(tx, { companyId: input.companyId, issueId: input.issueId }); + const [saved] = await tx.insert(agentWakeupRequests).values({ + companyId: input.companyId, agentId: input.agentId, source: input.source ?? "automation", + triggerDetail: input.triggerDetail ?? "system", reason: "issue_commented", status: "deferred_issue_execution", + requestedByActorType: "user", + requestedByActorId: input.requestedByActorId ?? live.find(row => row.id === latest)?.authorUserId ?? null, + payload: withQueuedCommentIdsInWakePayload({ issueId: input.issueId, mutation: "comment", commentId: latest, + ...(input.executionWait ? { executionWait: input.executionWait } : {}), + [DEFERRED_WAKE_CONTEXT_KEY]: { issueId: input.issueId, taskId: input.issueId, wakeReason: "issue_commented", + source: "issue.comment", wakeCommentIds: remaining, wakeCommentId: latest } }, remaining), + }).returning({ id: agentWakeupRequests.id }); + return saved?.id ?? null; + }); +} diff --git a/server/src/vendor/paperclip-runner/index.ts b/server/src/vendor/paperclip-runner/index.ts index a48b7da1ed7..38108ec7a08 100644 --- a/server/src/vendor/paperclip-runner/index.ts +++ b/server/src/vendor/paperclip-runner/index.ts @@ -218,6 +218,7 @@ export const CONFIGURED_ENVIRONMENT_KEYS = runner.CONFIGURED_ENVIRONMENT_KEYS; export const GENERATED_RUNTIME_ENVIRONMENT_KEYS = runner.GENERATED_RUNTIME_ENVIRONMENT_KEYS; export const configuredEnvironmentProjection = runner.configuredEnvironmentProjection; export const configuredEnvironment = runner.configuredEnvironment; +export const PROVIDER_CREDENTIAL_ENVIRONMENT_KEYS = runner.PROVIDER_CREDENTIAL_ENVIRONMENT_KEYS; export const probeNativeRunnerEnvironment = runner.probeNativeRunnerEnvironment; diff --git a/tests/restart-chaos/grader.test.ts b/tests/restart-chaos/grader.test.ts index ec65b2a3e5e..416fc214eb0 100644 --- a/tests/restart-chaos/grader.test.ts +++ b/tests/restart-chaos/grader.test.ts @@ -127,6 +127,27 @@ describe("R1–R6 grading on fixture timelines", () => { expect(gradeR4(record(afterSuccess)).detail).toMatch(/comment_consumed_twice: c1 by r1,r2/); }); + // s9401/t72: the message's run was claimed just before the first run ended, + // then cancelled as stale before dispatch; a later run delivered the message. + it("passes R4 for a stale continuation cancelled before dispatch whose message a later run delivered once", () => { + const samples = [sample(90_000, "done", [ + run("r1", "succeeded", 1_000, 40_000), + run("r2", "cancelled", 30_000, 40_060, { startedAt: iso(39_960), wakeCommentIds: ["c1"], + errorCode: "continuation_task_ownership_changed" }), + run("r3", "succeeded", 45_000, 80_000, { wakeCommentIds: ["c1"] }), + ])]; + expect(gradeR4(record(samples))).toMatchObject({ verdict: "pass" }); + // A poll that saw both running (claiming marks running before dispatch) + // does not change that. + const polled = [sample(39_990, "in_progress", [run("r1", "running", 1_000), run("r2", "running", 30_000, null, { startedAt: iso(39_960) })]), + ...samples]; + expect(gradeR4(record(polled))).toMatchObject({ verdict: "pass" }); + // Any other cancelled run that overlapped still executed. + const executed = [sample(90_000, "done", [run("r1", "succeeded", 1_000, 40_000), + run("r2", "cancelled", 30_000, 40_060, { startedAt: iso(39_960), errorCode: "cancelled" })])]; + expect(gradeR4(record(executed)).detail).toMatch(/concurrent_runs/); + }); + it("fails R5 on a silent stall longer than three minutes after ready, but not during outages or named waits", () => { const idle = (at: number, extra: Partial = {}) => sample(at, "in_progress", [run("r1", "failed", 1_000, 30_000)], extra); const stalled = [idle(60_000), idle(60_000 + STALL_LIMIT_MS / 2), idle(60_000 + STALL_LIMIT_MS + 5_000)]; diff --git a/tests/restart-chaos/grader.ts b/tests/restart-chaos/grader.ts index 4dd4d2cc109..aaead120c04 100644 --- a/tests/restart-chaos/grader.ts +++ b/tests/restart-chaos/grader.ts @@ -103,6 +103,9 @@ export function gradeR3(record: TaskRecord): RuleResult { return { verdict: "pass", detail: "one Retry started a run that completed the task" }; } +/** Run cancellations decided before adapter dispatch: the run never executed. */ +const PRE_DISPATCH_CANCELLATIONS = new Set(["continuation_task_ownership_changed"]); + /** * R4 is about duplicate execution. A queued or scheduled wake waiting behind a * running run (for example a follow-up comment, cancelled when the running run @@ -114,22 +117,32 @@ export function gradeR3(record: TaskRecord): RuleResult { * after a successful run is a replay. */ export function gradeR4(record: TaskRecord): RuleResult { - for (const sample of reachable(record.samples)) { - const running = (sample.runs ?? []).filter(run => run.status === "running"); - if (running.length > 1) return { verdict: "fail", detail: `concurrent_runs: ${running.map(run => `${run.id}(${run.status})`).join(",")}` }; - } const finalRuns = new Map[number]>(); for (const sample of reachable(record.samples)) for (const run of sample.runs ?? []) finalRuns.set(run.id, run); + // Claiming marks a run running before dispatch, so a run that ended as a + // pre-dispatch cancellation never executed, whatever a poll saw. + const neverExecuted = (id: string) => { + const final = finalRuns.get(id); + return final?.status === "cancelled" && PRE_DISPATCH_CANCELLATIONS.has(final.errorCode ?? ""); + }; + for (const sample of reachable(record.samples)) { + const running = (sample.runs ?? []).filter(run => run.status === "running" && !neverExecuted(run.id)); + if (running.length > 1) return { verdict: "fail", detail: `concurrent_runs: ${running.map(run => `${run.id}(${run.status})`).join(",")}` }; + } const begun = [...finalRuns.values()].filter(run => run.startedAt) .sort((a, b) => (ms(a.createdAt) || 0) - (ms(b.createdAt) || 0)); + // A continuation cancelled as stale before adapter dispatch was claimed but + // never executed (chaos s9401/t72), so it cannot overlap another run. Its + // message is saved and delivered by a later run, counted once below. + const executed = begun.filter(run => !(run.status === "cancelled" && PRE_DISPATCH_CANCELLATIONS.has(run.errorCode ?? ""))); // Execution windows seen between polls: started → finished (or still going at the last sample). const lastAt = Math.max(0, ...reachable(record.samples).map(sample => sample.at)); const window = (run: (typeof begun)[number]) => ({ start: ms(run.startedAt) || 0, end: run.finishedAt ? ms(run.finishedAt) || 0 : ACTIVE_RUN_STATUSES.has(run.status) ? lastAt : ms(run.startedAt) || 0 }); - for (let index = 0; index < begun.length; index += 1) for (let other = index + 1; other < begun.length; other += 1) { - const [a, b] = [window(begun[index]!), window(begun[other]!)]; + for (let index = 0; index < executed.length; index += 1) for (let other = index + 1; other < executed.length; other += 1) { + const [a, b] = [window(executed[index]!), window(executed[other]!)]; if (a.start < b.end && b.start < a.end) { - return { verdict: "fail", detail: `concurrent_runs: ${begun[index]!.id}(${begun[index]!.status}),${begun[other]!.id}(${begun[other]!.status}) executed together` }; + return { verdict: "fail", detail: `concurrent_runs: ${executed[index]!.id}(${executed[index]!.status}),${executed[other]!.id}(${executed[other]!.status}) executed together` }; } } const consumers = new Map();