fix(ocap-kernel): keep the kernel's account of a vat's life in one piece

packages/extension/test/e2e/control-panel.test.ts

@@ -189,6 +189,13 @@ test.describe('Control Panel', () => {
         popupPage.locator('[data-testid="message-output"]'),
       ).toContainText(value);
     }
+    // Asserted per checkpoint rather than from `v3Values`, which is used as a
+    // negative below: v3's root keeps a count for as long as v1 imports it, so
+    // it is not one of the keys that vanish with the vat. The value is the root
+    // pin plus that import.
+    await expect(
+      popupPage.locator('[data-testid="message-output"]'),
+    ).toContainText('{"key":"ko6.refCount","value":"2,2"}');
     await popupPage.click('button:text("Control Panel")');
     await popupPage.locator('[data-testid="accordion-header"]').first().click();
     await popupPage
@@ -214,6 +221,11 @@ test.describe('Control Panel', () => {
         popupPage.locator('[data-testid="message-output"]'),
       ).toContainText(value);
     }
+    // Terminating v3 released the pin its root was held by, leaving v1's import
+    // as the only holder.
+    await expect(
+      popupPage.locator('[data-testid="message-output"]'),
+    ).toContainText('{"key":"ko6.refCount","value":"1,1"}');
     await popupPage.click('button:text("Control Panel")');
 
     await popupPage.click('button:text("Collect Garbage")');
@@ -241,6 +253,11 @@ test.describe('Control Panel', () => {
     await expect(
       popupPage.locator('[data-testid="message-output"]'),
     ).toContainText('{"key":"kp4.refCount","value":"1"}');
+    // v3's cleanup took its own c-list, not v1's import, so the root survives
+    // its owner at the one count that import justifies.
+    await expect(
+      popupPage.locator('[data-testid="message-output"]'),
+    ).toContainText('{"key":"ko6.refCount","value":"1,1"}');
     await popupPage.click('button:text("Control Panel")');
     await popupPage.locator('[data-testid="accordion-header"]').first().click();
     // delete v1

packages/kernel-test/src/crank-rollback.test.ts

@@ -199,6 +199,33 @@ describe('crank rollback against a real database', () => {
     kernelStore.endCrank();
   });
 
+  // The set is not per-crank: only `collectGarbage` empties it, and that runs at
+  // the end of a crank that had an item. So a candidate created while the run
+  // loop was idle — `terminateVat` unpinning a root is the real path — is still
+  // owed a collection, and an unrelated crank's rollback must not cancel it.
+  it('keeps GC candidates that predate the crank it rolled back', async () => {
+    const { kernelStore } = await makeStore();
+    const idle = kernelStore.initKernelPromise()[0];
+    kernelStore.decrementRefCount(idle, 'test');
+
+    kernelStore.startCrank();
+    kernelStore.createCrankSavepoint('start');
+    const abandoned = kernelStore.initKernelPromise()[0];
+    kernelStore.decrementRefCount(abandoned, 'test');
+    kernelStore.rollbackCrank('start');
+    kernelStore.endCrank();
+
+    kernelStore.startCrank();
+    kernelStore.createCrankSavepoint('start');
+    kernelStore.collectGarbage();
+    kernelStore.endCrank();
+
+    // Collected, because it was owed before the abandoned crank began.
+    expect(() => kernelStore.getKernelPromise(idle)).toThrow(
+      'unknown kernel promise',
+    );
+  });
+
   // `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.

packages/kernel-test/src/garbage-collection.test.ts

@@ -241,26 +241,39 @@ describe('Garbage Collection', () => {
      * Give an importer a chance to notice a dropped object and tell the kernel,
      * then keep cranking until the resulting GC actions have all been consumed.
      *
+     * Waits for `done` as well as for an empty action set, because an empty set
+     * is also what "the vat has not told us anything yet" looks like. A vat
+     * reports a dropped import only once the engine has actually collected it,
+     * and `gcAndFinalize` can only provoke that, not guarantee it on the first
+     * try — so a round that reports nothing has to be retried rather than read
+     * as the end of the story. Reaped afresh each round for the same reason:
+     * the report rides on a `bringOutYourDead`.
+     *
      * @param vatId - The vat to reap.
      * @param rootKRef - That vat's root, to poke with cranks afterwards.
+     * @param done - The outcome being waited for.
      */
-    async function reapAndSettle(vatId: VatId, rootKRef: KRef): Promise<void> {
-      kernel.reapVats((id) => id === vatId);
-      // BOYD has to reach the vat, the vat has to answer, and the kernel has to
-      // act on the answer — but a round can queue more work, so loop until the
-      // queue is actually empty rather than guessing at a crank count.
+    async function reapAndSettle(
+      vatId: VatId,
+      rootKRef: KRef,
+      done: () => boolean,
+    ): Promise<void> {
       const maxRounds = 10;
       for (let round = 0; round < maxRounds; round++) {
+        kernel.reapVats((id) => id === vatId);
+        // BOYD has to reach the vat, the vat has to answer, and the kernel has
+        // to act on the answer — but a round can queue more work, so loop until
+        // the queue is actually empty rather than guessing at a crank count.
         await kernel.queueMessage(rootKRef, 'noop', []);
         await waitUntilQuiescent(500);
-        if ([...kernelStore.getGCActions()].length === 0) {
+        if ([...kernelStore.getGCActions()].length === 0 && done()) {
           return;
         }
       }
       throw Error(
-        `GC actions still pending after ${maxRounds} rounds: ${[
-          ...kernelStore.getGCActions(),
-        ].join(', ')}`,
+        `GC did not settle after ${maxRounds} rounds; actions pending: ${
+          [...kernelStore.getGCActions()].join(', ') || '(none)'
+        }`,
       );
     }
 
@@ -295,7 +308,11 @@ describe('Garbage Collection', () => {
       await kernel.queueMessage(importerKRef, 'makeWeak', [objectId]);
       await kernel.queueMessage(importerKRef, 'forgetImport', []);
       await waitUntilQuiescent();
-      await reapAndSettle(importerVatId, importerKRef);
+      await reapAndSettle(
+        importerVatId,
+        importerKRef,
+        () => !kernelStore.getImporters(sharedKRef).includes(importerVatId),
+      );
 
       // The exporter must not have been told to drop it: the second importer
       // legitimately still holds it
@@ -328,7 +345,11 @@ describe('Garbage Collection', () => {
       await kernel.queueMessage(secondImporterKRef, 'makeWeak', [objectId]);
       await kernel.queueMessage(secondImporterKRef, 'forgetImport', []);
       await waitUntilQuiescent();
-      await reapAndSettle(secondImporterVatId, secondImporterKRef);
+      await reapAndSettle(
+        secondImporterVatId,
+        secondImporterKRef,
+        () => kernelStore.getImporters(sharedKRef).length === 0,
+      );
 
       expect(kernelStore.getImporters(sharedKRef)).toStrictEqual([]);
       // Only the createObject result's stored value still names it