Keep reads working when storage.debugLongTransactions is enabled

[kriszyp]

Enabling storage.debugLongTransactions no longer breaks reads: with the flag on, any search() whose transaction did not open its own read handle threw TypeError: Cannot read properties of undefined (reading 'push') and returned a 500 (#2222). useReadTxn() recorded a stack trace unconditionally, but only getReadTxn()'s fresh-handle branch ever created the array it pushed onto — so a handle adopted by save(), a transaction past OPEN, or an ImmediateTransaction (whose getReadTxn() never returns a handle) crashed the read. The diagnostic broke exactly the reads an operator turns it on to diagnose.

Unresolved from review: the cross-model legs and I could not reproduce the reported 500 through an HTTP request — see Verification — so this is pinned at the unit level, not end-to-end. The adjudicator also surfaced a pre-existing, flag-independent gap in the same area (save()'s adopt branch seeds none of the per-handle bookkeeping getReadTxn() establishes, so readTxnsUsed goes undefined++ → NaN); filed separately as #2224 rather than folded in here, because seeding it changes refcount and long-transaction-monitor semantics.

For the human reviewer

1. Fix at the read site, not the adopt site (fix-layer-read-site-vs-adopt-site). The broken invariant is that getReadTxn()'s fresh branch establishes three things together — readTxnsUsed = 1, trackedTxns.add(this), stackTraces = [...] — and save()'s adopt branch establishes none. Seeding all three in save() would fix this crash and the NaN refcount at once, but it also puts write-first transactions under the long-transaction monitor for the first time (they'd become abortable per Force-committing an over-time transaction leaves orphaned secondary-index entries (atomicity violation) #1407) and changes what doneReadTxn()'s refcount means. That is a semantic change I'm not willing to make as a drive-by on a crash fix, so this PR guards the read site and save() adopts a read handle without the per-handle bookkeeping getReadTxn() establishes (readTxnsUsed becomes NaN) #2224 carries the root fix. Say the word if you'd rather have them together.
2. Trace only reads the monitor can dump (traces-only-with-a-handle). The guard is on this.stackTraces, not on the returned handle: the array is seeded by the same getReadTxn() branch that does trackedTxns.add(this), so its presence marks a transaction whose traces the monitor can actually dump. Untracked reads (adopted handle, ImmediateTransaction, closed transaction) capture nothing rather than paying an Error per read that nothing reads. I originally guarded on the handle so adopted handles would keep collecting traces in case they later became tracked; #2224 would seed all three pieces of bookkeeping together, so tracing resumes on its own and the extra collection buys nothing. Changed in response to the gemini-code-assist review — thread resolved.
3. Unit coverage only (unit-only-scope, require-cache-eviction-as-the-test-seam). Four cases: three that crash on origin/main with the reported TypeError, plus a positive control proving a monitor-registered transaction is still traced. The test flips the load-time DEBUG_LONG_TXNS const by re-requiring the module with its own cache record and restoring the shared one, rather than exporting a new test seam next to setTxnExpiration. A seam is API once exported; the trade is one idle unref()ed monitor timer per run over an empty set. If you'd rather have the seam, it's a small change.

Verification

• New unit test unitTests/resources/debugLongTransactionsReads.test.js pins the three crash shapes plus a positive control, and the three crash cases fail on origin/main sources (rebuilt dist, not incremental) with the exact reported error and frame: TypeError: Cannot read properties of undefined (reading 'push') at DatabaseTransaction.useReadTxn (resources/DatabaseTransaction.ts:421:41) — including ImmediateTransaction.useReadTxn, the frame in the issue report. 3 failing + 1 passing control before, 4 passing after.
• End-to-end route attempted and not achieved: I booted Harper with storage.debugLongTransactions: true and drove 12 request shapes against it (REST collection query, REST GET with select, relationship serialization in both directions, static search() with and without a context, write-then-search in one request transaction, cross-table write-then-search, SQL SELECT ... WHERE, search_by_value) — all returned 200. Every one of those opens its read handle through getReadTxn()'s fresh branch, which seeds stackTraces. Reaching useReadTxn() with an unseeded array needs the context's transaction reference to be released between transactional()'s open === OPEN check and txnForContext(), which I could not force deterministically from a fixture. The reporter's stack frame (ImmediateTransaction.useReadTxn) is reproduced directly in the unit test instead.
• npm run test:unit:resources — 1587 passing, 0 failing.
• npm run test:integration on the three suites that run with debugLongTransactions: true (longtxn-secondary-index, eviction-secondary-index, txn-overtime-atomicity) — 7 pass, 0 fail.
• npm run test:unit:main could not run on this machine: a stale mocha process (17 days old) holds ~/harper/database/system/LOCK, so the suite dies at module load before any test. Unrelated to this change; CI runs the gate.
• npm run typecheck, npm run lint:required, prettier --check all clean.

Complexity: easy

_{Review-Coverage: authored=claude; ran=codex,gemini; declined=cursor-grok,cursor-composer,domain; rounds=4 @ 2bf62cd}

_{Human-Review-Need: 3 @ 2bf62cd}

[gemini-code-assist]

Code Review

This pull request resolves an issue where useReadTxn() could throw when storage.debugLongTransactions is enabled, specifically for transactions whose read handles did not originate from getReadTxn()'s fresh-handle branch (such as adopted handles or closed transactions). The review feedback identifies a performance optimization on the read hot-path: since untracked transactions are never inspected by the long-transaction monitor, we should avoid capturing expensive stack traces for them entirely by guarding with this.stackTraces instead of initializing it with ??=. Corresponding test assertions should also be updated to reflect this optimization.

[claude]

Reviewed; no blockers found.