From 09a70c3bef3e55be7a4e6f90b07506737261da02 Mon Sep 17 00:00:00 2001 From: Dotta Date: Fri, 30 Oct 2026 22:55:58 -0500 Subject: [PATCH] fix(server): recheck reactive sources after status summary commit Co-Authored-By: Paperclip --- doc/plans/2026-10-30-status-cards-sleep.md | 3 + .../status-card-evaluation-work.test.ts | 100 ++++++++++++++++++ server/src/services/status-cards.ts | 8 +- 3 files changed, 105 insertions(+), 6 deletions(-) diff --git a/doc/plans/2026-10-30-status-cards-sleep.md b/doc/plans/2026-10-30-status-cards-sleep.md index 97566de3bc4..55976497dce 100644 --- a/doc/plans/2026-10-30-status-cards-sleep.md +++ b/doc/plans/2026-10-30-status-cards-sleep.md @@ -160,3 +160,6 @@ 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. 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. + + +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. diff --git a/server/src/__tests__/status-card-evaluation-work.test.ts b/server/src/__tests__/status-card-evaluation-work.test.ts index 7c0f010949d..55b6264fe04 100644 --- a/server/src/__tests__/status-card-evaluation-work.test.ts +++ b/server/src/__tests__/status-card-evaluation-work.test.ts @@ -112,6 +112,7 @@ const support = external ? { supported: true } : await getEmbeddedPostgresTestSu const created = await cards.create(companyId, { interestPrompt: "Blockers", titlePinned: false, agentId, refreshPolicy }, actor); const compile = await cards.requestCompile(created.id, actor); await writeback(created.id, compile.generatingIssue.id, true); + if (!coordinator && mode === "reactive") await cards.tickDueStatusCards(); return cards.getById(created.id).then(row => row!); } @@ -144,6 +145,104 @@ const support = external ? { supported: true } : await getEmbeddedPostgresTestSu expect(errors).toEqual([]); }); + it.each([true, false])("rechecks source hints consumed during summary writeback without forcing unchanged generation (relevant=%s)", async relevant => { + const evaluation = statusCardEvaluationService(db, { wakeup }); + const runs = vi.spyOn(evaluation, "run"), hints = vi.spyOn(evaluation, "changed"); + await start(evaluation); + const created = await card(); + await quiet(); + const cards = statusCardService(db); + const generation = await cards.requestRefresh(created.id, { trigger: "manual", actor }); + const [run] = await db.insert(heartbeatRuns).values({ companyId, agentId, status: "running" }).returning(); + await db.update(issues).set({ checkoutRunId: run!.id }).where(eq(issues.id, generation.generatingIssue!.id)); + const writer = { agentId, runId: run!.id }; + await settled(); + const entered = Promise.withResolvers(), release = Promise.withResolvers(); + const factory = companySearch.companySearchService; + let hold = true; + const searchedCompanies: string[] = []; + vi.spyOn(companySearch, "companySearchService").mockImplementation(owner => { + const search = factory(owner), actual = search.search; + search.search = async (...args) => { + searchedCompanies.push(args[0]); + const result = await actual(...args); + if (hold) { hold = false; entered.resolve(); await release.promise; } + return result; + }; + return search; + }); + const writing = cards.writeSummary(created.id, { markdown: "Current blockers", changeSummary: "Completed before source change", generationIssueId: generation.generatingIssue!.id }, writer); + await entered.promise; + const passes = runs.mock.calls.length; + hints.mockClear(); + try { + await issueService(db).create(companyId, { title: "Source committed after summary search", status: relevant ? "blocked" : "backlog", priority: "medium" }); + await vi.waitFor(async () => { + await vi.advanceTimersByTimeAsync(500); + await Promise.allSettled([...evaluations]); + expect(hints.mock.calls.some(([scope]) => scope?.has(`changes:${companyId}`))).toBe(true); + expect(runs.mock.calls.length).toBeGreaterThan(passes); + }, { timeout: 5000 }); + expect((await cards.getById(created.id))!.generatingIssueId).toBe(generation.generatingIssue!.id); + expect(await evaluation.next()).toBeNull(); + expect(wakeup).not.toHaveBeenCalled(); + expect(searchedCompanies).toEqual([companyId]); + } finally { release.resolve(); } + await writing; + await settled(); + if (relevant) { + const pending = (await cards.getById(created.id))!; + expect(pending.generatingIssueId).toBeNull(); + expect(pending.pendingChangeCount).toBe(1); + expect(pending.nextEvalAt).toBeInstanceOf(Date); + expect(wakeup).not.toHaveBeenCalled(); + await vi.advanceTimersByTimeAsync(Math.max(0, pending.nextEvalAt!.getTime() - Date.now())); + await settled(); + const generated = (await cards.getById(created.id))!; + expect(generated.generatingIssueId).toBeTruthy(); + expect(wakeup).toHaveBeenCalledOnce(); + expect(await db.select().from(agentWakeupRequests)).toHaveLength(1); + await writeback(created.id, generated.generatingIssueId!); + expect((await db.select().from(statusCardUpdates).where(eq(statusCardUpdates.generationIssueId, generated.generatingIssueId!)))) + .toMatchObject([{ status: "ok", finishedAt: expect.any(Date) }]); + } + await quiet(); + expect(wakeup).toHaveBeenCalledTimes(relevant ? 1 : 0); + if (!relevant) expect(searchedCompanies).toEqual([companyId, companyId]); + expect(new Set(searchedCompanies)).toEqual(new Set([companyId])); + expect(errors).toEqual([]); + }); + + it("rolls back the summary and its post-commit recheck obligation together", async () => { + await start(); const created = await card(); await quiet(); + const cards = statusCardService(db); + const generation = await cards.requestRefresh(created.id, { trigger: "manual", actor }); + const [run] = await db.insert(heartbeatRuns).values({ companyId, agentId, status: "running" }).returning(); + await db.update(issues).set({ checkoutRunId: run!.id }).where(eq(issues.id, generation.generatingIssue!.id)); + await settled(); + const before = (await cards.getById(created.id))!; + const revisions = await cards.listSummaryRevisions(created.id); + await expect(db.transaction(async tx => { + await statusCardService(tx as unknown as typeof db).writeSummary(created.id, { markdown: "Rolled back summary", changeSummary: "Rollback", generationIssueId: generation.generatingIssue!.id }, { agentId, runId: run!.id }); + throw new Error("summary rollback"); + })).rejects.toThrow("summary rollback"); + await quiet(); + expect((await cards.getById(created.id))!).toMatchObject({ generatingIssueId: before.generatingIssueId, nextEvalAt: before.nextEvalAt, nextEvalReason: before.nextEvalReason }); + expect(await cards.listSummaryRevisions(created.id)).toEqual(revisions); + expect(wakeup).not.toHaveBeenCalled(); expect(errors).toEqual([]); + }); + + it.each(["manual", "interval"] as const)("preserves %s writeback cadence without a reactive recheck", async mode => { + const created = await card(mode), cards = statusCardService(db); + const generation = await cards.requestRefresh(created.id, { trigger: "manual", actor }); + const [run] = await db.insert(heartbeatRuns).values({ companyId, agentId, status: "running" }).returning(); + await db.update(issues).set({ checkoutRunId: run!.id }).where(eq(issues.id, generation.generatingIssue!.id)); + const result = await cards.writeSummary(created.id, { markdown: "Current blockers", changeSummary: "Complete", generationIssueId: generation.generatingIssue!.id }, { agentId, runId: run!.id }); + expect(result.card.generatingIssueId).toBeNull(); + expect(result.card.nextEvalAt?.getTime() ?? null).toBe(mode === "interval" ? result.card.lastGeneratedAt!.getTime() + 15 * 60_000 : null); + expect(wakeup).not.toHaveBeenCalled(); + }); + it("keeps card deletion scoped to its company and a missing deletion quiet", async () => { const removed = await card(); await card(); @@ -448,6 +547,7 @@ const support = external ? { supported: true } : await getEmbeddedPostgresTestSu vi.setSystemTime(pending.nextEvalAt!); await restarted.run(new AbortController().signal); expect(wakeup).toHaveBeenCalledOnce(); await writeback(created.id, (await statusCardService(db).getById(created.id))!.generatingIssueId!); + await restarted.run(new AbortController().signal); expect((await statusCardService(db).getById(created.id))!.nextEvalReason).toBe("membership"); }); diff --git a/server/src/services/status-cards.ts b/server/src/services/status-cards.ts index 862d9d64ec4..6da4dadaf0d 100644 --- a/server/src/services/status-cards.ts +++ b/server/src/services/status-cards.ts @@ -969,12 +969,8 @@ export function statusCardService( pendingChangeCount: 0, pendingChangeHash: null, lastChangeAt: null, - nextEvalReason: current.refreshPolicy.mode === "reactive" && - filterStatusCardChanges(diffStatusCardFingerprint(snapshot, watchedNow), current.refreshPolicy).length > 0 - ? "refresh" : membershipAt ? "membership" : null, - nextEvalAt: current.refreshPolicy.mode === "reactive" && - filterStatusCardChanges(diffStatusCardFingerprint(snapshot, watchedNow), current.refreshPolicy).length > 0 - ? now : nextEvaluationAt(current, membershipAt, now), + nextEvalReason: current.refreshPolicy.mode === "reactive" ? "refresh" : membershipAt ? "membership" : null, + nextEvalAt: current.refreshPolicy.mode === "reactive" ? now : nextEvaluationAt(current, membershipAt, now), updatedAt: now, }).where(and(eq(statusCards.id, current.id), eq(statusCards.generatingIssueId, input.generationIssueId))).returning(); if (!next) throw conflict("Status-card generation was superseded by a newer task");