Merge decision target review repair into Grok dispatch migration

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* codex/sleep-12-decision-target-invalidation:
  fix(server): retain full target scans with scoped transaction hints
This commit is contained in:
DottaandPaperclip committed 2026-10-24 14:00:26 -07:00
commit ee83da2455
5 files changed
+56 -4

No files matched your search

+15
View File
@@ -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);
+1 -1
View File
@@ -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(