fix(txn): keep context.transaction usable after its transaction completes

[kriszyp]

Closes #2229.

releaseContext() (added in #2030, to stop a long-lived context pinning a completed transaction) set context.transaction = null. That introduced a third state for a published slot, and application code cannot interpret it — the next touch throws Cannot read properties of null (reading 'commit').

Committing the current transaction mid-handler and then continuing to use the context is documented in two places: release-notes/v5-lincoln/v5-migration.md (await getContext().transaction.commit()) and the 4.5.0 notes ("transactions can now be reused after calling transaction.commit()"). On 5.2.1+ that pattern 500s. It is what took out central-manager's fabric connect and cluster create/delete on 5.2.3 for any non-super-user, since getUserPermissions() commits the caller's transaction mid-request.

A completed transaction now leaves RELEASED_TRANSACTION in the slot: one frozen, process-wide completed transaction. commit()/abort() are no-ops and reads through it see the latest committed state — what the slot did before #2030 — while retention stays O(1) per process rather than O(live contexts), so #2030's measurement is kept. Because the instance is shared, every route that adopts a caller-supplied transaction or context refuses it (txnForContext, transactional()'s argument normalizer, Resource.create's own argument shift, transaction() itself), and addWrite()/setContext() throw.

Two more callers were broken by the same null and are fixed with it: transaction.commit(context) (a checkpointing loop that commits every Nth row fails on its second checkpoint on 5.2.3 — the intervening write is serviced by its own scope, whose final commit releases the slot), and passing a transaction where a context is expected (Table.delete(id, context.transaction)).

For the human reviewer

1. The slot is now permanently truthy after first attach, rather than null. The alternative was keeping null and changing the docs plus every caller to context.transaction?.commit(). That loses: the pattern is published, so the break reaches every app that followed our own docs, and it shipped in a patch. The cost of this direction is that any downstream if (context.transaction) flips meaning — I audited every core reader (transaction.ts, Resource.ts, Table.ts's two stale writebacks and txnForContext, HierarchicalNavigableSmallWorld.ts) and none is truthiness-sensitive, and harper-pro reads context.transaction nowhere outside core/. Downstream apps and plugins are the residual exposure: if (context.transaction) context.transaction.addWrite(op) now enters the branch and throws instead of silently skipping. Hardest thing to rule on, and the one I most want a second opinion on.
2. One shared instance rather than a per-context lightweight closed one. Sharing is what keeps the memory win. It also means the instance must own no mutable state — hence the small frozen surface rather than a DatabaseTransaction subclass, whose inherited writes array stayed mutable through context.transaction.writes.push(...) and whose setContext() could rebind the shared private context. Anything off that surface now fails loudly rather than inheriting behavior that writes through to every other context.
3. isReleasedTransaction() is checked at four call sites rather than being a state on the transaction. A future adoption site that forgets gets a TypeError from the frozen instance — loud, but still a crash. The alternative (brand it, or funnel every adopter through one place) is cheap now and invasive later; I went with the predicate because Resource.create keeps its own reference to the argument and reads context.transaction.startedFrom back inside the callback, so a funnel-only guard leaves that reference pointing at the frozen object.
4. "Absent" is standardized as "use the ambient context", not "a fresh empty context". Every route resolves the placeholder to undefined and falls through to contextStorage.getStore(). Review flagged that this changes Resource.create's shifted two-argument form, whose isolation on origin/main would be lost — so I measured what that form actually did with a real completed transaction in the slot, which is what 5.2.0 had. It resolved to the caller's own context, because DatabaseTransaction has a getContext() and create calls it. The private {} only ever appeared on 5.2.1–5.2.3, from context = record || {} with null; it is an artifact of the bug, not behavior to preserve. A shared placeholder cannot carry a back-reference to the context that released it — that is the point of releasing — so the ambient context is the nearest faithful restoration, and it is also what Table.create(record) with no second argument already does.
5. context.transaction.commit() stays callable and transaction.commit(context) stays a no-op on a completed transaction. Review pushed twice for the helper to throw instead (a silent no-op can mask a context-lifecycle bug). It threw on 5.2.3 only as a side effect of the null, and on 5.2.0 it was a no-op — so keeping the throw would have preserved half the regression, which is how the checkpoint-loop break above was found. Both documented forms now agree.
6. LMDB is not at parity and the interface comment says so. LMDBTransaction overrides commit()/abort() and never calls releaseContext(), so there the completed transaction stays in the slot — callable, but retained. That gap predates this change; documented rather than closed, since LMDB is deprecated and its LINGERING semantics are load-bearing.
7. Not in scope, worth knowing: writes made with a context whose transaction has completed are each committed immediately and individually — no atomicity, no rollback. Identical on 5.2.0, so not part of this regression, but it is why a failed deleteCluster can leave a cluster TERMINATED with its instances RUNNING, and it is the real content of central-manager#705. A follow-up makes those writes atomic while the owning scope's commit is still pending.

One claim I made early and had to retract: txnTime: 0 from the placeholder's commit() is not a fidelity loss. Measured on both paths — re-committing the real completed transaction also returns { txnTime: 0 }, because the commit that completed it reset its timestamp.

Verification

Regression range established by running one case on three checkouts — v5.2.0, v5.2.3 (origin/main) and this branch: context.transaction after a nested Resource-API call, then commit(). 5.2.0 leaves the completed CLOSED transaction and the call is a no-op; 5.2.3 throws the production TypeError; this branch matches 5.2.0. getReadTxn()'s open !== OPEN early return and save()'s post-commit immediate-commit path, which the original triage blamed, are byte-identical between 5.2.0 and 5.2.3.

Fails-on-base: the tests here fail on unfixed origin/main for the right reasons — the two restored commit in the middle calls and the ambient handler case with Cannot read properties of null (reading 'commit'), the checkpoint loop and the bare-transaction/create routes with No active transaction is available to commit.

Added: the ambient delivery path (processLocalTransaction, the function serverHandlers.js calls for every operation, so this is the real operations entry point — only the HTTP transport is absent), the bare-context regression shape, all four adoption routes plus ambient-identity preservation through them, a fresh multi-store chain after release, every route into the shared placeholder's state, and the transaction.commit()/abort() behavior on absent versus released slots.

Gates, locally: test:unit:resources 1593 passing, 0 failing on RocksDB and 1392 passing, 0 failing with HARPER_STORAGE_ENGINE=lmdb. npm run lint:required and prettier clean. test:unit:main and test:integration:all could not run in this checkout — long-running Harper processes from other worktrees hold ~/harper/database/system/LOCK, so both abort on Resource temporarily unavailable before any test executes.

Those two ran in CI and passed, which closes the one real gap in the local verification: 42 checks pass, 2 skipped — all six integration shards on Node 24, Windows, Bun and uWS, plus unit tests on Node 22/24/26. One Next.js adapter leg was cancelled on a 30-minute Install Playwright browsers download timeout, before reaching any test; it passed on re-run.

Not verified: MQTT long-lived-context reattach (the scenario motivating #2030 — untested before this change too), and an operations-HTTP or central-manager flow end to end.

Complexity: complicated

_{Review-Coverage: authored=unknown; ran=none; rounds=1 @ d746a30}

_{Human-Review-Need: 4 @ d746a30}

[gemini-code-assist]

Code Review

This pull request replaces the use of null with a shared, process-wide placeholder instance RELEASED_TRANSACTION to represent a completed and released database transaction. This placeholder safely implements no-op commit() and abort() methods, allowing subsequent calls on a completed transaction context to succeed gracefully while preventing writes or context attachments. The changes span transaction management, resource normalization, and table context resolution, and are accompanied by comprehensive unit tests. I have no feedback to provide as there are no review comments.

[claude]

Reviewed; no blockers found.

[kriszyp]

The red Integration Tests 5/6 (uWS HTTP) is #1827 (Flaky: "Harper produced no startup output" — recurring integration-test startup hang), not this PR. Evidence:

• record-caching-ttl-eviction.test.ts failed with HarperStartupError: Harper produced no startup output for 150000ms — Harper never started; no assertion failed.
• The identical failure mode occurs on main (e.g. run 32182927071, same 150000ms startup hang).
• This same SHA passed all six shards, including 5/6 uWS, on the first run of this PR.

Re-run in flight. Every other check is green: 42 pass / 2 skipped, covering all six integration shards on Node 24, Windows, Bun and uWS, plus unit tests on Node 22/24/26.

— Claude Opus 5

[kriszyp]

Follow-up filed: #2241 — collapse the ImmediateTransaction / RELEASED_TRANSACTION surface.

Kris asked how the placeholder this PR adds differs from the ImmediateTransaction that txnForContext already mints, since both mean "no owning transaction scope". Fair question — they differ in three load-bearing properties (open OPEN vs CLOSED, store-bound vs store-less, per-context vs per-process and frozen), but a store-less immutable ImmediateTransaction would be the placeholder, so the redundancy is real.

Not folded into this PR: the released marker started as a frozen DatabaseTransaction subclass here and was replaced in review for concrete reasons (shallow freeze left writes mutable, #private fields unaffected so setContext() could rebind the shared context, un-overridden inherited mutators threw opaque TypeErrors). Collapsing the surface properly means touching every site that recognizes the marker — the argument normalizer, Resource.create, transaction(), txnForContext — which is more than a patch should carry. #2241 has the constraints written down.

— Claude Opus 5