test: pin the transaction invariants #1005 left broken

[grypez]

Explanation

Eight tests, all currently failing, for three defects that landed with #1005. No production code changes: each test states the invariant a fix has to restore, so the diff that repairs them is the specification being met rather than a claim about it.

CI is expected to be red until the fixes land. That is the point of the branch — please don't read the failures as flakes.

Draft because the remedies are the author's call, not mine. Happy for these to be cherry-picked into a fix PR and this one closed, or for me to add the fixes here.

The three defects

1. releaseSavepoint was never hardened the way rollbackSavepoint was

#1005 fixed rollbackSavepoint to discard the enclosing transaction when ROLLBACK TO fails, with a comment explaining why: otherwise the savepoint stays on the stack and the transaction stays open with nothing that will ever commit or abort it, so every later write on that connection joins it, reports success, and vanishes on close().

releaseSavepoint has the identical shape and was left alone. The new tests sit directly beside their rollback counterparts so the asymmetry is visible in place.

endCrank gets the companion case. Settling its waitForCrank waiters in a finally is right — but it also leaves ctx.savepoints listing a savepoint the release failed to remove, so the next crank numbers its savepoint t1 while the database still has t0. From then on releaseAllSavepoints releases the wrong one and every rollback aims past the crank it meant to undo. rollbackCrank already does the right thing in its own finally; this mirrors it.

Before #1005 this wedged safely — endCrank threw before clearing inCrank, so every waiter hung and nothing proceeded. Now the kernel walks past it. In the daemon process.exit(1) follows, but the browser worker deliberately stays up, so panel-driven reset() / terminateAllVats() writes proceed into the orphaned transaction.

Test	File
releaseSavepoint discards the transaction when the release fails	kernel-store/src/sqlite/nodejs.test.ts
releaseSavepoint discards the transaction when the release fails	kernel-store/src/sqlite/wasm.test.ts
forgets its savepoints even if releasing them fails	ocap-kernel/src/store/methods/crank.test.ts

2. #processCrankResult does fallible work after the crank's transactional boundary

The PR's headline claim is that the crank the loop died in is rolled back. It isn't, in two ways:

• Success path. #flushCrankBuffer() calls #invokeKernelSubscription, which settles the promise enqueueMessage handed an external Kernel.queueMessage caller. Only then can await #terminateVat(...) or collectGarbage() throw and have the new inner catch roll the crank back. The caller keeps an answer computed from state the store discarded; a restart re-delivers the message and notifies every other subscriber again. The abort path already guards this hazard by clearing #resolvedWithKernelSubscription — the success path doesn't.
• Abort path. A successful rollbackCrank('start') empties the savepoint stack, which ends the transaction. #terminateVat and collectGarbage then run with no savepoint and no transaction, so their writes autocommit piecemeal. #crankRollbackAttempted correctly suppresses a second rollback, but there was nothing left to undo either way.

The first test proves both halves of the inconsistency at once: its expect(resolve).toHaveBeenCalledWith(...) assertion passes — the caller really does get its answer — and then the rollback assertion fails. It only goes green when those two stop co-occurring.

The parameterized pair states the invariant as the rollback is the last thing the crank asks of the store, rather than prescribing where #terminateVat lands relative to collectGarbage. Reordering and moving the fallible work inside the savepoint both satisfy it.

Test	File
does not roll back a crank whose result the caller already received	ocap-kernel/src/KernelQueue.test.ts
does no store work after rolling back 'an abort'	ocap-kernel/src/KernelQueue.test.ts
does no store work after rolling back 'an abort that also terminates'	ocap-kernel/src/KernelQueue.test.ts

3. The wasm driver can be left believing it is in a transaction

wasm.ts tracks _inTx itself rather than reading it from SQLite, so a failed abort inside #1005's new rollbackSavepoint catch is the one case that can leave it disagreeing with the database. The nodejs driver is immune — db.inTransaction comes from better-sqlite3.

Left true, beginIfNeeded is a no-op from then on and the next createSavepoint runs SAVEPOINT in autocommit mode, where the matching RELEASE commits (Agoric/agoric-sdk#8423, already cited two lines above the code) and no later rollback can undo the delivery — an { abort: true } crank silently keeps its writes. Reachable whenever SQLite has already auto-rolled-back as part of the error that made ROLLBACK TO fail in the first place.

The second test runs that next createSavepoint and asserts the BEGIN, so the corruption path is observable rather than argued.

Test	File
stops believing it is in a transaction when the abort fails too	kernel-store/src/sqlite/wasm.test.ts
begins a transaction for the next savepoint after a failed abort	kernel-store/src/sqlite/wasm.test.ts

Testing

Every new test fails for the mechanism it describes, not incidentally:

× releaseSavepoint discards the transaction when the release fails   (nodejs)
    expected [ 'point1' ] to strictly equal []
× releaseSavepoint discards the transaction when the release fails   (wasm)
    expected [ 'point1' ] to strictly equal []
× stops believing it is in a transaction when the abort fails too
    expected true to be false
× begins a transaction for the next savepoint after a failed abort
    expected "vi.fn()" to be called once, but got 0 times
× forgets its savepoints even if releasing them fails
    expected [ 'test' ] to strictly equal []
× does not roll back a crank whose result the caller already received
    expected "vi.fn()" to not be called at all, but actually been called 1 times
× does no store work after rolling back 'an abort'
    expected 'collectGarbage' to be 'rollbackCrank'
× does no store work after rolling back 'an abort that also terminates'
    expected 'collectGarbage' to be 'rollbackCrank'

All pre-existing tests in the touched files still pass — 78/78 in kernel-store, 61/61 across KernelQueue.test.ts and crank.test.ts. yarn lint is clean.

Incidental

wasm.test.ts's existing rollbackSavepoint reports the rollback failure even if the abort fails too ends in a bare mockDb._inTx = false; with no assertion. It is already redundant given the describe's beforeEach, and it exists only because production leaves _inTx true — it quietly documents defect 3 instead of asserting it. Left in place rather than folding unrelated edits into this branch; worth deleting alongside the fix.

Checklist

• I've updated the test suite for new or updated code as appropriate
• I've updated documentation (JSDoc, Markdown, etc.) as appropriate — n/a, tests only
• I've communicated my changes to consumers by [updating changelogs...] — n/a, no consumer-facing change
• I've prepared draft pull requests for cross repository changes — n/a

🤖 Generated with Claude Code