mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-01 15:35:12 +01:00
Preserve completed shared status-card refresh generations
Guard stale cleanup with the durable generation ledger and a transaction-bound canonical cancellation. Coalesce the original completed manual request, retain future manual work, and preserve one-connection and outer-commit effects. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
304e8dc62f
commit
9a9668425b
3 files changed
+177
-6
No files matched your search
@@ -163,5 +163,18 @@ Refresh decisions use the card's PostgreSQL row revision as a private compare-an
|
||||
|
||||
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.
|
||||
|
||||
A losing refresh cancels only an unclaimed task. It locks the exact company task
|
||||
in the canonical issue-first order, checks its generation ledger, and preserves
|
||||
terminal state and timestamps. Successful summary writeback records that ledger
|
||||
in the same transaction that clears the card's generation pointer. Because the
|
||||
failed card revision CAS excludes a future claim of this revision's task, the
|
||||
loser cannot mistake a completed shared winner for an unclaimed orphan. The same
|
||||
original request returns without enqueueing or reporting an active generation;
|
||||
a separate later manual request can still start a new revision. Canonical issue
|
||||
cancellation uses a transaction-bound service so its preflight reads also work
|
||||
with one connection. Activity publications and native-question actions run only
|
||||
after the outer transaction commits. Archive and truly unclaimed-task cleanup
|
||||
retain their existing effects.
|
||||
|
||||
|
||||
Every reactive summary writeback records one immediate card-owned refresh deadline in the same transaction that clears its generation. This preserves changes whose committed source hint was consumed while the old generation still owned the card, including changes after the summary's final watched-source read. The existing card commit hint restores that one deadline; it does not fan out to other companies or force generation. A fresh unchanged search disarms it. A relevant diff follows the existing policy, assignment, result and quiet path. Manual and interval writeback cadence stays unchanged. Rollback removes both the summary and its recheck obligation. Real PostgreSQL controls hold the actual summary search, commit an ordinary issue write, observe the worker consume that hint while generation is still owned, then verify exactly one needed generation or one unchanged recheck followed by SQL quiet.
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
companyMemberships,
|
||||
costEvents,
|
||||
createDb,
|
||||
databaseWorkPending,
|
||||
documentRevisions,
|
||||
documents,
|
||||
heartbeatRuns,
|
||||
@@ -19,6 +20,7 @@ import {
|
||||
issues,
|
||||
statusCards,
|
||||
statusCardUpdates,
|
||||
subscribeDatabaseWork,
|
||||
} from "@paperclipai/db";
|
||||
import {
|
||||
defaultStatusCardRefreshPolicy,
|
||||
@@ -33,6 +35,7 @@ import { instanceSettingsService } from "../services/instance-settings.js";
|
||||
import type { IssueAssignmentWakeupDeps } from "../services/issue-assignment-wakeup.js";
|
||||
import { issueService } from "../services/issues.js";
|
||||
import { statusCardService } from "../services/status-cards.js";
|
||||
import { DELIVERY_QUEUES } from "../services/delivery-work-notifications.js";
|
||||
import {
|
||||
getEmbeddedPostgresTestSupport,
|
||||
startEmbeddedPostgresTestDatabase,
|
||||
@@ -703,6 +706,132 @@ describeEmbeddedPostgres("status card routes", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it.each([
|
||||
...(["creator", "deduplicated"] as const).flatMap(winnerKind =>
|
||||
(["active", "done", "cancelled"] as const).map(outcome => ({ winnerKind, outcome, requestTrigger: "interval" as const }))),
|
||||
...(["creator", "deduplicated"] as const).map(winnerKind => ({ winnerKind, outcome: "active" as const, requestTrigger: "manual" as const })),
|
||||
])("preserves a same-revision $winnerKind winner after summary and $outcome via $requestTrigger when its loser resumes", async ({ winnerKind, outcome, requestTrigger }) => {
|
||||
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: "Completed winner 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 gates = [Promise.withResolvers<void>(), Promise.withResolvers<void>()];
|
||||
const reached: string[] = [];
|
||||
const start = (index: number) => statusCardService(db, { issuesSvc: {
|
||||
...realIssues,
|
||||
update: async (...args: Parameters<typeof realIssues.update>) => {
|
||||
const result = await realIssues.update(...args);
|
||||
if (args[1].description) { reached[index] = args[0]; await gates[index]!.promise; }
|
||||
return result;
|
||||
},
|
||||
} }).requestRefresh(card.id, { trigger: requestTrigger });
|
||||
const creator = start(0);
|
||||
let deduplicated: ReturnType<typeof start> | undefined;
|
||||
try {
|
||||
await vi.waitFor(() => expect(reached[0]).toBeTruthy());
|
||||
deduplicated = start(1);
|
||||
await vi.waitFor(() => expect(reached[1]).toBe(reached[0]));
|
||||
const winnerIndex = winnerKind === "creator" ? 0 : 1;
|
||||
gates[winnerIndex]!.resolve();
|
||||
const winner = await (winnerIndex === 0 ? creator : deduplicated);
|
||||
expect(winner.enqueued).toBe(true);
|
||||
expect(winner.alreadyGenerating).toBe(winnerKind === "deduplicated");
|
||||
const run = await seedRun(company.id, summarizer.id);
|
||||
await db.update(issues).set({ checkoutRunId: run.id }).where(eq(issues.id, reached[0]!));
|
||||
await service.writeSummary(card.id, {
|
||||
generationIssueId: reached[0]!, markdown: "Completed winner summary",
|
||||
changeSummary: "Summarized watched work", model: "test",
|
||||
}, { agentId: summarizer.id, runId: run.id });
|
||||
if (outcome !== "active") await realIssues.update(reached[0]!, { status: outcome });
|
||||
expect(await service.getById(card.id)).toMatchObject({ generatingIssueId: null });
|
||||
const completed = await realIssues.getById(reached[0]!);
|
||||
expect(completed).toMatchObject({ status: outcome === "active" ? "todo" : outcome });
|
||||
|
||||
gates[1 - winnerIndex]!.resolve();
|
||||
const loser = await (winnerIndex === 0 ? deduplicated : creator);
|
||||
expect(loser).toMatchObject({ enqueued: false, alreadyGenerating: false, generatingIssue: null });
|
||||
expect(await realIssues.getById(reached[0]!)).toMatchObject({
|
||||
status: completed!.status, completedAt: completed!.completedAt, cancelledAt: completed!.cancelledAt,
|
||||
});
|
||||
const history = await service.listUpdates(card.id);
|
||||
expect(history).toHaveLength(1);
|
||||
expect(history[0]).toMatchObject({ generationIssueId: reached[0], status: "ok", finishedAt: expect.any(Date) });
|
||||
expect(await service.getById(card.id)).toMatchObject({ generatingIssueId: null, state: "active" });
|
||||
if (requestTrigger === "manual") {
|
||||
const independent = await service.requestRefresh(card.id, { trigger: "manual" });
|
||||
expect(independent.enqueued).toBe(true);
|
||||
expect(independent.generatingIssue!.id).not.toBe(completed!.id);
|
||||
}
|
||||
} finally {
|
||||
gates.forEach(gate => gate.resolve());
|
||||
await Promise.allSettled([creator, ...(deduplicated ? [deduplicated] : [])]);
|
||||
}
|
||||
});
|
||||
|
||||
it("cancels an unclaimed archived refresh with one database connection", async () => {
|
||||
const company = await seedCompany();
|
||||
await seedSummarizer(company.id);
|
||||
const single = createDb(externalDatabaseUrl ?? tempDb!.connectionString, { maxConnections: 1 });
|
||||
const service = statusCardService(single);
|
||||
const realIssues = issueService(single);
|
||||
let pending: Promise<unknown> | undefined;
|
||||
let unsubscribe: (() => void) | undefined;
|
||||
const signals: Array<{ event: string; pending: boolean; hints: string[] }> = [];
|
||||
try {
|
||||
const card = await service.create(company.id, {
|
||||
interestPrompt: "One connection cleanup", titlePinned: false, refreshPolicy: defaultStatusCardRefreshPolicy,
|
||||
}, { agentId: null, userId: "board-user" });
|
||||
await single.update(statusCards).set({ state: "active",
|
||||
queries: [{ scope: "issues", status: ["blocked"], sort: "updated", limit: 20, offset: 0 }] as typeof card.queries,
|
||||
}).where(eq(statusCards.id, card.id));
|
||||
const racing = statusCardService(single, { issuesSvc: {
|
||||
...realIssues,
|
||||
update: async (...args: Parameters<typeof realIssues.update>) => {
|
||||
const result = await realIssues.update(...args);
|
||||
if (args[1].description) {
|
||||
await service.update((await service.getById(card.id))!, {
|
||||
archived: true,
|
||||
}, { agentId: null, userId: "board-user" });
|
||||
// Observe only the losing task's canonical cancellation, after the
|
||||
// independent archive transaction has settled.
|
||||
unsubscribe = subscribeDatabaseWork(single, DELIVERY_QUEUES.statusCardEvaluation, (event, hints) => {
|
||||
signals.push({ event, pending: databaseWorkPending(single, DELIVERY_QUEUES.statusCardEvaluation), hints: [...hints ?? []] });
|
||||
});
|
||||
}
|
||||
return result;
|
||||
},
|
||||
} });
|
||||
let settled = false;
|
||||
let rejection: unknown;
|
||||
pending = racing.requestRefresh(card.id).then(() => { settled = true; }, error => {
|
||||
rejection = error; settled = true;
|
||||
});
|
||||
await vi.waitFor(() => expect(settled).toBe(true), { timeout: 2_000 });
|
||||
expect(rejection).toMatchObject({ status: 409 });
|
||||
expect(await single.select().from(issues).where(eq(issues.companyId, company.id))).toEqual([
|
||||
expect.objectContaining({ status: "cancelled" }),
|
||||
]);
|
||||
expect(await service.listUpdates(card.id)).toHaveLength(0);
|
||||
const cancellation = signals.filter(signal => signal.hints.includes(`changes:${company.id}`));
|
||||
expect(cancellation).toContainEqual(expect.objectContaining({ event: "intent", pending: true }));
|
||||
expect(cancellation.filter(signal => signal.event === "settled")).toEqual([
|
||||
expect.objectContaining({ event: "settled", pending: false }),
|
||||
]);
|
||||
} finally {
|
||||
unsubscribe?.();
|
||||
await single.$client.end({ timeout: 1 });
|
||||
await pending;
|
||||
}
|
||||
});
|
||||
|
||||
it("finalizes cancelled generation tasks as failed ledger entries", async () => {
|
||||
const company = await seedCompany();
|
||||
await enableStatusCards();
|
||||
|
||||
@@ -27,7 +27,8 @@ import { logger } from "../middleware/logger.js";
|
||||
import { readBuiltInAgentMarker } from "../lib/built-in-agent-metadata.js";
|
||||
import { builtInAgentService } from "./built-in-agents.js";
|
||||
import { companySearchService } from "./company-search.js";
|
||||
import { issueService } from "./issues.js";
|
||||
import { executeIssuePostCommitActions, issueService, type IssuePostCommitAction } from "./issues.js";
|
||||
import { publishActivity, type ActivityPublication } from "./activity-log.js";
|
||||
import { SUMMARIZER_BUILT_IN_KEY } from "./summary-slots.js";
|
||||
import {
|
||||
buildStatusCardFingerprint,
|
||||
@@ -643,6 +644,33 @@ export function statusCardService(
|
||||
? membershipAt : interval;
|
||||
}
|
||||
|
||||
async function cancelUnclaimedRefreshTask(card: StatusCardRow, issueId: string) {
|
||||
const postCommitActions: IssuePostCommitAction[] = [];
|
||||
const publications: ActivityPublication[] = [];
|
||||
const admitted = await db.transaction(async tx => {
|
||||
// Match the canonical mutation's issue-first lock order. A terminal
|
||||
// transition cannot race this recheck and have its receipt rewritten.
|
||||
const [current] = await tx.select({ status: issues.status }).from(issues)
|
||||
.where(and(eq(issues.id, issueId), eq(issues.companyId, card.companyId))).for("update");
|
||||
if (!current) return false;
|
||||
const [generation] = await tx.select({ id: statusCardUpdates.id }).from(statusCardUpdates)
|
||||
.where(and(eq(statusCardUpdates.cardId, card.id), eq(statusCardUpdates.generationIssueId, issueId))).limit(1);
|
||||
// A failed revision CAS excludes any later claim of this revision's task.
|
||||
// A summary clears the claim and records its ledger atomically, so even
|
||||
// an active task whose winner already finished remains admitted work.
|
||||
if (generation) return true;
|
||||
if (TERMINAL_ISSUE_STATUSES.has(current.status)) return false;
|
||||
// Bind preflight readers to this transaction too: a root-pool reader
|
||||
// would wait forever when this transaction owns its only connection.
|
||||
await issueService(tx as unknown as Db).update(issueId,
|
||||
{ status: "cancelled", companyGuard: card.companyId }, tx, publications, postCommitActions);
|
||||
return false;
|
||||
});
|
||||
for (const publication of publications) publishActivity(publication);
|
||||
await executeIssuePostCommitActions(db, postCommitActions);
|
||||
return admitted;
|
||||
}
|
||||
|
||||
async function refreshCurrentCard(cardId: string, input: {
|
||||
full?: boolean;
|
||||
trigger?: "manual" | "interval" | "reactive" | "restore";
|
||||
@@ -782,14 +810,15 @@ export function statusCardService(
|
||||
if (!next) {
|
||||
const winner = await getById(card.id);
|
||||
if (!winner?.generatingIssueId) {
|
||||
if (!TERMINAL_ISSUE_STATUSES.has(generationIssue!.status)) {
|
||||
await issuesSvc().update(generationIssue!.id, { status: "cancelled" });
|
||||
const admitted = await cancelUnclaimedRefreshTask(card, generationIssue!.id);
|
||||
if (winner && !winner.archivedAt) {
|
||||
if (admitted) return { card: winner, generatingIssue: null, alreadyGenerating: false, enqueued: false, kind, changes };
|
||||
return undefined;
|
||||
}
|
||||
if (winner && !winner.archivedAt) return undefined;
|
||||
throw conflict("Status-card refresh claim was lost");
|
||||
}
|
||||
if (generationIssue!.id !== winner.generatingIssueId && !TERMINAL_ISSUE_STATUSES.has(generationIssue!.status)) {
|
||||
await issuesSvc().update(generationIssue!.id, { status: "cancelled" });
|
||||
if (generationIssue!.id !== winner.generatingIssueId) {
|
||||
await cancelUnclaimedRefreshTask(card, generationIssue!.id);
|
||||
}
|
||||
const winnerIssue = await db.select().from(issues).where(eq(issues.id, winner.generatingIssueId)).then((rows) => rows[0] ?? null);
|
||||
return { card: winner, generatingIssue: winnerIssue, alreadyGenerating: true, enqueued: false, kind, changes };
|
||||
|
||||
Reference in new issue
Block a user