fix(sec): gate SQLite SPAC lock through in-process mutex + auto-resolve oversized dead-letter#173
Open
sroussey wants to merge 2 commits into
Open
Conversation
…oid concurrent BEGIN IMMEDIATE crash) The SQLite branch issues `BEGIN IMMEDIATE` on the singleton `better-sqlite3` connection that `getDb()` returns. The pre-existing per-CIK keyed mutex (`withInProcessLock`) only serializes writers on the same CIK — distinct CIKs race past the mutex and hit BEGIN concurrently, and SQLite responds with "cannot start a transaction within a transaction". Wrapping the SQLite branch body in a process-wide gate keyed by a sentinel value (`SQLITE_GLOBAL_LOCK_KEY = 0`) forces every SPAC writer to queue at the connection regardless of CIK. The connection-level SQLite database lock is single-writer anyway, so this matches the backend's actual concurrency model. Postgres and the in-memory fallback are unchanged (per-CIK serialization is correct there). Test: five parallel `recordRegistration` calls across CIKs [100, 200, 300, 400, 500] under a real SQLite backend all resolve fulfilled, with zero throws containing "transaction within a transaction". The fix is essential — reverting only the SQLITE_GLOBAL_LOCK_KEY wrap causes this test to throw on the second concurrent BEGIN.
… (informational only) `processRedemption8K` records a `redemption-partial-oversized` dead-letter when at least one exhibit was dropped over the per-exhibit cap but a non-empty survivor set still ran through extraction. The entry exists so operators can triage filings whose largest exhibit was elided. It is purely informational — the drop is deterministic (the cap doesn't move between runs) so no retry recovers the dropped exhibit. Today the entry sits in the `pending` worklist forever and pollutes `sec extractor dead-letters redemption` output with rows that no extractor-version bump can clear. This call adds a `markResolved` immediately after the `record` so the entry lands in the `resolved` state and is excluded from `listEligible`. The `attempts` counter keeps incrementing on each replay (so the audit trail of how many times the cap was hit for a given accession is preserved), and `listPending` / `listEligible` queries never surface it again. Test: a filing with one in-cap + one oversized exhibit runs through `processRedemption8K` twice; after each run the entry exists at status="resolved", `listEligible` filtered to its section returns 0 entries, and `attempts` increments (1 then 2).
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
2 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.
Two follow-ups on PR #170.
Fix 1 (CRITICAL): SQLite
withSpacCikLocksingleton-connection crashThe SQLite branch in
withSpacCikLockissuesBEGIN IMMEDIATEon the singletonbetter-sqlite3connection returned bygetDb(). The existing per-CIK keyed mutex (withInProcessLock) only serializes writers on the same CIK — distinct CIKs race past the mutex and hit BEGIN concurrently, and SQLite throws"cannot start a transaction within a transaction".Wrapping the SQLite branch body in a process-wide gate keyed by a sentinel value (
SQLITE_GLOBAL_LOCK_KEY = 0) forces every SPAC writer to queue at the connection regardless of CIK. The connection-level SQLite database lock is single-writer anyway, so this matches the backend's actual concurrency model. Postgres + in-memory fallback are unchanged.New test: five parallel
recordRegistrationcalls across CIKs[100, 200, 300, 400, 500]under a real SQLite backend all resolve fulfilled, with zero throws containing"transaction within a transaction". Reverting only the wrap causes the second concurrent BEGIN to throw — the fix is essential.Fix 2 (HIGH): auto-resolve the
redemption-partial-oversizeddead-letterprocessRedemption8Krecords aredemption-partial-oversizedentry whenever at least one exhibit was dropped over the per-exhibit cap but a non-empty survivor set still ran through extraction. The entry is purely informational — the drop is deterministic (the cap doesn't move between runs) so no retry recovers the dropped exhibit, and yet today the entry sits inpendingforever, pollutingsec extractor dead-letters redemptionwith rows no version bump can clear.Adding
markResolvedimmediately afterrecordlands the entry inresolvedsolistEligible/listPendingexclude it. Theattemptscounter still increments on each replay (audit trail intact), so operators retain visibility into how often the cap fired for a given accession.Test plan
bun test src/storage/spac/— 49 tests across 10 files, 0 fail (new SQLite parallel-writer test included).bun test src/sec/forms/miscellaneous-filings/— 49 tests across 4 files, 0 fail (existing partial-oversized test extended + a new dual-run idempotency test).bun run build— clean.Generated by Claude Code