From cebcd9c427e58e0d6731102945b60603860f22dc Mon Sep 17 00:00:00 2001 From: Dotta Date: Sat, 24 Oct 2026 15:24:01 -0700 Subject: [PATCH] fix(server): retain database precision in effect recovery cursor Co-Authored-By: Paperclip --- doc/idle-decision-effect-recovery.md | 9 +++++++++ .../decision-effect-recovery-work.test.ts | 18 ++++++++++++++++++ .../src/services/decision-effect-recovery.ts | 6 +++--- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/doc/idle-decision-effect-recovery.md b/doc/idle-decision-effect-recovery.md index c710597f876..dfdcfeb3fd2 100644 --- a/doc/idle-decision-effect-recovery.md +++ b/doc/idle-decision-effect-recovery.md @@ -95,3 +95,12 @@ and earliest-deadline lookup use the partial index and read two buffers (0.012 m and 0.003 ms in the local probe). The probe transaction is rolled back. Full PR CI and review must pass separately before this slice is ready. + +A follow-up regression seeds a legacy invalid claim with a PostgreSQL-generated +microsecond timestamp ahead of a valid claim. With a one-row page, the original +Date cursor selected the invalid claim again and starved the valid effect. That +regression failed before repair. Recovery now retains the timestamp as database +text for cursor comparisons, leaving business grace deadlines unchanged. The +invalid claim remains durable debt and is revisited after paging wraps; the valid +claim progresses without repeating its completed effect. +All 18 effect-recovery lifecycle cases and server typechecking pass after repair. diff --git a/server/src/__tests__/decision-effect-recovery-work.test.ts b/server/src/__tests__/decision-effect-recovery-work.test.ts index bb2c0b6ac5a..058780b4bf3 100644 --- a/server/src/__tests__/decision-effect-recovery-work.test.ts +++ b/server/src/__tests__/decision-effect-recovery-work.test.ts @@ -223,6 +223,24 @@ const support = externalUrl ? { supported: true } : await getEmbeddedPostgresTes expect(await recovery.nextEffectRecoveryAt()).not.toBeNull(); }); + it("pages past invalid claims using database timestamp precision", async () => { + vi.stubEnv("PAPERCLIP_DECISIONS_SWEEP_BATCH_SIZE", "1"); + const invalid = await create(), valid = await create(); + await claim(invalid.id); await claim(valid.id, new Date(Date.now() - 120_000)); + await db.execute(sql`update decisions set decided_by_user_id = null, + updated_at = date_trunc('milliseconds', now() - interval '3 minutes') + interval '456 microseconds' + where id = ${invalid.id}`); + const recovery = service(); + expect(await recovery.recoverEffects()).toEqual({ resumed: 0 }); + expect(await recovery.recoverEffects()).toEqual({ resumed: 1 }); + expect(await comments()).toHaveLength(1); + expect((await recovery.get(invalid.id))?.executionStatus).toBe("running"); + expect(await recovery.nextEffectRecoveryAt()).not.toBeNull(); + expect(await recovery.recoverEffects()).toEqual({ resumed: 0 }); + expect(await recovery.recoverEffects()).toEqual({ resumed: 0 }); + expect(await comments()).toHaveLength(1); + }); + it("advances past malformed claims and retries them on a later pass", async () => { vi.stubEnv("PAPERCLIP_DECISIONS_SWEEP_BATCH_SIZE", "1"); const invalid = await create(), valid = await create(); diff --git a/server/src/services/decision-effect-recovery.ts b/server/src/services/decision-effect-recovery.ts index 089ac62bc53..62cc8dcf3c0 100644 --- a/server/src/services/decision-effect-recovery.ts +++ b/server/src/services/decision-effect-recovery.ts @@ -14,14 +14,14 @@ function recoveryGraceMs() { } export function decisionEffectRecoveryService(db: Db, resume: (id: string) => Promise) { - let cursor: { updatedAt: Date; id: string } | undefined; + let cursor: { updatedAt: string; id: string } | undefined; async function sweep(now = new Date(), limit = decisionSweepBatchSize(), signal?: AbortSignal) { let resumed = 0; if (signal?.aborted) return { resumed }; - const rows = await db.select({ id: decisions.id, updatedAt: decisions.updatedAt }).from(decisions) + const rows = await db.select({ id: decisions.id, updatedAt: sql`${decisions.updatedAt}::text` }).from(decisions) .where(and(running, lte(decisions.updatedAt, new Date(now.getTime() - recoveryGraceMs())), cursor - ? sql`(${decisions.updatedAt}, ${decisions.id}) > (${cursor.updatedAt.toISOString()}::timestamptz, ${cursor.id}::uuid)` : undefined)) + ? sql`(${decisions.updatedAt}, ${decisions.id}) > (${cursor.updatedAt}::timestamptz, ${cursor.id}::uuid)` : undefined)) .orderBy(asc(decisions.updatedAt), asc(decisions.id)).limit(limit); const errors: unknown[] = []; for (const row of rows) {