From a29ce265b0c00678ada3f29f533d79f80fe25654 Mon Sep 17 00:00:00 2001 From: Dotta Date: Sat, 24 Oct 2026 14:53:21 -0700 Subject: [PATCH] fix(server): preserve database precision in retention recovery cursors Co-Authored-By: Paperclip --- doc/idle-approval-retention.md | 4 ++- ...2026-10-24-recurring-server-sleep-stack.md | 35 +++++++++++++++++++ .../__tests__/approval-retention-work.test.ts | 25 +++++++++++-- server/src/services/approval-retention.ts | 8 ++--- 4 files changed, 64 insertions(+), 8 deletions(-) diff --git a/doc/idle-approval-retention.md b/doc/idle-approval-retention.md index 2c2536d57d3..7c79e02bcc1 100644 --- a/doc/idle-approval-retention.md +++ b/doc/idle-approval-retention.md @@ -22,7 +22,9 @@ agent-conversation tasks remain excluded. A waiting external conversation with a linked pending approval is active under that same shared visibility policy; its chat state alone cannot hide this approval. Tests exercise the Slack case. -Each pass visits at most 100 candidates in activity/id order. A failed row does +Each pass visits at most 100 candidates in activity/id order. The cursor keeps +the database timestamp as text so sub-millisecond precision cannot repeat a +failed page. A failed row does not prevent later pages from progressing. The failed durable record remains eligible for recovery. Each candidate is re-read and locked before archiving, so a resubmission committed while the worker waited cannot archive the new diff --git a/doc/plans/2026-10-24-recurring-server-sleep-stack.md b/doc/plans/2026-10-24-recurring-server-sleep-stack.md index 22724b067c2..dacc8cc4e19 100644 --- a/doc/plans/2026-10-24-recurring-server-sleep-stack.md +++ b/doc/plans/2026-10-24-recurring-server-sleep-stack.md @@ -794,3 +794,38 @@ budget case), and 201 tool-gateway cases. Workspace typecheck, production build, migration numbering/safety, fresh replay and idempotent index replay pass. The implementation is committed as `ea3dd40cdd`; remote publication, CI and review remain required. The earlier failures remain recorded above. + + +## Slice 14 publication and precision review repair + +Approval retention is [#17497](https://github.com/paperclipai/paperclip/pull/17497), +initial head `2f334c678b`, based on #17490. It is attached. Initial CI run +40135489716 completed without a failed CI job. Greptile gave 4/5 with one P2: +JavaScript Date truncation can repeat a full failed page whose source timestamp +contains microseconds (comment 4368633789, thread PRRT_kwDORclPyM6v79u7). + +A database-generated timestamp regression failed before repair: all 100 failed +rows repeated and blocked row 101. The cursor now preserves the database text +timestamp. All 21 lifecycle cases pass afterward, and server typecheck passes. +During the full regression run, the earlier quiet-window test crossed its real +deadline after settlement. It now measures an explicit window before that +deadline; it still asserts no SQL and eventual archival. Total selected coverage +is now 318 unique tests. Fresh remote checks/review are required after this fix. + +Related precision audit is required before final stack acceptance: the earlier +TTL worker (#17479) and effect-recovery worker (#17482) retain Date cursors too. +Add database-precision regressions and repair those owning PRs, then propagate +parent updates through the stack. The already-merged terminal-workspace sweep +also uses a Date keyset cursor; verify and address it as a focused follow-up if +confirmed. These are concrete remaining verification duties, not waived by the +earlier green checks. + +Branch `codex/sleep-15-join-retention` exists at the initial slice-14 head, with +no implementation yet. Its producer audit found request writes in access routes +(accept, replay, auto-approve, approve, reject, key claim), Slack identity access +creation, and company deletion. Eligibility is pending_approval plus matching +invite/company and active company. Invite expiry/revocation do not remove the +existing feed item. Preserve that policy. Retention controls and company status +changes must notify this owner. Decision retention remains separately pending; +its visibility dependencies differ, and normal decisions already have a +maximum 30-day TTL before the 90-day archive threshold. diff --git a/server/src/__tests__/approval-retention-work.test.ts b/server/src/__tests__/approval-retention-work.test.ts index 239591587d2..d2725689139 100644 --- a/server/src/__tests__/approval-retention-work.test.ts +++ b/server/src/__tests__/approval-retention-work.test.ts @@ -86,15 +86,17 @@ const delay = DEFAULT_DECISION_ARCHIVE_DAYS * 86_400_000; it("is quiet with no approvals and archives unseen durable approvals at their actual deadline", async () => { await start(); await quiet(); - const deadline = Date.now() + 120_000; + const deadline = Date.now() + 600_000; const row = await create(new Date(deadline - delay)); await settle(); expect(await state(row.id)).toBeNull(); expect(coordinator!.nextWakeAt()).toBe(deadline); + const quietWindow = deadline - Date.now() - 30_000; + expect(quietWindow).toBeGreaterThan(0); const reads = vi.spyOn(db, "select"); - await vi.advanceTimersByTimeAsync(30_000); + await vi.advanceTimersByTimeAsync(quietWindow); expect(reads).not.toHaveBeenCalled(); reads.mockRestore(); - await vi.advanceTimersByTimeAsync(120_000); + await vi.advanceTimersByTimeAsync(60_000); await archived(row.id); await quiet(); expect((await db.select().from(approvals).where(eq(approvals.id, row.id)))[0].status).toBe("pending"); }); @@ -322,4 +324,21 @@ const delay = DEFAULT_DECISION_ARCHIVE_DAYS * 86_400_000; ]); }); + + it("advances past a failed page with full database timestamp precision", async () => { + await db.execute(sql`insert into approvals(company_id, type, payload, updated_at) + select ${companyId}, 'request_board_approval', jsonb_build_object('retentionTestFailure', n <= 100), + date_trunc('milliseconds', now() - interval '91 days') + interval '456 microseconds' + + case when n <= 100 then interval '0 seconds' else interval '1 second' end + from generate_series(1, 101) n`); + await failArchives(); + const service = approvalRetentionService(db); + await expect(service.sweep()).rejects.toThrow("Approval retention failed"); + expect(await service.sweep()).toEqual({ archived: 1 }); + await db.execute(sql`drop trigger approval_retention_fail on decision_retention`); + expect(await service.sweep()).toEqual({ archived: 100 }); + expect(await service.sweep()).toEqual({ archived: 0 }); + expect(await service.next()).toBeNull(); + }); + }); diff --git a/server/src/services/approval-retention.ts b/server/src/services/approval-retention.ts index 19bf334bd6d..32203fd7c24 100644 --- a/server/src/services/approval-retention.ts +++ b/server/src/services/approval-retention.ts @@ -18,9 +18,9 @@ const eligibleLink = sql`not exists ( )`; export function approvalRetentionService(db: Db) { - let cursor: { at: Date; id: string } | undefined; + let cursor: { at: string; id: string } | undefined; function candidates(database: Db, condition?: SQL) { - return database.select({ id: approvals.id, companyId: approvals.companyId, at: approvals.updatedAt }).from(approvals) + return database.select({ id: approvals.id, companyId: approvals.companyId, at: approvals.updatedAt, cursorAt: sql`${approvals.updatedAt}::text` }).from(approvals) .innerJoin(companies, eq(companies.id, approvals.companyId)) .leftJoin(decisionRetention, and(eq(decisionRetention.companyId, approvals.companyId), eq(decisionRetention.sourceKind, "approval"), sql`${decisionRetention.sourceId} = ${approvals.id}::text`)) @@ -31,7 +31,7 @@ export function approvalRetentionService(db: Db) { async function sweep(now = new Date(), signal?: AbortSignal) { const cutoff = new Date(now.getTime() - archiveDelay); const rows = await candidates(db, and(lte(approvals.updatedAt, cutoff), cursor - ? sql`(${approvals.updatedAt}, ${approvals.id}) > (${cursor.at.toISOString()}::timestamptz, ${cursor.id}::uuid)` : undefined)).limit(100); + ? sql`(${approvals.updatedAt}, ${approvals.id}) > (${cursor.at}::timestamptz, ${cursor.id}::uuid)` : undefined)).limit(100); let archived = 0; const errors: unknown[] = []; for (const candidate of rows) { @@ -51,7 +51,7 @@ export function approvalRetentionService(db: Db) { }] }); }); } catch (error) { errors.push(error); } - cursor = { at: candidate.at, id: candidate.id }; + cursor = { at: candidate.cursorAt, id: candidate.id }; } if (rows.length < 100) cursor = undefined; if (errors.length === 1) throw errors[0];