From 2a85da67f7a024bf31250ad7ee6671330658509e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 13 Aug 2026 12:07:39 +0200 Subject: [PATCH] Stop claiming a summary is pending when auto-read is off The upload path enqueued a summary job and stamped PENDING without consulting the owner's auto-read opt-in; the job's opt-out branch then deliberately left the state alone, so a document uploaded with the opt-in off said "summary is being generated" forever. Two writers, contradictory assumptions. Both ends of the contract now agree. enqueueDocumentSummary checks the opt-in first and refuses to mint a job (or a PENDING) it knows the job cannot resolve; the job keeps its own check for a pref flipped between enqueue and run and heals a stranded PENDING back to NONE, scoped so READY / WITHHELD / UNAVAILABLE stay untouched. Migration 0333 drains the rows already orphaned. The view needs nothing: NONE already renders the manual summary action. Refs #788 --- .../migration.sql | 12 +++ .../jobs/__tests__/document-summary.test.ts | 75 ++++++++++++++++++- src/lib/jobs/document-summary.ts | 48 +++++++++--- 3 files changed, 123 insertions(+), 12 deletions(-) create mode 100644 prisma/migrations/0333_summary_pending_heal_index_outcome/migration.sql diff --git a/prisma/migrations/0333_summary_pending_heal_index_outcome/migration.sql b/prisma/migrations/0333_summary_pending_heal_index_outcome/migration.sql new file mode 100644 index 000000000..9acfc02ee --- /dev/null +++ b/prisma/migrations/0333_summary_pending_heal_index_outcome/migration.sql @@ -0,0 +1,12 @@ +-- Refs #788 — heal phantom summary PENDING rows. +-- `enqueueDocumentSummary` used to claim PENDING even when the owner's +-- auto-read opt-in was OFF; the job's opt-out branch then left the state +-- alone, so the detail view said "being generated" forever. Both writers now +-- agree (the enqueue end no longer claims what the job cannot resolve, the +-- job end heals PENDING→NONE on opt-out); this one-time data fix drains the +-- rows already stranded. Safe by construction: READY / WITHHELD / UNAVAILABLE +-- are never touched, and a job that is genuinely mid-flight rewrites its +-- terminal state after this UPDATE anyway. +UPDATE "inbound_documents" +SET "summary_state" = 'NONE' +WHERE "summary_state" = 'PENDING'; diff --git a/src/lib/jobs/__tests__/document-summary.test.ts b/src/lib/jobs/__tests__/document-summary.test.ts index 6cf1db45c..a88010654 100644 --- a/src/lib/jobs/__tests__/document-summary.test.ts +++ b/src/lib/jobs/__tests__/document-summary.test.ts @@ -55,8 +55,12 @@ vi.mock("@/lib/ai/ai-budgets", () => ({ AI_BUDGETS: { documentSummary: { temperature: 0.3, maxTokens: 600 } }, })); -import { runDocumentSummaryJob } from "../document-summary"; +import { + enqueueDocumentSummary, + runDocumentSummaryJob, +} from "../document-summary"; import { prisma } from "@/lib/db"; +import { getGlobalBoss } from "@/lib/jobs/boss-instance"; import { documentAutoReadEnabled } from "@/lib/documents/document-settings"; import { resolveDocumentVisionProvider } from "@/lib/documents/provider-order"; import { @@ -124,14 +128,19 @@ beforeEach(() => { }); describe("runDocumentSummaryJob — gating", () => { - it("does NOTHING when the opt-in is OFF (no provider call, no write)", async () => { + it("does no provider work when the opt-in is OFF (only the PENDING heal may write)", async () => { vi.mocked(documentAutoReadEnabled).mockResolvedValue(false); await runDocumentSummaryJob({ userId: "user-1", documentId: "doc-1" }); expect(resolveDocumentVisionProvider).not.toHaveBeenCalled(); expect(runDocumentSummary).not.toHaveBeenCalled(); - expect(prisma.inboundDocument.updateMany).not.toHaveBeenCalled(); + // The single permitted write is the PENDING→NONE heal; nothing may claim + // an attempt (UNAVAILABLE/WITHHELD) or a summary (READY) here. + const calls = vi.mocked(prisma.inboundDocument.updateMany).mock.calls; + for (const [arg] of calls) { + expect(arg.data).toEqual({ summaryState: "NONE" }); + } }); it("generates and PERSISTS the summary encrypted when opt-in is ON + a provider is configured", async () => { @@ -248,4 +257,64 @@ describe("runDocumentSummaryJob — gating", () => { }), ); }); + + it("heals a phantom PENDING back to NONE when the opt-in is OFF", async () => { + vi.mocked(documentAutoReadEnabled).mockResolvedValue(false); + + await runDocumentSummaryJob({ userId: "user-1", documentId: "doc-1" }); + + // The opt-out branch may not leave a claimed PENDING standing — the view + // reads PENDING as "being generated" and this run is the only writer that + // can resolve it. The heal is scoped to PENDING: UNAVAILABLE / WITHHELD + // record real attempts and stay, READY is never touched. + expect(prisma.inboundDocument.updateMany).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ summaryState: "PENDING" }), + data: { summaryState: "NONE" }, + }), + ); + // Still strictly no provider work. + expect(resolveDocumentVisionProvider).not.toHaveBeenCalled(); + expect(runDocumentSummary).not.toHaveBeenCalled(); + }); +}); + +describe("enqueueDocumentSummary — the pending contract", () => { + it("does not enqueue and does not claim PENDING when the opt-in is OFF", async () => { + vi.mocked(documentAutoReadEnabled).mockResolvedValue(false); + const send = vi.fn().mockResolvedValue("job-1"); + vi.mocked(getGlobalBoss).mockReturnValue({ send } as never); + + const result = await enqueueDocumentSummary("user-1", "doc-1"); + + // With auto-read OFF the job's only move is the opt-out no-op, so a + // PENDING claimed here could never be resolved to a real outcome. No job, + // no state write — the view keeps offering the manual action. + expect(result).toEqual({ enqueued: false }); + expect(send).not.toHaveBeenCalled(); + expect(prisma.inboundDocument.updateMany).not.toHaveBeenCalled(); + }); + + it("enqueues and claims PENDING when the opt-in is ON and a job was created", async () => { + const send = vi.fn().mockResolvedValue("job-1"); + vi.mocked(getGlobalBoss).mockReturnValue({ send } as never); + + const result = await enqueueDocumentSummary("user-1", "doc-1"); + + expect(result).toEqual({ enqueued: true }); + expect(send).toHaveBeenCalledTimes(1); + expect(prisma.inboundDocument.updateMany).toHaveBeenCalledWith( + expect.objectContaining({ data: { summaryState: "PENDING" } }), + ); + }); + + it("claims nothing when the boss dropped the send (no job id)", async () => { + const send = vi.fn().mockResolvedValue(null); + vi.mocked(getGlobalBoss).mockReturnValue({ send } as never); + + const result = await enqueueDocumentSummary("user-1", "doc-1"); + + expect(result).toEqual({ enqueued: false }); + expect(prisma.inboundDocument.updateMany).not.toHaveBeenCalled(); + }); }); diff --git a/src/lib/jobs/document-summary.ts b/src/lib/jobs/document-summary.ts index 83bd6e022..ab3bff03b 100644 --- a/src/lib/jobs/document-summary.ts +++ b/src/lib/jobs/document-summary.ts @@ -109,8 +109,23 @@ export async function runDocumentSummaryJob( // on-demand route stays the only way to summarise). Fail-closed on a missing // row. if (!(await documentAutoReadEnabled(userId))) { - // Opting out is not a failed attempt — leave the state alone so the view - // keeps offering the manual action rather than claiming we tried. + // Opting out is not a failed attempt — but a claimed PENDING may not + // outlive this run either. PENDING promises the reader "a job will resolve + // this", and this job IS that resolution: with the opt-in OFF the honest + // terminal state is NONE (never attempted), which is what the enqueue end + // of the contract (`enqueueDocumentSummary`) also concludes when the pref + // is off. The heal is scoped to PENDING only — NONE needs no write, + // UNAVAILABLE / WITHHELD record real attempts, READY is a stored summary. + // This also drains any phantom PENDING left by a pref flip mid-flight. + await prisma.inboundDocument.updateMany({ + where: { + id: documentId, + userId, + deletedAt: null, + summaryState: "PENDING", + }, + data: { summaryState: "NONE" }, + }); annotate({ action: { name: "documents.summary.autoSkipped" }, meta: { documentId, reason: "opt-out" }, @@ -282,14 +297,29 @@ export async function enqueueDocumentSummary( userId: string, documentId: string, ): Promise<{ enqueued: boolean }> { - const boss = getGlobalBoss(); - if (!boss) return { enqueued: false }; - const payload: DocumentSummaryPayload = { - userId, - documentId, - enqueuedAt: new Date().toISOString(), - }; try { + // Contract, enqueue end: PENDING may only be claimed for work the job can + // terminally resolve. With the auto-read opt-in OFF the job's only branch + // is the opt-out no-op, so enqueueing would mint a job whose sole purpose + // is to undo this function's own PENDING — don't. No job, no state write; + // the document stays NONE and the view offers the manual action. The job + // keeps its own pref check (belt-and-braces for a pref flipped between + // enqueue and run) and heals PENDING→NONE there — the other end of this + // contract lives in `runDocumentSummaryJob`'s opt-out branch. + if (!(await documentAutoReadEnabled(userId))) { + annotate({ + action: { name: "documents.summary.autoSkipped" }, + meta: { documentId, reason: "opt-out" }, + }); + return { enqueued: false }; + } + const boss = getGlobalBoss(); + if (!boss) return { enqueued: false }; + const payload: DocumentSummaryPayload = { + userId, + documentId, + enqueuedAt: new Date().toISOString(), + }; const jobId = await boss.send(DOCUMENT_SUMMARY_QUEUE, payload, { retryLimit: 2, retryDelay: 30,