From 67aa87a93bb1bbc4559105497863f541f9fb4e9b Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Sun, 25 Oct 2026 12:58:50 -0700 Subject: [PATCH] perf(recovery): skip unchanged retained workspace evidence writes (#17650) Guard the retained-evidence update with its existing JSONB comparison. Reuse the full first result when evidence is unchanged, while preserving new evidence delivery, source-bound retention, cleanup, and recovery holds. Verified with 199 recovery and retention tests, full workspace typechecking, the server build, green CI, and exact-head Apex5/5. Co-Authored-By: Paperclip --- doc/DATABASE.md | 5 ++- .../legacy-workspace-restore-recovery.test.ts | 35 +++++++++++++++++++ .../src/services/legacy-execution-recovery.ts | 9 ++--- 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/doc/DATABASE.md b/doc/DATABASE.md index c7b525877fc..137b7fe1810 100644 --- a/doc/DATABASE.md +++ b/doc/DATABASE.md @@ -99,7 +99,7 @@ process-field update remain in the same transaction. Legacy terminalization locks runtime mode, status, full result evidence, and the status-delivery token without loading unused input, profile, or log bodies. -It keeps the existing lock, writes, full returned run, workspace-retention +It keeps the existing lock, full returned run, workspace-retention receipts, and source-bound recovery holds. Stored payloads remain unchanged. Conversation ownership checks select process identity and full recovery result @@ -281,6 +281,9 @@ Legacy recovery also retains the pending delivery when status is unchanged and no patch was supplied. Explicit patches, new terminal statuses, and newly retained workspace evidence still create a delivery. File retention, ownership checks, failed write conditions, lock cleanup, and recovery holds still apply. +The retention-evidence write returns another full run only when that evidence +changes. An unchanged retry keeps the full run returned by the first write and +still processes retained leases and recovery holds. Agent lists read the complete company roster once and use that same snapshot for termination filtering and organization-chain normalization. Detail and plugin runtime reads diff --git a/server/src/__tests__/legacy-workspace-restore-recovery.test.ts b/server/src/__tests__/legacy-workspace-restore-recovery.test.ts index 84da47b84b7..65e0e0cc147 100644 --- a/server/src/__tests__/legacy-workspace-restore-recovery.test.ts +++ b/server/src/__tests__/legacy-workspace-restore-recovery.test.ts @@ -73,6 +73,41 @@ const support = externalTestDatabaseUrl ? { supported: true } : await getEmbedde expect(JSON.stringify(events)).not.toContain(bulk.slice(0, 64)); }); + it.each(["legacy", "native"])("returns one complete run when retained workspace evidence is unchanged for %s", async (runtimeMode) => { + const f = await seed(); + const bulk = randomBytes(128 * 1024).toString("base64"); + const [saved] = await db.update(heartbeatRuns).set({ runtimeMode, status: "failed", finishedAt: new Date(), + contextSnapshot: { ...f.run.contextSnapshot, savedInput: bulk }, runnerProfileJson: { savedProfile: bulk }, + stdoutExcerpt: bulk, stderrExcerpt: bulk, + }).where(eq(heartbeatRuns.id, f.runId)).returning(); + const recordRestoreFailureOnly = runtimeMode === "native"; + const failed = (await terminalizeLegacyExecution({ db, run: saved!, status: "failed", recordRestoreFailureOnly, + workspaceRestoreSource: f.lease, + patch: { resultJson: { workspaceRestoreFailure: "restore_failed", futureResult: bulk } }, + }))!; + const events: Array> = [], now = Date.now(); + const capture = createDatabaseResponseCapture({ start: now - 100, end: now + 59_000 }, event => events.push(event)); + installDatabaseResponseCapture(db, { heartbeatRuns }, capture); + let returned: Awaited>; + try { returned = await terminalizeLegacyExecution({ db, run: failed, status: "failed", recordRestoreFailureOnly, + workspaceRestoreSource: f.lease, + ...(recordRestoreFailureOnly ? { patch: { resultJson: failed.resultJson } } : {}), + }); } + finally { capture.flush(); } + const writes = events.filter(event => event.event === "paperclip.db_response_capture.entry.v1" + && event.table === "heartbeat_runs" && event.operation === "update"); + expect(writes.reduce((sum, group) => sum + Number(group.rows), 0)).toBe(1); + expect(returned).toMatchObject({ id: f.runId, status: "failed", finishedAt: failed.finishedAt, + contextSnapshot: failed.contextSnapshot, runnerProfileJson: failed.runnerProfileJson, resultJson: failed.resultJson, + stdoutExcerpt: bulk, stderrExcerpt: bulk }); + if (!recordRestoreFailureOnly) expect(returned!.executionStatusDeliveryId).toBe(failed.executionStatusDeliveryId); + expect((await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, f.runId)))[0]).toEqual(returned); + expect(await readLease(f.lease.id)).toMatchObject({ status: "pending_cleanup", leasePolicy: "retain_on_failure" }); + expect(await db.select().from(issueRecoveryActions).where(eq(issueRecoveryActions.sourceIssueId, f.issueId))) + .toEqual([expect.objectContaining({ evidence: expect.objectContaining({ runId: f.runId }), ownerType: "board" })]); + expect(JSON.stringify(events)).not.toContain(bulk.slice(0, 64)); + }); + it("preserves pending and acknowledged status delivery on unpatched legacy recovery retries", async () => { await deliverExecutionStatuses(db, { publish: vi.fn() }); const f = await seed(); diff --git a/server/src/services/legacy-execution-recovery.ts b/server/src/services/legacy-execution-recovery.ts index 5d5fe783b3f..9a3041c88d8 100644 --- a/server/src/services/legacy-execution-recovery.ts +++ b/server/src/services/legacy-execution-recovery.ts @@ -156,11 +156,12 @@ export async function terminalizeLegacyExecution(input: { ...updated.resultJson, workspaceRestoreRecovery: { schema: LEGACY_WORKSPACE_RECOVERY_SCHEMA, leaseIds: [...retainedLeaseIds].sort() }, }; - [updated] = await tx.update(heartbeatRuns).set({ + const [retained] = await tx.update(heartbeatRuns).set({ resultJson: retainedResult, - executionStatusDeliveryId: sql`case when ${heartbeatRuns.resultJson} is distinct from ${JSON.stringify(retainedResult)}::jsonb - then ${randomUUID()}::uuid else ${heartbeatRuns.executionStatusDeliveryId} end`, - }).where(and(eq(heartbeatRuns.id, updated.id), eq(heartbeatRuns.companyId, updated.companyId))).returning(); + executionStatusDeliveryId: randomUUID(), + }).where(and(eq(heartbeatRuns.id, updated.id), eq(heartbeatRuns.companyId, updated.companyId), + sql`${heartbeatRuns.resultJson} is distinct from ${JSON.stringify(retainedResult)}::jsonb`)).returning(); + if (retained) updated = retained; } const needsRecovery = input.recordRestoreFailureOnly ? retainedLeaseIds.length > 0 || hasRequiredWorkspaceRecovery(updated.resultJson)