From aabed6a9203508e01e08f5a957e8b1d85eee4a49 Mon Sep 17 00:00:00 2001
From: grypez <143971198+grypez@users.noreply.github.com>
Date: Thu, 6 Aug 2026 07:17:49 -0400
Subject: [PATCH 01/10] test: pin the transaction invariants #1005 left broken
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 .../kernel-store/src/sqlite/nodejs.test.ts    |  23 ++++
 packages/kernel-store/src/sqlite/wasm.test.ts |  69 ++++++++++++
 packages/ocap-kernel/src/KernelQueue.test.ts  | 106 ++++++++++++++++++
 .../src/store/methods/crank.test.ts           |  17 +++
 4 files changed, 215 insertions(+)

diff --git a/packages/kernel-store/src/sqlite/nodejs.test.ts b/packages/kernel-store/src/sqlite/nodejs.test.ts
index a62392fe1..3bf0db5f6 100644
--- a/packages/kernel-store/src/sqlite/nodejs.test.ts
+++ b/packages/kernel-store/src/sqlite/nodejs.test.ts
@@ -360,6 +360,29 @@ describe('makeSQLKernelDatabase', () => {
       expect(mockDb._spStack).toStrictEqual([]);
     });
 
+    // The same hazard `rollbackSavepoint` guards against, by the other door: a
+    // RELEASE that throws leaves the savepoint on the stack and the transaction
+    // open with nothing to ever commit or abort it, so every later write on this
+    // connection joins it, reports success, and vanishes on close.
+    it('releaseSavepoint discards the transaction when the release fails', async () => {
+      const db = await makeSQLKernelDatabase({});
+      mockDb.inTransaction = true;
+      mockDb._spStack = ['point1'];
+      mockStatement.run.mockClear();
+      mockDb.exec.mockImplementationOnce(() => {
+        throw new Error('disk I/O error');
+      });
+
+      expect(() => db.releaseSavepoint('point1')).toThrowError(
+        'disk I/O error',
+      );
+
+      expect(mockDb._spStack).toStrictEqual([]);
+      // The abort is the only prepared statement this path runs.
+      expect(mockStatement.run).toHaveBeenCalledOnce();
+      mockDb.inTransaction = false;
+    });
+
     it('supports nested savepoints', async () => {
       const db = await makeSQLKernelDatabase({});
       db.createSavepoint('outer');
diff --git a/packages/kernel-store/src/sqlite/wasm.test.ts b/packages/kernel-store/src/sqlite/wasm.test.ts
index 2cbc96d65..e0e6be1c4 100644
--- a/packages/kernel-store/src/sqlite/wasm.test.ts
+++ b/packages/kernel-store/src/sqlite/wasm.test.ts
@@ -518,6 +518,75 @@ describe('makeSQLKernelDatabase', () => {
       expect(mockDb._inTx).toBe(false);
     });
 
+    // The same hazard `rollbackSavepoint` guards against, by the other door: a
+    // RELEASE that throws leaves the savepoint on the stack and the transaction
+    // open with nothing to ever commit or abort it, so every later write on this
+    // connection joins it, reports success, and vanishes on close.
+    it('releaseSavepoint discards the transaction when the release fails', async () => {
+      const db = await makeSQLKernelDatabase({});
+      mockDb._inTx = true;
+      mockDb._spStack = ['point1'];
+      mockDb.exec.mockImplementationOnce(() => {
+        throw new Error('disk I/O error');
+      });
+
+      expect(() => db.releaseSavepoint('point1')).toThrowError(
+        'disk I/O error',
+      );
+
+      expect(mockDb._spStack).toStrictEqual([]);
+      expect(mockDb._inTx).toBe(false);
+    });
+
+    // `_inTx` is tracked here rather than read from SQLite, so a failed abort is
+    // the one case that can leave it disagreeing with the database. Left true,
+    // `beginIfNeeded` becomes a no-op forever after.
+    it('stops believing it is in a transaction when the abort fails too', async () => {
+      const db = await makeSQLKernelDatabase({});
+      mockDb._inTx = true;
+      mockDb._spStack = ['point1'];
+      mockDb.exec.mockImplementationOnce(() => {
+        throw new Error('disk I/O error');
+      });
+      mockStatement.step.mockImplementationOnce(() => {
+        throw new Error('cannot rollback');
+      });
+
+      expect(() => db.rollbackSavepoint('point1')).toThrowError(
+        'disk I/O error',
+      );
+
+      expect(mockDb._inTx).toBe(false);
+    });
+
+    // The consequence of the above, and the reason it is worth asserting: a
+    // savepoint created outside a transaction autocommits when released
+    // (Agoric/agoric-sdk#8423), so no later rollback can undo the delivery — an
+    // aborted crank would silently keep its writes.
+    it('begins a transaction for the next savepoint after a failed abort', async () => {
+      const db = await makeSQLKernelDatabase({});
+      mockDb._inTx = true;
+      mockDb._spStack = ['point1'];
+      mockDb.exec.mockImplementationOnce(() => {
+        throw new Error('disk I/O error');
+      });
+      mockStatement.step.mockImplementationOnce(() => {
+        throw new Error('cannot rollback');
+      });
+      expect(() => db.rollbackSavepoint('point1')).toThrowError(
+        'disk I/O error',
+      );
+
+      mockDb.exec.mockClear();
+      mockStatement.step.mockClear();
+      db.createSavepoint('next');
+
+      // BEGIN is the only prepared statement `createSavepoint` runs; the
+      // SAVEPOINT itself goes through `exec`.
+      expect(mockStatement.step).toHaveBeenCalledOnce();
+      expect(mockDb.exec).toHaveBeenCalledWith('SAVEPOINT next');
+    });
+
     it('supports nested savepoints', async () => {
       const db = await makeSQLKernelDatabase({});
       db.createSavepoint('outer');
diff --git a/packages/ocap-kernel/src/KernelQueue.test.ts b/packages/ocap-kernel/src/KernelQueue.test.ts
index d7beb0e2f..073c08d31 100644
--- a/packages/ocap-kernel/src/KernelQueue.test.ts
+++ b/packages/ocap-kernel/src/KernelQueue.test.ts
@@ -194,6 +194,112 @@ describe('KernelQueue', () => {
       expect(kernelStore.collectGarbage).toHaveBeenCalled();
       expect(kernelStore.endCrank).toHaveBeenCalled();
     });
+
+    // `#flushCrankBuffer` settles the promise `enqueueMessage` handed an external
+    // caller, reading the resolution out of the store on the way. Rolling the
+    // crank back afterwards un-resolves that promise in the store and restores
+    // the run queue item, so a restart delivers the message a second time and
+    // notifies every other subscriber again — while the original caller has
+    // already been told the first answer.
+    it('does not roll back a crank whose result the caller already received', async () => {
+      const mockItem: RunQueueItem = {
+        type: 'send',
+        target: 'ko123',
+        message: { result: 'kp1' } as KernelMessage,
+      };
+      (kernelStore.runQueueLength as unknown as MockInstance)
+        .mockReturnValueOnce(1)
+        .mockReturnValue(0);
+      (kernelStore.dequeueRun as unknown as MockInstance).mockReturnValueOnce(
+        mockItem,
+      );
+
+      // A caller is awaiting this message's result.
+      const resolve = vi.fn();
+      const reject = vi.fn();
+      kernelQueue.subscriptions.set('kp1', { resolve, reject });
+
+      // The crank succeeds, so the flush hands that caller its answer...
+      (
+        kernelStore.flushCrankBuffer as unknown as MockInstance
+      ).mockReturnValueOnce([
+        { type: 'notify', endpointId: 'v1', kpid: 'kp1' },
+      ]);
+      (kernelStore.getKernelPromise as unknown as MockInstance).mockReturnValue(
+        {
+          state: 'fulfilled',
+          value: { body: '"answer"', slots: [] },
+        },
+      );
+
+      // ...and only then does the kernel die, in work that runs after the flush.
+      const terminationError = new Error('vat worker already gone');
+      (terminateVat as unknown as MockInstance).mockRejectedValueOnce(
+        terminationError,
+      );
+      const deliver = vi.fn().mockResolvedValue({
+        terminate: { vatId: 'v1', info: { body: '"exit"', slots: [] } },
+      });
+
+      await expect(kernelQueue.run(deliver)).rejects.toBe(terminationError);
+
+      expect(resolve).toHaveBeenCalledWith({ body: '"answer"', slots: [] });
+      expect(kernelStore.rollbackCrank).not.toHaveBeenCalled();
+    });
+
+    // `rollbackCrank('start')` rolls back the crank's outermost savepoint, which
+    // ends the transaction — so anything the crank does to the store afterwards
+    // autocommits piecemeal and no rollback can reach it. Whatever the ordering,
+    // the rollback has to be the last thing the crank asks of the store.
+    it.each([
+      { label: 'an abort', crankResult: { abort: true } },
+      {
+        label: 'an abort that also terminates',
+        crankResult: {
+          abort: true,
+          terminate: { vatId: 'v1', info: { body: '"exit"', slots: [] } },
+        },
+      },
+    ])(
+      'does no store work after rolling back $label',
+      async ({ crankResult }) => {
+        const mockItem: RunQueueItem = {
+          type: 'send',
+          target: 'ko123',
+          message: { result: 'kp99' } as KernelMessage,
+        };
+        (kernelStore.runQueueLength as unknown as MockInstance)
+          .mockReturnValueOnce(1)
+          .mockReturnValue(0);
+        (kernelStore.dequeueRun as unknown as MockInstance).mockReturnValueOnce(
+          mockItem,
+        );
+
+        const storeCalls: string[] = [];
+        (
+          kernelStore.rollbackCrank as unknown as MockInstance
+        ).mockImplementation(() => {
+          storeCalls.push('rollbackCrank');
+        });
+        (terminateVat as unknown as MockInstance).mockImplementation(
+          async () => {
+            storeCalls.push('terminateVat');
+          },
+        );
+        (
+          kernelStore.collectGarbage as unknown as MockInstance
+        ).mockImplementation(() => {
+          storeCalls.push('collectGarbage');
+          throw new Error(STOP_RUN_LOOP);
+        });
+
+        const deliver = vi.fn().mockResolvedValue(crankResult);
+        await expect(kernelQueue.run(deliver)).rejects.toThrow(STOP_RUN_LOOP);
+
+        expect(storeCalls).toContain('rollbackCrank');
+        expect(storeCalls.at(-1)).toBe('rollbackCrank');
+      },
+    );
   });
 
   describe('getRunLoopStatus', () => {
diff --git a/packages/ocap-kernel/src/store/methods/crank.test.ts b/packages/ocap-kernel/src/store/methods/crank.test.ts
index db3de8645..8fd6cd1fe 100644
--- a/packages/ocap-kernel/src/store/methods/crank.test.ts
+++ b/packages/ocap-kernel/src/store/methods/crank.test.ts
@@ -206,6 +206,23 @@ describe('crank methods', () => {
       expect(context.resolveCrank).toBeUndefined();
       expect(await waiter).toBeUndefined();
     });
+
+    // What `rollbackCrank` already does in its own `finally`. Settling the crank
+    // regardless means callers proceed, so a savepoint left listed here has the
+    // next crank number 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.
+    it('forgets its savepoints even if releasing them fails', () => {
+      crankMethods.startCrank();
+      context.savepoints = ['test'];
+      vi.mocked(kdb.releaseSavepoint).mockImplementationOnce(() => {
+        throw new Error('database is gone');
+      });
+
+      expect(() => crankMethods.endCrank()).toThrow('database is gone');
+
+      expect(context.savepoints).toStrictEqual([]);
+    });
   });
 
   describe('releaseAllSavepoints', () => {

From 601504d68159e9bc8740cfd48b1a846164a9ad04 Mon Sep 17 00:00:00 2001
From: Dimitris Marlagkoutsos <info@sirtimid.com>
Date: Thu, 6 Aug 2026 16:23:28 +0200
Subject: [PATCH 02/10] fix: keep a crank's store work inside one transaction
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 .../kernel-store/src/sqlite/nodejs.test.ts    | 19 ++++
 packages/kernel-store/src/sqlite/nodejs.ts    | 17 +++-
 packages/kernel-store/src/sqlite/wasm.test.ts | 19 +++-
 packages/kernel-store/src/sqlite/wasm.ts      | 29 +++++-
 packages/ocap-kernel/src/KernelQueue.test.ts  | 99 ++++++++++++-------
 packages/ocap-kernel/src/KernelQueue.ts       | 46 ++++++---
 .../ocap-kernel/src/store/methods/crank.ts    | 13 ++-
 7 files changed, 186 insertions(+), 56 deletions(-)

diff --git a/packages/kernel-store/src/sqlite/nodejs.test.ts b/packages/kernel-store/src/sqlite/nodejs.test.ts
index 3bf0db5f6..e38655d9a 100644
--- a/packages/kernel-store/src/sqlite/nodejs.test.ts
+++ b/packages/kernel-store/src/sqlite/nodejs.test.ts
@@ -383,6 +383,25 @@ describe('makeSQLKernelDatabase', () => {
       mockDb.inTransaction = false;
     });
 
+    it('releaseSavepoint reports the release failure even if the abort fails too', async () => {
+      const db = await makeSQLKernelDatabase({});
+      mockDb.inTransaction = true;
+      mockDb._spStack = ['point1'];
+      mockDb.exec.mockImplementationOnce(() => {
+        throw new Error('disk I/O error');
+      });
+      mockStatement.run.mockImplementationOnce(() => {
+        throw new Error('cannot rollback');
+      });
+
+      expect(() => db.releaseSavepoint('point1')).toThrowError(
+        'disk I/O error',
+      );
+
+      expect(mockDb._spStack).toStrictEqual([]);
+      mockDb.inTransaction = false;
+    });
+
     it('supports nested savepoints', async () => {
       const db = await makeSQLKernelDatabase({});
       db.createSavepoint('outer');
diff --git a/packages/kernel-store/src/sqlite/nodejs.ts b/packages/kernel-store/src/sqlite/nodejs.ts
index ec863edc7..d0a644244 100644
--- a/packages/kernel-store/src/sqlite/nodejs.ts
+++ b/packages/kernel-store/src/sqlite/nodejs.ts
@@ -321,7 +321,22 @@ export async function makeSQLKernelDatabase({
       throw new Error(`No such savepoint: ${name}`);
     }
     const query = SQL_QUERIES.RELEASE_SAVEPOINT.replace('%NAME%', name);
-    db.exec(query);
+    try {
+      db.exec(query);
+    } catch (error) {
+      // The hazard `rollbackSavepoint` guards against, by the other door: left as
+      // it was, the savepoint stays on the stack and the transaction open with
+      // nothing to ever commit or abort it, so every later write on this
+      // connection joins it, reports success, and vanishes on close. There is no
+      // committing this transaction now, so discard it.
+      db._spStack.length = 0;
+      try {
+        rollbackIfNeeded();
+      } catch {
+        // The release failure below is the one worth reporting.
+      }
+      throw error;
+    }
     db._spStack.splice(idx);
     if (db._spStack.length === 0) {
       commitIfNeeded();
diff --git a/packages/kernel-store/src/sqlite/wasm.test.ts b/packages/kernel-store/src/sqlite/wasm.test.ts
index e0e6be1c4..25403caa4 100644
--- a/packages/kernel-store/src/sqlite/wasm.test.ts
+++ b/packages/kernel-store/src/sqlite/wasm.test.ts
@@ -488,7 +488,6 @@ describe('makeSQLKernelDatabase', () => {
       );
 
       expect(mockDb._spStack).toStrictEqual([]);
-      mockDb._inTx = false;
     });
 
     it('releaseSavepoint validates savepoint exists', async () => {
@@ -538,6 +537,24 @@ describe('makeSQLKernelDatabase', () => {
       expect(mockDb._inTx).toBe(false);
     });
 
+    it('releaseSavepoint reports the release failure even if the abort fails too', async () => {
+      const db = await makeSQLKernelDatabase({});
+      mockDb._inTx = true;
+      mockDb._spStack = ['point1'];
+      mockDb.exec.mockImplementationOnce(() => {
+        throw new Error('disk I/O error');
+      });
+      mockStatement.step.mockImplementationOnce(() => {
+        throw new Error('cannot rollback');
+      });
+
+      expect(() => db.releaseSavepoint('point1')).toThrowError(
+        'disk I/O error',
+      );
+
+      expect(mockDb._inTx).toBe(false);
+    });
+
     // `_inTx` is tracked here rather than read from SQLite, so a failed abort is
     // the one case that can leave it disagreeing with the database. Left true,
     // `beginIfNeeded` becomes a no-op forever after.
diff --git a/packages/kernel-store/src/sqlite/wasm.ts b/packages/kernel-store/src/sqlite/wasm.ts
index c0c32b8a7..beda14811 100644
--- a/packages/kernel-store/src/sqlite/wasm.ts
+++ b/packages/kernel-store/src/sqlite/wasm.ts
@@ -210,10 +210,18 @@ export async function makeSQLKernelDatabase({
    */
   function rollbackIfNeeded(): void {
     if (db._inTx) {
-      sqlAbortTransaction.step();
-      sqlAbortTransaction.reset();
+      // Out of the transaction as far as this driver is concerned before the
+      // abort is even attempted. Unlike the nodejs driver, which reads
+      // `inTransaction` from SQLite, `_inTx` is tracked here — and an abort
+      // typically fails because SQLite already rolled back on its own as part of
+      // whatever went wrong. Left true, `beginIfNeeded` is a no-op from then on
+      // and the next `createSavepoint` runs in autocommit mode, where its
+      // `RELEASE` commits (see `createSavepoint`) and no later rollback can undo
+      // the delivery.
       db._inTx = false;
       db._spStack.length = 0;
+      sqlAbortTransaction.step();
+      sqlAbortTransaction.reset();
     }
   }
 
@@ -403,7 +411,22 @@ export async function makeSQLKernelDatabase({
       throw new Error(`No such savepoint: ${name}`);
     }
     const query = SQL_QUERIES.RELEASE_SAVEPOINT.replace('%NAME%', name);
-    db.exec(query);
+    try {
+      db.exec(query);
+    } catch (error) {
+      // The hazard `rollbackSavepoint` guards against, by the other door: left as
+      // it was, the savepoint stays on the stack and the transaction open with
+      // nothing to ever commit or abort it, so every later write on this
+      // connection joins it, reports success, and vanishes on close. There is no
+      // committing this transaction now, so discard it.
+      db._spStack.length = 0;
+      try {
+        rollbackIfNeeded();
+      } catch {
+        // The release failure below is the one worth reporting.
+      }
+      throw error;
+    }
     db._spStack.splice(idx);
     if (db._spStack.length === 0) {
       commitIfNeeded();
diff --git a/packages/ocap-kernel/src/KernelQueue.test.ts b/packages/ocap-kernel/src/KernelQueue.test.ts
index 073c08d31..ff82eca91 100644
--- a/packages/ocap-kernel/src/KernelQueue.test.ts
+++ b/packages/ocap-kernel/src/KernelQueue.test.ts
@@ -91,6 +91,23 @@ describe('KernelQueue', () => {
     };
   };
 
+  /**
+   * Stop the run loop by failing the *next* crank's start, so that the crank
+   * under test runs to completion. Throwing from one of a crank's own store calls
+   * cuts it short, which hides everything the crank does after that call.
+   */
+  const stopAfterOneCrank = (): void => {
+    let cranks = 0;
+    (kernelStore.startCrank as unknown as MockInstance).mockImplementation(
+      () => {
+        cranks += 1;
+        if (cranks > 1) {
+          throw new Error(STOP_RUN_LOOP);
+        }
+      },
+    );
+  };
+
   /**
    * Run a single crank whose delivery blows up, killing the run loop.
    *
@@ -127,7 +144,8 @@ describe('KernelQueue', () => {
       const deliver = vi.fn().mockRejectedValue(deliverError);
       await expect(kernelQueue.run(deliver)).rejects.toBe(deliverError);
       expect(kernelStore.startCrank).toHaveBeenCalled();
-      expect(kernelStore.createCrankSavepoint).toHaveBeenCalledWith('start');
+      expect(kernelStore.createCrankSavepoint).toHaveBeenCalledWith('crank');
+      expect(kernelStore.createCrankSavepoint).toHaveBeenCalledWith('delivery');
       expect(processGCActionSetSpy).toHaveBeenCalled();
       expect(kernelStore.nextReapAction).toHaveBeenCalled();
       expect(kernelStore.nextTerminatedVatCleanup).toHaveBeenCalled();
@@ -155,9 +173,9 @@ describe('KernelQueue', () => {
       });
       await expect(kernelQueue.run(deliver)).rejects.toThrow(STOP_RUN_LOOP);
       expect(kernelStore.startCrank).toHaveBeenCalled();
-      expect(kernelStore.createCrankSavepoint).toHaveBeenCalledWith('start');
+      expect(kernelStore.createCrankSavepoint).toHaveBeenCalledWith('delivery');
       expect(deliver).toHaveBeenCalledWith(mockItem);
-      expect(kernelStore.rollbackCrank).toHaveBeenCalledWith('start');
+      expect(kernelStore.rollbackCrank).toHaveBeenCalledWith('delivery');
       expect(kernelStore.collectGarbage).toHaveBeenCalled();
       expect(kernelStore.endCrank).toHaveBeenCalled();
     });
@@ -196,12 +214,13 @@ describe('KernelQueue', () => {
     });
 
     // `#flushCrankBuffer` settles the promise `enqueueMessage` handed an external
-    // caller, reading the resolution out of the store on the way. Rolling the
-    // crank back afterwards un-resolves that promise in the store and restores
-    // the run queue item, so a restart delivers the message a second time and
-    // notifies every other subscriber again — while the original caller has
-    // already been told the first answer.
-    it('does not roll back a crank whose result the caller already received', async () => {
+    // caller, reading the resolution out of the store on the way. Were the crank
+    // rolled back after that, the store would un-resolve the promise and restore
+    // the run queue item, so a restart would deliver the message a second time
+    // and notify every other subscriber again — while the original caller had
+    // already been told the first answer. So the flush comes last, after
+    // everything that could still fail.
+    it('answers no caller from a crank it then rolls back', async () => {
       const mockItem: RunQueueItem = {
         type: 'send',
         target: 'ko123',
@@ -219,7 +238,7 @@ describe('KernelQueue', () => {
       const reject = vi.fn();
       kernelQueue.subscriptions.set('kp1', { resolve, reject });
 
-      // The crank succeeds, so the flush hands that caller its answer...
+      // The delivery succeeds and its result is there for the flush to hand over...
       (
         kernelStore.flushCrankBuffer as unknown as MockInstance
       ).mockReturnValueOnce([
@@ -232,7 +251,7 @@ describe('KernelQueue', () => {
         },
       );
 
-      // ...and only then does the kernel die, in work that runs after the flush.
+      // ...but the crank still has fallible work left, and it dies there.
       const terminationError = new Error('vat worker already gone');
       (terminateVat as unknown as MockInstance).mockRejectedValueOnce(
         terminationError,
@@ -243,26 +262,39 @@ describe('KernelQueue', () => {
 
       await expect(kernelQueue.run(deliver)).rejects.toBe(terminationError);
 
-      expect(resolve).toHaveBeenCalledWith({ body: '"answer"', slots: [] });
-      expect(kernelStore.rollbackCrank).not.toHaveBeenCalled();
+      expect(kernelStore.rollbackCrank).toHaveBeenCalledWith('delivery');
+      expect(resolve).not.toHaveBeenCalled();
+      // Told the result will never come, rather than left waiting on a crank the
+      // store no longer has any record of.
+      expect(reject).toHaveBeenCalledWith(
+        expect.objectContaining({ cause: terminationError }),
+      );
     });
 
-    // `rollbackCrank('start')` rolls back the crank's outermost savepoint, which
-    // ends the transaction — so anything the crank does to the store afterwards
-    // autocommits piecemeal and no rollback can reach it. Whatever the ordering,
-    // the rollback has to be the last thing the crank asks of the store.
+    // Rolling back to the crank's *outermost* savepoint discards the enclosing
+    // transaction (see `rollbackSavepoint`), which would leave the work an aborted
+    // crank still owes — terminating the vat whose delivery failed, collecting
+    // garbage — autocommitting statement by statement, beyond the reach of any
+    // later rollback. That work has to follow the rollback, since the worker is
+    // already gone and the store must not go on believing the vat is alive, so it
+    // is the rollback that has to spare the transaction.
     it.each([
-      { label: 'an abort', crankResult: { abort: true } },
+      {
+        label: 'an abort',
+        crankResult: { abort: true },
+        storeOrder: ['rollbackCrank', 'collectGarbage'],
+      },
       {
         label: 'an abort that also terminates',
         crankResult: {
           abort: true,
           terminate: { vatId: 'v1', info: { body: '"exit"', slots: [] } },
         },
+        storeOrder: ['rollbackCrank', 'terminateVat', 'collectGarbage'],
       },
     ])(
-      'does no store work after rolling back $label',
-      async ({ crankResult }) => {
+      'keeps the crank transactional after rolling back $label',
+      async ({ crankResult, storeOrder }) => {
         const mockItem: RunQueueItem = {
           type: 'send',
           target: 'ko123',
@@ -296,8 +328,13 @@ describe('KernelQueue', () => {
         const deliver = vi.fn().mockResolvedValue(crankResult);
         await expect(kernelQueue.run(deliver)).rejects.toThrow(STOP_RUN_LOOP);
 
-        expect(storeCalls).toContain('rollbackCrank');
-        expect(storeCalls.at(-1)).toBe('rollbackCrank');
+        expect(storeCalls).toStrictEqual(storeOrder);
+        expect(
+          (
+            kernelStore.createCrankSavepoint as unknown as MockInstance
+          ).mock.calls.flat(),
+        ).toStrictEqual(['crank', 'delivery']);
+        expect(kernelStore.rollbackCrank).not.toHaveBeenCalledWith('crank');
       },
     );
   });
@@ -392,7 +429,7 @@ describe('KernelQueue', () => {
       await killRunLoop(new Error('crank exploded'));
       // Without this, endCrank's savepoint release commits the half-finished
       // crank and the dequeued item is lost.
-      expect(kernelStore.rollbackCrank).toHaveBeenCalledWith('start');
+      expect(kernelStore.rollbackCrank).toHaveBeenCalledWith('delivery');
     });
 
     it('does not roll back when the savepoint was never created', async () => {
@@ -952,7 +989,7 @@ describe('KernelQueue', () => {
         throw new Error(STOP_RUN_LOOP);
       });
       await expect(kernelQueue.run(deliver)).rejects.toThrow(STOP_RUN_LOOP);
-      expect(kernelStore.rollbackCrank).toHaveBeenCalledWith('start');
+      expect(kernelStore.rollbackCrank).toHaveBeenCalledWith('delivery');
       expect(rejectSpy).toHaveBeenCalledWith(terminateInfo);
       expect(kernelQueue.subscriptions.has('kp99')).toBe(false);
     });
@@ -988,7 +1025,7 @@ describe('KernelQueue', () => {
         throw new Error(STOP_RUN_LOOP);
       });
       await expect(kernelQueue.run(deliver)).rejects.toThrow(STOP_RUN_LOOP);
-      expect(kernelStore.rollbackCrank).toHaveBeenCalledWith('start');
+      expect(kernelStore.rollbackCrank).toHaveBeenCalledWith('delivery');
       expect(rejectedAfterAbort).toBe(false);
       expect(resolveSpy).not.toHaveBeenCalled();
       expect(subscribedAfterAbort).toBe(true);
@@ -1058,11 +1095,7 @@ describe('KernelQueue', () => {
         mockItem,
       );
       const deliver = vi.fn().mockResolvedValue(undefined);
-      (
-        kernelStore.collectGarbage as unknown as MockInstance
-      ).mockImplementation(() => {
-        throw new Error(STOP_RUN_LOOP);
-      });
+      stopAfterOneCrank();
       await expect(kernelQueue.run(deliver)).rejects.toThrow(STOP_RUN_LOOP);
       expect(rejectSpy).toHaveBeenCalledWith(rejectedValue);
       expect(resolveSpy).not.toHaveBeenCalled();
@@ -1097,11 +1130,7 @@ describe('KernelQueue', () => {
         mockItem,
       );
       const deliver = vi.fn().mockResolvedValue(undefined);
-      (
-        kernelStore.collectGarbage as unknown as MockInstance
-      ).mockImplementation(() => {
-        throw new Error(STOP_RUN_LOOP);
-      });
+      stopAfterOneCrank();
       await expect(kernelQueue.run(deliver)).rejects.toThrow(STOP_RUN_LOOP);
       expect(resolveSpy).toHaveBeenCalledWith(fulfilledValue);
       expect(rejectSpy).not.toHaveBeenCalled();
diff --git a/packages/ocap-kernel/src/KernelQueue.ts b/packages/ocap-kernel/src/KernelQueue.ts
index 4148300ff..48bbdeeee 100644
--- a/packages/ocap-kernel/src/KernelQueue.ts
+++ b/packages/ocap-kernel/src/KernelQueue.ts
@@ -130,7 +130,15 @@ export class KernelQueue {
       this.#kernelStore.startCrank();
       this.#crankRollbackAttempted = false;
       try {
-        this.#kernelStore.createCrankSavepoint('start');
+        // Two savepoints, because the crank's transaction has to outlive the
+        // delivery's rollback. Rolling back to the outermost savepoint discards
+        // the enclosing transaction (see `rollbackSavepoint`), and the work an
+        // aborted crank still owes — terminating the vat whose delivery failed,
+        // collecting garbage — would then autocommit statement by statement,
+        // beyond the reach of any later rollback. Only `delivery` is ever rolled
+        // back; releasing `crank` in `endCrank` is this crank's one commit point.
+        this.#kernelStore.createCrankSavepoint('crank');
+        this.#kernelStore.createCrankSavepoint('delivery');
 
         // The savepoint exists from here on, so a throw can be undone. Without
         // this, `endCrank`'s savepoint release commits the half-finished crank:
@@ -158,7 +166,7 @@ export class KernelQueue {
           // savepoint" over the real error.
           if (!this.#crankRollbackAttempted) {
             try {
-              this.#kernelStore.rollbackCrank('start');
+              this.#kernelStore.rollbackCrank('delivery');
             } catch (rollbackError) {
               // The original failure stays the `cause`, since that is the root
               // cause an operator needs; the rollback failure is named here.
@@ -304,7 +312,7 @@ export class KernelQueue {
       // For active vats, this allows the message to be retried in a future crank.
       // For terminated vats, the message will just go splat.
       try {
-        this.#kernelStore.rollbackCrank('start');
+        this.#kernelStore.rollbackCrank('delivery');
       } finally {
         // Set even when the rollback threw. `rollbackCrank` forgets the
         // savepoint in its own `finally`, so "attempted" and "the savepoint is
@@ -333,17 +341,24 @@ export class KernelQueue {
       // TODO: Currently all errors terminate the vat, but instead we could
       // restart it and terminate the vat only after a certain number of failed
       // retries. This is probably where we should implement the vat restart logic.
-    } else {
-      // Upon on successful crank completion, enqueue buffered vat outputs for delivery.
-      this.#flushCrankBuffer();
     }
     // Vat termination during delivery is triggered by an illegal syscall
-    // or by syscall.exit().
+    // or by syscall.exit(). Its store writes have to survive the rollback above:
+    // the worker is already gone, so a store that still believed the vat was
+    // alive would relaunch it after a restart and redeliver what killed it.
     if (crankResult?.terminate) {
       const { vatId, info } = crankResult.terminate;
       await this.#terminateVat(vatId, info);
     }
     this.#kernelStore.collectGarbage();
+    if (!crankResult?.abort) {
+      // The crank survived, so hand its buffered outputs on — last, once nothing
+      // fallible remains. The flush settles the promise `enqueueMessage` gave an
+      // external caller, reading the result out of the store; were the crank
+      // rolled back after that, the caller would keep an answer computed from
+      // state the store discarded, and a restart would deliver the message again.
+      this.#flushCrankBuffer();
+    }
   }
 
   /**
@@ -366,21 +381,24 @@ export class KernelQueue {
    */
   #flushCrankBuffer(): void {
     const items = this.#kernelStore.flushCrankBuffer();
+    const resolved: KRef[] = [];
     for (const item of items) {
       this.#enqueueRun(item);
       if (item.type === 'notify') {
-        // Invoke kernel subscription callback if any, reading resolution
-        // data from the (now committed) promise state
-        this.#invokeKernelSubscription(item.kpid);
+        resolved.push(item.kpid);
       }
     }
+    // Also promises resolved during this crank that don't have kernel-level
+    // subscribers (e.g., promises from enqueueMessage).
+    resolved.push(...this.#resolvedWithKernelSubscription);
+    this.#resolvedWithKernelSubscription = [];
 
-    // Invoke kernel subscriptions for promises resolved during this crank
-    // that don't have kernel-level subscribers (e.g., promises from enqueueMessage)
-    for (const kpid of this.#resolvedWithKernelSubscription) {
+    // Callbacks only once every store write is done. Each hands an external
+    // caller a result read out of the store, and a write that threw in between
+    // would have the crank rolled back underneath answers already given.
+    for (const kpid of resolved) {
       this.#invokeKernelSubscription(kpid);
     }
-    this.#resolvedWithKernelSubscription = [];
   }
 
   /**
diff --git a/packages/ocap-kernel/src/store/methods/crank.ts b/packages/ocap-kernel/src/store/methods/crank.ts
index 87d2bc65b..24957bce4 100644
--- a/packages/ocap-kernel/src/store/methods/crank.ts
+++ b/packages/ocap-kernel/src/store/methods/crank.ts
@@ -77,8 +77,17 @@ export function getCrankMethods(ctx: StoreContext, kdb: KernelDatabase) {
    */
   function releaseAllSavepoints(): void {
     if (ctx.savepoints.length > 0) {
-      kdb.releaseSavepoint('t0');
-      ctx.savepoints.length = 0;
+      try {
+        kdb.releaseSavepoint('t0');
+      } finally {
+        // Forget the savepoints even if the release failed, as `rollbackCrank`
+        // does. A failed release discards the whole transaction (see
+        // `releaseSavepoint`), so the database has no savepoints left either;
+        // leaving them listed here would have the next crank number its savepoint
+        // `t1`, and from then on every release and rollback would aim one crank
+        // past the one it meant to end.
+        ctx.savepoints.length = 0;
+      }
     }
   }
 

From f7ad5d64bf318be44a666bdbf1bffab73bc60758 Mon Sep 17 00:00:00 2001
From: Dimitris Marlagkoutsos <info@sirtimid.com>
Date: Thu, 6 Aug 2026 16:23:44 +0200
Subject: [PATCH 03/10] test(kernel-test): reap until the vat's GC is visible,
 not three times
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`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>
---
 .../src/garbage-collection.test.ts            | 45 ++++++++++++-------
 1 file changed, 30 insertions(+), 15 deletions(-)

diff --git a/packages/kernel-test/src/garbage-collection.test.ts b/packages/kernel-test/src/garbage-collection.test.ts
index 268ed7920..953a95ac2 100644
--- a/packages/kernel-test/src/garbage-collection.test.ts
+++ b/packages/kernel-test/src/garbage-collection.test.ts
@@ -105,6 +105,28 @@ describe('Garbage Collection', () => {
     expect(parseReplyBody(useResult.body)).toBe(objectId);
   });
 
+  /**
+   * Reap the importer vat until the kernel's bookkeeping catches up with the
+   * vat's own garbage collection, or the attempts run out.
+   *
+   * `bringOutYourDead` can only report an import as dropped once the engine has
+   * collected the vat's presence and run its finalizer, which forcing a GC pass
+   * does not guarantee on the first attempt. Reaping once and then cranking
+   * buys one attempt rather than several, because `scheduleReap` dedupes — so
+   * each attempt schedules its own reap, and a message to the vat wakes the run
+   * loop to consume it.
+   *
+   * @param settled - Whether the state under test has arrived.
+   */
+  async function reapImporterUntil(settled: () => boolean): Promise<void> {
+    const isImporter = (vatId: VatId): boolean => vatId === importerVatId;
+    for (let attempt = 0; attempt < 5 && !settled(); attempt += 1) {
+      kernel.reapVats(isImporter);
+      await kernel.queueMessage(importerKRef, 'noop', []);
+      await waitUntilQuiescent(500);
+    }
+  }
+
   it('should trigger GC syscalls through bringOutYourDead', async () => {
     // Create an object in the exporter vat with a known ID
     const objectId = 'test-object';
@@ -149,14 +171,10 @@ describe('Garbage Collection', () => {
     await kernel.queueMessage(importerKRef, 'makeWeak', [objectId]);
     await waitUntilQuiescent();
 
-    // Schedule reap to trigger bringOutYourDead on next crank
-    kernel.reapVats((vatId) => vatId === importerVatId);
-
-    // Run 3 cranks to allow bringOutYourDead to be processed
-    for (let i = 0; i < 3; i++) {
-      await kernel.queueMessage(importerKRef, 'noop', []);
-      await waitUntilQuiescent(500);
-    }
+    // Reap until the importer reports the drop
+    await reapImporterUntil(
+      () => kernelStore.getObjectRefCount(createObjectRef).reachable === 1,
+    );
 
     // Check reference counts after dropImports
     const afterWeakRefCounts = kernelStore.getObjectRefCount(createObjectRef);
@@ -168,13 +186,10 @@ describe('Garbage Collection', () => {
     await kernel.queueMessage(importerKRef, 'forgetImport', []);
     await waitUntilQuiescent();
 
-    // Schedule another reap
-    kernel.reapVats((vatId) => vatId === importerVatId);
-
-    for (let i = 0; i < 3; i++) {
-      await kernel.queueMessage(importerKRef, 'noop', []);
-      await waitUntilQuiescent(500);
-    }
+    // Reap until the importer reports the retirement
+    await reapImporterUntil(
+      () => kernelStore.getObjectRefCount(createObjectRef).recognizable === 1,
+    );
 
     // Check reference counts after retireImports
     const afterForgetRefCounts = kernelStore.getObjectRefCount(createObjectRef);

From cd6bdda9a63fe128e09024c3ae43c10dcb8f046b Mon Sep 17 00:00:00 2001
From: Dimitris Marlagkoutsos <info@sirtimid.com>
Date: Thu, 6 Aug 2026 16:33:20 +0200
Subject: [PATCH 04/10] test: fix rollback crank test

---
 .../kernel-test/src/crank-rollback.test.ts    | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/packages/kernel-test/src/crank-rollback.test.ts b/packages/kernel-test/src/crank-rollback.test.ts
index cd2a708aa..dce56b322 100644
--- a/packages/kernel-test/src/crank-rollback.test.ts
+++ b/packages/kernel-test/src/crank-rollback.test.ts
@@ -138,6 +138,46 @@ describe('crank rollback against a real database', () => {
     expect(kdb.kernelKVStore.get('second')).toBe('yes');
   });
 
+  // An aborted crank rolls its delivery back and then still has work to do —
+  // terminating the vat whose delivery failed, collecting garbage — whose writes
+  // have to survive that rollback, since the vat's worker is already gone.
+  it('keeps the writes a crank makes after rolling its delivery back', async () => {
+    const { kernelStore, kdb } = await makeStore();
+
+    kernelStore.startCrank();
+    kernelStore.createCrankSavepoint('crank');
+    kernelStore.createCrankSavepoint('delivery');
+    kdb.kernelKVStore.set('delivered', 'yes');
+
+    kernelStore.rollbackCrank('delivery');
+    kdb.kernelKVStore.set('terminated', 'yes');
+    kernelStore.endCrank();
+
+    expect(kdb.kernelKVStore.get('delivered')).toBeUndefined();
+    expect(kdb.kernelKVStore.get('terminated')).toBe('yes');
+  });
+
+  // And they survive it *as part of the crank's transaction*, which is why the
+  // delivery gets a savepoint of its own rather than rolling back the crank's:
+  // rolling back the outermost savepoint discards the transaction, after which
+  // those writes would autocommit one statement at a time. Nothing in the run
+  // loop rolls the crank's own savepoint back — it is the only way from here to
+  // observe that the writes are still undoable at all.
+  it('holds those writes in the transaction rather than autocommitting them', async () => {
+    const { kernelStore, kdb } = await makeStore();
+
+    kernelStore.startCrank();
+    kernelStore.createCrankSavepoint('crank');
+    kernelStore.createCrankSavepoint('delivery');
+    kernelStore.rollbackCrank('delivery');
+    kdb.kernelKVStore.set('terminated', 'yes');
+
+    kernelStore.rollbackCrank('crank');
+    kernelStore.endCrank();
+
+    expect(kdb.kernelKVStore.get('terminated')).toBeUndefined();
+  });
+
   // `createCrankSavepoint` records the name only once the database has the
   // savepoint. Asking to roll back one that was never created must therefore say
   // so, rather than releasing someone else's savepoint.

From 82b88ce62163dcc5f41a0227105da08928dd0697 Mon Sep 17 00:00:00 2001
From: Dimitris Marlagkoutsos <info@sirtimid.com>
Date: Thu, 6 Aug 2026 17:37:12 +0200
Subject: [PATCH 05/10] fix(ocap-kernel): forget every savepoint when a crank
 rollback fails
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 .../src/store/methods/crank.test.ts           | 33 ++++++++++++++++---
 .../ocap-kernel/src/store/methods/crank.ts    | 28 +++++++++-------
 2 files changed, 45 insertions(+), 16 deletions(-)

diff --git a/packages/ocap-kernel/src/store/methods/crank.test.ts b/packages/ocap-kernel/src/store/methods/crank.test.ts
index 8fd6cd1fe..389afd7a0 100644
--- a/packages/ocap-kernel/src/store/methods/crank.test.ts
+++ b/packages/ocap-kernel/src/store/methods/crank.test.ts
@@ -153,6 +153,29 @@ describe('crank methods', () => {
 
       expect(mockCrankBuffer).toHaveLength(0);
     });
+
+    // A failed rollback discards the whole transaction, so the enclosing
+    // savepoints are gone from the database too — not just the one rolled back
+    // to. Truncating to the ordinal would leave `endCrank` releasing a `t0` the
+    // database no longer has, and that "No such savepoint" would be thrown from
+    // the run loop's `finally`, over whatever really killed the kernel.
+    it('forgets every savepoint when the rollback fails', () => {
+      context.inCrank = true;
+      crankMethods.createCrankSavepoint('crank');
+      crankMethods.createCrankSavepoint('delivery');
+      vi.mocked(kdb.rollbackSavepoint).mockImplementationOnce(() => {
+        throw new Error('disk I/O error');
+      });
+
+      expect(() => crankMethods.rollbackCrank('delivery')).toThrow(
+        'disk I/O error',
+      );
+
+      expect(context.savepoints).toStrictEqual([]);
+      // The release `endCrank` would otherwise attempt, and throw over.
+      crankMethods.endCrank();
+      expect(kdb.releaseSavepoint).not.toHaveBeenCalled();
+    });
   });
 
   describe('endCrank', () => {
@@ -207,11 +230,11 @@ describe('crank methods', () => {
       expect(await waiter).toBeUndefined();
     });
 
-    // What `rollbackCrank` already does in its own `finally`. Settling the crank
-    // regardless means callers proceed, so a savepoint left listed here has the
-    // next crank number 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.
+    // What `rollbackCrank` already does when its own rollback fails. Settling
+    // the crank regardless means callers proceed, so a savepoint left listed
+    // here has the next crank number 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.
     it('forgets its savepoints even if releasing them fails', () => {
       crankMethods.startCrank();
       context.savepoints = ['test'];
diff --git a/packages/ocap-kernel/src/store/methods/crank.ts b/packages/ocap-kernel/src/store/methods/crank.ts
index 24957bce4..b26219cbb 100644
--- a/packages/ocap-kernel/src/store/methods/crank.ts
+++ b/packages/ocap-kernel/src/store/methods/crank.ts
@@ -51,13 +51,19 @@ export function getCrankMethods(ctx: StoreContext, kdb: KernelDatabase) {
       if (ctx.savepoints[ordinal] === savepoint) {
         try {
           kdb.rollbackSavepoint(`t${ordinal}`);
-        } finally {
-          // Forget the savepoint even if the rollback failed. Leaving it listed
-          // would have `endCrank`'s release commit the crank we just abandoned —
-          // the half-finished state this rollback exists to discard. A failed
-          // rollback discards the whole transaction instead (see
-          // `rollbackSavepoint`), which for a crank is the same boundary.
+          // Forget the savepoint. Leaving it listed would have `endCrank`'s
+          // release commit the crank we just abandoned — the half-finished state
+          // this rollback exists to discard.
           ctx.savepoints.length = ordinal;
+        } catch (error) {
+          // A failed rollback discards the whole transaction (see
+          // `rollbackSavepoint`), taking every savepoint with it, not just this
+          // one. Truncating to `ordinal` would leave the enclosing savepoints
+          // listed against a database that no longer has them, and `endCrank`
+          // would then throw "No such savepoint: t0" from the run loop's
+          // `finally` — burying the failure that actually killed the kernel.
+          ctx.savepoints.length = 0;
+          throw error;
         }
         // The rollback reverted DB state but in-memory caches are stale.
         // Recreate the run queue so its cached head/tail are re-read from DB.
@@ -81,11 +87,11 @@ export function getCrankMethods(ctx: StoreContext, kdb: KernelDatabase) {
         kdb.releaseSavepoint('t0');
       } finally {
         // Forget the savepoints even if the release failed, as `rollbackCrank`
-        // does. A failed release discards the whole transaction (see
-        // `releaseSavepoint`), so the database has no savepoints left either;
-        // leaving them listed here would have the next crank number its savepoint
-        // `t1`, and from then on every release and rollback would aim one crank
-        // past the one it meant to end.
+        // does when its own rollback fails. A failed release discards the whole
+        // transaction (see `releaseSavepoint`), so the database has no savepoints
+        // left either; leaving them listed here would have the next crank number
+        // its savepoint `t1`, and from then on every release and rollback would
+        // aim one crank past the one it meant to end.
         ctx.savepoints.length = 0;
       }
     }

From b8b25ab2c06a6cd5e9d4dfbd482d29746738ba2d Mon Sep 17 00:00:00 2001
From: Dimitris Marlagkoutsos <info@sirtimid.com>
Date: Thu, 6 Aug 2026 17:37:23 +0200
Subject: [PATCH 06/10] fix(kernel-store): log an abort that fails while
 discarding a transaction

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>
---
 packages/kernel-store/src/sqlite/nodejs.ts | 22 ++++++++++++++++++----
 packages/kernel-store/src/sqlite/wasm.ts   | 22 ++++++++++++++++++----
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/packages/kernel-store/src/sqlite/nodejs.ts b/packages/kernel-store/src/sqlite/nodejs.ts
index d0a644244..5b7e777b0 100644
--- a/packages/kernel-store/src/sqlite/nodejs.ts
+++ b/packages/kernel-store/src/sqlite/nodejs.ts
@@ -298,8 +298,15 @@ export async function makeSQLKernelDatabase({
       db._spStack.length = 0;
       try {
         rollbackIfNeeded();
-      } catch {
-        // The rollback failure below is the one worth reporting.
+      } catch (abortError) {
+        // The rollback failure below is the one worth reporting, but a failed
+        // abort leaves SQLite holding a transaction this driver has stopped
+        // tracking, which the next crank would silently write into. Nothing here
+        // can repair that, so at least say so.
+        logger?.error(
+          'failed to discard transaction after rollback',
+          abortError,
+        );
       }
       throw error;
     }
@@ -332,8 +339,15 @@ export async function makeSQLKernelDatabase({
       db._spStack.length = 0;
       try {
         rollbackIfNeeded();
-      } catch {
-        // The release failure below is the one worth reporting.
+      } catch (abortError) {
+        // The release failure below is the one worth reporting, but a failed
+        // abort leaves SQLite holding a transaction this driver has stopped
+        // tracking, which the next crank would silently write into. Nothing here
+        // can repair that, so at least say so.
+        logger?.error(
+          'failed to discard transaction after release',
+          abortError,
+        );
       }
       throw error;
     }
diff --git a/packages/kernel-store/src/sqlite/wasm.ts b/packages/kernel-store/src/sqlite/wasm.ts
index beda14811..65d9915e4 100644
--- a/packages/kernel-store/src/sqlite/wasm.ts
+++ b/packages/kernel-store/src/sqlite/wasm.ts
@@ -388,8 +388,15 @@ export async function makeSQLKernelDatabase({
       db._spStack.length = 0;
       try {
         rollbackIfNeeded();
-      } catch {
-        // The rollback failure below is the one worth reporting.
+      } catch (abortError) {
+        // The rollback failure below is the one worth reporting. `_inTx` is
+        // already false by then, so the next `beginIfNeeded` issues its `BEGIN`
+        // and SQLite says loudly if it really is still in a transaction — but
+        // that is a crank away, and this is where the evidence is.
+        logger?.error(
+          'failed to discard transaction after rollback',
+          abortError,
+        );
       }
       throw error;
     }
@@ -422,8 +429,15 @@ export async function makeSQLKernelDatabase({
       db._spStack.length = 0;
       try {
         rollbackIfNeeded();
-      } catch {
-        // The release failure below is the one worth reporting.
+      } catch (abortError) {
+        // The release failure below is the one worth reporting. `_inTx` is
+        // already false by then, so the next `beginIfNeeded` issues its `BEGIN`
+        // and SQLite says loudly if it really is still in a transaction — but
+        // that is a crank away, and this is where the evidence is.
+        logger?.error(
+          'failed to discard transaction after release',
+          abortError,
+        );
       }
       throw error;
     }

From e02b61998ac678528d492a7c62b03054dde515bf Mon Sep 17 00:00:00 2001
From: Dimitris Marlagkoutsos <info@sirtimid.com>
Date: Thu, 6 Aug 2026 17:38:10 +0200
Subject: [PATCH 07/10] test(ocap-kernel): pin the flush's ordering against a
 failing enqueue
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 packages/ocap-kernel/src/KernelQueue.test.ts | 51 ++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/packages/ocap-kernel/src/KernelQueue.test.ts b/packages/ocap-kernel/src/KernelQueue.test.ts
index ff82eca91..072a302df 100644
--- a/packages/ocap-kernel/src/KernelQueue.test.ts
+++ b/packages/ocap-kernel/src/KernelQueue.test.ts
@@ -271,6 +271,57 @@ describe('KernelQueue', () => {
       );
     });
 
+    // The same invariant one level down, inside the flush itself: moving every
+    // buffered item onto the run queue is store work too, and it can fail
+    // part-way. Answering the first caller while the second enqueue is still
+    // ahead would hand out a result the crank's rollback then discards.
+    it('answers no caller until every buffered item is enqueued', async () => {
+      const mockItem: RunQueueItem = {
+        type: 'send',
+        target: 'ko123',
+        message: { result: 'kp1' } as KernelMessage,
+      };
+      (kernelStore.runQueueLength as unknown as MockInstance)
+        .mockReturnValueOnce(1)
+        .mockReturnValue(0);
+      (kernelStore.dequeueRun as unknown as MockInstance).mockReturnValueOnce(
+        mockItem,
+      );
+
+      const resolve = vi.fn();
+      const reject = vi.fn();
+      kernelQueue.subscriptions.set('kp1', { resolve, reject });
+
+      // Two resolutions to hand over, the caller's first.
+      (
+        kernelStore.flushCrankBuffer as unknown as MockInstance
+      ).mockReturnValueOnce([
+        { type: 'notify', endpointId: 'v1', kpid: 'kp1' },
+        { type: 'notify', endpointId: 'v2', kpid: 'kp2' },
+      ]);
+      (kernelStore.getKernelPromise as unknown as MockInstance).mockReturnValue(
+        { state: 'fulfilled', value: { body: '"answer"', slots: [] } },
+      );
+
+      // The second enqueue is the write that fails.
+      const enqueueError = new Error('database is gone');
+      let enqueued = 0;
+      (kernelStore.enqueueRun as unknown as MockInstance).mockImplementation(
+        () => {
+          enqueued += 1;
+          if (enqueued > 1) {
+            throw enqueueError;
+          }
+        },
+      );
+
+      const deliver = vi.fn().mockResolvedValue(undefined);
+      await expect(kernelQueue.run(deliver)).rejects.toBe(enqueueError);
+
+      expect(kernelStore.rollbackCrank).toHaveBeenCalledWith('delivery');
+      expect(resolve).not.toHaveBeenCalled();
+    });
+
     // Rolling back to the crank's *outermost* savepoint discards the enclosing
     // transaction (see `rollbackSavepoint`), which would leave the work an aborted
     // crank still owes — terminating the vat whose delivery failed, collecting

From dd20fbde381fcd9b294375e540b22b710d97459c Mon Sep 17 00:00:00 2001
From: Dimitris Marlagkoutsos <info@sirtimid.com>
Date: Thu, 6 Aug 2026 17:38:24 +0200
Subject: [PATCH 08/10] docs: correct the transaction claims review found wrong
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 packages/kernel-store/CHANGELOG.md            |  6 +++
 packages/kernel-store/src/sqlite/wasm.ts      | 16 ++++----
 .../src/garbage-collection.test.ts            | 15 ++++---
 packages/ocap-kernel/CHANGELOG.md             |  5 +++
 packages/ocap-kernel/src/KernelQueue.ts       | 39 ++++++++++++-------
 5 files changed, 56 insertions(+), 25 deletions(-)

diff --git a/packages/kernel-store/CHANGELOG.md b/packages/kernel-store/CHANGELOG.md
index 5cfe39eb5..5e1957b5e 100644
--- a/packages/kernel-store/CHANGELOG.md
+++ b/packages/kernel-store/CHANGELOG.md
@@ -12,6 +12,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `rollbackSavepoint` discards the enclosing transaction when `ROLLBACK TO` itself fails, instead of leaving the savepoint on its stack and the transaction open ([#1005](https://github.com/MetaMask/ocap-kernel/pull/1005))
   - Nothing would ever commit or abort that transaction, so every later write on the connection silently joined it, reported success, and vanished on close. Discarding it is no wider than the caller asked for: the transaction begins with the outermost savepoint, so it holds only the work the rollback was abandoning
   - The rollback failure is still what gets thrown, even if aborting the transaction fails too
+- `releaseSavepoint` discards the enclosing transaction when `RELEASE` itself fails, the same way `rollbackSavepoint` already did when `ROLLBACK TO` failed ([#1012](https://github.com/MetaMask/ocap-kernel/pull/1012))
+  - The same hazard by the other door: the savepoint stayed on the stack and the transaction stayed open with nothing left that would ever commit or abort it. The release failure is what gets thrown, even if aborting fails too
+- The wasm driver leaves `_inTx` false when aborting a transaction throws, rather than believing it is still in one ([#1012](https://github.com/MetaMask/ocap-kernel/pull/1012))
+  - It tracks `_inTx` itself instead of reading it from SQLite, so a throwing abort was the one case that could leave the two disagreeing. Left true, `beginIfNeeded` became a permanent no-op and later writes autocommitted a statement at a time; left false, the next `BEGIN` fails loudly if SQLite really is still in a transaction. The nodejs driver reads `db.inTransaction` and was never affected
+- An abort that fails while recovering from a failed savepoint operation is logged, in both drivers ([#1012](https://github.com/MetaMask/ocap-kernel/pull/1012))
+  - The savepoint failure is still the one thrown, but the abandoned transaction it leaves behind was previously silent
 
 ## [0.6.0]
 
diff --git a/packages/kernel-store/src/sqlite/wasm.ts b/packages/kernel-store/src/sqlite/wasm.ts
index 65d9915e4..45313869f 100644
--- a/packages/kernel-store/src/sqlite/wasm.ts
+++ b/packages/kernel-store/src/sqlite/wasm.ts
@@ -211,13 +211,15 @@ export async function makeSQLKernelDatabase({
   function rollbackIfNeeded(): void {
     if (db._inTx) {
       // Out of the transaction as far as this driver is concerned before the
-      // abort is even attempted. Unlike the nodejs driver, which reads
-      // `inTransaction` from SQLite, `_inTx` is tracked here — and an abort
-      // typically fails because SQLite already rolled back on its own as part of
-      // whatever went wrong. Left true, `beginIfNeeded` is a no-op from then on
-      // and the next `createSavepoint` runs in autocommit mode, where its
-      // `RELEASE` commits (see `createSavepoint`) and no later rollback can undo
-      // the delivery.
+      // abort is even attempted, because the abort can throw. Unlike the nodejs
+      // driver, which reads `inTransaction` from SQLite, `_inTx` is tracked here,
+      // so a throw is the one thing that can leave the two disagreeing. Left
+      // true, `beginIfNeeded` is a no-op from then on: writes outside a savepoint
+      // autocommit one statement at a time, and the outermost `RELEASE` of a
+      // savepoint created in autocommit mode commits rather than nesting (see
+      // `createSavepoint`). Setting it false instead means the next `BEGIN`
+      // throws if SQLite really is still in a transaction, which is the failure
+      // worth having.
       db._inTx = false;
       db._spStack.length = 0;
       sqlAbortTransaction.step();
diff --git a/packages/kernel-test/src/garbage-collection.test.ts b/packages/kernel-test/src/garbage-collection.test.ts
index 953a95ac2..5ec22e72b 100644
--- a/packages/kernel-test/src/garbage-collection.test.ts
+++ b/packages/kernel-test/src/garbage-collection.test.ts
@@ -110,13 +110,18 @@ describe('Garbage Collection', () => {
    * vat's own garbage collection, or the attempts run out.
    *
    * `bringOutYourDead` can only report an import as dropped once the engine has
-   * collected the vat's presence and run its finalizer, which forcing a GC pass
+   * collected the vat's presence and run its finalizer, which `gcAndFinalize`
    * does not guarantee on the first attempt. Reaping once and then cranking
-   * buys one attempt rather than several, because `scheduleReap` dedupes — so
-   * each attempt schedules its own reap, and a message to the vat wakes the run
-   * loop to consume it.
+   * repeatedly buys one attempt rather than several, because `nextReapAction`
+   * shifts the single scheduled entry off and the later cranks find nothing to
+   * do — so each attempt has to schedule its own reap, with a message to the vat
+   * to wake the run loop and consume it.
    *
-   * @param settled - Whether the state under test has arrived.
+   * Gives up after five attempts and lets the caller's assertion report the
+   * failure, which names the refcount that never arrived.
+   *
+   * @param settled - Predicate answering whether the state under test has
+   * arrived.
    */
   async function reapImporterUntil(settled: () => boolean): Promise<void> {
     const isImporter = (vatId: VatId): boolean => vatId === importerVatId;
diff --git a/packages/ocap-kernel/CHANGELOG.md b/packages/ocap-kernel/CHANGELOG.md
index 87dcabb83..2919f89b6 100644
--- a/packages/ocap-kernel/CHANGELOG.md
+++ b/packages/ocap-kernel/CHANGELOG.md
@@ -48,6 +48,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Roll back the crank the run loop died in instead of committing it, so a restart resumes from a consistent boundary ([#1005](https://github.com/MetaMask/ocap-kernel/pull/1005))
   - Because the killing item is no longer consumed, a restart re-dequeues it; an item that reliably kills a crank needs `clearState`/`reset` rather than a restart
   - Store state only — a crank that had already flushed its buffer settled JS-side subscriptions irreversibly
+- Keep a crank's store work inside one transaction, so the writes an aborted crank still owes are committed or undone as a unit ([#1012](https://github.com/MetaMask/ocap-kernel/pull/1012))
+  - A crank now takes two savepoints, `crank` and `delivery`, and rolls back only `delivery`. Rolling back the outermost savepoint ends the enclosing transaction, so terminating the vat and collecting garbage — which follow the rollback, and must survive it — were autocommitting a statement at a time, beyond the reach of any later rollback
+  - Buffered vat outputs are flushed after that work rather than before it. The flush settles the promise `queueMessage` handed an external caller, so a later failure used to roll the crank back underneath an answer already given. Termination still settles the dying vat's own promises immediately
+- Forget a crank's savepoints when the store call that ends them fails, in both `rollbackCrank` and `endCrank` ([#1012](https://github.com/MetaMask/ocap-kernel/pull/1012))
+  - A failed rollback or release discards the whole transaction, taking every savepoint with it. Keeping them listed had the next crank number its savepoint `t1` while the database had none, and had `endCrank` throw `No such savepoint: t0` from the run loop's `finally` — over whatever really killed the kernel
 - Refuse inbound remote deliveries once the run loop is dead, rolling back without acknowledging them, so the peer retries and gives up instead of waiting on a kernel that will never deliver ([#1005](https://github.com/MetaMask/ocap-kernel/pull/1005))
   - Covers `bringOutYourDead` as well as `message` and `notify`: a reap is queue work too, consumed only by the run loop. The remaining GC arms need no guard, since they only touch refcounts
 - Refuse `launchSubcluster` once the run loop is dead ([#1005](https://github.com/MetaMask/ocap-kernel/pull/1005))
diff --git a/packages/ocap-kernel/src/KernelQueue.ts b/packages/ocap-kernel/src/KernelQueue.ts
index 48bbdeeee..53256be03 100644
--- a/packages/ocap-kernel/src/KernelQueue.ts
+++ b/packages/ocap-kernel/src/KernelQueue.ts
@@ -135,8 +135,11 @@ export class KernelQueue {
         // the enclosing transaction (see `rollbackSavepoint`), and the work an
         // aborted crank still owes — terminating the vat whose delivery failed,
         // collecting garbage — would then autocommit statement by statement,
-        // beyond the reach of any later rollback. Only `delivery` is ever rolled
-        // back; releasing `crank` in `endCrank` is this crank's one commit point.
+        // beyond the reach of any later rollback. The run loop only ever rolls
+        // back `delivery`; `crank` is released by `endCrank`, which is this
+        // crank's one commit point. That release names the *first* savepoint
+        // created here, by ordinal — see `releaseAllSavepoints` — so `crank` has
+        // to stay first.
         this.#kernelStore.createCrankSavepoint('crank');
         this.#kernelStore.createCrankSavepoint('delivery');
 
@@ -342,21 +345,30 @@ export class KernelQueue {
       // restart it and terminate the vat only after a certain number of failed
       // retries. This is probably where we should implement the vat restart logic.
     }
-    // Vat termination during delivery is triggered by an illegal syscall
-    // or by syscall.exit(). Its store writes have to survive the rollback above:
-    // the worker is already gone, so a store that still believed the vat was
-    // alive would relaunch it after a restart and redeliver what killed it.
+    // Vat termination during delivery is triggered by an illegal syscall or by
+    // syscall.exit(). This call is what kills the worker, and its store writes
+    // have to survive the rollback above: once the worker is gone, a store that
+    // still believed the vat was alive would relaunch it after a restart and
+    // redeliver what killed it. Hence the rollback goes only as far as
+    // `delivery`, leaving these writes inside the crank's transaction.
     if (crankResult?.terminate) {
       const { vatId, info } = crankResult.terminate;
       await this.#terminateVat(vatId, info);
     }
     this.#kernelStore.collectGarbage();
     if (!crankResult?.abort) {
-      // The crank survived, so hand its buffered outputs on — last, once nothing
-      // fallible remains. The flush settles the promise `enqueueMessage` gave an
-      // external caller, reading the result out of the store; were the crank
-      // rolled back after that, the caller would keep an answer computed from
-      // state the store discarded, and a restart would deliver the message again.
+      // The crank survived, so hand its buffered outputs on — after the store
+      // work above, which can still fail. The flush settles the promise
+      // `enqueueMessage` gave an external caller, reading the result out of the
+      // store; were the crank rolled back after that, the caller would keep an
+      // answer computed from state the store discarded, and a restart would
+      // deliver the message again.
+      //
+      // Not airtight: `#terminateVat` resolves the promises the dying vat was
+      // deciding via `resolvePromises`, which defaults to `immediate` and so
+      // invokes their kernel subscriptions before `collectGarbage` runs. Closing
+      // that would mean deferring those too, which is a change to termination
+      // semantics rather than to crank ordering.
       this.#flushCrankBuffer();
     }
   }
@@ -388,8 +400,9 @@ export class KernelQueue {
         resolved.push(item.kpid);
       }
     }
-    // Also promises resolved during this crank that don't have kernel-level
-    // subscribers (e.g., promises from enqueueMessage).
+    // Plus promises resolved during this crank that produced no notify of their
+    // own — nothing in the store was subscribed to them — but that the kernel
+    // itself is waiting on (e.g., promises from `enqueueMessage`).
     resolved.push(...this.#resolvedWithKernelSubscription);
     this.#resolvedWithKernelSubscription = [];
 

From ea64b9136aee4c25d33f41e3add6e68ca24eb246 Mon Sep 17 00:00:00 2001
From: Dimitris Marlagkoutsos <info@sirtimid.com>
Date: Thu, 6 Aug 2026 17:50:32 +0200
Subject: [PATCH 09/10] docs: cut the padding from this branch's comments and
 changelogs

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>
---
 packages/kernel-store/CHANGELOG.md            | 11 ++-
 .../kernel-store/src/sqlite/nodejs.test.ts    |  5 +-
 packages/kernel-store/src/sqlite/nodejs.ts    | 10 +--
 packages/kernel-store/src/sqlite/wasm.test.ts | 17 ++--
 packages/kernel-store/src/sqlite/wasm.ts      | 30 +++----
 .../kernel-test/src/crank-rollback.test.ts    | 14 ++-
 .../src/garbage-collection.test.ts            | 16 ++--
 packages/ocap-kernel/CHANGELOG.md             |  8 +-
 packages/ocap-kernel/src/KernelQueue.test.ts  | 24 ++---
 packages/ocap-kernel/src/KernelQueue.ts       | 88 +++++++------------
 .../src/store/methods/crank.test.ts           | 16 ++--
 .../ocap-kernel/src/store/methods/crank.ts    | 25 +++---
 12 files changed, 98 insertions(+), 166 deletions(-)

diff --git a/packages/kernel-store/CHANGELOG.md b/packages/kernel-store/CHANGELOG.md
index 5e1957b5e..f49786942 100644
--- a/packages/kernel-store/CHANGELOG.md
+++ b/packages/kernel-store/CHANGELOG.md
@@ -12,12 +12,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `rollbackSavepoint` discards the enclosing transaction when `ROLLBACK TO` itself fails, instead of leaving the savepoint on its stack and the transaction open ([#1005](https://github.com/MetaMask/ocap-kernel/pull/1005))
   - Nothing would ever commit or abort that transaction, so every later write on the connection silently joined it, reported success, and vanished on close. Discarding it is no wider than the caller asked for: the transaction begins with the outermost savepoint, so it holds only the work the rollback was abandoning
   - The rollback failure is still what gets thrown, even if aborting the transaction fails too
-- `releaseSavepoint` discards the enclosing transaction when `RELEASE` itself fails, the same way `rollbackSavepoint` already did when `ROLLBACK TO` failed ([#1012](https://github.com/MetaMask/ocap-kernel/pull/1012))
-  - The same hazard by the other door: the savepoint stayed on the stack and the transaction stayed open with nothing left that would ever commit or abort it. The release failure is what gets thrown, even if aborting fails too
-- The wasm driver leaves `_inTx` false when aborting a transaction throws, rather than believing it is still in one ([#1012](https://github.com/MetaMask/ocap-kernel/pull/1012))
-  - It tracks `_inTx` itself instead of reading it from SQLite, so a throwing abort was the one case that could leave the two disagreeing. Left true, `beginIfNeeded` became a permanent no-op and later writes autocommitted a statement at a time; left false, the next `BEGIN` fails loudly if SQLite really is still in a transaction. The nodejs driver reads `db.inTransaction` and was never affected
-- An abort that fails while recovering from a failed savepoint operation is logged, in both drivers ([#1012](https://github.com/MetaMask/ocap-kernel/pull/1012))
-  - The savepoint failure is still the one thrown, but the abandoned transaction it leaves behind was previously silent
+- `releaseSavepoint` discards the enclosing transaction when `RELEASE` fails, as `rollbackSavepoint` already did for `ROLLBACK TO` ([#1012](https://github.com/MetaMask/ocap-kernel/pull/1012))
+  - Otherwise the savepoint stayed on the stack and the transaction open with nothing left to commit or abort it. The release failure is still what gets thrown
+- The wasm driver clears `_inTx` when aborting a transaction throws, instead of believing it is still in one ([#1012](https://github.com/MetaMask/ocap-kernel/pull/1012))
+  - Left true, `beginIfNeeded` became a permanent no-op and later writes autocommitted. The nodejs driver reads `db.inTransaction` and was never affected
+- An abort that fails while recovering from a failed savepoint operation is now logged in both drivers ([#1012](https://github.com/MetaMask/ocap-kernel/pull/1012))
 
 ## [0.6.0]
 
diff --git a/packages/kernel-store/src/sqlite/nodejs.test.ts b/packages/kernel-store/src/sqlite/nodejs.test.ts
index e38655d9a..270f018ae 100644
--- a/packages/kernel-store/src/sqlite/nodejs.test.ts
+++ b/packages/kernel-store/src/sqlite/nodejs.test.ts
@@ -360,10 +360,7 @@ describe('makeSQLKernelDatabase', () => {
       expect(mockDb._spStack).toStrictEqual([]);
     });
 
-    // The same hazard `rollbackSavepoint` guards against, by the other door: a
-    // RELEASE that throws leaves the savepoint on the stack and the transaction
-    // open with nothing to ever commit or abort it, so every later write on this
-    // connection joins it, reports success, and vanishes on close.
+    // The hazard `rollbackSavepoint` guards against, by the other door.
     it('releaseSavepoint discards the transaction when the release fails', async () => {
       const db = await makeSQLKernelDatabase({});
       mockDb.inTransaction = true;
diff --git a/packages/kernel-store/src/sqlite/nodejs.ts b/packages/kernel-store/src/sqlite/nodejs.ts
index 5b7e777b0..fa754bb88 100644
--- a/packages/kernel-store/src/sqlite/nodejs.ts
+++ b/packages/kernel-store/src/sqlite/nodejs.ts
@@ -300,9 +300,8 @@ export async function makeSQLKernelDatabase({
         rollbackIfNeeded();
       } catch (abortError) {
         // The rollback failure below is the one worth reporting, but a failed
-        // abort leaves SQLite holding a transaction this driver has stopped
-        // tracking, which the next crank would silently write into. Nothing here
-        // can repair that, so at least say so.
+        // abort leaves SQLite holding a transaction the next crank would
+        // silently write into. Nothing here can repair that.
         logger?.error(
           'failed to discard transaction after rollback',
           abortError,
@@ -341,9 +340,8 @@ export async function makeSQLKernelDatabase({
         rollbackIfNeeded();
       } catch (abortError) {
         // The release failure below is the one worth reporting, but a failed
-        // abort leaves SQLite holding a transaction this driver has stopped
-        // tracking, which the next crank would silently write into. Nothing here
-        // can repair that, so at least say so.
+        // abort leaves SQLite holding a transaction the next crank would
+        // silently write into. Nothing here can repair that.
         logger?.error(
           'failed to discard transaction after release',
           abortError,
diff --git a/packages/kernel-store/src/sqlite/wasm.test.ts b/packages/kernel-store/src/sqlite/wasm.test.ts
index 25403caa4..8e82a00bc 100644
--- a/packages/kernel-store/src/sqlite/wasm.test.ts
+++ b/packages/kernel-store/src/sqlite/wasm.test.ts
@@ -517,10 +517,7 @@ describe('makeSQLKernelDatabase', () => {
       expect(mockDb._inTx).toBe(false);
     });
 
-    // The same hazard `rollbackSavepoint` guards against, by the other door: a
-    // RELEASE that throws leaves the savepoint on the stack and the transaction
-    // open with nothing to ever commit or abort it, so every later write on this
-    // connection joins it, reports success, and vanishes on close.
+    // The hazard `rollbackSavepoint` guards against, by the other door.
     it('releaseSavepoint discards the transaction when the release fails', async () => {
       const db = await makeSQLKernelDatabase({});
       mockDb._inTx = true;
@@ -555,9 +552,8 @@ describe('makeSQLKernelDatabase', () => {
       expect(mockDb._inTx).toBe(false);
     });
 
-    // `_inTx` is tracked here rather than read from SQLite, so a failed abort is
-    // the one case that can leave it disagreeing with the database. Left true,
-    // `beginIfNeeded` becomes a no-op forever after.
+    // A failed abort is the one case that can leave `_inTx` disagreeing with the
+    // database. Left true, `beginIfNeeded` is a no-op forever after.
     it('stops believing it is in a transaction when the abort fails too', async () => {
       const db = await makeSQLKernelDatabase({});
       mockDb._inTx = true;
@@ -576,10 +572,9 @@ describe('makeSQLKernelDatabase', () => {
       expect(mockDb._inTx).toBe(false);
     });
 
-    // The consequence of the above, and the reason it is worth asserting: a
-    // savepoint created outside a transaction autocommits when released
-    // (Agoric/agoric-sdk#8423), so no later rollback can undo the delivery — an
-    // aborted crank would silently keep its writes.
+    // Why that matters: a savepoint created outside a transaction commits when
+    // released (Agoric/agoric-sdk#8423), so an aborted crank would keep its
+    // writes.
     it('begins a transaction for the next savepoint after a failed abort', async () => {
       const db = await makeSQLKernelDatabase({});
       mockDb._inTx = true;
diff --git a/packages/kernel-store/src/sqlite/wasm.ts b/packages/kernel-store/src/sqlite/wasm.ts
index 45313869f..a43092732 100644
--- a/packages/kernel-store/src/sqlite/wasm.ts
+++ b/packages/kernel-store/src/sqlite/wasm.ts
@@ -210,16 +210,12 @@ export async function makeSQLKernelDatabase({
    */
   function rollbackIfNeeded(): void {
     if (db._inTx) {
-      // Out of the transaction as far as this driver is concerned before the
-      // abort is even attempted, because the abort can throw. Unlike the nodejs
-      // driver, which reads `inTransaction` from SQLite, `_inTx` is tracked here,
-      // so a throw is the one thing that can leave the two disagreeing. Left
-      // true, `beginIfNeeded` is a no-op from then on: writes outside a savepoint
-      // autocommit one statement at a time, and the outermost `RELEASE` of a
-      // savepoint created in autocommit mode commits rather than nesting (see
-      // `createSavepoint`). Setting it false instead means the next `BEGIN`
-      // throws if SQLite really is still in a transaction, which is the failure
-      // worth having.
+      // Cleared before the abort is attempted, because the abort can throw and
+      // `_inTx` is tracked here rather than read from SQLite as the nodejs driver
+      // does. Left true, `beginIfNeeded` is a no-op forever after and writes
+      // autocommit one statement at a time (see `createSavepoint`). Cleared, a
+      // still-open transaction surfaces as a failed `BEGIN` — the louder
+      // failure.
       db._inTx = false;
       db._spStack.length = 0;
       sqlAbortTransaction.step();
@@ -391,10 +387,9 @@ export async function makeSQLKernelDatabase({
       try {
         rollbackIfNeeded();
       } catch (abortError) {
-        // The rollback failure below is the one worth reporting. `_inTx` is
-        // already false by then, so the next `beginIfNeeded` issues its `BEGIN`
-        // and SQLite says loudly if it really is still in a transaction — but
-        // that is a crank away, and this is where the evidence is.
+        // The rollback failure below is the one worth reporting. The next
+        // `BEGIN` will fail if SQLite really is still in a transaction, but that
+        // is a crank away and this is where the evidence is.
         logger?.error(
           'failed to discard transaction after rollback',
           abortError,
@@ -432,10 +427,9 @@ export async function makeSQLKernelDatabase({
       try {
         rollbackIfNeeded();
       } catch (abortError) {
-        // The release failure below is the one worth reporting. `_inTx` is
-        // already false by then, so the next `beginIfNeeded` issues its `BEGIN`
-        // and SQLite says loudly if it really is still in a transaction — but
-        // that is a crank away, and this is where the evidence is.
+        // The release failure below is the one worth reporting. The next
+        // `BEGIN` will fail if SQLite really is still in a transaction, but that
+        // is a crank away and this is where the evidence is.
         logger?.error(
           'failed to discard transaction after release',
           abortError,
diff --git a/packages/kernel-test/src/crank-rollback.test.ts b/packages/kernel-test/src/crank-rollback.test.ts
index dce56b322..edb4fd0e6 100644
--- a/packages/kernel-test/src/crank-rollback.test.ts
+++ b/packages/kernel-test/src/crank-rollback.test.ts
@@ -138,9 +138,8 @@ describe('crank rollback against a real database', () => {
     expect(kdb.kernelKVStore.get('second')).toBe('yes');
   });
 
-  // An aborted crank rolls its delivery back and then still has work to do —
-  // terminating the vat whose delivery failed, collecting garbage — whose writes
-  // have to survive that rollback, since the vat's worker is already gone.
+  // An aborted crank still owes work after the rollback — terminating the vat,
+  // collecting garbage — whose writes have to survive it.
   it('keeps the writes a crank makes after rolling its delivery back', async () => {
     const { kernelStore, kdb } = await makeStore();
 
@@ -157,12 +156,9 @@ describe('crank rollback against a real database', () => {
     expect(kdb.kernelKVStore.get('terminated')).toBe('yes');
   });
 
-  // And they survive it *as part of the crank's transaction*, which is why the
-  // delivery gets a savepoint of its own rather than rolling back the crank's:
-  // rolling back the outermost savepoint discards the transaction, after which
-  // those writes would autocommit one statement at a time. Nothing in the run
-  // loop rolls the crank's own savepoint back — it is the only way from here to
-  // observe that the writes are still undoable at all.
+  // And survive it *inside the crank's transaction*, not as autocommitted
+  // statements. Rolling back `crank` is the only way to observe that from here;
+  // the run loop never does it.
   it('holds those writes in the transaction rather than autocommitting them', async () => {
     const { kernelStore, kdb } = await makeStore();
 
diff --git a/packages/kernel-test/src/garbage-collection.test.ts b/packages/kernel-test/src/garbage-collection.test.ts
index 5ec22e72b..8496d051b 100644
--- a/packages/kernel-test/src/garbage-collection.test.ts
+++ b/packages/kernel-test/src/garbage-collection.test.ts
@@ -109,19 +109,15 @@ describe('Garbage Collection', () => {
    * Reap the importer vat until the kernel's bookkeeping catches up with the
    * vat's own garbage collection, or the attempts run out.
    *
-   * `bringOutYourDead` can only report an import as dropped once the engine has
+   * `bringOutYourDead` reports an import as dropped only once the engine has
    * collected the vat's presence and run its finalizer, which `gcAndFinalize`
-   * does not guarantee on the first attempt. Reaping once and then cranking
-   * repeatedly buys one attempt rather than several, because `nextReapAction`
-   * shifts the single scheduled entry off and the later cranks find nothing to
-   * do — so each attempt has to schedule its own reap, with a message to the vat
-   * to wake the run loop and consume it.
+   * does not guarantee on the first attempt. Each attempt needs its own reap —
+   * `nextReapAction` shifts the one scheduled entry off, so cranking again finds
+   * nothing to do — plus a message to wake the run loop and consume it.
    *
-   * Gives up after five attempts and lets the caller's assertion report the
-   * failure, which names the refcount that never arrived.
+   * Gives up after five attempts; the caller's assertion reports the failure.
    *
-   * @param settled - Predicate answering whether the state under test has
-   * arrived.
+   * @param settled - Whether the state under test has arrived yet.
    */
   async function reapImporterUntil(settled: () => boolean): Promise<void> {
     const isImporter = (vatId: VatId): boolean => vatId === importerVatId;
diff --git a/packages/ocap-kernel/CHANGELOG.md b/packages/ocap-kernel/CHANGELOG.md
index 2919f89b6..ac82b159f 100644
--- a/packages/ocap-kernel/CHANGELOG.md
+++ b/packages/ocap-kernel/CHANGELOG.md
@@ -48,11 +48,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Roll back the crank the run loop died in instead of committing it, so a restart resumes from a consistent boundary ([#1005](https://github.com/MetaMask/ocap-kernel/pull/1005))
   - Because the killing item is no longer consumed, a restart re-dequeues it; an item that reliably kills a crank needs `clearState`/`reset` rather than a restart
   - Store state only — a crank that had already flushed its buffer settled JS-side subscriptions irreversibly
-- Keep a crank's store work inside one transaction, so the writes an aborted crank still owes are committed or undone as a unit ([#1012](https://github.com/MetaMask/ocap-kernel/pull/1012))
-  - A crank now takes two savepoints, `crank` and `delivery`, and rolls back only `delivery`. Rolling back the outermost savepoint ends the enclosing transaction, so terminating the vat and collecting garbage — which follow the rollback, and must survive it — were autocommitting a statement at a time, beyond the reach of any later rollback
-  - Buffered vat outputs are flushed after that work rather than before it. The flush settles the promise `queueMessage` handed an external caller, so a later failure used to roll the crank back underneath an answer already given. Termination still settles the dying vat's own promises immediately
+- Keep a crank's store work inside one transaction ([#1012](https://github.com/MetaMask/ocap-kernel/pull/1012))
+  - A crank now takes two savepoints, `crank` and `delivery`, and rolls back only `delivery`. Rolling back the outermost one ends the transaction, so terminating the vat and collecting garbage — which follow the rollback and must survive it — were autocommitting a statement at a time
+  - Buffered vat outputs are flushed after that work rather than before it, so a later failure can no longer roll the crank back underneath an answer already given. Termination still settles the dying vat's own promises immediately
 - Forget a crank's savepoints when the store call that ends them fails, in both `rollbackCrank` and `endCrank` ([#1012](https://github.com/MetaMask/ocap-kernel/pull/1012))
-  - A failed rollback or release discards the whole transaction, taking every savepoint with it. Keeping them listed had the next crank number its savepoint `t1` while the database had none, and had `endCrank` throw `No such savepoint: t0` from the run loop's `finally` — over whatever really killed the kernel
+  - A failed rollback or release discards the whole transaction, savepoints included. Keeping them listed had `endCrank` throw `No such savepoint: t0` from the run loop's `finally`, over whatever really killed the kernel
 - Refuse inbound remote deliveries once the run loop is dead, rolling back without acknowledging them, so the peer retries and gives up instead of waiting on a kernel that will never deliver ([#1005](https://github.com/MetaMask/ocap-kernel/pull/1005))
   - Covers `bringOutYourDead` as well as `message` and `notify`: a reap is queue work too, consumed only by the run loop. The remaining GC arms need no guard, since they only touch refcounts
 - Refuse `launchSubcluster` once the run loop is dead ([#1005](https://github.com/MetaMask/ocap-kernel/pull/1005))
diff --git a/packages/ocap-kernel/src/KernelQueue.test.ts b/packages/ocap-kernel/src/KernelQueue.test.ts
index 072a302df..1af58d81f 100644
--- a/packages/ocap-kernel/src/KernelQueue.test.ts
+++ b/packages/ocap-kernel/src/KernelQueue.test.ts
@@ -213,13 +213,8 @@ describe('KernelQueue', () => {
       expect(kernelStore.endCrank).toHaveBeenCalled();
     });
 
-    // `#flushCrankBuffer` settles the promise `enqueueMessage` handed an external
-    // caller, reading the resolution out of the store on the way. Were the crank
-    // rolled back after that, the store would un-resolve the promise and restore
-    // the run queue item, so a restart would deliver the message a second time
-    // and notify every other subscriber again — while the original caller had
-    // already been told the first answer. So the flush comes last, after
-    // everything that could still fail.
+    // Why the flush comes after the crank's fallible work: see
+    // `#processCrankResult`. Here the terminate is what fails.
     it('answers no caller from a crank it then rolls back', async () => {
       const mockItem: RunQueueItem = {
         type: 'send',
@@ -271,10 +266,8 @@ describe('KernelQueue', () => {
       );
     });
 
-    // The same invariant one level down, inside the flush itself: moving every
-    // buffered item onto the run queue is store work too, and it can fail
-    // part-way. Answering the first caller while the second enqueue is still
-    // ahead would hand out a result the crank's rollback then discards.
+    // The same invariant inside the flush: `#enqueueRun` is store work and can
+    // fail part-way, so no caller may be answered until all of it lands.
     it('answers no caller until every buffered item is enqueued', async () => {
       const mockItem: RunQueueItem = {
         type: 'send',
@@ -322,13 +315,8 @@ describe('KernelQueue', () => {
       expect(resolve).not.toHaveBeenCalled();
     });
 
-    // Rolling back to the crank's *outermost* savepoint discards the enclosing
-    // transaction (see `rollbackSavepoint`), which would leave the work an aborted
-    // crank still owes — terminating the vat whose delivery failed, collecting
-    // garbage — autocommitting statement by statement, beyond the reach of any
-    // later rollback. That work has to follow the rollback, since the worker is
-    // already gone and the store must not go on believing the vat is alive, so it
-    // is the rollback that has to spare the transaction.
+    // Why two savepoints: see `#runLoop`. This pins that the rollback spares the
+    // transaction, so the work an aborted crank still owes stays inside it.
     it.each([
       {
         label: 'an abort',
diff --git a/packages/ocap-kernel/src/KernelQueue.ts b/packages/ocap-kernel/src/KernelQueue.ts
index 53256be03..c29e334e6 100644
--- a/packages/ocap-kernel/src/KernelQueue.ts
+++ b/packages/ocap-kernel/src/KernelQueue.ts
@@ -56,14 +56,10 @@ export class KernelQueue {
 
   /**
    * Whether this crank's savepoint has already been handed to `rollbackCrank`.
-   * Attempted, not necessarily succeeded: `rollbackCrank` forgets the savepoint
-   * whether or not the database call throws, so after either outcome a second
-   * attempt can only report "no such savepoint" over the real error.
-   *
-   * This has to be recorded at the moment of the attempt rather than returned
-   * from `#processCrankResult`, because that method can throw after rolling back
-   * (`#terminateVat`, `collectGarbage`), and the catch below must still know not
-   * to ask twice.
+   * Attempted, not necessarily succeeded: it is forgotten either way, so a second
+   * attempt could only report "no such savepoint" over the real error. Recorded
+   * at the attempt rather than returned, because `#processCrankResult` can throw
+   * after rolling back.
    */
   #crankRollbackAttempted: boolean = false;
 
@@ -130,24 +126,18 @@ export class KernelQueue {
       this.#kernelStore.startCrank();
       this.#crankRollbackAttempted = false;
       try {
-        // Two savepoints, because the crank's transaction has to outlive the
-        // delivery's rollback. Rolling back to the outermost savepoint discards
-        // the enclosing transaction (see `rollbackSavepoint`), and the work an
-        // aborted crank still owes — terminating the vat whose delivery failed,
-        // collecting garbage — would then autocommit statement by statement,
-        // beyond the reach of any later rollback. The run loop only ever rolls
-        // back `delivery`; `crank` is released by `endCrank`, which is this
-        // crank's one commit point. That release names the *first* savepoint
-        // created here, by ordinal — see `releaseAllSavepoints` — so `crank` has
-        // to stay first.
+        // Two savepoints, because rolling back the outermost one discards the
+        // enclosing transaction (see `rollbackSavepoint`) and an aborted crank
+        // still has writes to make. Only `delivery` is ever rolled back;
+        // releasing `crank` in `endCrank` is this crank's one commit point.
+        // `releaseAllSavepoints` names it by ordinal, so `crank` must stay first.
         this.#kernelStore.createCrankSavepoint('crank');
         this.#kernelStore.createCrankSavepoint('delivery');
 
         // The savepoint exists from here on, so a throw can be undone. Without
-        // this, `endCrank`'s savepoint release commits the half-finished crank:
-        // the item this crank dequeued is gone for good, refcount increments
-        // stick, and promises resolved during it stay resolved while their
-        // notifies die unflushed. A restart would resume from that.
+        // this, `endCrank`'s release commits the half-finished crank: the
+        // dequeued item is gone for good, refcount increments stick, and
+        // resolved promises keep their unflushed notifies.
         try {
           const queueItem = this.#getNextRunQueueItem();
           if (queueItem) {
@@ -164,15 +154,14 @@ export class KernelQueue {
             wakeUpPromise = promise;
           }
         } catch (error) {
-          // An aborted crank already asked, and `rollbackCrank` discards the
-          // savepoint either way; asking again could only throw "no such
-          // savepoint" over the real error.
+          // An aborted crank already asked; asking again could only throw "no
+          // such savepoint" over the real error.
           if (!this.#crankRollbackAttempted) {
             try {
               this.#kernelStore.rollbackCrank('delivery');
             } catch (rollbackError) {
-              // The original failure stays the `cause`, since that is the root
-              // cause an operator needs; the rollback failure is named here.
+              // The original failure stays the `cause`; the rollback failure is
+              // named here.
               throw new Error(
                 `Run loop died and its crank could not be rolled back: ${String(rollbackError)}`,
                 { cause: error },
@@ -317,11 +306,9 @@ export class KernelQueue {
       try {
         this.#kernelStore.rollbackCrank('delivery');
       } finally {
-        // Set even when the rollback threw. `rollbackCrank` forgets the
-        // savepoint in its own `finally`, so "attempted" and "the savepoint is
-        // gone" now coincide exactly — and a second attempt from the run loop's
-        // catch would report a missing savepoint as the reason the kernel died,
-        // burying the database error that actually killed it.
+        // Set even when the rollback threw: the savepoint is gone either way, so
+        // a second attempt would report a missing savepoint as the reason the
+        // kernel died, burying the error that actually killed it.
         this.#crankRollbackAttempted = true;
       }
       // Discard kernel subscriptions that were queued for invocation
@@ -345,30 +332,23 @@ export class KernelQueue {
       // restart it and terminate the vat only after a certain number of failed
       // retries. This is probably where we should implement the vat restart logic.
     }
-    // Vat termination during delivery is triggered by an illegal syscall or by
-    // syscall.exit(). This call is what kills the worker, and its store writes
-    // have to survive the rollback above: once the worker is gone, a store that
-    // still believed the vat was alive would relaunch it after a restart and
-    // redeliver what killed it. Hence the rollback goes only as far as
-    // `delivery`, leaving these writes inside the crank's transaction.
+    // This call kills the worker, so its writes must outlive the rollback above:
+    // a store that still believed the vat was alive would relaunch it after a
+    // restart and redeliver what killed it.
     if (crankResult?.terminate) {
       const { vatId, info } = crankResult.terminate;
       await this.#terminateVat(vatId, info);
     }
     this.#kernelStore.collectGarbage();
     if (!crankResult?.abort) {
-      // The crank survived, so hand its buffered outputs on — after the store
-      // work above, which can still fail. The flush settles the promise
-      // `enqueueMessage` gave an external caller, reading the result out of the
-      // store; were the crank rolled back after that, the caller would keep an
-      // answer computed from state the store discarded, and a restart would
-      // deliver the message again.
+      // After the fallible work above, not before it. The flush settles the
+      // promise `enqueueMessage` gave an external caller, so a later rollback
+      // would discard the state that answer was computed from.
       //
-      // Not airtight: `#terminateVat` resolves the promises the dying vat was
-      // deciding via `resolvePromises`, which defaults to `immediate` and so
-      // invokes their kernel subscriptions before `collectGarbage` runs. Closing
-      // that would mean deferring those too, which is a change to termination
-      // semantics rather than to crank ordering.
+      // Not airtight: `#terminateVat` resolves the dying vat's promises through
+      // `resolvePromises`, which defaults to `immediate` and invokes their
+      // subscriptions before `collectGarbage`. Deferring those too would change
+      // termination semantics, not crank ordering.
       this.#flushCrankBuffer();
     }
   }
@@ -400,15 +380,13 @@ export class KernelQueue {
         resolved.push(item.kpid);
       }
     }
-    // Plus promises resolved during this crank that produced no notify of their
-    // own — nothing in the store was subscribed to them — but that the kernel
-    // itself is waiting on (e.g., promises from `enqueueMessage`).
+    // Plus promises with no vat subscriber to notify, which the kernel is
+    // nonetheless waiting on (e.g. from `enqueueMessage`).
     resolved.push(...this.#resolvedWithKernelSubscription);
     this.#resolvedWithKernelSubscription = [];
 
-    // Callbacks only once every store write is done. Each hands an external
-    // caller a result read out of the store, and a write that threw in between
-    // would have the crank rolled back underneath answers already given.
+    // Callbacks only once every `#enqueueRun` is done: one that threw partway
+    // would roll the crank back underneath answers already given.
     for (const kpid of resolved) {
       this.#invokeKernelSubscription(kpid);
     }
diff --git a/packages/ocap-kernel/src/store/methods/crank.test.ts b/packages/ocap-kernel/src/store/methods/crank.test.ts
index 389afd7a0..08a1e4925 100644
--- a/packages/ocap-kernel/src/store/methods/crank.test.ts
+++ b/packages/ocap-kernel/src/store/methods/crank.test.ts
@@ -154,11 +154,9 @@ describe('crank methods', () => {
       expect(mockCrankBuffer).toHaveLength(0);
     });
 
-    // A failed rollback discards the whole transaction, so the enclosing
-    // savepoints are gone from the database too — not just the one rolled back
-    // to. Truncating to the ordinal would leave `endCrank` releasing a `t0` the
-    // database no longer has, and that "No such savepoint" would be thrown from
-    // the run loop's `finally`, over whatever really killed the kernel.
+    // A failed rollback discards every savepoint, not just this one. Truncating
+    // to the ordinal would have `endCrank` release a `t0` the database lacks and
+    // throw over whatever really killed the kernel.
     it('forgets every savepoint when the rollback fails', () => {
       context.inCrank = true;
       crankMethods.createCrankSavepoint('crank');
@@ -230,11 +228,9 @@ describe('crank methods', () => {
       expect(await waiter).toBeUndefined();
     });
 
-    // What `rollbackCrank` already does when its own rollback fails. Settling
-    // the crank regardless means callers proceed, so a savepoint left listed
-    // here has the next crank number 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.
+    // As `rollbackCrank` does. Left listed, the next crank numbers its savepoint
+    // `t1` against a database that has none, and every later release and rollback
+    // aims one crank past its target.
     it('forgets its savepoints even if releasing them fails', () => {
       crankMethods.startCrank();
       context.savepoints = ['test'];
diff --git a/packages/ocap-kernel/src/store/methods/crank.ts b/packages/ocap-kernel/src/store/methods/crank.ts
index b26219cbb..887ef2bee 100644
--- a/packages/ocap-kernel/src/store/methods/crank.ts
+++ b/packages/ocap-kernel/src/store/methods/crank.ts
@@ -51,17 +51,14 @@ export function getCrankMethods(ctx: StoreContext, kdb: KernelDatabase) {
       if (ctx.savepoints[ordinal] === savepoint) {
         try {
           kdb.rollbackSavepoint(`t${ordinal}`);
-          // Forget the savepoint. Leaving it listed would have `endCrank`'s
-          // release commit the crank we just abandoned — the half-finished state
-          // this rollback exists to discard.
+          // Left listed, `endCrank`'s release would commit the crank we just
+          // abandoned.
           ctx.savepoints.length = ordinal;
         } catch (error) {
-          // A failed rollback discards the whole transaction (see
-          // `rollbackSavepoint`), taking every savepoint with it, not just this
-          // one. Truncating to `ordinal` would leave the enclosing savepoints
-          // listed against a database that no longer has them, and `endCrank`
-          // would then throw "No such savepoint: t0" from the run loop's
-          // `finally` — burying the failure that actually killed the kernel.
+          // A failed rollback discards the whole transaction, so every savepoint
+          // is gone, not just this one. Truncating to `ordinal` would have
+          // `endCrank` release a `t0` the database lacks and throw over whatever
+          // really killed the kernel.
           ctx.savepoints.length = 0;
           throw error;
         }
@@ -86,12 +83,10 @@ export function getCrankMethods(ctx: StoreContext, kdb: KernelDatabase) {
       try {
         kdb.releaseSavepoint('t0');
       } finally {
-        // Forget the savepoints even if the release failed, as `rollbackCrank`
-        // does when its own rollback fails. A failed release discards the whole
-        // transaction (see `releaseSavepoint`), so the database has no savepoints
-        // left either; leaving them listed here would have the next crank number
-        // its savepoint `t1`, and from then on every release and rollback would
-        // aim one crank past the one it meant to end.
+        // A failed release discards the transaction too, so the database has no
+        // savepoints left either. Left listed, the next crank would number its
+        // savepoint `t1` and aim every later release and rollback one crank past
+        // the one it meant to end.
         ctx.savepoints.length = 0;
       }
     }

From 5f3ca97d4b1c543a4aea67a76405d03173bf0cf2 Mon Sep 17 00:00:00 2001
From: Dimitris Marlagkoutsos <info@sirtimid.com>
Date: Thu, 6 Aug 2026 18:28:00 +0200
Subject: [PATCH 10/10] chore: trigger CI

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