mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-01 15:35:12 +01:00
Scope status-card refresh tasks to their claim revision
Keep concurrent refreshes of one revision deduplicated while preventing a superseded refresh from cancelling a newer revision task before its claim. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
8179a607ea
commit
1d800abd49
3 files changed
+72
-3
No files matched your search
@@ -159,7 +159,7 @@ eviction, skipped/repeated hours, half-hour DST, date-line changes, rollback,
|
||||
strict boundaries, and actual twenty-card source delivery followed by quiet.
|
||||
|
||||
|
||||
Refresh decisions use the card's PostgreSQL row revision as a private compare-and-swap fence. If a card changes during watched-source reads, the owner rereads it and reconsiders the current policy, for at most three attempts. A replacement future interval deadline remains unchanged; a replacement manual policy does not run automatically. The same fence protects no-change, paused-policy and generation claims. Existing issue idempotency, generation ownership and losing-claim cancellation still apply. The row revision is not exposed through card APIs or shared scheduling contracts.
|
||||
Refresh decisions use the card's PostgreSQL row revision as a private compare-and-swap fence. If a card changes during watched-source reads, the owner rereads it and reconsiders the current policy, for at most three attempts. A replacement future interval deadline remains unchanged; a replacement manual policy does not run automatically. The same fence protects no-change, paused-policy and generation claims. Refresh task idempotency includes that captured revision: concurrent requests for the same revision still share one task, while a later revision gets an independent task even when its watched fingerprint is unchanged. A superseded refresh can therefore cancel its losing task without cancelling a newer revision's not-yet-claimed task. Compile task identity is unchanged. The row revision is not exposed through card APIs or shared scheduling contracts.
|
||||
|
||||
Company source hints do not preempt a future deadline owned by a failed evaluation (`retry`). Membership and policy deadlines remain eligible for source-driven rechecks, and actual due retries still run. An explicit policy edit can replace the retry through the existing card mutation path. Real PostgreSQL regressions hold the actual search while committing an ordinary policy edit, then verify the saved interval still fires. Persistent search failures stay quiet through issue hints and restart, retry at their saved deadline, and recover through an ordinary policy edit.
|
||||
|
||||
|
||||
@@ -2,7 +2,7 @@ import { resolveExternalTestDatabaseUrl } from "./helpers/external-test-database
|
||||
import { randomUUID } from "node:crypto";
|
||||
import express from "express";
|
||||
import request from "supertest";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
|
||||
import { eq } from "drizzle-orm";
|
||||
import {
|
||||
activityLog,
|
||||
@@ -638,6 +638,71 @@ describeEmbeddedPostgres("status card routes", () => {
|
||||
expect(refreshIssue).toMatchObject({ status: "cancelled" });
|
||||
});
|
||||
|
||||
it("keeps the current refresh writable after an older revision loses its claim", async () => {
|
||||
const company = await seedCompany();
|
||||
const summarizer = await seedSummarizer(company.id);
|
||||
const service = statusCardService(db);
|
||||
const realIssues = issueService(db);
|
||||
await realIssues.create(company.id, { title: "Watched blocker", status: "blocked", priority: "medium" });
|
||||
const card = await service.create(company.id, {
|
||||
interestPrompt: "Revision race", titlePinned: false,
|
||||
refreshPolicy: { ...defaultStatusCardRefreshPolicy, mode: "interval", intervalMinutes: 15 },
|
||||
}, { agentId: null, userId: "board-user" });
|
||||
await db.update(statusCards).set({
|
||||
state: "active", fingerprint: {},
|
||||
queries: [{ scope: "issues", status: ["blocked"], sort: "updated", limit: 20, offset: 0 }] as typeof card.queries,
|
||||
}).where(eq(statusCards.id, card.id));
|
||||
|
||||
const oldGate = Promise.withResolvers<void>();
|
||||
const currentGate = Promise.withResolvers<void>();
|
||||
let oldIssueId: string | undefined;
|
||||
let currentIssueId: string | undefined;
|
||||
const blockedIssues = (which: "old" | "current") => ({
|
||||
...realIssues,
|
||||
update: async (...args: Parameters<typeof realIssues.update>) => {
|
||||
const result = await realIssues.update(...args);
|
||||
if (args[1].description) {
|
||||
if (which === "old") { oldIssueId = args[0]; await oldGate.promise; }
|
||||
else { currentIssueId = args[0]; await currentGate.promise; }
|
||||
}
|
||||
return result;
|
||||
},
|
||||
});
|
||||
const oldRefresh = statusCardService(db, { issuesSvc: blockedIssues("old") })
|
||||
.requestRefresh(card.id, { trigger: "interval" });
|
||||
let currentRefresh: ReturnType<typeof service.requestRefresh> | undefined;
|
||||
try {
|
||||
await vi.waitFor(() => expect(oldIssueId).toBeTruthy());
|
||||
// Ordinary policy editing changes the card revision without changing the
|
||||
// watched fingerprint. Both refreshes have created their task before the
|
||||
// old loser cleans up; the current refresh has not yet claimed the card.
|
||||
await service.update((await service.getById(card.id))!, {
|
||||
refreshPolicy: defaultStatusCardRefreshPolicy,
|
||||
}, { agentId: null, userId: "board-user" });
|
||||
currentRefresh = statusCardService(db, { issuesSvc: blockedIssues("current") })
|
||||
.requestRefresh(card.id, { trigger: "manual" });
|
||||
await vi.waitFor(() => expect(currentIssueId).toBeTruthy());
|
||||
oldGate.resolve();
|
||||
await oldRefresh;
|
||||
currentGate.resolve();
|
||||
const current = await currentRefresh;
|
||||
const savedIssue = await realIssues.getById(current.generatingIssue!.id);
|
||||
expect(savedIssue).toMatchObject({ status: "todo" });
|
||||
expect(current.generatingIssue!.id).not.toBe(oldIssueId);
|
||||
const run = await seedRun(company.id, summarizer.id);
|
||||
await db.update(issues).set({ checkoutRunId: run.id }).where(eq(issues.id, savedIssue!.id));
|
||||
await service.writeSummary(card.id, {
|
||||
generationIssueId: savedIssue!.id, markdown: "Current revision summary",
|
||||
changeSummary: "Summarized the current revision", model: "test",
|
||||
}, { agentId: summarizer.id, runId: run.id });
|
||||
expect(await service.getById(card.id)).toMatchObject({ generatingIssueId: null });
|
||||
expect(await realIssues.getById(oldIssueId!)).toMatchObject({ status: "cancelled" });
|
||||
} finally {
|
||||
oldGate.resolve(); currentGate.resolve();
|
||||
await Promise.allSettled([oldRefresh, ...(currentRefresh ? [currentRefresh] : [])]);
|
||||
}
|
||||
});
|
||||
|
||||
it("finalizes cancelled generation tasks as failed ledger entries", async () => {
|
||||
const company = await seedCompany();
|
||||
await enableStatusCards();
|
||||
|
||||
@@ -753,7 +753,11 @@ export function statusCardService(
|
||||
createdByAgentId: input.actor?.agentId ?? null,
|
||||
createdByUserId: input.actor?.userId ?? null,
|
||||
hiddenAt: now,
|
||||
idempotencyKey: `status-card-update:${card.id}:${fingerprintHash}`,
|
||||
// Claim ownership is revision-scoped too. A superseded refresh may cancel
|
||||
// its unclaimed task; it must not share that task with a newer revision
|
||||
// which has the same watched fingerprint but has not claimed the card yet.
|
||||
// Concurrent refreshes of this revision still deduplicate normally.
|
||||
idempotencyKey: `status-card-update:${card.id}:${revision}:${fingerprintHash}`,
|
||||
onDeduplicated: (reason) => { deduplicated = reason === "idempotency_key"; },
|
||||
});
|
||||
const reopened = deduplicated && TERMINAL_ISSUE_STATUSES.has(created.status)
|
||||
|
||||
Reference in new issue
Block a user