mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-29 21:07:40 +01:00
fix(server): preserve database precision in retention recovery cursors
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
2f334c678b
commit
a29ce265b0
4 files changed
+64
-8
No files matched your search
@@ -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
|
||||
|
||||
@@ -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.
|
||||
@@ -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();
|
||||
});
|
||||
|
||||
});
|
||||
@@ -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<string>`${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];
|
||||
|
||||
Reference in new issue
Block a user