mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-29 21:07:40 +01:00
fix(server): retain full target scans with scoped transaction hints
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
5062efb368
commit
1bbfc69590
5 files changed
+56
-4
No files matched your search
@@ -107,3 +107,18 @@ A local query probe with 40,000 completed decisions, one open decision, and 40,0
|
||||
target links used existing indexes: scoped lookup read five buffers (0.012 ms),
|
||||
full open-page lookup two (0.003 ms). The probe was rolled back. No new schema
|
||||
index is needed for this change. Full PR CI and fresh review remain required.
|
||||
|
||||
## Review and CI repair
|
||||
|
||||
Review found that an absent deletion hint was dropped when the outer transaction
|
||||
also carried a scoped hint. Both delete-before-update and update-before-delete
|
||||
regressions reproduced the missing expiry after the target links cascaded away.
|
||||
Deletion now sends the explicit `all` hint, which survives alongside scoped hints.
|
||||
The repaired target suite passes 20 cases; the TTL suite passes 14 cases.
|
||||
|
||||
The initial CI run failed the TTL worker's drain assertion. It measured SQL
|
||||
before already-admitted sweeps had finished. The test now joins those specific
|
||||
sweep promises before measuring passive SQL; unresolved-intent holds remain
|
||||
intact. Waiting for the entire idle count to reach zero was tested and rejected:
|
||||
that count correctly includes the retained recovery holds. No production drain
|
||||
or retry behavior changed for this test repair.
|
||||
@@ -602,3 +602,22 @@ run hit a PostgreSQL lock timeout, and the following test saw the experimental
|
||||
flag left enabled. No CI retry has been used. Reproduce the two tests on the
|
||||
exact parent head before classifying a retry or repair. This parent is not yet
|
||||
claimed green. The full original inventory remains in scope.
|
||||
|
||||
Slice 11 #17482 now has all checks passing at `a5b5994c1d`, Greptile 5/5,
|
||||
no unresolved findings, and no merge conflict. Its one browser-shard retry passed
|
||||
after both affected cases passed locally on the exact head (2/2). The original
|
||||
fixture-cleanup lock timeout remains documented in the PR.
|
||||
|
||||
Slice 12 is #17486 at initial head `5062efb368`. Review found that deletion's
|
||||
absent hint could be dropped when the same outer transaction included a scoped
|
||||
hint. Both operation orders failed new regressions before the explicit `all`
|
||||
hint repair. The repaired target/TTL selection passes 34 cases. CI also exposed
|
||||
a TTL test race: passive-SQL measurement now joins already-admitted sweeps,
|
||||
without dropping unresolved recovery holds. Fresh CI/review remain required.
|
||||
|
||||
The fresh master audit at `991c47bc6e` identified an additional recurring-server
|
||||
owner: the Grok routine dispatcher starts a one-second settings/dispatch scan
|
||||
when public MCP is configured, even when Grok is disabled. Include this in the
|
||||
full stack as slice 13, separately from decision retention. The disabled baseline
|
||||
issued three settings queries in 3.2 seconds. Its durable prepared dispatches,
|
||||
atomic claim, and existing uncertain-outcome policy must remain authoritative.
|
||||
@@ -59,8 +59,10 @@ const support = externalUrl ? { supported: true } : await getEmbeddedPostgresTes
|
||||
async function start() {
|
||||
coordinator = createDeliveryWorkCoordinator({ owner: db, canRun: () => !draining,
|
||||
canReconcile: () => true, onError: error => errors.push(error) });
|
||||
await registerDecisionContinuationWork(coordinator, service(), () => !draining).ready;
|
||||
await registerDecisionExpiryWork(coordinator, service(), () => !draining).ready;
|
||||
const continuation = service(), expiry = service();
|
||||
await registerDecisionContinuationWork(coordinator, continuation, () => !draining).ready;
|
||||
await registerDecisionExpiryWork(coordinator, expiry, () => !draining).ready;
|
||||
return { continuation, expiry };
|
||||
}
|
||||
async function quiet() {
|
||||
await vi.waitFor(async () => {
|
||||
@@ -151,12 +153,14 @@ const support = externalUrl ? { supported: true } : await getEmbeddedPostgresTes
|
||||
});
|
||||
|
||||
it("keeps a failed expiry retry through drain and preserves pending state", async () => {
|
||||
const row = await create(new Date(Date.now() + 30_000)); await start();
|
||||
const row = await create(new Date(Date.now() + 30_000)); const workers = await start();
|
||||
const sweeps = [vi.spyOn(workers.expiry, "sweepTtlExpiry"), vi.spyOn(workers.continuation, "sweepContinuations")];
|
||||
await db.execute(sql`create function decision_expiry_fail() returns trigger language plpgsql as $$ begin
|
||||
if NEW.status = 'expired' then raise exception 'expiry unavailable'; end if; return NEW; end $$`);
|
||||
await db.execute(sql`create trigger decision_expiry_fail before update on decisions for each row execute function decision_expiry_fail()`);
|
||||
await vi.waitFor(async () => { await vi.advanceTimersByTimeAsync(30_000); expect(errors.length).toBeGreaterThan(0); });
|
||||
draining = true; expect((await service().get(row.id))?.status).toBe("open");
|
||||
await Promise.allSettled(sweeps.flatMap(sweep => sweep.mock.results.filter(result => result.type === "return").map(result => result.value)));
|
||||
const reads = vi.spyOn(db, "select"); await vi.advanceTimersByTimeAsync(60_000);
|
||||
expect(reads).not.toHaveBeenCalled(); reads.mockRestore();
|
||||
await db.execute(sql`drop trigger decision_expiry_fail on decisions`);
|
||||
|
||||
@@ -165,6 +165,20 @@ const support = externalUrl ? { supported: true } : await getEmbeddedPostgresTes
|
||||
await expired(row.id); await quiet(); expect(wake).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it.each([true, false])("preserves a deletion mixed with a scoped hint in one transaction, deleteFirst=%s", async deleteFirst => {
|
||||
const row = await create(); const otherId = randomUUID();
|
||||
await db.insert(issues).values({ id: otherId, companyId, title: "Unrelated", status: "todo", responsibleUserId: userId });
|
||||
await start(); await quiet();
|
||||
await db.transaction(async tx => {
|
||||
const scoped = issueService(tx as unknown as typeof db);
|
||||
if (deleteFirst) await scoped.remove(targetId);
|
||||
await scoped.update(otherId, { status: "done", actorUserId: userId });
|
||||
if (!deleteFirst) await scoped.remove(targetId);
|
||||
});
|
||||
expect(await db.select().from(decisionTargetIssues).where(eq(decisionTargetIssues.decisionId, row.id))).toHaveLength(0);
|
||||
await expired(row.id); await quiet(); expect(wake).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("retains a first target change through drain without passive SQL", async () => {
|
||||
const row = await create(); await start(); await quiet(); draining = true; await change("cancelled");
|
||||
const reads = vi.spyOn(db, "select"); await vi.advanceTimersByTimeAsync(120_000);
|
||||
|
||||
@@ -3,7 +3,7 @@ import { DELIVERY_QUEUES, notifyDeliveryWork } from "./delivery-work-notificatio
|
||||
import type { decisionService } from "./decisions.js";
|
||||
|
||||
export function notifyDecisionTargetWork(transaction: object, hint?: `issue:${string}` | `decision:${string}`) {
|
||||
return notifyDeliveryWork(transaction, DELIVERY_QUEUES.decisionTargets, hint);
|
||||
return notifyDeliveryWork(transaction, DELIVERY_QUEUES.decisionTargets, hint ?? "all");
|
||||
}
|
||||
|
||||
export function registerDecisionTargetWork(
|
||||
|
||||
Reference in new issue
Block a user