test: cross-examine the six transaction claims #1021 rests on - #1039

Prettier wanted a blank line before the entry following a nested bullet,
and the entries still cited #1010, which this PR replaces.

…ring

`retireKernelObjects` deletes an object and queues a `retireImport` for each
importer in the same breath, so until that action is delivered an importer's
c-list entry names a kref the kernel has already dropped. The audit counted
those entries as holders and reported a violation against the collector's own
output — and since `assertRefCountsIfAuditing` throws from inside the crank,
that killed the run loop for good.

Reachable from an ordinary `terminateVat` while a surviving vat holds the
dying vat's export in liveslots' dropped-but-recognizable state. No current
test produced it; found by Cursor Bugbot on #1020 and reproduced against the
real store.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Rebasing the baseline to (0, 0) made every reference explicit, which
exposed the holders that were never references at all. An ocap URL
carries its kref inside an encrypted bearer token and nothing else, so
the kernel cannot discover from its own state that a holder exists:
`issueOcapURL` took no reference of any kind. Under the old baseline
nothing exported was collectable and it never showed; at (0, 0) the
target is collected as soon as the message that carried it to the issuer
is delivered, and the URL names a dead capability. The audit is silent
on it by construction — the object genuinely has no holder it can see.

Retain the target when the URL is issued, before the token exists, since
the token is unretractable once it does. One pin per kref however many
URLs name it, and no release: the token is persistent and unexpiring, so
`revoke` is how the capability dies. Pinning also puts the holder inside
the reference graph, so the audit can see it rather than being taught to
excuse it.

The same shape had a second door. `incrementRefCount` has no
`kernelRefExists` guard where `decrementRefCount` does, so importing a
deleted kref read its missing counts as (0, 0) and wrote them back,
resurrecting a live-looking object with no owner — deliverable to by
nobody, and endorsed by the audit, since the new c-list entry is a
legitimate holder for exactly the count it finds. Reached by redeeming a
URL issued for an object since collected. Guard the point of corruption,
`translateRefKtoE`, rather than `incrementRefCount` itself: creating an
entry for a deleted kref is the invariant, and releasing a reference to
something already gone is how GC teardown is allowed to race deletion.

Also release a vat's root pin when `deleteSubcluster` retires vats that
never ran here. It bypasses `stopVat`, so nothing released the pin
`launchVat` took in the incarnation that did run them, leaving the root's
count permanently above zero and `pinnedObjects` naming a vat that no
longer exists. `stopVat` and `deleteSubcluster` now share
`releaseVatRootPin`.

Vat root pinning had no unit coverage at all, so pin-on-launch,
release-on-terminate and keep-across-restart are asserted now; the last
is what the comment claims and what would break silently. Restores the
`maybeFreeKrefs` assertion on `forgetEndpointImports`' ownership-migrated
branch, which lost its `not.toHaveBeenCalled` when that branch stopped
returning early.

Corrects three claims that the (0, 0) birth falsified and that shipped as
documentation: both `KernelServiceManager` comments asserting its delete
branch cannot fire, when it now does, and a changelog entry asserting
(1, 1) birth two dozen lines above one asserting (0, 0). `recomputeRefCounts`
no longer describes itself as a migration; nothing calls it, and opening
an existing store does not migrate one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Retaining before minting is right: minting awaits, so a collection crank can run in that window. But nothing undid the retention when minting then failed. A rejected kernel-service call is reported to the caller rather than thrown out of the crank, so the crank commits and the pin outlives the kernel that took it, naming a URL that never existed.

retainForOcapURL now reports whether this call took the pin, and undoOcapURLRetention unwinds one that never backed a URL. Guarded on the ledger rather than the pin list, so it can only remove the pin it put there: a kref some live URL already names keeps the pin that URL depends on, and a vat root keeps its lifetime pin.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The retention was deduplicated by kref, and the failure path undid it if this
call was the one that took it. Minting awaits, though, so issuances for the
same target overlap: a second `issue` can mint a URL while the first is still
in flight, having taken no retention of its own because the ledger already
named the kref. If the first then fails it unwinds the retention the second's
live URL depends on, and collection can take the capability out from under it.

The ledger is a multiset now, one entry and one pin per issuance, so a failed
mint releases only what it took. Pins were already a multiset, and each pin
here is either released by its own failure or held by its own live URL, so
none is left unreleasable — the concern that motivated deduplicating.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

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

`releaseSavepoint` was never hardened the way `rollbackSavepoint` was in
that PR. A RELEASE that throws leaves the savepoint on the stack and the
transaction open with nothing that will ever commit or abort it, so every
later write on the connection joins it, reports success, and vanishes on
close — verbatim the failure mode #1005 documents for the other door. The
driver tests sit beside their rollback counterparts so the asymmetry is
visible in place. `endCrank` gets the companion case: it now settles its
waiters in a `finally`, which is right, but it also leaves the savepoint
listed, so the next crank numbers its savepoint `t1` against a database
that still has `t0`.

`#processCrankResult` does fallible work after the crank's transactional
boundary has already been crossed. On the success path `#flushCrankBuffer`
settles the promise `enqueueMessage` handed an external caller, and only
then can `#terminateVat` throw and have the new catch roll the crank back
— so the caller keeps an answer computed from state the store discarded,
and a restart delivers the message again. On the abort path the rollback
ends the transaction, so `#terminateVat` and `collectGarbage` autocommit
piecemeal and the second rollback the flag correctly suppresses would
have had nothing left to undo either way. The invariant is stated as "the
rollback is the last thing the crank asks of the store", which leaves the
choice of remedy open.

The wasm driver tracks `_inTx` itself rather than reading it from SQLite,
so a failed abort inside the new catch is the one case that can leave it
disagreeing with the database. Left true, `beginIfNeeded` is a no-op from
then on and the next `createSavepoint` runs in autocommit mode, where the
matching RELEASE commits (Agoric/agoric-sdk#8423, already cited two lines
above the code) and no rollback can undo the delivery. The second test
runs that next `createSavepoint` and asserts the BEGIN, so the corruption
path is observable instead of argued.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Three transaction-integrity defects, all in the same family: a store call
fails, and the layer above goes on as though its bookkeeping still matched
the database.

- `releaseSavepoint` (both SQLite drivers) discards the enclosing
  transaction when `RELEASE` fails, as `rollbackSavepoint` already does
  when `ROLLBACK TO` fails. Left as it was, the savepoint stayed on the
  stack and the transaction open with nothing to ever commit or abort it,
  so every later write on the connection joined it, reported success, and
  vanished on `close()`.
- `releaseAllSavepoints` forgets its savepoints even if the release
  throws, as `rollbackCrank` already does. A savepoint left listed had the
  next crank number its savepoint `t1` while the database still had `t0`,
  from which point every release and rollback aimed one crank past the one
  it meant to end.
- The wasm driver stops believing it is in a transaction when an abort
  fails. `_inTx` is tracked in the driver rather than read from SQLite, and
  an abort usually fails because SQLite already rolled back on its own.
  Left true, `beginIfNeeded` was a no-op from then on and the next
  `createSavepoint` ran in autocommit mode, where its `RELEASE` commits
  (Agoric/agoric-sdk#8423) and no later rollback could undo the delivery.

And the crank boundary itself, in two parts:

- A crank now takes two savepoints. Rolling back to the outermost one
  discards the enclosing transaction, so the work an aborted crank still
  owes — terminating the vat whose delivery failed, collecting garbage —
  was autocommitting statement by statement, beyond the reach of any later
  rollback. That work has to follow the rollback, since the worker is gone
  and the store must not go on believing the vat is alive, so it is the
  rollback that spares the transaction. Releasing the outer savepoint in
  `endCrank` is now a crank's one commit point.
- `#flushCrankBuffer` runs last, after everything that can still fail.
  It settles the promise `enqueueMessage` handed an external caller,
  reading the result out of the store; rolling the crank back after that
  left the caller holding an answer computed from state the store had
  discarded, and a restart would deliver the message again.

Tests for the first three defects are Ryan's, from #1011. The two crank
tests there specify the remedy as "the rollback is the last thing the
crank asks of the store", which reordering the fallible work before it
would satisfy — but that rollback would then undo the vat termination.
They are restated here as the invariant the fix does hold.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

`should trigger GC syscalls through bringOutYourDead` scheduled one reap
and then ran three cranks. `scheduleReap` dedupes, so that bought one
`bringOutYourDead`, not three — and an import is only reported as dropped
once the engine has collected the vat's presence and run its finalizer,
which the forced GC pass inside `bringOutYourDead` cannot guarantee on the
first attempt. When it hadn't, no further reap was ever scheduled and the
refcount stayed where it was: `expected 2 to be 1`, as on main in
31081630878.

Each attempt now schedules its own reap and stops as soon as the kernel's
bookkeeping catches up, so the common case is one crank rather than three.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

A failed `ROLLBACK TO` discards the whole transaction, taking every
savepoint with it — not just the one rolled back to. `rollbackCrank`
truncated `ctx.savepoints` to the rolled-back ordinal regardless, which
was correct while a crank took one savepoint at ordinal 0 and cleared the
list, but leaves `['crank']` listed now that the delivery sits at ordinal
1.

`endCrank` then releases a `t0` the database no longer has, and throws
"No such savepoint: t0" from the run loop's `finally` — replacing the
failure that actually killed the kernel, with no `cause`. That is the
masking this branch's own error-preservation exists to prevent.

Clear the list on the throwing path, truncate to the ordinal only on
success.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…tion

Both drivers recover from a failed savepoint operation by discarding the
enclosing transaction, and swallow any error from that abort so the
savepoint failure stays the one reported. That part is right, but it left
the abandoned transaction entirely silent: on the nodejs driver, where
`inTransaction` is read from SQLite, the next crank's `beginIfNeeded`
sees the transaction still open, skips its `BEGIN`, and commits the dead
crank's writes alongside the new crank's.

Nothing here can repair that, so at least record it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Moving `#invokeKernelSubscription` out of the enqueue loop and after it
was the one production change on this branch with no test: reverting
`#flushCrankBuffer` to its interleaved form left all 2412 ocap-kernel
tests passing.

Same hazard as the crank-level ordering a few tests up, one level down —
`#enqueueRun` is store work and can fail part-way, so answering the first
caller while the second enqueue is still ahead hands out a result the
crank's rollback then discards.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Five comments on this branch asserted more than the code holds:

- `wasm.ts` claimed a stale `_inTx` meant "no later rollback can undo the
  delivery". False: a savepoint created in autocommit mode does open a
  transaction, and an inner savepoint still rolls back. The real cost is
  that writes outside a savepoint autocommit one statement at a time, and
  the outermost `RELEASE` commits. The "an abort typically fails because
  SQLite already rolled back" premise was unsupported and isn't the
  reason for the reorder — the reason is simply that the abort can throw.
- `#processCrankResult` said "the worker is already gone" ahead of the
  call that kills the worker.
- The flush was described as running "once nothing fallible remains".
  It doesn't: `#terminateVat` resolves the dying vat's promises through
  `resolvePromises`, which defaults to `immediate` and invokes their
  kernel subscriptions before `collectGarbage`. Reachable without an
  abort, via a clean `exitVat`. Recorded rather than fixed — closing it
  changes termination semantics, not crank ordering.
- "Only `delivery` is ever rolled back" is true of the run loop but not
  of the tests. Scoped, and the ordinal coupling it depends on is now
  stated: `endCrank` releases `t0` by position, so `crank` must stay
  first.
- `reapImporterUntil` credited `scheduleReap` deduping for the old
  one-BOYD behaviour; it was `nextReapAction` shifting the single entry
  off, leaving the later cranks nothing to do.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Comment the non-obvious why, in the shortest form that carries it. The
two-savepoint rationale was re-argued in full in four places; the tests
now point at `#runLoop` and `#processCrankResult` instead of restating
them, and the hazard block duplicated across both driver test files is a
line. No reasoning removed, only the retelling.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…lback

A database rollback cannot reach two pieces of state, so `rollbackCrank`
now reverts both itself.

Every `provideCachedStoredValue` answers reads from a closure and only
writes through to kv. Reverting the database therefore left the closure
holding the abandoned crank's value, and the next `set` persisted it.
`processGCActionSet` takes an action out of the set before delivering it,
so an aborted delivery lost the action outright rather than retrying it.
`reapQueue` was exposed the same way.

`maybeFreeKrefs` lives in RAM, so nothing reverted it either. Its entries
are collection candidates only because of the decrements the rollback
undid, and a later `collectGarbage` threw outright on a promise the
rollback had deleted, killing the run loop.

No live bug either way: every `abort` `#deliverGCAction` returns is paired
with a `terminate`, which is what made losing the action harmless. The
comment there claimed the rollback restored the action, which is the thing
a future reader would trust when adding an abort path that isn't paired
with a termination; it now states the real causality.

The cached values are declared once so that initialization and the
refresher cannot disagree about which ones exist.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Failing repro, not a fix.

## The issue

`rollbackIfNeeded` was corrected in #1012 to clear `_inTx` *before* stepping
the abort, because the abort can throw and `_inTx` is tracked in the driver
rather than read from SQLite. `commitIfNeeded` has the identical shape and was
left alone:

    function commitIfNeeded(): void {
      if (db._inTx && db._spStack.length === 0) {
        sqlCommitTransaction.step();   // can throw
        sqlCommitTransaction.reset();
        db._inTx = false;              // ...so this never runs
      }
    }

A COMMIT that throws leaves `_inTx` true against a database that may hold no
transaction. `beginIfNeeded` is then a no-op forever after, so the next
`createSavepoint` issues its SAVEPOINT outside a transaction — and a savepoint
taken outside a transaction commits when it is released
(Agoric/agoric-sdk#8423). That is the hazard the whole `beginIfNeeded` dance
exists to prevent, and `commitIfNeeded` is reached from `releaseSavepoint`,
which is the crank's commit point. The writes that leak are a whole crank's.

The nodejs driver is unaffected, for the same reason it was unaffected by the
abort case: it reads `db.inTransaction` live from SQLite.

Worth noting that the comment introduced above `stops believing it is in a
transaction when the abort fails too` asserts that a failed abort is "the one
case that can leave `_inTx` disagreeing with the database". This is the second
case, so that comment needs correcting along with the code.

## What we hope to see instead

`releaseSavepoint` still throws the COMMIT failure, but `_inTx` is false
afterwards, so the next `createSavepoint` opens a transaction of its own
instead of creating a bare savepoint. Same two-line reorder as
`rollbackIfNeeded`, and the "one case" comment updated.

## Current failure

    AssertionError: expected true to be false
      packages/kernel-store/src/sqlite/wasm.test.ts
      > stops believing it is in a transaction when the commit fails

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Failing repro, not a fix.

## The issue

#1012 fixes one error-masking path at the start of a dying crank and opens
another at its end.

Before the two-savepoint scheme, `rollbackCrank('start')` emptied
`ctx.savepoints`, so `endCrank` -> `releaseAllSavepoints` was a guaranteed
no-op on the dying path: nothing to release, nothing that could throw. Now
`rollbackCrank('delivery')` truncates to the ordinal and leaves `['crank']`
behind (crank.ts:56, deliberately — that is what keeps the transaction open for
the work an aborted crank still owes). So `endCrank` issues a real
`RELEASE t0`, which commits, which can fail.

`#runLoop` calls it from a bare `finally`:

    } finally {
      this.#kernelStore.endCrank();
      ...
    }

A throw there replaces the pending exception. The disk error that actually
killed the kernel is discarded — not demoted to `cause`, discarded — and
`run()` rejects with the release failure instead. `#failRunLoop` records that,
so `getRunLoopStatus().detail` loses the root cause too, and
`onRunLoopFailure` — what the daemon logs as fatal — gets the wrong error.

A/B against origin/main with the same repro: main reports `crank exploded`,
this branch reports `database is gone` with `cause: undefined`.

This is the same class of bug as the `No such savepoint: t0` masking that
82b88ce fixes, and the same class the `reports both failures when the
rollback also fails` test above already guards on the other path.

## What we hope to see instead

Whatever names the release failure, the error that killed the crank stays
reachable. The rollback path already has the shape to copy:

    throw new Error(
      `Run loop died and its crank could not be rolled back: ${...}`,
      { cause: error },
    );

The assertion is deliberately fix-agnostic — it walks the `cause` chain — so
either wrapping `endCrank`'s failure with the original as `cause`, or reporting
it and rethrowing the original, will satisfy it.

## Current failure

    AssertionError: expected [ Error: database is gone ]
      to include Error: crank exploded
      packages/ocap-kernel/src/KernelQueue.test.ts
      > reports both failures when endCrank also fails

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Failing repro, not a fix.

## The issue

#1012 replaces four silently-swallowed aborts with `logger?.error(...)` in the
SQLite drivers, and its description says: "Four swallowed aborts were silent.
Now logged." They are not. No production call site passes a `logger` to
`makeSQLKernelDatabase`, so every one of those calls is dead code:

    packages/kernel-node-runtime/src/kernel/make-kernel.ts:63
    packages/kernel-browser-runtime/src/kernel-worker/kernel-worker.ts:47
    packages/kernel-test-local/src/lms-chat.ts:30
    packages/kernel-node-runtime/test/helpers/remote-comms.ts:172

`make-kernel.ts` is the clearest case: it builds a `rootLogger` and hands
sub-loggers to `NodejsPlatformServices` and to `Kernel.make`, then constructs
the store with `{ dbFilename }` alone. The store is the one collaborator that
gets no logger. Nor does any test pass one, which is why the gap survived
review.

This matters more than a missing log line. On the nodejs driver a failed abort
leaves `db.inTransaction` true with nothing that will ever commit or abort it,
so later writes on that connection join a transaction that vanishes on close.
The driver's own comment concedes "Nothing here can repair that" — the log is
the entire remedy, and it does not reach anyone.

`logger?.error` is the right convention for this package; the injection is what
is missing.

## What we hope to see instead

`makeKernel` passes a tagged sub-logger to `makeSQLKernelDatabase`, as it
already does for its other collaborators — something like
`rootLogger.subLogger({ tags: ['store'] })`. The other three call sites want the
same treatment, and are worth covering once this one is fixed.

## Current failure

    AssertionError: expected "vi.fn()" to be called with arguments:
      [ ObjectContaining{…} ]
    -     "logger": Any<Logger>,
      packages/kernel-node-runtime/src/kernel/make-kernel.test.ts
      > gives the kernel store a logger

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Failing repro, not a fix.

## The issue

#1012 hardens `releaseSavepoint` so that a failed `RELEASE` discards the
enclosing transaction, clearing the driver's `_spStack` on the way. Two callers
it does not touch depend on the old behaviour, and both are now worse off than
before the change.

`RemoteHandle.handleRemoteMessage` releases inside the `try` and rolls back in
the `catch`:

    this.#kernelStore.setRemoteHighestReceivedSeq(this.remoteId, seq);
    this.#kernelStore.releaseSavepoint(savepointName);   // fails
  } catch (error) {
    this.#kernelStore.rollbackSavepoint(savepointName);  // "No such savepoint"
    throw error;                                        // never reached
  }

Since the release already cleared the stack, the rollback throws
`No such savepoint: receive_r0_1`, which escapes the `catch` and replaces the
real failure. Not demoted to `cause` — replaced. `RemoteManager` has the same
shape at its `peerIncarnation_*` savepoint.

A/B verified against origin/main with a real driver: main's rollback succeeds
and `database or disk is full` propagates; on this branch the caller gets the
missing-savepoint error instead. So the PR description's "the release failure
still propagates" holds for the crank path it fixed and not for these two.

`crank.ts:57-63` shows the author recognised exactly this hazard — a stale
savepoint list producing `No such savepoint` over the real error — and fixed it
for the crank only. The remote paths were missed because nothing exercised them.

Note the secondary effect these tests don't reach: `ctx.savepoints` still lists
the crank's own savepoints after this, so the next `endCrank` throws
`No such savepoint: t0` over whatever is left of the failure.

## What we hope to see instead

The failure the database reported is what reaches the caller. Any of these does
it, and the assertion doesn't care which:

- move the release out of the `try`, so a release failure isn't followed by a
  rollback attempt at all
- have the `catch` tolerate a rollback that reports a savepoint already
  discarded, rethrowing the original either way
- make the driver's discard leave the name rollback-able as a no-op

The mock models the drivers' bookkeeping rather than the expected outcome, so it
is `RemoteHandle`'s error handling under test, not the mock's.

## Current failure

    AssertionError: expected Error: No such savepoint: receive_r0_1
      to be Error: database or disk is full
      packages/ocap-kernel/src/remotes/kernel/RemoteHandle.test.ts
      > reports the release failure rather than a missing savepoint

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

No change to what any of them proves; all four still fail for the reasons
their own commits describe.

- `RemoteHandle`: assert the rollback is still *attempted*. Without this,
  deleting the rollback from the `catch` outright would turn the test green,
  which is not the fix — a `RELEASE` that failed for a reason of its own may
  well have left the savepoint standing.
- `RemoteHandle`: drop an unnecessary `as KernelStore` cast, and say why the
  store is replaced wholesale rather than having its methods assigned over
  (`makeKernelStore` hardens what it returns).
- `make-kernel`: note that `kernel-worker.ts` omits the logger too, so the wasm
  driver's pair of `logger?.error` calls stays dead even once this test passes.
  Use `vi.mocked`, as the sibling `make-kernel-options.test.ts` does.
- `causeChain` returns `Error[]`; every element is already narrowed by the loop
  guard.
- Drop "see the commit message for this test" from the four comment blocks: each
  stands alone, and the reference would not survive a squash-merge. Restate the
  claim the wasm comment made by citing a neighbouring test's title, which would
  have broken silently on rename.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

`rollbackIfNeeded` was corrected for this ordering already; `commitIfNeeded`
still stepped the COMMIT first. `_inTx` is tracked in the driver rather than
read from SQLite, so a throwing COMMIT wedged it true: `beginIfNeeded` became a
permanent no-op, and the next savepoint was created bare — where its RELEASE
commits (Agoric/agoric-sdk#8423) and no later rollback could undo the delivery.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…un loop

Now that the delivery rollback spares the `crank` savepoint, `endCrank`'s
release is a real RELEASE and COMMIT on the dying path, where it used to be a
no-op. `#runLoop` called it from a bare `finally`, so a failing one silently
replaced whatever killed the kernel — and only `error.message` crosses the
wire, so the real failure reached neither `getStatus` nor the daemon log.

Report it with the crank's failure as the `cause`, the shape the rollback path
already uses. The in-flight error is boxed rather than left `undefined`, so a
crank that threw `undefined` stays distinguishable from one that did not throw.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…vepoint

`RemoteHandle.handleRemoteMessage` releases its savepoint inside the `try` and
rolls back in the `catch`. Now that a failed RELEASE discards the whole
savepoint stack, that rollback names a savepoint that is already gone and threw
`No such savepoint` out of the `catch` in place of the database failure that
brought it there — not even as `cause`.

Log the rollback failure instead of throwing it. The rollback is still
attempted, because a release that failed for a reason of its own may well have
left the savepoint standing.

`RemoteManager`'s `peerIncarnation_*` savepoint has the identical shape and had
no coverage of it at all, so a fix applied here and forgotten there would have
left its suite green. Fixed alike, and the savepoint-stack model both tests
drive the drivers with is now shared.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…llback

`rollbackCrank` gained two pieces of work that compose the wrong way round if
the failure path simply rethrows: forgetting every savepoint, and reverting the
caches a database rollback cannot reach. A failed rollback discards the whole
transaction, so the database has moved back at least as far as a successful
rollback would have taken it and those caches are at least as stale — the one
case where skipping the revert leaves the consumed GC action lost and krefs
queued for a collection that then kills the run loop.

The second test pins the other direction: reverting must not become a way to
lose the database error either.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

No production call site passed one, so every `logger?.` call in the SQLite
drivers was dead code — including the aborts they report while discarding a
transaction, which fire on exactly the path where the kernel is already dying
and a diagnostic is worth most.

The browser worker has a module-level `Logger` already, so both drivers are
covered rather than only the nodejs one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The crank-transaction and rollback work landed here rather than in #1012,
which is closed and replaced. #1021 is a placeholder until the PR exists.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Follow-up to the c-list accounting fix, addressing defects found in review.

An owner that stops naming its own export left the object behind. Both the
delivered `retireExport` and the `retireExports`/`abandonExports` syscalls tore
down the owner's c-list entry but left `owner` and `refCount` in place, with no
path that could ever reclaim them: `cleanupTerminatedVat` finds krefs by walking
the owner's c-list, and the collector only revisits krefs in `maybeFreeKrefs`.
The records leaked, and the next collection to visit such a kref read the
owner's deleted entry through `getRequired` and took the run loop down with it.
New `orphanKernelObject` drops the owner mapping and hands the object to the
collector, which already knows how to retire an orphan. `collectGarbage` also
treats an owner with no c-list entry as orphaned rather than trusting the
mapping.

Reporting a dead run loop belongs to #1005, which landed on main first. It is
what makes the audit usable at all: `assertRefCountsIfAuditing` throws from
inside a crank, so with the failure logged and swallowed a violation's sole
symptom was a test hanging to its timeout with no mention of reference counts.
The `kernel-test` case here asserts that shape — the caller is told the run loop
died, and the audit error rides along as the `cause`.

Also: GC action delivery survives a vanished endpoint or a failed delivery
instead of stopping the loop; `launchVat` tears down a worker whose kernel-side
registration failed rather than stranding it; `RefCountViolation` discriminates
on `kind` instead of sentinel-matching `stored`; and the store context's
auditing flag no longer shares a name with `auditRefCounts()`.

Tests cover the crash path, the orphan-and-collect sequence, retiring
stragglers, GC-action robustness, and that a violation reaches a caller. The
`item.target` charge and both `deliver|notify` early returns now have assertions
that fail if the fix is reverted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Review of the previous commit found that four of the five error handlers it
added turned a crash into a state the kernel can no longer detect. Corrects
that, and closes a hole the orphaning opened.

`orphanKernelObject` took an object's owner mapping on trust. Nothing upstream
of `performExportCleanup` checks that the vref it was handed is even an export —
`translateSyscallVtoK` maps both directions alike — so a vat could pass an
import to `abandonExports`, which needs no precondition at all, and erase a
different live vat's claim to an object it was still exporting. Sends to that
object then went splat with OBJECT_DELETED, terminating the victim tripped
`cleanupTerminatedVat`'s ownership assertion and took the run loop with it, and
the audit could not see any of it, because an export entry carries no count.
Disowning is now the owner's own doing: the expected owner is a required
argument and must match, and the syscall path rejects a mismatch outright.

The vanished-endpoint catch returned before the teardown, but
`processGCActionSet` had already consumed the action, so neither the kernel nor
the durable set remembered the object — a permanent leak, also invisible to the
audit. The kernel's side is now released whether or not anyone is left to tell,
and krefs whose entries a cleanup already removed are skipped rather than
assumed present.

The delivery-failure catch committed the teardown after the endpoint had failed
to hear about it, so the endpoint would go on to mint a fresh kref for an object
the kernel believed it had let go of — the same object with two identities. It
now aborts, which restores both the entries and the action, and terminates the
vat that could not accept the delivery.

`launchVat`'s cleanup path stopped the worker without marking the vat
terminated, so nothing ever reclaimed the records a partial launch had written.

The audit counted an importer's c-list entry as a holder during the window
between `retireKernelObjects` deleting an object and delivering the matching
`retireImport`, so the collector's own output failed the end-of-crank check. The
missing assertion in the test covering that sequence is now present.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…very

Aborting a failed GC delivery restores the action to the durable set, and
`processGCActionSet` is consulted ahead of all other run-queue work. For a vat
that is fine, because terminating it is what stops the restored action from
coming back. A remote cannot be terminated, so the same item would be selected
every crank and nothing else would ever run. A remote is a separate kernel
across a link that can drop messages anyway, and it reconciles on the next
incarnation change, so its failures no longer abort.

Also stop `orphanKernelObject` throwing on an object that is already orphaned.
Disowning something nobody owns is a no-op, not an error: only a mismatch with a
different, live owner is, which is the case the check exists for. Same for the
syscall path.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Only `deleteVat` removes `vatConfig.<vatId>`, and `cleanupTerminatedVat`
sweeps `${vatId}.`-prefixed keys, which never match it. The writes making up
a vat's death were interleaved with awaits across `VatManager.stopVat`,
`#endVat`'s `finally`, `VatHandle.terminate` and a lambda in `Kernel.ts`, so a
throw part-way left the vat marked terminated with its config alive — which
reads as *active* again as soon as cleanup drops the mark, killing the run
loop over the disagreement and resurrecting the vat on the next process start.

`VatManager.#retireVat` now makes all four writes with no await between them,
modelled on SwingSet's synchronous prelude in `kernel.js` `terminateVat`;
worker teardown follows and is best-effort. `#endVat` and `#abandonVat` go as
duplicates of it, and `VatHandle.terminate` is left with only its own channel
to close.

A vat whose stream fails is retired by the manager, via a new
`onCriticalFailure`, rather than tearing itself down: that left the handle in
the manager and the vat live in the store, so the next delivery went to a
worker that could not answer and, the vat RPC client having no timeout, the
crank never completed while the run loop still reported itself running.

`makeGCAndFinalize` drains the queues before sweeping, since a pending
continuation still holds its closure's objects, so a vat reports its dropped
imports on the `bringOutYourDead` that provoked them rather than a later one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…tial launch terminated

Two conflicts the rebase onto the GC-hardening stack surfaced, both real
disagreements rather than textual ones.

`revertStateBeneathRollback` cleared `maybeFreeKrefs` outright. The set is not
per-crank — only `collectGarbage` empties it — so a candidate added while the
run loop was idle, which `terminateVat` unpinning a root produces, was owed a
collection and lost it to an unrelated crank's rollback. It now restores the
savepoint's snapshot, which discards the abandoned crank's additions and keeps
everything that predates it. The unit test had encoded the old behaviour and is
updated to distinguish the two cases.

`launchVat`'s cleanup relied on `stopVat` reaching `#retireVat` to record the
death, but `stopVat` refuses a vat the kernel has no handle for and the store
does not call active — which is what a partial launch looks like. The mark is
asserted directly again, as it was before this branch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Recording the vat's death only saves the deliveries that come after it. The one
in flight when the worker died stays parked on an RPC client with no timeout, so
its crank never completes — the same hang `onCriticalFailure` exists to prevent,
one delivery earlier. The worker was left running too, since nothing else would
stop it once the handle was off the books.

Found by Cursor Bugbot on #1023.

Also reverts this branch's additions to the extension control-panel e2e test.
They asserted `ko6.refCount` directly, and which vat owns `ko6` is not stable:
order. The behaviour they checked is covered by the refcount audit, which runs
on every kernel `kernel-test` builds.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

… exists

`onCriticalFailure` closed over the binding `VatHandle.make` was still to return, so a stream that broke during `#init` threw on the temporal dead zone — after the vat had been retired, and before anything rejected the pending `initVat` that nothing else will ever settle. The handle is now passed to the callback, and `runVat` refuses to put a handle on the books for a vat retired while it was being made.

The teardown also awaited the worker kill before `terminate`, which is what rejects the vat's pending RPCs. A worker slow to die — or one that never does — kept the parked delivery parked, which is the hang this path exists to clear. The two now run alongside each other, `terminate` first.

Found by Cursor Bugbot on #1023.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…point

`KernelQueue.#runLoop` calls releasing its `crank` savepoint "this crank's one
commit point". `releaseAllSavepoints` releases `t0`, which is the outermost
savepoint only when the crank opened the first one, and two production paths
open savepoints through `KernelStore.createSavepoint` -- invisible to the
ordinal numbering, uncoordinated with the crank, one of them held across an
await.

Real SQLite through the real driver, one test per interleaving.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

A failed `ROLLBACK TO` is taken to discard the whole transaction, which is what
makes truncating the savepoint list to zero match the database. That holds only
when the compensating abort succeeds, and both drivers catch and log one that
does not. This driver reads `db.inTransaction` from SQLite, so it cannot wedge a
flag -- and also cannot end an ownerless transaction.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Clearing `_inTx` before stepping the COMMIT stops a throwing COMMIT wedging the
flag true, and leaves nothing able to end the transaction it left open:
`rollbackIfNeeded` reads the false flag and returns, and `releaseSavepoint`
reaches `commitIfNeeded` with nothing wrapping it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…s callers

The flush is last of the crank's own work so that no external caller is answered
before the fallible work is done, and the reference count audit runs after the
flush so that buffered items are not read as leaks. The audit is itself
fallible, so the two orderings contradict each other.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

`maybeFreeKrefs` is not per-crank: only `collectGarbage` empties it, so a
candidate added while no crank was open must survive an unrelated crank's
rollback. `RemoteManager.#handlePeerIncarnation` is such a producer, and the
objects it abandons are invisible to the reference count audit once lost.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>