[Architecture] feat: durable job queues — BullMQ-primary with Postgres fallback, worker threads, async-on for new installs - #294
Open
amal66 wants to merge 15 commits into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
amal66
marked this pull request as draft
August 6, 2026 03:31
amal66
force-pushed
the
olp-pr/durable-queues
branch
from
August 18, 2026 13:43
3a3ecbf to
fecd946
Compare
|
|
This was referenced Aug 19, 2026
amal66
force-pushed
the
olp-pr/durable-queues
branch
from
August 21, 2026 17:23
fecd946 to
c65e343
Compare
amal66
added a commit
to amal66/mike
that referenced
this pull request
Aug 21, 2026
…ewrites clear their stale renditions
WHY THIS MATTERS
The first two commits put ASYNC_DOCUMENT_CONVERSION in front of the five
LibreOffice call sites on the upload/version routes — but a whole-tree
survey found two more sites hiding inside the chat tools, reached through
the SSE stream itself:
1. replicate_document converts the source inline when it has no PDF
rendition to copy (toolDispatcher.ts), and
2. generate_ppt converts the generated PPTX inline
(documentOps.ts / persistGeneratedFile).
With the flag on, a deployment reasonably believes "LibreOffice no longer
runs on my request threads" — yet a chat turn that replicates or generates
a deck still paid the 3-15s LibreOffice cold start in-band, and a
conversion failure was swallowed (devLog) leaving the document permanently
rendition-less with no retry path. Closing these keeps the flag's promise
honest: it now covers every LibreOffice call site in the app.
WHAT IS A RENDITION AND WHO CONSUMES IT
A "rendition" is the per-version converted PDF (pdf_storage_path) that
/single-documents/:id/display serves in place of raw Office bytes. The
frontend routes DOCX to DocxView by FILENAME (docx-preview over raw bytes),
so DOCX renditions are almost never displayed — but PPTX has no dedicated
viewer and renders exclusively through its rendition via PdfView, which is
why generate_ppt's silent conversion failure was a real product gap.
HOW IT WORKS
Both sites follow the exact pattern of the five route sites:
- flag off → inline docxToPdf, byte-for-byte the historical behavior.
- flag on → the document/copy is inserted "ready" with
pdf_storage_path: null and one conversion job per new version is
enqueued (deduped on convert:<versionId>, attempts: 3, exponential
backoff). finalizeDocumentStatus: false because these documents are
usable from their raw bytes — a rendition failure must never flip a
healthy document to "error".
- an enqueue failure degrades to the sync path's conversion-failure
behavior (usable document, no rendition) instead of failing the tool.
THE INVARIANT HALF (accept/reject + in-place re-edit)
Accept/reject of a tracked change and an in-place assistant re-edit both
rewrite a version's DOCX bytes at its existing storage path. Any rendition
recorded for that version now describes bytes that no longer exist — and a
stale rendition is not hypothetical decoration: /display would serve it,
and replicate_document COPIES it onto every replica. Both rewrite sites now
null pdf_storage_path in the same update that re-hashes the content, the
same ordering discipline as the content_sha256 clear-then-set that
surrounds the byte write. In today's flows assistant_edit versions never
carry a rendition, so this is an invariant made explicit, not a behavior
change.
DELIBERATELY NOT DONE
edit_document / generate_docx versions do NOT get renditions enqueued:
DOCX renders through DocxView from raw bytes everywhere (the tracked-
changes UI depends on it), so a rendition would cost a LibreOffice run per
chat edit and display nothing. Recorded here so the omission reads as a
decision, not an oversight.
TESTS
- replicateRenditionQueue.test.ts: flag off → one inline conversion, no
queue; flag on → zero in-band LibreOffice, one job per copy with
per-copy pdfKey and finalize:false; enqueue failure → copies still
usable.
- documentOps.generatedRendition.test.ts: same contract for generate_ppt,
plus xlsx never converts or enqueues (spreadsheets are served raw).
Part of the durable-queues row (#40 → olp PR open-legal-products#294).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JDev46QdqBsdyRCRBxcvk2
amal66
added a commit
to amal66/mike
that referenced
this pull request
Aug 21, 2026
…rminal writes + cross-process job cancellation
WHY THIS MATTERS
POST /tabular-review/:id/clear-cells resets a row's cells to "pending" so a
user can blank bad results and start over. But extraction runs
concurrently with it — inline on another request (sync mode) or on a
worker that survives disconnects (async mode, introduced by this PR). A
run that finished AFTER the clear would write "done" (or "error") straight
over the user's reset, and in async mode a queued-but-unstarted job would
happily re-fill the freshly cleared row seconds later. Durability made the
old race WIDER, so this PR owns the fix.
WHAT IS A LOST UPDATE
Two writers race the same row: the user's reset (status = pending) and the
extraction's terminal write (status = done). Without a guard the last
writer wins, and the extraction — which started before the reset and knows
nothing of it — silently undoes the user's action. The classic cure is an
optimistic condition: make the terminal write assert the state it believes
it owns.
HOW IT WORKS — three layers, weakest to strongest
1. GUARDED TERMINAL WRITES (both modes). Every terminal cell write now
carries AND status = 'generating': the shared core's done-write, the
sync route's missing→error writes, the sync regenerate-cell writes, and
the worker's permanent-failure cleanup. An extraction only ever
finalizes a claim it still holds; if clear-cells revoked the claim, the
write matches zero rows and the SSE/Redis announce is skipped too — a
tailing stream never shows a "done" the DB doesn't hold. The model's
result still counts as "received", so the caller neither marks the
cleared cell "error" nor retries over it.
2. QUEUE CANCELLATION (async mode, flag-gated so sync deployments never
dial Redis). clear-cells addresses every deterministic jobId for the
cleared rows — extract:<review>:<row> and each :<col> variant — and
REMOVES jobs still waiting/delayed, so they never start.
3. PERSISTED CANCEL MARKER for jobs already active. BullMQ's Job#discard()
is only an in-memory flag on the worker's own Job instance — calling it
from the API process is a silent no-op (a live-Redis smoke caught
exactly this: the "discarded" job's retry was scheduled anyway).
Instead the job's data is marked canceled: true via updateData(), which
IS persisted to Redis; each retry attempt re-fetches job data, and
runExtractionJob now returns immediately on the marker instead of
re-claiming the cleared cells from scratch.
TRADEOFF, FLAGGED
The permanent-failure handler now flips only cells still claimed
("generating") to "error". A job that dies before ever claiming its cells
(e.g. the settings lookup fails on all 3 attempts) leaves them "pending" —
a blank, re-runnable state rather than a red one; the resume stream then
runs to its 15-minute cap instead of resolving early. Chosen deliberately:
silently overwriting a user's reset is worse than a quieter failure under
already-broken infrastructure.
TESTS
- tabular.extractRow: a done-write that matches zero rows (cell cleared
mid-flight) suppresses the announce and is not reported "missing".
- extractionWorker: canceled jobs touch nothing; permanent failure leaves
cleared (pending) cells alone while still erroring claimed ones.
- extractionQueue: waiting jobs removed; active jobs get the PERSISTED
updateData marker (never Job#discard); remove() losing the race falls
back to the marker; per-job failures are swallowed (best-effort by
design — the write guards are the correctness layer).
- Live-Redis smoke (real BullMQ): dedupe, remove-waiting, and
cancel-active verified end-to-end — attempt 1 active during the cancel,
attempt 2 saw canceled: true and completed without touching the DB.
Part of the durable-queues row (#40 → olp PR open-legal-products#294).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JDev46QdqBsdyRCRBxcvk2
amal66
changed the title
amal66
added a commit
to amal66/mike
that referenced
this pull request
Aug 21, 2026
…ewrites clear their stale renditions
WHY THIS MATTERS
The first two commits put ASYNC_DOCUMENT_CONVERSION in front of the five
LibreOffice call sites on the upload/version routes — but a whole-tree
survey found two more sites hiding inside the chat tools, reached through
the SSE stream itself:
1. replicate_document converts the source inline when it has no PDF
rendition to copy (toolDispatcher.ts), and
2. generate_ppt converts the generated PPTX inline
(documentOps.ts / persistGeneratedFile).
With the flag on, a deployment reasonably believes "LibreOffice no longer
runs on my request threads" — yet a chat turn that replicates or generates
a deck still paid the 3-15s LibreOffice cold start in-band, and a
conversion failure was swallowed (devLog) leaving the document permanently
rendition-less with no retry path. Closing these keeps the flag's promise
honest: it now covers every LibreOffice call site in the app.
WHAT IS A RENDITION AND WHO CONSUMES IT
A "rendition" is the per-version converted PDF (pdf_storage_path) that
/single-documents/:id/display serves in place of raw Office bytes. The
frontend routes DOCX to DocxView by FILENAME (docx-preview over raw bytes),
so DOCX renditions are almost never displayed — but PPTX has no dedicated
viewer and renders exclusively through its rendition via PdfView, which is
why generate_ppt's silent conversion failure was a real product gap.
HOW IT WORKS
Both sites follow the exact pattern of the five route sites:
- flag off → inline docxToPdf, byte-for-byte the historical behavior.
- flag on → the document/copy is inserted "ready" with
pdf_storage_path: null and one conversion job per new version is
enqueued (deduped on convert:<versionId>, attempts: 3, exponential
backoff). finalizeDocumentStatus: false because these documents are
usable from their raw bytes — a rendition failure must never flip a
healthy document to "error".
- an enqueue failure degrades to the sync path's conversion-failure
behavior (usable document, no rendition) instead of failing the tool.
THE INVARIANT HALF (accept/reject + in-place re-edit)
Accept/reject of a tracked change and an in-place assistant re-edit both
rewrite a version's DOCX bytes at its existing storage path. Any rendition
recorded for that version now describes bytes that no longer exist — and a
stale rendition is not hypothetical decoration: /display would serve it,
and replicate_document COPIES it onto every replica. Both rewrite sites now
null pdf_storage_path in the same update that re-hashes the content, the
same ordering discipline as the content_sha256 clear-then-set that
surrounds the byte write. In today's flows assistant_edit versions never
carry a rendition, so this is an invariant made explicit, not a behavior
change.
DELIBERATELY NOT DONE
edit_document / generate_docx versions do NOT get renditions enqueued:
DOCX renders through DocxView from raw bytes everywhere (the tracked-
changes UI depends on it), so a rendition would cost a LibreOffice run per
chat edit and display nothing. Recorded here so the omission reads as a
decision, not an oversight.
TESTS
- replicateRenditionQueue.test.ts: flag off → one inline conversion, no
queue; flag on → zero in-band LibreOffice, one job per copy with
per-copy pdfKey and finalize:false; enqueue failure → copies still
usable.
- documentOps.generatedRendition.test.ts: same contract for generate_ppt,
plus xlsx never converts or enqueues (spreadsheets are served raw).
Part of the durable-queues row (#40 → olp PR open-legal-products#294).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JDev46QdqBsdyRCRBxcvk2
amal66
added a commit
to amal66/mike
that referenced
this pull request
Aug 21, 2026
…rminal writes + cross-process job cancellation
WHY THIS MATTERS
POST /tabular-review/:id/clear-cells resets a row's cells to "pending" so a
user can blank bad results and start over. But extraction runs
concurrently with it — inline on another request (sync mode) or on a
worker that survives disconnects (async mode, introduced by this PR). A
run that finished AFTER the clear would write "done" (or "error") straight
over the user's reset, and in async mode a queued-but-unstarted job would
happily re-fill the freshly cleared row seconds later. Durability made the
old race WIDER, so this PR owns the fix.
WHAT IS A LOST UPDATE
Two writers race the same row: the user's reset (status = pending) and the
extraction's terminal write (status = done). Without a guard the last
writer wins, and the extraction — which started before the reset and knows
nothing of it — silently undoes the user's action. The classic cure is an
optimistic condition: make the terminal write assert the state it believes
it owns.
HOW IT WORKS — three layers, weakest to strongest
1. GUARDED TERMINAL WRITES (both modes). Every terminal cell write now
carries AND status = 'generating': the shared core's done-write, the
sync route's missing→error writes, the sync regenerate-cell writes, and
the worker's permanent-failure cleanup. An extraction only ever
finalizes a claim it still holds; if clear-cells revoked the claim, the
write matches zero rows and the SSE/Redis announce is skipped too — a
tailing stream never shows a "done" the DB doesn't hold. The model's
result still counts as "received", so the caller neither marks the
cleared cell "error" nor retries over it.
2. QUEUE CANCELLATION (async mode, flag-gated so sync deployments never
dial Redis). clear-cells addresses every deterministic jobId for the
cleared rows — extract:<review>:<row> and each :<col> variant — and
REMOVES jobs still waiting/delayed, so they never start.
3. PERSISTED CANCEL MARKER for jobs already active. BullMQ's Job#discard()
is only an in-memory flag on the worker's own Job instance — calling it
from the API process is a silent no-op (a live-Redis smoke caught
exactly this: the "discarded" job's retry was scheduled anyway).
Instead the job's data is marked canceled: true via updateData(), which
IS persisted to Redis; each retry attempt re-fetches job data, and
runExtractionJob now returns immediately on the marker instead of
re-claiming the cleared cells from scratch.
TRADEOFF, FLAGGED
The permanent-failure handler now flips only cells still claimed
("generating") to "error". A job that dies before ever claiming its cells
(e.g. the settings lookup fails on all 3 attempts) leaves them "pending" —
a blank, re-runnable state rather than a red one; the resume stream then
runs to its 15-minute cap instead of resolving early. Chosen deliberately:
silently overwriting a user's reset is worse than a quieter failure under
already-broken infrastructure.
TESTS
- tabular.extractRow: a done-write that matches zero rows (cell cleared
mid-flight) suppresses the announce and is not reported "missing".
- extractionWorker: canceled jobs touch nothing; permanent failure leaves
cleared (pending) cells alone while still erroring claimed ones.
- extractionQueue: waiting jobs removed; active jobs get the PERSISTED
updateData marker (never Job#discard); remove() losing the race falls
back to the marker; per-job failures are swallowed (best-effort by
design — the write guards are the correctness layer).
- Live-Redis smoke (real BullMQ): dedupe, remove-waiting, and
cancel-active verified end-to-end — attempt 1 active during the cancel,
attempt 2 saw canceled: true and completed without touching the DB.
Part of the durable-queues row (#40 → olp PR open-legal-products#294).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JDev46QdqBsdyRCRBxcvk2
amal66
added a commit
to amal66/mike
that referenced
this pull request
Aug 21, 2026
…turn audit WHY THIS MATTERS Some workloads must be durable in EVERY deployment — audit trails, account erasure, storage cleanup, export builds. The BullMQ queues (open-legal-products#294) are deliberately opt-in because they require Redis; making THESE workloads depend on an opt-in would mean the default deployment keeps losing audit rows and leaking storage. Every deployment — web, Word add-in stack, Mac app Docker stack — already runs Postgres, so this queue is built on the database that is already there and runs BY DEFAULT with zero new infrastructure or configuration. DB_JOBS_ENABLED=false exists only as an operational escape hatch. WHAT IS A POSTGRES JOB QUEUE A db_jobs table plus one claim function built on FOR UPDATE SKIP LOCKED — the standard Postgres idiom for work queues: concurrent claimers lock the rows they take and SKIP rows locked by others, so any number of backend replicas partition the work with no coordinator and no double-claims. State machine per job: pending → running → done ├— error → pending (run_at pushed back: 30s/90s/270s… ≤30min) └— attempts exhausted → failed (kept for inspection) Crash recovery is folded into the claim itself: a "running" job whose claimed_at is older than the stale threshold was orphaned by a dead worker and gets re-claimed — no separate reaper to keep in sync. Optional dedupe_key (partial unique index over live jobs only) makes double submits collapse race-free; attempts increment at CLAIM time so a crash-looping job cannot retry forever. HOW THE PIECES FIT - migrations/20260821_01_db_jobs.sql (+ schema.sql, kept in lockstep for the drift check): table, partial indexes, claim_db_jobs(), service_role only. - lib/dbq/enqueue.ts — enqueueDbJob (unique-violation on the dedupe key is reported as success: the work is already scheduled) and enqueueStorageCleanup (never throws; falls back to today's inline best-effort deletes if the enqueue itself fails). - lib/dbq/runner.ts — 5s poll (DB_JOBS_POLL_MS), batch claim, per-job outcome writes, hourly retention sweep (done 7d, failed 30d, export artifacts 24h — artifact file deleted BEFORE its row, which is the only pointer to it). A missing table (migration not yet applied) logs and retries next tick; it never crashes the server. - lib/dbq/handlers.ts — audit.chat_turn, account.delete, storage.cleanup, export.build. All idempotent; a throw is the retry signal. Their call sites land in the follow-up commits. - index.ts — runner starts unconditionally at boot; graceful shutdown waits for the in-flight tick. Also: the workflow add-on catalog now syncs at boot instead of inside the first unlucky GET /workflow-addons after a deploy (the lazy latch stays as fallback). FIRST CONSUMER: CHAT-TURN AUDIT recordChatTurn ran 1+N sequential fire-and-forget inserts AFTER the SSE stream's [DONE] — the most likely moment for a process to be torn down — and swallowed every failure. The chat routes now enqueue ONE small job (audit.chat_turn) instead; the worker fans out the rows with retries that survive restarts. The row mapping is extracted into pure chatTurnAuditEvents() so the direct fallback path and the handler cannot drift. At-least-once caveat, on purpose: a retry after a partial fan-out can duplicate a row — for an audit trail a rare duplicate beats a silent gap. If the enqueue itself fails, the code falls back to the old direct inserts, so audit can never break the user path. TESTED Unit: state machine (done/retry/terminal/unknown-kind), backoff curve, batch isolation, retention ordering, dedupe-as-success, storage-cleanup fallback, all four handlers. Live against real Postgres/PostgREST (local Supabase): enqueue→claim→done, dedupe unique-violation, retry backoff not re-claimed early, stale-running re-claim, and two CONCURRENT claims over 10 jobs partitioning 5/5 with zero overlap (SKIP LOCKED proven, not assumed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDev46QdqBsdyRCRBxcvk2
amal66
force-pushed
the
olp-pr/durable-queues
branch
from
August 21, 2026 20:46
b982233 to
2f3bca5
Compare
This was referenced Aug 24, 2026
fix: bound client upload concurrency and surface per-file outcomes (issue #8 follow-up to #294)
#381
Draft
amal66
added a commit
to amal66/mike
that referenced
this pull request
Aug 24, 2026
…ewrites clear their stale renditions
WHY THIS MATTERS
The first two commits put ASYNC_DOCUMENT_CONVERSION in front of the five
LibreOffice call sites on the upload/version routes — but a whole-tree
survey found two more sites hiding inside the chat tools, reached through
the SSE stream itself:
1. replicate_document converts the source inline when it has no PDF
rendition to copy (toolDispatcher.ts), and
2. generate_ppt converts the generated PPTX inline
(documentOps.ts / persistGeneratedFile).
With the flag on, a deployment reasonably believes "LibreOffice no longer
runs on my request threads" — yet a chat turn that replicates or generates
a deck still paid the 3-15s LibreOffice cold start in-band, and a
conversion failure was swallowed (devLog) leaving the document permanently
rendition-less with no retry path. Closing these keeps the flag's promise
honest: it now covers every LibreOffice call site in the app.
WHAT IS A RENDITION AND WHO CONSUMES IT
A "rendition" is the per-version converted PDF (pdf_storage_path) that
/single-documents/:id/display serves in place of raw Office bytes. The
frontend routes DOCX to DocxView by FILENAME (docx-preview over raw bytes),
so DOCX renditions are almost never displayed — but PPTX has no dedicated
viewer and renders exclusively through its rendition via PdfView, which is
why generate_ppt's silent conversion failure was a real product gap.
HOW IT WORKS
Both sites follow the exact pattern of the five route sites:
- flag off → inline docxToPdf, byte-for-byte the historical behavior.
- flag on → the document/copy is inserted "ready" with
pdf_storage_path: null and one conversion job per new version is
enqueued (deduped on convert:<versionId>, attempts: 3, exponential
backoff). finalizeDocumentStatus: false because these documents are
usable from their raw bytes — a rendition failure must never flip a
healthy document to "error".
- an enqueue failure degrades to the sync path's conversion-failure
behavior (usable document, no rendition) instead of failing the tool.
THE INVARIANT HALF (accept/reject + in-place re-edit)
Accept/reject of a tracked change and an in-place assistant re-edit both
rewrite a version's DOCX bytes at its existing storage path. Any rendition
recorded for that version now describes bytes that no longer exist — and a
stale rendition is not hypothetical decoration: /display would serve it,
and replicate_document COPIES it onto every replica. Both rewrite sites now
null pdf_storage_path in the same update that re-hashes the content, the
same ordering discipline as the content_sha256 clear-then-set that
surrounds the byte write. In today's flows assistant_edit versions never
carry a rendition, so this is an invariant made explicit, not a behavior
change.
DELIBERATELY NOT DONE
edit_document / generate_docx versions do NOT get renditions enqueued:
DOCX renders through DocxView from raw bytes everywhere (the tracked-
changes UI depends on it), so a rendition would cost a LibreOffice run per
chat edit and display nothing. Recorded here so the omission reads as a
decision, not an oversight.
TESTS
- replicateRenditionQueue.test.ts: flag off → one inline conversion, no
queue; flag on → zero in-band LibreOffice, one job per copy with
per-copy pdfKey and finalize:false; enqueue failure → copies still
usable.
- documentOps.generatedRendition.test.ts: same contract for generate_ppt,
plus xlsx never converts or enqueues (spreadsheets are served raw).
Part of the durable-queues row (#40 → olp PR open-legal-products#294).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JDev46QdqBsdyRCRBxcvk2
amal66
force-pushed
the
olp-pr/durable-queues
branch
from
August 24, 2026 18:39
234a119 to
f59aee9
Compare
amal66
added a commit
to amal66/mike
that referenced
this pull request
Aug 24, 2026
…rminal writes + cross-process job cancellation
WHY THIS MATTERS
POST /tabular-review/:id/clear-cells resets a row's cells to "pending" so a
user can blank bad results and start over. But extraction runs
concurrently with it — inline on another request (sync mode) or on a
worker that survives disconnects (async mode, introduced by this PR). A
run that finished AFTER the clear would write "done" (or "error") straight
over the user's reset, and in async mode a queued-but-unstarted job would
happily re-fill the freshly cleared row seconds later. Durability made the
old race WIDER, so this PR owns the fix.
WHAT IS A LOST UPDATE
Two writers race the same row: the user's reset (status = pending) and the
extraction's terminal write (status = done). Without a guard the last
writer wins, and the extraction — which started before the reset and knows
nothing of it — silently undoes the user's action. The classic cure is an
optimistic condition: make the terminal write assert the state it believes
it owns.
HOW IT WORKS — three layers, weakest to strongest
1. GUARDED TERMINAL WRITES (both modes). Every terminal cell write now
carries AND status = 'generating': the shared core's done-write, the
sync route's missing→error writes, the sync regenerate-cell writes, and
the worker's permanent-failure cleanup. An extraction only ever
finalizes a claim it still holds; if clear-cells revoked the claim, the
write matches zero rows and the SSE/Redis announce is skipped too — a
tailing stream never shows a "done" the DB doesn't hold. The model's
result still counts as "received", so the caller neither marks the
cleared cell "error" nor retries over it.
2. QUEUE CANCELLATION (async mode, flag-gated so sync deployments never
dial Redis). clear-cells addresses every deterministic jobId for the
cleared rows — extract:<review>:<row> and each :<col> variant — and
REMOVES jobs still waiting/delayed, so they never start.
3. PERSISTED CANCEL MARKER for jobs already active. BullMQ's Job#discard()
is only an in-memory flag on the worker's own Job instance — calling it
from the API process is a silent no-op (a live-Redis smoke caught
exactly this: the "discarded" job's retry was scheduled anyway).
Instead the job's data is marked canceled: true via updateData(), which
IS persisted to Redis; each retry attempt re-fetches job data, and
runExtractionJob now returns immediately on the marker instead of
re-claiming the cleared cells from scratch.
TRADEOFF, FLAGGED
The permanent-failure handler now flips only cells still claimed
("generating") to "error". A job that dies before ever claiming its cells
(e.g. the settings lookup fails on all 3 attempts) leaves them "pending" —
a blank, re-runnable state rather than a red one; the resume stream then
runs to its 15-minute cap instead of resolving early. Chosen deliberately:
silently overwriting a user's reset is worse than a quieter failure under
already-broken infrastructure.
TESTS
- tabular.extractRow: a done-write that matches zero rows (cell cleared
mid-flight) suppresses the announce and is not reported "missing".
- extractionWorker: canceled jobs touch nothing; permanent failure leaves
cleared (pending) cells alone while still erroring claimed ones.
- extractionQueue: waiting jobs removed; active jobs get the PERSISTED
updateData marker (never Job#discard); remove() losing the race falls
back to the marker; per-job failures are swallowed (best-effort by
design — the write guards are the correctness layer).
- Live-Redis smoke (real BullMQ): dedupe, remove-waiting, and
cancel-active verified end-to-end — attempt 1 active during the cancel,
attempt 2 saw canceled: true and completed without touching the DB.
Part of the durable-queues row (#40 → olp PR open-legal-products#294).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JDev46QdqBsdyRCRBxcvk2
amal66
added a commit
to amal66/mike
that referenced
this pull request
Aug 24, 2026
…turn audit WHY THIS MATTERS Some workloads must be durable in EVERY deployment — audit trails, account erasure, storage cleanup, export builds. The BullMQ queues (open-legal-products#294) are deliberately opt-in because they require Redis; making THESE workloads depend on an opt-in would mean the default deployment keeps losing audit rows and leaking storage. Every deployment — web, Word add-in stack, Mac app Docker stack — already runs Postgres, so this queue is built on the database that is already there and runs BY DEFAULT with zero new infrastructure or configuration. DB_JOBS_ENABLED=false exists only as an operational escape hatch. WHAT IS A POSTGRES JOB QUEUE A db_jobs table plus one claim function built on FOR UPDATE SKIP LOCKED — the standard Postgres idiom for work queues: concurrent claimers lock the rows they take and SKIP rows locked by others, so any number of backend replicas partition the work with no coordinator and no double-claims. State machine per job: pending → running → done ├— error → pending (run_at pushed back: 30s/90s/270s… ≤30min) └— attempts exhausted → failed (kept for inspection) Crash recovery is folded into the claim itself: a "running" job whose claimed_at is older than the stale threshold was orphaned by a dead worker and gets re-claimed — no separate reaper to keep in sync. Optional dedupe_key (partial unique index over live jobs only) makes double submits collapse race-free; attempts increment at CLAIM time so a crash-looping job cannot retry forever. HOW THE PIECES FIT - migrations/20260821_01_db_jobs.sql (+ schema.sql, kept in lockstep for the drift check): table, partial indexes, claim_db_jobs(), service_role only. - lib/dbq/enqueue.ts — enqueueDbJob (unique-violation on the dedupe key is reported as success: the work is already scheduled) and enqueueStorageCleanup (never throws; falls back to today's inline best-effort deletes if the enqueue itself fails). - lib/dbq/runner.ts — 5s poll (DB_JOBS_POLL_MS), batch claim, per-job outcome writes, hourly retention sweep (done 7d, failed 30d, export artifacts 24h — artifact file deleted BEFORE its row, which is the only pointer to it). A missing table (migration not yet applied) logs and retries next tick; it never crashes the server. - lib/dbq/handlers.ts — audit.chat_turn, account.delete, storage.cleanup, export.build. All idempotent; a throw is the retry signal. Their call sites land in the follow-up commits. - index.ts — runner starts unconditionally at boot; graceful shutdown waits for the in-flight tick. Also: the workflow add-on catalog now syncs at boot instead of inside the first unlucky GET /workflow-addons after a deploy (the lazy latch stays as fallback). FIRST CONSUMER: CHAT-TURN AUDIT recordChatTurn ran 1+N sequential fire-and-forget inserts AFTER the SSE stream's [DONE] — the most likely moment for a process to be torn down — and swallowed every failure. The chat routes now enqueue ONE small job (audit.chat_turn) instead; the worker fans out the rows with retries that survive restarts. The row mapping is extracted into pure chatTurnAuditEvents() so the direct fallback path and the handler cannot drift. At-least-once caveat, on purpose: a retry after a partial fan-out can duplicate a row — for an audit trail a rare duplicate beats a silent gap. If the enqueue itself fails, the code falls back to the old direct inserts, so audit can never break the user path. TESTED Unit: state machine (done/retry/terminal/unknown-kind), backoff curve, batch isolation, retention ordering, dedupe-as-success, storage-cleanup fallback, all four handlers. Live against real Postgres/PostgREST (local Supabase): enqueue→claim→done, dedupe unique-violation, retry backoff not re-claimed early, stale-running re-claim, and two CONCURRENT claims over 10 jobs partitioning 5/5 with zero overlap (SKIP LOCKED proven, not assumed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDev46QdqBsdyRCRBxcvk2
Collaborator
Author
Manual cross-transport test round — 2026-08-24 (post-rebase
|
| # | Scenario | Result | Evidence |
|---|---|---|---|
| A1 | Fresh boot contract | ✅ | workers: thread, all 3 BullMQ workers in the worker thread, [dbq] runner (poll 60000ms, driver redis), workflow-sync one-shot imported 141 workflows (#376 pathway intact beside Redis) |
| A2 | DOCX upload → queued conversion | ✅ | Upload returned 201 instantly; [conversion-worker] converted; PDF rendition present in document_versions |
| A3 | Tabular run, async queue + lease | ✅ | 4 rows × 2 cols; live SSE cell updates in the grid; cells done with real LLM content; lease taken and released; per-row job ids extract_<review>_<row> — no colons (the b71a56c fix, observed live) |
| A4 | Kill tab mid-run → durability | ✅ | Tab killed immediately after Run; workers finished all 8 cells anyway; reopened page shows the completed grid |
| A5 | Lease 409 semantics | ✅ | Stale expected_updated_at → 409 review_stale; two concurrent generates → one SSE stream + one 409 review_running |
| A6 | clear-cells + queued regenerate-cell | ✅ | 8 error-cells → pending (cancellation runs under the lease); regenerate-cell via queue returned the cell JSON in 2.6 s; lease released, zero stamps left |
| A7 | Durable chat-turn audit (outbox) | ✅ | audit.chat_turn flipped done in 128 ms; chat.message event fanned out; History shows it with model + display-name column (main's #374/#365 features intact) |
| A8 | Async exports | ✅ | History CSV: POST /user/exports 202 → poll → download; header byte-identical to main (created_at,user,action,status,title,application,project_id,model); documents-zip = valid ZIP (PK, 34.8 KB/4 docs); account JSON 47.8 KB |
| A10 | Storage-failure fault injection | ✅ | MinIO stopped → document delete still succeeds (row-first); storage.cleanup job fails 3/3 deletes failed, re-queues with backoff; MinIO restarted → attempt 3 → done |
| A11 | Account erasure | ✅ | account.delete done in 1 attempt; all user rows cascaded to zero (docs/reviews/chats/workflows/auth.users); login → invalid_credentials; storage cleanup job done |
| — | Failure semantics (bonus, found by accident) | ✅ | With an invalid provider key, jobs retried then marked cells error, cleared stamps, released the lease — no stuck spinners, no held lease |
B. Postgres fallback transport
| # | Scenario | Result | Evidence |
|---|---|---|---|
| B1 | QUEUE_DRIVER=postgres + crash recovery |
✅ | Boot: driver postgres, poll 5000ms, no BullMQ workers. Run started, backend docker killed mid-run → 3 cells stranded generating, 2 jobs stranded running. After restart: pending jobs picked in ≤5 s; stale running claims re-claimed (attempts=2) — all 8 cells done. (Stale window simulated by aging claimed_at 11 min via SQL rather than waiting out the 600 s default.) |
| B2 | Upgrade-in-place invariance | ✅ | No REDIS_URL, no ASYNC_* flags → driver postgres, no BullMQ workers, no Redis dial, clean SIGTERM shutdown. (First attempt used a wrong flag name and usefully confirmed the legacy ASYNC_DOCUMENT_CONVERSION=true → redis resolution rule.) |
C. Main-regression spot checks (features that landed on main during the rebase window)
Signup → #365 onboarding (profile + practice + skip) ✅ · #374 tabular UX and is_running plumbing ✅ (exercised throughout A3–A6) · #376/#380 DB-backed workflow catalog: all 141 workflows + packs render, add-on Import works ✅ · #335 dark mode toggles ✅ · History page + filters ✅
Not covered live (and why)
- Word add-in
surface:"word"audit — needs a Word host; covered by the branch's unit tests (wordChataudit enqueue). - MCP token-refresh sweep — needs a connected MCP OAuth grant; covered by unit tests (4xx=dead-grant / 5xx-retry classification).
- Watching the resume stream reattach mid-flight — runs on this fixture complete in ~2 s, too fast to reattach visually; durability itself is proven by A4 + B1.
- The 600 s stale-claim window elapsing naturally (simulated via SQL aging in B1).
CI
All 10 checks green on f59aee93 (CodeQL, frontend coverage ratchet, and playwright — the three that were red before the rebase — now pass). The three prior CodeQL highs are fixed in 5264b007 with regression tests; frontend coverage floors raised per the config's own rule in f59aee93.
Test account was a throwaway (qa-294@test.local) created and erased as part of the round.
amal66
marked this pull request as ready for review
August 24, 2026 19:28
amal66
added a commit
to amal66/mike
that referenced
this pull request
Aug 24, 2026
WHY NOW The module split moves route code wholesale, so CodeQL treats every moved line as new and re-flags patterns that predate this PR. Four js/tainted-format-string highs fired — all the same shape open-legal-products#294 already fixed in routes/documents.ts (5264b00): a user-controlled filename interpolated into console.error's format-string position, where a name containing %s/%d would eat the error argument. THE FIX Same convention as 5264b00: the message is a constant string and the filename travels as a structured argument, at the four moved sites — documents.upload, documents.versions (upload + replace), and projects.documents. The [versions/copy] site already carries the fix from open-legal-products#294's commit; these four bring the moved code up to the same rule. Gates: backend tsc clean, 858 tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0132PfwQ6VviSeCRgdGhiq9Z
amal66
added a commit
to amal66/mike
that referenced
this pull request
Aug 24, 2026
Express 4 does not forward async handler rejections, and the routes that open-legal-products#295 left outside modules/ were unguarded — a throw in any of them meant an unhandled rejection and a socket held open until client timeout. Only the workflows router defended itself. Its asyncRoute wrapper is now a shared middleware (middleware/asyncRoute.ts) applied to audit, quickActions, sourceDocuments, wordChat and workflowAddons, so a rejection reaches app.ts's handleUnhandledError instead of nothing. routerErrorHandler now only attributes the failure to a router in the log and delegates the response to handleUnhandledError. Answering with a router-specific 500 body would have contradicted the internal-error contract main established: body-parser's 400/413 keep their own status and code, everything else is the opaque {code:"internal_error"} body. Also fixed in those route files, where errors were being silently converted to success or not-found: - quickActions: DELETE returned 204 with no row matched (now selects the deleted ids and 404s when none); DB failures on the workflow lookup read as 404 (now 500, via the now-exported workflows.service.resolveWorkflowAccess instead of a drifted private copy). - workflowAddons: the import response hand-rebuilt the workflow shape and had drifted from GET /workflows/:id on four fields — now uses the exported withDatabaseWorkflow; a failed add-on lookup answered 404 (now 500). - audit: lib/auditExport's private project-access helper duplicated lib/access.listAccessibleProjectIds and had drifted from it (it re-counted the caller's own shared projects and used a different `contains` encoding). - sourceDocuments: a missing or rejected CourtListener token answered 502, sending the user to check a service that was fine; credential failures now answer 400 with a fixed message that never echoes the upstream body. The history page's surface filter learns the "word" label, so the durable Word chat audit rows open-legal-products#294 introduced (surface "word") are filterable rather than showing a raw key. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…xtraction WHY THIS MATTERS Two workloads in Mike are expensive and can outlive the HTTP request that started them: DOCX -> PDF conversion (LibreOffice) and tabular-review cell extraction (one LLM call per row). Today both run inline on the request thread, so a closed laptop lid, a dropped connection, or a server restart mid-run silently loses the work — the review grid is left with spinners that never resolve, and a large upload blocks its request on LibreOffice. WHAT IS A DURABLE JOB QUEUE A job queue moves work out of the request/response cycle: the request records WHAT should happen (a small JSON payload in Redis) and returns; a worker process picks the job up, runs it, and retries it with exponential backoff if it fails. "Durable" means the job survives the death of the thing that created it — the queue (BullMQ on Redis) holds the job until a worker finishes it, no matter what happens to the original HTTP request or even the server process (BullMQ re-queues jobs whose worker crashed via its stalled-job detection). The classic hazard of queues is the DOUBLE SUBMIT: a client that reconnects and re-POSTs would enqueue the same work twice. This design pushes correctness into the queue's identity model — every job's id is derived deterministically from the work itself (`convert:<versionId>`, `extract:<reviewId>:<rowId>`), so BullMQ collapses a duplicate submit into the already-in-flight job. Durable STATE lives only in Postgres (documents.status, tabular_cells); jobs re-read that state when they run and skip columns already done, which is what makes retries idempotent. HOW IT WORKS - Both queues are OFF by default and opt-in per deployment: ASYNC_DOCUMENT_CONVERSION / ASYNC_TABULAR_EXTRACTION (default "false"). With the flags off the server never dials Redis — the queue connection is created lazily and only reached via enqueue/startWorkers, so a fresh clone still runs fully synchronously with zero new infrastructure. - lib/queue/: a shared lazy Redis connection (maxRetriesPerRequest: null, which BullMQ's blocking commands require), the two queues with deterministic jobIds + retry/backoff, and runProgress — a Redis pub/sub bridge that carries per-cell progress frames from workers to any HTTP request that is watching. - workers/: conversionWorker (DOCX->PDF off the request thread; conversion failure finalizes the document without a PDF rendition, matching the sync path) and extractionWorker (throws on incomplete extraction so BullMQ retries; after the last retry a permanent-failure handler flips surviving cells to "error" so the grid never shows an eternal spinner). A declarative registry + startWorkers()/stopWorkers() lifecycle, started from index.ts only when a flag is on, with graceful SIGTERM/SIGINT drain. - lib/tabular/: the extraction core factored out of routes/tabular.ts so the synchronous route and the async worker share ONE loop (extractRowColumns). The unit of work is the review ROW — one document, or a folder of source documents extracted together — matching the row model main adopted for folder-grouped reviews. tabular.rows.ts carries the row loaders (loadReviewRows / loadRowDocumentText) that both the routes and the worker need. - POST /:reviewId/generate keeps its exact synchronous behavior by default; with the flag on it enqueues one job per row, subscribes to the review's progress channel BEFORE enqueuing (so a fast worker cannot publish into the void), and forwards the same cell_update SSE frames the sync path emits. A 3-second DB-poll backstop reconciles any missed pub/sub frame, so a dropped message can never hang the stream. A new GET /:reviewId/generate/stream lets a disconnected client reattach to a running generation without re-triggering work. Ported from amal66/mike (upstream-pr/durable-queues, #40) and re-derived against current main: the extraction core is row-based (not document-based) to match the folder-grouped row model (open-legal-products#274) and db pagination (open-legal-products#263) that landed after the original branch, and the moved helper bodies match main's current copies byte-for-byte (multi-document citation prompts, Ollama key exemption). Tests: 510 passing (was 499), including queue jobId determinism, worker idempotency/retry/permanent-failure policy, row extraction core, and the pending-cell targeting used by the reconnectable stream. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…enerate-cell, stale-work reaper, and a frontend that can consume it
WHY THIS MATTERS
The first commit made two workloads durable, but a feature flag is only
real if flipping it on produces a working product. Three gaps stood in
the way. First, ASYNC_DOCUMENT_CONVERSION covered one of the five places
that spawn LibreOffice — project uploads, added versions, replaced
versions and document-to-version copies still blocked their requests for
seconds to minutes. Second, regenerate-cell ran a full LLM extraction
inline even with the extraction queue enabled, and a crash mid-call
stranded the cell in "generating" forever. Third, the frontend had no
way to see async results: nothing polled a "processing" document, and
the reconnectable generate stream had zero callers — enabling the flags
produced spinners that never resolved.
WHAT IS AN ORPHANED TRANSIENT STATE
Transient statuses ("processing", "generating") encode a promise: some
running code will eventually write a terminal state. A crash in the
window between the transient write and the terminal write breaks the
promise, and because nothing else owns the row, the lie persists
forever. The fix has two halves: narrow the windows (hand the work to a
queue that retries and survives restarts) and add an owner of last
resort (a reaper that flips provably-orphaned rows to "error").
HOW IT WORKS
- Conversion queue covers all five LibreOffice call sites. Version flows
pass a per-version pdfKey (renditions of different versions must not
collide on the document-level key) and finalizeDocumentStatus: false —
their document is already "ready", so a rendition failure must not
flip a healthy document to "error"; only the initial-upload flow parks
the document "processing" and lets the worker finalize it. Terminal
conversion jobs are now removed immediately (same rationale as
extraction): replace-file reuses the versionId, and a lingering
completed job record would silently dedupe the re-conversion.
- Regenerate-cell becomes a single-cell job: payload gains columnIndex,
jobId gains a column suffix (extract:<review>:<row>:<col>) so it never
dedupes against a full-row job, and the worker narrows to that one
column. The route keeps its synchronous JSON contract by waiting on
the cell's terminal state (pub/sub + DB-poll backstop); if the wait
budget elapses it answers 202 {status:"generating"} — the job keeps
running and the client catches up through the resume stream. The
disconnect-divergence bug (client marks error, backend later writes
done) is gone: the DB is the only authority.
- Stale-work reaper (lib/maintenance/staleWork.ts, swept at boot + every
10 min): documents "processing" past a 30-minute age gate with no live
conversion job flip to "error"; "generating" cells with no live job
flip to "error" (async mode only — cells have no timestamp column, so
in sync mode a live inline run is indistinguishable from an orphan).
Job existence is the liveness signal, which immediate job removal
makes trustworthy.
- Frontend catch-up: GET /single-documents/:documentId exists so the
client can poll one document instead of refetching the collection;
DocTable polls pending/processing rows every 3s and merges status
changes through the existing update path. The tabular view now aborts
its generate stream on unmount, reconnects once through
GET /generate/stream on a dropped stream, resumes an in-flight run
found at mount (cells still "generating"), and treats regenerate's
202 as "keep the skeleton, tail the stream" instead of an error.
- The GET stream view no longer dials Redis in synchronous deployments
(the subscribe is flag-gated; the DB-poll backstop does the resolving
there), so the no-Redis-by-default invariant holds for every new path.
Tests: backend 523 passing (+13: payload passthrough, per-version pdf
keys, finalize semantics, single-cell narrowing in worker + failure
handler, reaper liveness/age-gate/no-op cases); frontend 174 passing,
tsc and production build clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ewrites clear their stale renditions
WHY THIS MATTERS
The first two commits put ASYNC_DOCUMENT_CONVERSION in front of the five
LibreOffice call sites on the upload/version routes — but a whole-tree
survey found two more sites hiding inside the chat tools, reached through
the SSE stream itself:
1. replicate_document converts the source inline when it has no PDF
rendition to copy (toolDispatcher.ts), and
2. generate_ppt converts the generated PPTX inline
(documentOps.ts / persistGeneratedFile).
With the flag on, a deployment reasonably believes "LibreOffice no longer
runs on my request threads" — yet a chat turn that replicates or generates
a deck still paid the 3-15s LibreOffice cold start in-band, and a
conversion failure was swallowed (devLog) leaving the document permanently
rendition-less with no retry path. Closing these keeps the flag's promise
honest: it now covers every LibreOffice call site in the app.
WHAT IS A RENDITION AND WHO CONSUMES IT
A "rendition" is the per-version converted PDF (pdf_storage_path) that
/single-documents/:id/display serves in place of raw Office bytes. The
frontend routes DOCX to DocxView by FILENAME (docx-preview over raw bytes),
so DOCX renditions are almost never displayed — but PPTX has no dedicated
viewer and renders exclusively through its rendition via PdfView, which is
why generate_ppt's silent conversion failure was a real product gap.
HOW IT WORKS
Both sites follow the exact pattern of the five route sites:
- flag off → inline docxToPdf, byte-for-byte the historical behavior.
- flag on → the document/copy is inserted "ready" with
pdf_storage_path: null and one conversion job per new version is
enqueued (deduped on convert:<versionId>, attempts: 3, exponential
backoff). finalizeDocumentStatus: false because these documents are
usable from their raw bytes — a rendition failure must never flip a
healthy document to "error".
- an enqueue failure degrades to the sync path's conversion-failure
behavior (usable document, no rendition) instead of failing the tool.
THE INVARIANT HALF (accept/reject + in-place re-edit)
Accept/reject of a tracked change and an in-place assistant re-edit both
rewrite a version's DOCX bytes at its existing storage path. Any rendition
recorded for that version now describes bytes that no longer exist — and a
stale rendition is not hypothetical decoration: /display would serve it,
and replicate_document COPIES it onto every replica. Both rewrite sites now
null pdf_storage_path in the same update that re-hashes the content, the
same ordering discipline as the content_sha256 clear-then-set that
surrounds the byte write. In today's flows assistant_edit versions never
carry a rendition, so this is an invariant made explicit, not a behavior
change.
DELIBERATELY NOT DONE
edit_document / generate_docx versions do NOT get renditions enqueued:
DOCX renders through DocxView from raw bytes everywhere (the tracked-
changes UI depends on it), so a rendition would cost a LibreOffice run per
chat edit and display nothing. Recorded here so the omission reads as a
decision, not an oversight.
TESTS
- replicateRenditionQueue.test.ts: flag off → one inline conversion, no
queue; flag on → zero in-band LibreOffice, one job per copy with
per-copy pdfKey and finalize:false; enqueue failure → copies still
usable.
- documentOps.generatedRendition.test.ts: same contract for generate_ppt,
plus xlsx never converts or enqueues (spreadsheets are served raw).
Part of the durable-queues row (#40 → olp PR open-legal-products#294).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JDev46QdqBsdyRCRBxcvk2
…rminal writes + cross-process job cancellation
WHY THIS MATTERS
POST /tabular-review/:id/clear-cells resets a row's cells to "pending" so a
user can blank bad results and start over. But extraction runs
concurrently with it — inline on another request (sync mode) or on a
worker that survives disconnects (async mode, introduced by this PR). A
run that finished AFTER the clear would write "done" (or "error") straight
over the user's reset, and in async mode a queued-but-unstarted job would
happily re-fill the freshly cleared row seconds later. Durability made the
old race WIDER, so this PR owns the fix.
WHAT IS A LOST UPDATE
Two writers race the same row: the user's reset (status = pending) and the
extraction's terminal write (status = done). Without a guard the last
writer wins, and the extraction — which started before the reset and knows
nothing of it — silently undoes the user's action. The classic cure is an
optimistic condition: make the terminal write assert the state it believes
it owns.
HOW IT WORKS — three layers, weakest to strongest
1. GUARDED TERMINAL WRITES (both modes). Every terminal cell write now
carries AND status = 'generating': the shared core's done-write, the
sync route's missing→error writes, the sync regenerate-cell writes, and
the worker's permanent-failure cleanup. An extraction only ever
finalizes a claim it still holds; if clear-cells revoked the claim, the
write matches zero rows and the SSE/Redis announce is skipped too — a
tailing stream never shows a "done" the DB doesn't hold. The model's
result still counts as "received", so the caller neither marks the
cleared cell "error" nor retries over it.
2. QUEUE CANCELLATION (async mode, flag-gated so sync deployments never
dial Redis). clear-cells addresses every deterministic jobId for the
cleared rows — extract:<review>:<row> and each :<col> variant — and
REMOVES jobs still waiting/delayed, so they never start.
3. PERSISTED CANCEL MARKER for jobs already active. BullMQ's Job#discard()
is only an in-memory flag on the worker's own Job instance — calling it
from the API process is a silent no-op (a live-Redis smoke caught
exactly this: the "discarded" job's retry was scheduled anyway).
Instead the job's data is marked canceled: true via updateData(), which
IS persisted to Redis; each retry attempt re-fetches job data, and
runExtractionJob now returns immediately on the marker instead of
re-claiming the cleared cells from scratch.
TRADEOFF, FLAGGED
The permanent-failure handler now flips only cells still claimed
("generating") to "error". A job that dies before ever claiming its cells
(e.g. the settings lookup fails on all 3 attempts) leaves them "pending" —
a blank, re-runnable state rather than a red one; the resume stream then
runs to its 15-minute cap instead of resolving early. Chosen deliberately:
silently overwriting a user's reset is worse than a quieter failure under
already-broken infrastructure.
TESTS
- tabular.extractRow: a done-write that matches zero rows (cell cleared
mid-flight) suppresses the announce and is not reported "missing".
- extractionWorker: canceled jobs touch nothing; permanent failure leaves
cleared (pending) cells alone while still erroring claimed ones.
- extractionQueue: waiting jobs removed; active jobs get the PERSISTED
updateData marker (never Job#discard); remove() losing the race falls
back to the marker; per-job failures are swallowed (best-effort by
design — the write guards are the correctness layer).
- Live-Redis smoke (real BullMQ): dedupe, remove-waiting, and
cancel-active verified end-to-end — attempt 1 active during the cancel,
attempt 2 saw canceled: true and completed without touching the DB.
Part of the durable-queues row (#40 → olp PR open-legal-products#294).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JDev46QdqBsdyRCRBxcvk2
…turn audit WHY THIS MATTERS Some workloads must be durable in EVERY deployment — audit trails, account erasure, storage cleanup, export builds. The BullMQ queues (open-legal-products#294) are deliberately opt-in because they require Redis; making THESE workloads depend on an opt-in would mean the default deployment keeps losing audit rows and leaking storage. Every deployment — web, Word add-in stack, Mac app Docker stack — already runs Postgres, so this queue is built on the database that is already there and runs BY DEFAULT with zero new infrastructure or configuration. DB_JOBS_ENABLED=false exists only as an operational escape hatch. WHAT IS A POSTGRES JOB QUEUE A db_jobs table plus one claim function built on FOR UPDATE SKIP LOCKED — the standard Postgres idiom for work queues: concurrent claimers lock the rows they take and SKIP rows locked by others, so any number of backend replicas partition the work with no coordinator and no double-claims. State machine per job: pending → running → done ├— error → pending (run_at pushed back: 30s/90s/270s… ≤30min) └— attempts exhausted → failed (kept for inspection) Crash recovery is folded into the claim itself: a "running" job whose claimed_at is older than the stale threshold was orphaned by a dead worker and gets re-claimed — no separate reaper to keep in sync. Optional dedupe_key (partial unique index over live jobs only) makes double submits collapse race-free; attempts increment at CLAIM time so a crash-looping job cannot retry forever. HOW THE PIECES FIT - migrations/20260821_01_db_jobs.sql (+ schema.sql, kept in lockstep for the drift check): table, partial indexes, claim_db_jobs(), service_role only. - lib/dbq/enqueue.ts — enqueueDbJob (unique-violation on the dedupe key is reported as success: the work is already scheduled) and enqueueStorageCleanup (never throws; falls back to today's inline best-effort deletes if the enqueue itself fails). - lib/dbq/runner.ts — 5s poll (DB_JOBS_POLL_MS), batch claim, per-job outcome writes, hourly retention sweep (done 7d, failed 30d, export artifacts 24h — artifact file deleted BEFORE its row, which is the only pointer to it). A missing table (migration not yet applied) logs and retries next tick; it never crashes the server. - lib/dbq/handlers.ts — audit.chat_turn, account.delete, storage.cleanup, export.build. All idempotent; a throw is the retry signal. Their call sites land in the follow-up commits. - index.ts — runner starts unconditionally at boot; graceful shutdown waits for the in-flight tick. Also: the workflow add-on catalog now syncs at boot instead of inside the first unlucky GET /workflow-addons after a deploy (the lazy latch stays as fallback). FIRST CONSUMER: CHAT-TURN AUDIT recordChatTurn ran 1+N sequential fire-and-forget inserts AFTER the SSE stream's [DONE] — the most likely moment for a process to be torn down — and swallowed every failure. The chat routes now enqueue ONE small job (audit.chat_turn) instead; the worker fans out the rows with retries that survive restarts. The row mapping is extracted into pure chatTurnAuditEvents() so the direct fallback path and the handler cannot drift. At-least-once caveat, on purpose: a retry after a partial fan-out can duplicate a row — for an audit trail a rare duplicate beats a silent gap. If the enqueue itself fails, the code falls back to the old direct inserts, so audit can never break the user path. TESTED Unit: state machine (done/retry/terminal/unknown-kind), backoff curve, batch isolation, retention ordering, dedupe-as-success, storage-cleanup fallback, all four handlers. Live against real Postgres/PostgREST (local Supabase): enqueue→claim→done, dedupe unique-violation, retry backoff not re-claimed early, stale-running re-claim, and two CONCURRENT claims over 10 jobs partitioning 5/5 with zero overlap (SKIP LOCKED proven, not assumed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDev46QdqBsdyRCRBxcvk2
… deletes
WHY THIS MATTERS
Deletion was the least reliable operation in the app, in two distinct ways:
1. DELETE /user/account ran an unbounded multi-table cascade plus N storage
deletes INLINE in the request, then deleted the auth user. A crash,
restart, or client timeout partway left a half-deleted account that
still existed — the worst possible GDPR posture.
2. Every storage-object delete in the tree was fire-and-forget
(`deleteFile(...).catch(() => {})`): one storage hiccup and the bytes
leaked forever, invisibly — document versions, project documents,
library folders, workflow reference files, failed-upload rollbacks.
WHAT CHANGES
- DELETE /user/account REVERSES its ordering: the auth user is deleted
first (from the user's view the account is gone instantly, sessions
revoked, and a failure here changes nothing — cleanly retriable), then
the data cascade is enqueued as a durable account.delete job (deduped
per user, up to 20 attempts). The cascade is all deletes — idempotent —
so a crash mid-run simply re-runs. If even the enqueue fails, the old
inline cascade runs as fallback rather than stranding the data. The
handler also erases the user's leftovers in the queue itself (export
artifacts hold a full copy of their data; queued audit payloads hold
titles and prompts) and account erasure now purges the exports/<user>/
storage prefix too.
- Every fire-and-forget storage delete becomes a durable storage.cleanup
job, with a consistent ROWS FIRST, FILES SECOND discipline: if the row
delete fails, no file has been touched and the data stays intact; if the
process dies after it, the queued job still removes the files with
retries. The handler deletes what it can each attempt and throws so the
retry re-runs the idempotent remainder. Converted sites: single-document
delete, project-document delete, library bulk/folder delete,
deleteUserProjects (collect paths before the version rows go away),
workflow deletion fan-out, workflow reference upload/replace rollbacks
and reference deletes (which previously deleted the file BEFORE the row
— a failure order that could leave a row pointing at deleted bytes).
WHY A QUEUE AND NOT A TRANSACTION
The DB cascade and the storage deletes span two systems (Postgres + object
storage) that cannot commit atomically. The job gives the cross-system
half what a transaction cannot: at-least-once completion with retries that
survive restarts, in every deployment (the DB queue needs only Postgres —
see the previous commit).
TESTED
Handler tests cover the cascade + queue-leftover purge and the
partial-failure-then-retry contract of storage.cleanup;
enqueueStorageCleanup's fall-back-to-inline path is pinned so a queue
outage degrades to exactly today's behavior, never worse.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JDev46QdqBsdyRCRBxcvk2
…request builds
WHY THIS MATTERS
The account/chats/tabular exports built their entire JSON payload in
memory INSIDE the GET request. A large account meant a slow response
racing the HTTP timeout; a dropped tab threw the whole build away; a
re-click started a second full build. Nothing was retryable and nothing
survived a restart.
HOW IT WORKS
Exports now ride the DB queue end to end:
- POST /user/exports { type } enqueues an export.build job (202 with the
job id). Deduped per (user, type) via the queue's live dedupe key, so
double clicks and impatient retries collapse into the running build —
and a build that outlives the tab is simply found again by the next
click.
- The worker builds the export off the request thread (3 attempts with
backoff), parks the artifact under exports/<userId>/<jobId>-<file>.json
in storage, and records the same export.* audit action the sync route
wrote.
- GET /user/exports/:id reports pending/done/failed; GET
/user/exports/:id/download streams the artifact with an attachment
disposition. Both are authenticated, MFA-gated like the old routes, and
ownership-checked per request — deliberately NOT the signed
/download/:token route, which only serves paths backed by a live
document_versions row and would 404 on an export artifact.
- Artifacts expire after 24 hours: the runner's retention sweep deletes
the file first, then the row (the row is the only pointer to the file),
and account erasure purges the whole exports/<userId>/ prefix plus the
user's export jobs.
- The frontend (Settings → Privacy & Data) schedules, polls every 2s (the
DocTable pattern), then downloads via the same blob-anchor flow as
before — same buttons, same MFA retry handling, same UX, just durable
underneath. The legacy synchronous GET /user/*/export routes remain for
curl users and older clients.
DELIBERATELY KEPT SYNCHRONOUS
The audit-history CSV (bounded by EXPORT_LIMIT and filter params) and the
document ZIP (bounded by the user's selection, immediate-download UX).
Queueing those would add polling friction to their common small case;
they are listed as follow-up candidates instead.
TESTED
Handler unit tests (artifact path shape, content type, audit action,
malformed payload rejection) and a frontend test that pins the wiring:
schedule → poll (pending → done) → download, and a failed build surfacing
an error with no download. Live smoke against real Postgres exercised the
full enqueue→claim→done path this flow rides.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JDev46QdqBsdyRCRBxcvk2
…tic Postgres fallback WHY THIS MATTERS The PR so far had two queue systems with disjoint coverage: BullMQ (conversion/extraction, opt-in, Redis-required) and the Postgres DB queue (audit/deletes/exports, default-on). That split forced a workload to pick its transport at design time. This commit turns them into ONE contract — every queued workload runs on whichever transport the deployment has: Redis configured -> BullMQ delivers instantly (+ live pub/sub progress) no Redis -> the Postgres queue carries the same jobs, unchanged so new installs (which ship Redis) get BullMQ for EVERYTHING while a bare-metal deployment that never configures Redis still gets every durability guarantee, just with poll latency. WHAT IS A TRANSACTIONAL OUTBOX For the registry jobs (audit, deletion, cleanup, exports, …) the db_jobs row stays the single durable record — the "outbox". With Redis available, enqueue ALSO hands the row's id to a new app-jobs BullMQ queue for instant pickup; the worker then claims the row through a new claim_db_job(id) RPC before running it. Claiming through Postgres — never trusting the delivery — is what makes this safe: a duplicated delivery (BullMQ replay, the poll backstop racing it, an operator re-enqueue) matches zero rows on the conditional claim and becomes a no-op, and a LOST delivery is recovered by the poller, which drops to a 60s backstop cadence in Redis mode (5s remains the primary cadence without Redis). Retries are redelivered through BullMQ at their backoff time so they don't wait for the backstop. Delivery jobs carry attempts: 1 — the durable record and the poller ARE the retry mechanism; BullMQ is never a second source of truth. DRIVER RESOLUTION (lib/dbq/driver.ts) QUEUE_DRIVER=redis|postgres wins; else REDIS_URL set -> redis; else a legacy ASYNC_* flag on -> redis (those flags always meant "BullMQ against REDIS_URL", so flag-on installs keep their semantics); else postgres — which is every pre-existing default deployment, untouched. CONVERSION/EXTRACTION ON THE FALLBACK enqueueConversion/enqueueExtraction route to the DB queue under the postgres driver with the SAME identity (the BullMQ jobId doubles as the dedupe key), the same retry budget, and the same job bodies (runConversionJob/runExtractionJob). Domain-level permanent-failure semantics are reproduced via per-kind failure hooks (document -> "error" only in the finalize flow; markExtractionFailed for cells). Live progress publish becomes a silent no-op without Redis — the SSE views' DB-poll backstops already resolve every cell (they had to, for missed pub/sub frames), so the UX degrades to 3s-granularity updates rather than breaking. clear-cells cancellation goes through a new cancel_db_jobs(keys) RPC: pending rows deleted, running rows get the persisted `canceled` payload marker — the exact analogue of the BullMQ remove + updateData split. The stale-work reaper checks job liveness on whichever transport is active. TESTED Unit: outbox delivery on enqueue (and enqueue surviving delivery failure), claim-through-Postgres no-op on duplicates, retry redelivery timing, postgres-driver routing for both workloads incl. the cancel RPC. Live on real Redis + Postgres: enqueue -> BullMQ delivery -> claim -> done in under a second, duplicate delivery claiming zero rows. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDev46QdqBsdyRCRBxcvk2
…point, async-on for new installs
WHY THIS MATTERS
Three deployment-shape problems, one seam:
1. Queue workers shared the API process's event loop — a CPU-heavy job
(zip building, export serialization, pdf parsing) could starve HTTP
requests.
2. There was no way to run workers on separate hardware without code
changes.
3. New installs got the SYNCHRONOUS defaults even though their compose
stack could trivially ship Redis — while flipping the code defaults
would break every existing deployment that upgrades in place (git/
docker pull picks up new code; only forks pinned to old commits would
be safe). Defaults therefore move in the BOOTSTRAP ARTIFACTS, never in
code.
HOW IT WORKS
- workerRuntime.ts bundles everything that processes background work
(BullMQ workers, DB-queue runner, stale-work reaper, MCP token-refresh
sweep, catalog boot sync) behind one startAllWorkers/stopAllWorkers
pair, so the same code runs in any of three homes selected by
WORKERS_MODE:
thread (default) — a worker_thread inside the API process. Dev (tsx)
spawns the .ts entry through tsx's CJS require hook; prod spawns the
compiled .js. A crashed thread respawns after 5s — durable state
(db_jobs, Redis) means nothing is lost across the gap.
inline — the historical single-thread behavior (escape hatch).
none — the API runs no workers; a standalone process (src/worker.ts,
`node dist/worker.js`) runs them instead: a separate container or
machine on the same Postgres/Redis. Scale-out is safe by
construction — BullMQ partitions per connection and the DB queue
claims with FOR UPDATE SKIP LOCKED, so N workers divide jobs, never
duplicate them. This is the "async computers" seam.
- Graceful shutdown is coordinated across homes: SIGTERM drains HTTP,
posts "shutdown" to the thread (or stops inline workers), and the
standalone worker has its own SIGTERM handling with a force-exit guard.
- docker-compose.yml (the new-install path) now ships a Redis service
(AOF persistence so queued jobs survive a Redis restart) and sets
REDIS_URL + both ASYNC_* flags on the backend AT THE COMPOSE LEVEL:
fresh `docker compose up` deployments run BullMQ for everything, while
non-compose deployments that pull this commit see zero change. A
commented `worker` service documents the dedicated-worker mode.
VERIFIED LIVE (all three homes)
Dev tsx + thread: boots, DB runner inside the thread, clean SIGTERM.
Prod compiled + thread: same through dist/. Standalone worker process:
boots the right driver-gated worker set and shuts down cleanly. Redis
driver boots all three BullMQ workers with the poller at backstop cadence;
postgres driver boots pollers only.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JDev46QdqBsdyRCRBxcvk2
…Word audit, MCP refresh, text precompute WHY THIS MATTERS The whole-app survey found workloads still running inline, fire-and-forget, or not at all. This commit brings each of them onto the queue contract, so "async by default" now genuinely covers the application. 1. AUDIT CSV + DOCUMENT ZIP AS ASYNC EXPORTS. The History CSV built its rows and the bulk zip buffered every document's bytes inside one request. Both become export.build types: the CSV logic moves to lib/auditExport.ts (the sync route keeps working by calling it; a handler must not import an Express router), the zip job re-verifies per-document access AT BUILD TIME and loads files sequentially — this path exists precisely because concurrent fetches are what blow the memory ceiling. Results carry content_type so the download endpoint serves csv/zip/json correctly. Frontend: History export uses the schedule→poll→download flow (same filters, same UX); DocTable keeps the instant sync zip for ≤10 documents and rides the queue above that — small selections keep their immediate download, large ones stop racing timeouts. Filtered exports carry NO dedupe key (different filters/ selections must not collapse). The lib also carries main's display-name CSV format (user column prefers the profile display name), so the sync route and the async job render one identical CSV. 2. WORD ADD-IN TURNS ENTER THE AUDIT TRAIL. Word chats were recorded nowhere. The same durable enqueueChatTurnAudit now fires where chat.ts fires it, with an explicit surface: "word" (ChatTurnAuditBase gained an optional surface override; derivation for existing callers unchanged). Privacy line held deliberately: the audit title is the chat/document title, NEVER the prompt — local-storage-mode turns produce exactly one metadata row and no conversation content server-side. 3. WORKFLOW ADD-ON ROLLBACK RIDES storage.cleanup. The add-on import's failure rollback now cleans its uploaded copies through the durable storage.cleanup job instead of fire-and-forget deletes. (An earlier version of this series also queued a boot-time workflow-catalog sync; main's open-legal-products#376 since moved catalog ingestion to an operator-run `npm run sync:workflows` step, which removes the boot-time sync concept entirely — so that job is gone rather than ported.) 4. MCP OAUTH TOKENS REFRESH PROACTIVELY. A 5-minute sweep enqueues mcp.refresh_token (deduped per connector) for tokens expiring within 15 minutes, so users stop hitting "reconnect your connector" for a transient hiccup at exactly the wrong moment. Failure classification was inverted deliberately: a 4xx from a token endpoint means the grant is dead (RFC 6749 puts invalid_grant at 400) — permanent, log and stop; only 5xx/429 retry. Being wrong toward "permanent" costs one skipped background refresh (the lazy refresh in oauthBearerToken remains the last line of defense); wrong the other way replays a dead grant at the authorization server forever. A 24h expired-age floor keeps one abandoned connector from enqueueing a doomed job every 5 minutes for the life of the deployment. 5. LEGACY OFFICE TEXT IS PRECOMPUTED ONCE. read_document paid a full LibreOffice conversion PER CHAT TURN for .doc/.ppt sources. A document.precompute_text job (enqueued at upload, and on first cache miss) extracts the text once — through the exact same extractor the read path uses, so cache and source cannot drift — into extracted-text/<versionId>.txt; the read path checks the cache first. The cache key is swept by document/account deletion and invalidated at the in-place byte-rewrite sites. TESTED Backend tsc clean, 855 tests; frontend tsc clean, 595 tests + production build. One pre-existing assertion was tightened, not weakened: "local Word chats insert no rows" now asserts no word_chat rows AND that the single new insert is the audit job whose title excludes the prompt. Note: lib/dbq/handlers.ts in this commit carries the registry accumulated across the whole series (export types, conversion/extraction fallback, and these kinds) — it is the one file every workload registers into. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDev46QdqBsdyRCRBxcvk2
…le to every unit test WHY THIS MATTERS The first end-to-end browser run of this branch failed its very first upload with 500 "Custom Id cannot contain :". Every unit test was green, the earlier live Redis smokes were green — and both were telling the truth, which is what makes this bug worth documenting. WHAT BULLMQ ACTUALLY VALIDATES ':' is BullMQ's own Redis key separator, so custom jobIds may not contain it — EXCEPT that, for backwards compatibility with old repeatable jobs, exactly-three-segment ids are still tolerated (bullmq Job#addJob: split on ':' must yield length 3 or it throws; the code carries a TODO to ban ':' outright in the next breaking release). Our id scheme happened to straddle that exception: extract:<review>:<row> 3 segments -> allowed (smoke passed!) dbjob:<id>:<attempt> 3 segments -> allowed (outbox worked!) convert:<versionId> 2 segments -> THROWS (upload broke) extract:<review>:<row>:<col> 4 segments -> THROWS (regenerate broke) So the exact operations the smokes exercised were the two that landed in the legacy carve-out, and the two that didn't were only reachable through real user flows. The unit tests mock the Queue class, so the validation never ran there at all. THE FIX Underscore separators everywhere a custom id is minted: conversionJobId -> convert_<versionId>, extractionJobId -> extract_<review>_<row>[_<col>], delivery ids -> dbjob_<id>_<attempt>. These strings double as the DB queue's dedupe keys, so both transports keep one identity per unit of work; nothing persists old-format ids (BullMQ jobs are removed on completion and dedupe keys die with their jobs), so there is no migration concern. VERIFIED Same browser flows re-run against the rebuilt stack: upload -> converted by the queue, full-row extraction, and single-cell regenerate (the 4-segment case) all green; unit suites updated to pin the new format. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDev46QdqBsdyRCRBxcvk2
…imports need the backend sources WHY THIS MATTERS `docker compose up` — the project's one-command install — has been unable to build the frontend image since the AI-SDK merge (open-legal-products#368) landed on main today. This branch's new-install story ships through that compose file, so the fix rides here; it is a MAIN regression, not one this branch introduced (verified by building the image from pristine origin/main: same failure). WHAT BREAKS AND WHY CI NEVER SAW IT frontend/src/app/components/shared/types.ts type-imports AskInputsEvent et al from ../../../../../backend/src/lib/chat/types — a relative import that reaches OUTSIDE the frontend package. On a developer machine (and in CI, which builds with `npm run build --prefix frontend` on the host) the backend directory exists, so the types resolve. Inside the Docker build, the context was ./frontend only: the import cannot resolve, TypeScript degrades the imported union members, Extract<AssistantEvent, {type: "ask_inputs"}> collapses, and `next build`'s type check fails with the misleading "Parameter 'item' implicitly has an 'any' type". Nothing was wrong with that file — its types simply weren't in the image. THE FIX Build the frontend image from the REPO ROOT context and copy backend/src to /backend/src, which is exactly where the five-levels-up relative path lands from /app. Only backend *sources* are copied and only for the type check — the imports are `import type` and fully erased from the bundle. A per-Dockerfile ignore (frontend/Dockerfile.dockerignore, honoured by BuildKit) keeps the enlarged context lean: no node_modules, no .git, no word-addin/desktop trees. VERIFIED Image builds clean; the full compose stack (fresh volumes, schema.sql bootstrap, Redis, worker thread) boots and served every flow of this branch's live verification pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDev46QdqBsdyRCRBxcvk2
…; last, un-taint the conversion log
WHY THESE THREE
CodeQL flagged the PR's new code paths: the DOCX→markdown sanitizer
(js/incomplete-multi-character-sanitization and js/double-escaping) and the
version-copy conversion error log (js/tainted-format-string).
THE SANITIZER (lib/tabular/tabular.extract.ts)
A single `.replace(/<[^>]+>/g, "")` pass over attacker-influenced HTML can
REASSEMBLE a tag: "<scr<script>ipt>" loses its inner tag and leaves
"<script>" standing. The strip now loops to a fixed point — the output
cannot contain a tag no matter how the input nests them.
Decoding order matters for the entities: decoding "&" BEFORE "<"
turns "&lt;" (the escaped TEXT "<") into a live "<" — a classic
double-unescape. "&" now decodes last, so escaped text stays text.
Covered by a new test file pinning both properties plus the plain-entity
happy path.
THE LOG (routes/documents.ts)
`console.error(`...${filename}:`, err)` puts a user-controlled filename in
console's format-string position — a filename containing %s/%d would eat
the error argument. The filename now travels as a structured argument.
…chet to match
The rebase's new frontend surface (asyncExport polling helper, the async
exports API wrappers, getDocument) sat below the coverage ratchet. New
tests pin: the schedule→poll→download happy path and its three filename
arms, the failed-status throw (no download attempted), the 150-poll
timeout, the non-test 2s poll cadence (fake timers + NODE_ENV stub),
startUserExport's params-present and params-absent bodies, the encoded
export-id round-trip, and the drag-payload legacy fallback in
docTableSelection.
Per the config's own rule ("floors only go up: when you add tests, raise
them in the same PR"), statements moves 99 → 100 to match the new
measurement. Branches stays at 97 (measured 97.66 — the fraction is the
pre-existing dev-logging and `?? null` arms the config comment already
carves out, not headroom worth gating away).
amal66
added a commit
that referenced
this pull request
Aug 26, 2026
WHY NOW The module split moves route code wholesale, so CodeQL treats every moved line as new and re-flags patterns that predate this PR. Four js/tainted-format-string highs fired — all the same shape #294 already fixed in routes/documents.ts (5264b00): a user-controlled filename interpolated into console.error's format-string position, where a name containing %s/%d would eat the error argument. THE FIX Same convention as 5264b00: the message is a constant string and the filename travels as a structured argument, at the four moved sites — documents.upload, documents.versions (upload + replace), and projects.documents. The [versions/copy] site already carries the fix from #294's commit; these four bring the moved code up to the same rule. Gates: backend tsc clean, 858 tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0132PfwQ6VviSeCRgdGhiq9Z
amal66
force-pushed
the
olp-pr/durable-queues
branch
from
August 26, 2026 07:01
f59aee9 to
171188a
Compare
amal66
added a commit
to amal66/mike
that referenced
this pull request
Aug 26, 2026
Express 4 does not forward async handler rejections, and the routes that open-legal-products#295 left outside modules/ were unguarded — a throw in any of them meant an unhandled rejection and a socket held open until client timeout. Only the workflows router defended itself. Its asyncRoute wrapper is now a shared middleware (middleware/asyncRoute.ts) applied to audit, quickActions, sourceDocuments, wordChat and workflowAddons, so a rejection reaches app.ts's handleUnhandledError instead of nothing. routerErrorHandler now only attributes the failure to a router in the log and delegates the response to handleUnhandledError. Answering with a router-specific 500 body would have contradicted the internal-error contract main established: body-parser's 400/413 keep their own status and code, everything else is the opaque {code:"internal_error"} body. Also fixed in those route files, where errors were being silently converted to success or not-found: - quickActions: DELETE returned 204 with no row matched (now selects the deleted ids and 404s when none); DB failures on the workflow lookup read as 404 (now 500, via the now-exported workflows.service.resolveWorkflowAccess instead of a drifted private copy). - workflowAddons: the import response hand-rebuilt the workflow shape and had drifted from GET /workflows/:id on four fields — now uses the exported withDatabaseWorkflow; a failed add-on lookup answered 404 (now 500). - audit: lib/auditExport's private project-access helper duplicated lib/access.listAccessibleProjectIds and had drifted from it (it re-counted the caller's own shared projects and used a different `contains` encoding). - sourceDocuments: a missing or rejected CourtListener token answered 502, sending the user to check a service that was fine; credential failures now answer 400 with a fixed message that never echoes the upstream body. The history page's surface filter learns the "word" label, so the durable Word chat audit rows open-legal-products#294 introduced (surface "word") are filterable rather than showing a raw key. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Existing deployments apply only migration files that sort after their recorded version, in filename order. Main now ships 20260825_01_auth_handoff_tickets.sql, so a deployment upgraded to that point would skip this branch's 20260824_01_db_jobs.sql entirely and never create the db_jobs table — fresh installs (schema.sql) would be fine, which makes the breakage easy to miss. Rename it to 20260826_01_db_jobs.sql: 20260825_02..05 and _10/_11 are already claimed by the in-flight organizations, connectors, and chat-permissions branches, so the next calendar date avoids both the ordering hazard and a slot collision. The schema.sql and dbq/types.ts cross-references move with it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PaBfaTyuVJPTdYkMYd3w2S
amal66
added a commit
that referenced
this pull request
Aug 26, 2026
WHY NOW The module split moves route code wholesale, so CodeQL treats every moved line as new and re-flags patterns that predate this PR. Four js/tainted-format-string highs fired — all the same shape #294 already fixed in routes/documents.ts (5264b00): a user-controlled filename interpolated into console.error's format-string position, where a name containing %s/%d would eat the error argument. THE FIX Same convention as 5264b00: the message is a constant string and the filename travels as a structured argument, at the four moved sites — documents.upload, documents.versions (upload + replace), and projects.documents. The [versions/copy] site already carries the fix from #294's commit; these four bring the moved code up to the same rule. Gates: backend tsc clean, 858 tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0132PfwQ6VviSeCRgdGhiq9Z
amal66
added a commit
to amal66/mike
that referenced
this pull request
Aug 26, 2026
Express 4 does not forward async handler rejections, and the routes that open-legal-products#295 left outside modules/ were unguarded — a throw in any of them meant an unhandled rejection and a socket held open until client timeout. Only the workflows router defended itself. Its asyncRoute wrapper is now a shared middleware (middleware/asyncRoute.ts) applied to audit, quickActions, sourceDocuments, wordChat and workflowAddons, so a rejection reaches app.ts's handleUnhandledError instead of nothing. routerErrorHandler now only attributes the failure to a router in the log and delegates the response to handleUnhandledError. Answering with a router-specific 500 body would have contradicted the internal-error contract main established: body-parser's 400/413 keep their own status and code, everything else is the opaque {code:"internal_error"} body. Also fixed in those route files, where errors were being silently converted to success or not-found: - quickActions: DELETE returned 204 with no row matched (now selects the deleted ids and 404s when none); DB failures on the workflow lookup read as 404 (now 500, via the now-exported workflows.service.resolveWorkflowAccess instead of a drifted private copy). - workflowAddons: the import response hand-rebuilt the workflow shape and had drifted from GET /workflows/:id on four fields — now uses the exported withDatabaseWorkflow; a failed add-on lookup answered 404 (now 500). - audit: lib/auditExport's private project-access helper duplicated lib/access.listAccessibleProjectIds and had drifted from it (it re-counted the caller's own shared projects and used a different `contains` encoding). - sourceDocuments: a missing or rejected CourtListener token answered 502, sending the user to check a service that was fine; credential failures now answer 400 with a fixed message that never echoes the upstream body. The history page's surface filter learns the "word" label, so the durable Word chat audit rows open-legal-products#294 introduced (surface "word") are filterable rather than showing a raw key. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Row amal66#40 of the reference index (#205), grown into the app's full background-job architecture. Supersedes and absorbs #369.
Stack mechanics (read this first)
This is the bottom of a three-PR architecture stack. Each PR is based on the one below it, so each diff shows only its own commits:
mainolp-pr/durable-queuesolp-pr/service-layersolp-pr/durable-queues,olp-pr/service-layers) exist in this repo purely as mirrors of the identical branches on theamal66/mikefork, so that GitHub can compute a clean stacked diff. They are force-pushed together on every rebase — origin and fork tips are kept byte-identical (both currentlyf59aee93/b82bbbed). Nothing else should branch off them.main, [Architecture] refactor: per-domain modules with service layers — pure motion, zero new deps (stacked on #294) #295 should be retargeted tomain(its diff is already only its own 2 commits, so nothing changes about what it contains), and so on down the stack. GitHub will do the retarget automatically for [Architecture] refactor: per-domain modules with service layers — pure motion, zero new deps (stacked on #294) #295 ifolp-pr/durable-queuesis deleted on merge — please don't delete it until [Architecture] refactor: per-domain modules with service layers — pure motion, zero new deps (stacked on #294) #295 has been retargeted, or [Quality] Repo-wide audit fixes: error containment, drifted forks, silent failures (stacked on #294→#295) #356's base disappears with it.olp-pr/durable-queuesontomain, then rebaseolp-pr/service-layersonto it, thenolp-pr/code-quality, then force-push all three to the fork and the two mirror branches to origin. Head tip today:f59aee93, rebased ontomain@54681b55.Design (ADR)
Context. Two kinds of workloads need to leave the request thread: user-attended heavy work (DOCX→PDF conversion, tabular extraction) and work that must be durable in every deployment (audit trails, account erasure, storage cleanup, export builds, token refresh). The first kind benefits from instant pickup and live progress; the second kind must not depend on infrastructure a default deployment doesn't have.
Decision: one queue contract, two interchangeable transports.
db_jobsPostgres row stays the single durable record; enqueue also hands the row id to anapp-jobsBullMQ queue; the worker claims the row through a conditionalclaim_db_job()RPC, so a duplicated delivery matches zero rows and a lost delivery is recovered by the poller (which drops to a 60s backstop). BullMQ is never a second source of truth.claim_db_jobs()usesFOR UPDATE SKIP LOCKEDso claims partition work safely across replicas; crash recovery is folded into the claim (rows stuckrunningpastp_stale_seconds, default 600s, get re-claimed — there is no separate reaper).cancel_db_jobs(text[])gives the Postgres transport the cross-process cancellation that BullMQ gets natively. Live progress degrades to the SSE views' existing DB-poll backstops (5s poll vs the 60s Redis-mode backstop).backend/src/lib/dbq/driver.ts): explicitQUEUE_DRIVER=redis|postgreswins; elseREDIS_URLset → redis; else a legacyASYNC_DOCUMENT_CONVERSION/ASYNC_TABULAR_EXTRACTIONflag on → redis (those flags always meant BullMQ); else postgres. (QUEUE_DRIVER=autois documentation shorthand for "unset" — any unrecognized value falls through the same ladder.)Where workers run (
WORKERS_MODE,backend/src/index.ts):thread(default) — a worker_thread inside the API process, off the HTTP event loop, respawned on crash;inline— the historical single-thread mode;none— a standalone worker process (node dist/worker.js, sourcebackend/src/worker.ts) on the same Postgres/Redis: a separate container or machine, N instances safe by construction. Graceful shutdown is coordinated in every mode (SIGTERM → drain → 15s force-exit).New installs vs existing installs. Code defaults never flipped — flipping them would break every deployment that upgrades in place (only forks pinned to old commits would be safe). Instead the bootstrap artifacts changed:
docker-compose.ymlships a Redis service (redis:7-alpine,--appendonly yesAOF, loopback-only) withASYNC_*on at the compose level, so freshdocker compose upinstalls run BullMQ for everything, while an existing bare-metal deployment that pulls this branch sees byte-identical behavior until it opts in.Alternatives considered. Always-Redis (rejected: breaks the £20-VPS deployment and every in-place upgrade); Postgres-only (rejected: loses instant pickup and live progress where Redis exists); LISTEN/NOTIFY instead of polling for the fallback (not reachable through supabase-js/PostgREST); separate worker deployment as the default (rejected: single-box stays single-box; the seam exists for those who want it).
What runs on the queue now
Eight durable job kinds (
backend/src/lib/dbq/handlers.ts):conversion.convert,extraction.extract,audit.chat_turn,account.delete,storage.cleanup,export.build,mcp.refresh_token,document.precompute_text— plus theapp-jobsBullMQ delivery queue and the two native conversion/extraction queues.main)enqueueConversionsites incl. the two hiding in chat tools)await docxToPdf(...)inline in the request; failures swallowedconvert_<versionId>— either transportcancel_db_jobsin Postgres mode)surface: "word")void recordChatTurn(...)— 1+N fire-and-forget inserts after[DONE]; Word: nothingdeleteFile(path).catch(() => {})sites, incl. the 6 in workflows/workflow-addons import rollback)storage.cleanupjob.doc/.ppttext forread_documentextracted-text/<versionId>.txt(enqueued at upload / first miss), swept on deletionAlso in this diff, and worth a reviewer's attention beyond the queue itself:
routes/tabular.tsis decomposed intobackend/src/lib/tabular/{extract,extractRow,generate,generateStream,prompt,rows,shared}.ts(~1,540 new lines, net −701 in the route) so the SSE route and the async worker provably share one extraction core. [Architecture] refactor: per-domain modules with service layers — pure motion, zero new deps (stacked on #294) #295 later moves this whole directory intomodules/tabular/.backend/src/lib/maintenance/staleWork.ts): documents stuckprocessingand cells stuckgeneratingflip toerror(30 min stale / 10 min sweep, first sweep 30s after boot, boundedlimit 500).lib/queue/runProgress.ts(Redis pub/sub cell progress),workers/registry.ts(driver-gated worker registry),workerThread.ts,workerRuntime.ts,lib/sseHeartbeat.ts,lib/auditExport.ts,lib/pdfjs.ts.frontend/src/app/lib/asyncExport.ts(schedule → poll → download, 2s cadence, 150-poll ceiling) wired into the history page, DocTable, TabularReviewView, and privacy/data settings.bullmq ^5.34.0,ioredis ^5.11.1. One new migration:backend/migrations/20260824_01_db_jobs.sql, mirrored intobackend/schema.sql.Base-case replication — see the problems on
mainAll of these are on pristine
origin/main; none needs this branch.git grep -n "await docxToPdf" origin/main -- backend/src/routes/documents.ts→ the upload handler awaits LibreOffice inline (lines 650, 864). Upload a large DOCX from the web app with the Network tab open: thePOST /documentsrequest stays pending for the whole LibreOffice run. Behind Cloudflare, a bulk upload of these hits the 100s edge timeout and surfaces as a phantom CORS error — that is issue Uploads fail under bulk load with 524 timeouts and misleading CORS errors #8.git grep -n "void recordChatTurn" origin/main→routes/chat.ts:686,702,routes/projectChat.ts:357. Run a chat turn anddocker killthe backend the instant the stream emits[DONE]: the turn happened, noaudit_eventsrow exists, and nothing ever retries. On this branch the same kill leaves adb_jobsrow that completes after restart.git show origin/main:backend/src/lib/userDataCleanup.ts | sed -n 115p→deleteFile(path).catch(() => {}). Stop MinIO/S3, delete a document from the UI: the row disappears, the object leaks permanently, and nothing is recorded.mainand close the tab → the run stops; there is no server-side job to resume.main(recordChatTurnhas nowordChat.tscaller).mainholds one HTTP connection for its entire build.PR replication — see it working
A. Fresh install, Redis transport.
docker compose up --buildwith fresh volumes.workers: thread, three BullMQ workers started in the worker thread, and[dbq] runner (poll 60000ms, driver redis).POSTreturns 201 instantly;[conversion-worker] convertedfollows; PDF rendition lands indocument_versions.extract_<reviewId>_<rowId>[_<col>](underscores, no colons — see the tradeoff note below).select kind, status from db_jobs order by created_at desc limit 1showsaudit.chat_turnflipping todonesub-second (outbox delivery, not the 60s poll).docker stopMinIO, delete a document → the delete still succeeds (rows first), thestorage.cleanupjob fails and re-queues with backoff; restart MinIO → the job reachesdoneon a later attempt.B. Postgres transport, no Redis at all.
QUEUE_DRIVER=postgres(or simply unsetREDIS_URLand theASYNC_*flags).driver postgres, poll 5000ms, no BullMQ workers, no Redis dial.docker killthe backend mid-run → cells strandedgenerating, jobs strandedrunning. Restart: pending jobs are claimed within ~5s; the strandedrunningrows are re-claimed onceclaimed_atis older than 600s (to test without waiting, age it:update db_jobs set claimed_at = claimed_at - interval '11 minutes' where status = 'running';). All cells reachdone.C. Upgrade-in-place invariance (the property that matters most). On a deployment with no
REDIS_URLand noASYNC_*flags, boot the backend: no Redis dial (verify with an unroutableREDIS_URL— every module still imports cleanly), conversion/extraction stay inline exactly as onmain, and the Postgres queue quietly carries audit/deletes/exports. The only behavioral deltas vsmainare the durability fixes.D. Worker placement. Default boot logs
workers: thread+[worker-thread] background workers started.WORKERS_MODE=noneon the API plusnode dist/worker.jsin a second container (compose has a commentedworkerservice) moves the same worker set out of process. SIGTERM in every mode ends withShutdown complete.Tradeoffs & design decisions (flagged)
attempts: 1) — the durable row + poller are the retry mechanism; two retry systems fighting is how jobs run twice._, never:— BullMQ rejects most colon-containing custom jobIds (all but a legacy 3-segment form). This is a real constraint the schema now encodes, not a style choice; see the live-verification note below.requireMfaIfEnrolled(the export routes' gate) while the small sync ZIP only requires auth. Flagged, not hidden; unifying either way is a small follow-up.chat.message,surface: "word"); the title is the chat/document title and never the prompt, holding the no-server-side-conversation contract. Easy to gate onpersistChatif the maintainers prefer zero rows — say the word and I'll add the gate here rather than in a follow-up. This is the one deliberate privacy judgement call in the PR, so it deserves a maintainer's opinion.invalid_grantat 400). Wrong toward "permanent" costs one skipped background refresh; wrong the other way replays a dead grant forever. A 24h expired-age floor stops abandoned connectors from enqueueing doomed jobs indefinitely.versionId— sound today (legacy.docnever gains assistant-edit versions, which are DOCX); flagged as the one soft spot in cache identity.b0e52648) — it is not queue work.docker compose upis broken on currentmain(a Migrate backend LLM providers to Vercel AI SDK #368 regression): the frontend type-imports frombackend/src, which the frontend-only Docker context doesn't contain, sonext build's type check fails inside the image while host builds and CI pass. Fixed here by building the frontend image from the repo root with aDockerfile.dockerignore. Called out explicitly because it is an unrelated fix in this diff — happy to split it out if you'd rather take it separately.erroron rendition failure;edit_document/generate_docxdeliberately get no renditions; flag-on PPTX has a first-open gap until its rendition lands.Testing evidence
Automated. Backend
tscclean, 80 test files / ~820 cases. Frontendtscclean, 97 test files / ~520 cases, production build green. (Case counts includeit.eachexpansion; file counts are exact —mainhas 64 backend / 95 frontend test files, so this PR adds 16 backend and 2 frontend test files.) All CI checks are green onf59aee93— CodeQL, gitleaks, backend, frontend, playwright, Supabase stack tests, eval harness, and the "Fresh install vs upgraded deployment" schema-drift job that builds both install paths.Live cross-transport test round — posted publicly as comment 5400174994. That comment is the primary evidence for this PR and is worth reading in full: a fresh
docker compose up --build(fresh volumes), Chrome driven over the DevTools protocol, a real Anthropic key, with Postgres and container logs cross-checked at every step. It covers 11 Redis-transport scenarios (A1–A11: fresh-boot contract, queued conversion, tabular run with live SSE, kill-tab durability, clear-cells + queued regenerate-cell, sub-second outbox audit delivery, async CSV/ZIP/JSON exports, MinIO-stopped fault injection with recovery on restart, full account erasure) and 2 Postgres-fallback scenarios (B1 crash-recovery with stale-claim reclaim, B2 upgrade-in-place invariance), plus main-regression spot checks for the features that landed during the rebase window (#365 onboarding, #374 tabular UX, #376/#380 workflow catalog, #335 dark mode). It also lists what was not covered live and why (Word-host audit, MCP grant refresh, mid-flight stream reattach).Earlier rounds, still valid. Live claim-RPC semantics on real Supabase Postgres + Redis (dedupe unique-violation, backoff-not-reclaimed-early, stale-
runningreclaim, two concurrent claimers partitioning 10 jobs 5/5 with zero overlap); outbox end-to-end incl. duplicate-delivery no-op; BullMQ cancellation semantics (the smoke that caughtJob#discard()being an in-memory no-op cross-process); boot smokes for all threeWORKERS_MODEhomes in dev-tsx and compiled prod, both drivers, clean SIGTERM. Gated Supabase stack tests 15/15 against real GoTrue + RLS. The Word add-in Playwright suite (22 spec files, chromium + webkit) was run as a regression check and is green — this PR changes zero files underword-addin/; its Word work is entirely server-side inbackend/src/routes/wordChat.ts.Two real defects were found by hand-testing and fixed on the branch — both invisible to every unit test:
52e5a212). Ourconvert:<id>and 4-segment regenerate ids threw at the first real upload, whileextract:<a>:<b>had sailed through the smokes inside BullMQ's legacy 3-segment carve-out. All custom ids now use underscores. (The live comment cites this asb71a56c1— that is the pre-rebase hash of the same commit; on the current branch it is52e5a212.)docker compose upbroken onmain(commitb0e52648) — see the tradeoff note above.CodeQL. The three highs CodeQL raised on this branch are fixed in
5264b007with a pinning regression test (backend/src/lib/tabular/__tests__/tabular.extract.sanitize.test.ts): tag-stripping now loops to a fixed point (<scr<script>ipt>reassembly), entity decoding does&last (double-unescaping), and a user-controlled filename moved out ofconsole.error's format-string position.f59aee93then raised the frontend coverage floor that the new tests made stale (statements 99 → 100; branches stays 97 at a measured 97.66%).Provenance
Re-derived from amal66#40 against current row-based
main(see earlier revisions of this description for the port details); the fork's embedding queue still rides with the RAG row. The Postgres queue, outbox, worker-thread placement, and full-app coverage are new in this PR. The Mac desktop shell's compose (separate branch) inherits the Redis service when it rebases.Related issues
Addresses the root cause of #8 (bulk uploads → Cloudflare 524 timeouts surfacing as phantom CORS errors): synchronous LibreOffice conversion inside the upload request leaves this PR's queue, so upload responses return immediately. Note the rollout caveat for existing deployments — async conversion is on by default only for fresh compose installs; an in-place upgrade must set
ASYNC_DOCUMENT_CONVERSION=true(with Redis, orQUEUE_DRIVER=postgres). The client-side half of #8 (unbounded parallel uploads, per-file outcomes) is fixed separately in #381.Maintenance
Same commitment as the other index rows: triage within 48h on anything this breaks. Escape hatches at every layer:
QUEUE_DRIVER=postgres,DB_JOBS_ENABLED=false,WORKERS_MODE=inline, and the whole feature set degrades tomain's behavior when the relevant enqueue can't reach its backend.🤖 Generated with Claude Code