mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-29 21:07:40 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A completed decision can owe a wake to the agent that proposed it. > - The decision row already stores the pending continuation. > - The main maintenance tick scans this debt even when no continuation is ready. > - Committed changes can activate recovery and leave settled history quiet. > - This pull request gives continuation delivery its own worker and shares foreground delivery ownership. > - Pending work stays recoverable without an unconditional continuation scan. ## Linked Issues or Issue Description Targets master after #17475 merged. This is slice 09 of the recurring server work migration. Each PR owns one reason for background work. The remaining inventory is in `doc/plans/2026-10-24-recurring-server-sleep-stack.md`. **What existing behavior does this improve?** Decision continuation delivery and recovery in the server. **Current behavior** Foreground paths send a wake and clear a pending flag. The combined decision sweep scans pending continuations on every maintenance tick. It can overlap foreground delivery. **Proposed behavior** Terminal decision writes notify a dedicated worker through the existing transaction boundary. Foreground and recovery calls share one in-flight delivery per decision and DB owner. Recovery re-reads readiness, pages past failures, and retries only unresolved debt or errors. **Reason and benefit** Open decisions, running effects, and delivered history no longer cause an independent continuation scan. No queue table is added. Decision expiry, target invalidation, interrupted effects, and retention remain separate duties. ## What Changed - Extract continuation readiness, delivery, and paged recovery into the decision owner. Preserve the wake payload and at-least-once crash behavior. - Signal the ten decision update sites before SQL in their existing transaction scope. Keep effects and wake calls outside those transactions. - Explicitly create the continuation obligation on all three archive-proposal failure settlements, including recovered running rows without inherited pending metadata. - Serialize concurrent decision effect execution through final settlement. Coalesce foreground and worker wakes. Re-read the durable flag before sending. Fence effect completion to running state so competing completion cannot restore acknowledged debt. - Expose an opaque DB work-scope key. Child pools and nested transactions share that identity; independent roots remain separate. Existing signal consumers keep their behavior. - Bound each recovery pass to 100 candidates, retain a fair cursor across failures, and stop between deliveries on shutdown. Forward the existing AbortSignal through the shared deadline adapter. - Register enabled-heartbeat continuation recovery before decision housekeeping. Remove only its independent scan from the main tick. Preserve the public combined sweep and disabled-heartbeat behavior. - Add generated migration `0388_ambitious_micromax.sql`, an idempotent concurrent partial index for pending continuations, and keep five generated snapshots. - Add real producer, transaction, failure, concurrency, paging, quiet, and shutdown tests. Document ownership and remaining limits in `doc/idle-decision-continuations.md`. ## Verification Current candidate: `18f682863cadb91281a86b4da2553a4579ee054a`, normally integrated with landed archive squash `5785090ff1`. All 31 independent incoming master files are preserved exactly, including controller-authority recovery. All 97 decision/startup/deadline cases plus server types/build pass after the required Runner vendor rebuild. The preceding full five-suite selection passed 293 cases; the continuation owner behavior is unchanged by master reconciliation. - **97 cases pass at this master integration** across decision, startup and scoped-deadline suites, plus server types/build. The preceding parent candidate passed the full 293-case decision/gateway/archive/startup/deadline selection; those results remain evidence for that revision. All selected cases executed with separate PostgreSQL fixtures where needed. - Tenant-driven global scan amplification reproduced six sweeps for six sequential real cancellations. Existing thirty-second notification coalescing now permits one recovery sweep per window while retaining pending work. The first green fixture wrongly expected the intent hold to clear before coalesced reconciliation; the hold remains and quiet is checked after settlement. Foreground delivery stays immediate. - The archive contention test uses a signaled dedicated single-connection read and two FIFO SQL round trips instead of a fixed delay. Removing effect coalescing makes it fail with two contender reads; restoring production code passes. No production test hook was added. - **Three new real PostgreSQL regressions failed before repair**: recovered archive proposals with invalid signatures, mismatched manifests or archive execution failures and no inherited pending flag. Each now persists failure, wakes once, acknowledges continuation delivery and remains quiet on replay. Running-state fencing is preserved. - The unmerged continuation index is regenerated as `0388_ambitious_micromax.sql` after parent migrations 386/387. All parent journal entries are unchanged, five generated snapshots remain, numbering/safety and snapshot/schema-drift checks pass. Fresh database migration apply and replay pass; the concurrent index is valid. - Existing lifecycle coverage includes foreground/worker coalescing, child pools, transaction commit/rollback, failed acknowledgement, restart, drain/resume, shutdown and fair 100-row recovery. Earlier bounded-pass and concurrent archive-result regressions and their repairs remain recorded in [the owning behavior audit](doc/idle-decision-continuations.md). - Failure accounting: the first combined command used an incorrect retention fixture variable and skipped 15 cases. The corrected separate external-PostgreSQL retention run passed all 15. The historical nine embedded-only work-signal cases were not executed by this selection. Extra upstream embedded-only suites attempted on the parent could not initialize locally; they remain CI verification, as recorded in the parent PR. Earlier reused-fixture, assertion and compile failures remain in the behavior audit and stack ledger; they are not erased by later passing checks. Commands: `pnpm --filter @paperclipai/server typecheck`, `pnpm --filter @paperclipai/server build`, server Vitest for `decisions-service`, `decision-retention-service`, `server-startup-feedback-export` and `scoped-deadline-work`; `pnpm --filter @paperclipai/db check:migrations`, migration apply/replay, and the generated snapshot-drift test. Local targeted evidence does not establish a full local repository run, deployed sleep or fleet savings. Fresh exact-head CI and both automated reviews remain merge gates. ## Risks - Notification-driven recovery remains global but is coalesced to thirty seconds. This bounds request-driven scans; it does not introduce company-scoped selection. Foreground delivery remains immediate. - Delivery remains at least once. A crash or acknowledgement failure after accepting a wake can send it again. Local coalescing is not a cross-process claim. - Supported writers share the application DB signal owner. Independent SQL writers or multiple active server processes need a separate wake contract. Wake-producing service methods do not gain a new guarantee when called inside an uncommitted caller transaction. - The concurrent index needs top-level migration execution; it is registered with the existing migration owner and can be replayed safely. - No Cloud sleep blocker or external alarm coverage changes. Worker quiet does not prove deployed app or database suspension. ## Model Used OpenAI Codex, GPT-6 family, with reasoning and code execution. The exact serving model identifier and context window were not exposed. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
210 lines
11 KiB
TypeScript
210 lines
11 KiB
TypeScript
import { sql } from "drizzle-orm";
|
|
import { drizzle } from "drizzle-orm/postgres-js";
|
|
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
|
|
import { createDb, withDedicatedDbConnection } from "./client.js";
|
|
import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./test-embedded-postgres.js";
|
|
import { databaseWorkScopeKey, databaseWorkPending, installDatabaseWorkSignals, reconcileDatabaseWork, signalDatabaseWork, subscribeDatabaseWork } from "./work-signals.js";
|
|
|
|
const support = await getEmbeddedPostgresTestSupport();
|
|
(support.supported ? describe : describe.skip)("database work signals", () => {
|
|
let db: ReturnType<typeof createDb>;
|
|
let temporary: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>>;
|
|
beforeAll(async () => {
|
|
temporary = await startEmbeddedPostgresTestDatabase("work-signals-");
|
|
db = createDb(temporary.connectionString);
|
|
await db.execute(sql`create table work_signal_test (id text primary key)`);
|
|
}, 90_000);
|
|
afterAll(async () => { await temporary?.cleanup(); });
|
|
|
|
it("defers nested savepoint notifications until the outer commit and preserves return values", async () => {
|
|
const events: string[] = [];
|
|
const unsubscribe = subscribeDatabaseWork(db, "nested", event => events.push(event));
|
|
try {
|
|
const result = await db.transaction(async tx => {
|
|
await tx.transaction(async nested => {
|
|
await signalDatabaseWork(nested, "nested");
|
|
await nested.execute(sql`insert into work_signal_test values ('nested')`);
|
|
});
|
|
expect(events).toEqual(["intent"]);
|
|
expect(databaseWorkPending(db, "nested")).toBe(true);
|
|
expect(await db.execute(sql`select * from work_signal_test where id = 'nested'`)).toHaveLength(0);
|
|
return 123;
|
|
});
|
|
expect(result).toBe(123);
|
|
expect(events).toEqual(["intent", "settled"]);
|
|
expect(databaseWorkPending(db, "nested")).toBe(false);
|
|
expect(await db.execute(sql`select * from work_signal_test where id = 'nested'`)).toHaveLength(1);
|
|
} finally { unsubscribe(); }
|
|
});
|
|
|
|
it("reconciles a rollback without leaving a permanent pending flag", async () => {
|
|
await expect(db.transaction(async tx => {
|
|
await signalDatabaseWork(tx, "rollback");
|
|
await tx.execute(sql`insert into work_signal_test values ('rollback')`);
|
|
throw new Error("abort");
|
|
})).rejects.toThrow("abort");
|
|
await reconcileDatabaseWork(db, "rollback");
|
|
expect(databaseWorkPending(db, "rollback")).toBe(false);
|
|
expect(await db.execute(sql`select * from work_signal_test where id = 'rollback'`)).toHaveLength(0);
|
|
});
|
|
|
|
it("coalesces scoped hints and publishes them only when the outer transaction settles", async () => {
|
|
const settled: string[][] = [];
|
|
const unsubscribe = subscribeDatabaseWork(db, "scoped", (event, hints) => {
|
|
if (event === "settled") settled.push([...(hints ?? [])]);
|
|
});
|
|
try {
|
|
await db.transaction(async tx => {
|
|
await signalDatabaseWork(tx, "scoped", "connection-a");
|
|
await tx.transaction(async nested => {
|
|
await signalDatabaseWork(nested, "scoped", "connection-b");
|
|
await signalDatabaseWork(nested, "scoped", "connection-a");
|
|
});
|
|
expect(settled).toEqual([]);
|
|
});
|
|
expect(settled).toEqual([["connection-a", "connection-b"]]);
|
|
} finally { unsubscribe(); }
|
|
});
|
|
|
|
it("keeps outer ownership after a caught savepoint rollback", async () => {
|
|
const events: string[] = [];
|
|
const unsubscribe = subscribeDatabaseWork(db, "savepoint", event => events.push(event));
|
|
try {
|
|
await db.transaction(async tx => {
|
|
await expect(tx.transaction(async nested => {
|
|
await signalDatabaseWork(nested, "savepoint");
|
|
await nested.execute(sql`insert into work_signal_test values ('savepoint')`);
|
|
throw new Error("rollback savepoint");
|
|
})).rejects.toThrow("rollback savepoint");
|
|
expect(databaseWorkPending(db, "savepoint")).toBe(true);
|
|
expect(events).toEqual(["intent"]);
|
|
await signalDatabaseWork(tx, "savepoint");
|
|
await tx.execute(sql`insert into work_signal_test values ('savepoint-retry')`);
|
|
});
|
|
expect(events).toEqual(["intent", "intent", "settled"]);
|
|
expect(databaseWorkPending(db, "savepoint")).toBe(false);
|
|
} finally { unsubscribe(); }
|
|
});
|
|
|
|
it.each(["commit", "abort"])("does not clear a late %s until PostgreSQL reports a settled transaction", async outcome => {
|
|
// Inject a lost response at the client boundary while a REAL transaction
|
|
// remains open on PostgreSQL. The status probe and row visibility are real.
|
|
const raw = drizzle(db.$client);
|
|
const transaction = raw.transaction.bind(raw);
|
|
let release!: () => void;
|
|
const gate = new Promise<void>(resolve => { release = resolve; });
|
|
let wrote!: () => void;
|
|
const written = new Promise<void>(resolve => { wrote = resolve; });
|
|
let serverDone!: Promise<unknown>;
|
|
raw.transaction = ((callback: any) => {
|
|
serverDone = transaction(async tx => {
|
|
const result = await callback(tx);
|
|
wrote();
|
|
await gate;
|
|
if (outcome === "abort") throw new Error("server abort");
|
|
return result;
|
|
});
|
|
// Observe rejection immediately; tests await the same result below.
|
|
void serverDone.catch(() => {});
|
|
return written.then(() => { throw new Error("lost transaction response"); });
|
|
}) as typeof raw.transaction;
|
|
const owner = installDatabaseWorkSignals(raw);
|
|
const topic = `late-${outcome}`;
|
|
const settled: string[][] = [];
|
|
const unsubscribe = subscribeDatabaseWork(owner, topic, (event, hints) => {
|
|
if (event === "settled") settled.push([...(hints ?? [])]);
|
|
});
|
|
try {
|
|
await expect(owner.transaction(async tx => {
|
|
await signalDatabaseWork(tx, topic, "connection-late");
|
|
await tx.execute(sql`insert into work_signal_test values (${topic})`);
|
|
})).rejects.toThrow("lost transaction response");
|
|
await reconcileDatabaseWork(owner, topic);
|
|
expect(databaseWorkPending(owner, topic)).toBe(true);
|
|
expect(await db.execute(sql`select * from work_signal_test where id = ${topic}`)).toHaveLength(0);
|
|
settled.length = 0;
|
|
release(); await serverDone.catch(() => {});
|
|
await reconcileDatabaseWork(owner, topic);
|
|
expect(settled).toEqual([["connection-late"]]);
|
|
expect(databaseWorkPending(owner, topic)).toBe(false);
|
|
expect(await db.execute(sql`select * from work_signal_test where id = ${topic}`)).toHaveLength(outcome === "commit" ? 1 : 0);
|
|
} finally { release(); await serverDone?.catch(() => {}); unsubscribe(); }
|
|
});
|
|
|
|
it("recovers after a real backend disconnect without replaying the queue write", async () => {
|
|
await expect(db.transaction(async tx => {
|
|
await signalDatabaseWork(tx, "disconnect");
|
|
await tx.execute(sql`insert into work_signal_test values ('disconnect')`);
|
|
const [{ pid }] = await tx.execute(sql`select pg_backend_pid() as pid`);
|
|
await db.execute(sql`select pg_terminate_backend(${pid as number})`);
|
|
await tx.execute(sql`select 1`);
|
|
})).rejects.toThrow();
|
|
await reconcileDatabaseWork(db, "disconnect");
|
|
expect(databaseWorkPending(db, "disconnect")).toBe(false);
|
|
expect(await db.execute(sql`select * from work_signal_test where id = 'disconnect'`)).toHaveLength(0);
|
|
});
|
|
|
|
it("shares notifications with dedicated child pools but isolates unrelated roots and topics", async () => {
|
|
const listener = vi.fn();
|
|
const other = createDb(temporary.connectionString);
|
|
const unsubscribe = subscribeDatabaseWork(db, "dedicated", listener);
|
|
try {
|
|
await other.transaction(tx => signalDatabaseWork(tx, "dedicated"));
|
|
await db.transaction(tx => signalDatabaseWork(tx, "other"));
|
|
expect(listener).not.toHaveBeenCalled();
|
|
await withDedicatedDbConnection(db, dedicated => dedicated.transaction(tx => signalDatabaseWork(tx, "dedicated")));
|
|
expect(listener.mock.calls.map(([event]) => event)).toEqual(["intent", "settled"]);
|
|
} finally { unsubscribe(); await other.$client.end(); }
|
|
});
|
|
|
|
it("never changes transaction success because a listener throws", async () => {
|
|
const warn = vi.spyOn(process, "emitWarning").mockImplementation(() => {});
|
|
const unsubscribe = subscribeDatabaseWork(db, "listener", () => { throw new Error("listener"); });
|
|
try {
|
|
await expect(db.transaction(async tx => { await signalDatabaseWork(tx, "listener"); return 7; })).resolves.toBe(7);
|
|
expect(warn).toHaveBeenCalledTimes(2);
|
|
} finally { unsubscribe(); warn.mockRestore(); }
|
|
});
|
|
});
|
|
|
|
describe("work signal query boundaries", () => {
|
|
it("adds no SQL to transactions which do not register queue work", async () => {
|
|
const execute = vi.fn();
|
|
const connection = installDatabaseWorkSignals({ execute, transaction: async (callback: any) => callback({ execute, transaction: async () => {} }) });
|
|
await expect(connection.transaction(async () => 19)).resolves.toBe(19);
|
|
expect(execute).not.toHaveBeenCalled();
|
|
});
|
|
it("fails before the write if XID acquisition fails, without retaining uncertainty", async () => {
|
|
const execute = vi.fn().mockRejectedValue(new Error("disconnected"));
|
|
const connection = installDatabaseWorkSignals({ execute, transaction: async (callback: any) => callback({ execute, transaction: async () => {} }) });
|
|
const write = vi.fn();
|
|
await expect(connection.transaction(async (tx: any) => { await signalDatabaseWork(tx, "failed"); write(); })).rejects.toThrow("disconnected");
|
|
expect(write).not.toHaveBeenCalled();
|
|
expect(databaseWorkPending(connection, "failed")).toBe(false);
|
|
});
|
|
it("rejects registration on an autocommit root", async () => {
|
|
const connection = installDatabaseWorkSignals({ execute: vi.fn(), transaction: vi.fn() });
|
|
await expect(signalDatabaseWork(connection, "bad")).rejects.toThrow("inside a transaction");
|
|
});
|
|
});
|
|
|
|
|
|
describe("database work scope identity", () => {
|
|
type TestConnection = { execute: () => Promise<unknown[]>; transaction: (callback: (tx: TestConnection) => unknown) => Promise<unknown> };
|
|
function connection(): TestConnection {
|
|
return { execute: vi.fn(async () => []), transaction: async callback => callback(connection()) };
|
|
}
|
|
it("shares owner identity through child pools and nested transactions, but isolates independent owners", async () => {
|
|
const first = installDatabaseWorkSignals(connection());
|
|
const second = installDatabaseWorkSignals(connection());
|
|
const child = installDatabaseWorkSignals(connection(), first);
|
|
const key = databaseWorkScopeKey(first);
|
|
expect(databaseWorkScopeKey(second)).not.toBe(key);
|
|
expect(databaseWorkScopeKey(child)).toBe(key);
|
|
await child.transaction(async tx => {
|
|
expect(databaseWorkScopeKey(tx)).toBe(key);
|
|
await tx.transaction(async nested => { expect(databaseWorkScopeKey(nested)).toBe(key); });
|
|
});
|
|
});
|
|
});
|