fix(ocap-kernel): boot past a vat whose code can no longer be loaded

.gitignore

@@ -2,6 +2,7 @@
 dist/
 coverage/
 docs/*
+reviews/
 !docs/*.md
 !docs/contributing
 
@@ -96,4 +97,4 @@ test-results
 
 # Claude
 **/.claude/settings.local.json
-.playwright-mcp/
+.playwright-mcp/

packages/kernel-test/src/persistence.test.ts

@@ -2,7 +2,8 @@ import type { CapData } from '@endo/marshal';
 import { makeSQLKernelDatabase } from '@metamask/kernel-store/sqlite/nodejs';
 import { waitUntilQuiescent } from '@metamask/kernel-utils';
 import { kunser } from '@metamask/ocap-kernel';
-import { unlink } from 'node:fs/promises';
+import { copyFile, readFile, unlink, writeFile } from 'node:fs/promises';
+import { fileURLToPath, pathToFileURL } from 'node:url';
 import { describe, expect, it, beforeEach, afterEach } from 'vitest';
 
 import {
@@ -18,19 +19,27 @@ const v1Root = 'ko4';
 describe('persistent storage', { timeout: 20_000 }, () => {
   let logger: ReturnType<typeof makeTestLogger>;
   let databasePath: string;
+  let doomedBundlePath: string;
 
   beforeEach(async () => {
     // Create a unique database file for each test in the current directory
     databasePath = `./persistence-test-${Date.now()}-${Math.random()}.db`;
+    // A bundle one test takes away mid-scenario. Declared here so it is removed
+    // even when that test fails partway: the working directory is the monorepo
+    // root, where `*.bundle` is gitignored, so a leaked copy is invisible to
+    // `git status` and survives `yarn clean`.
+    doomedBundlePath = `./doomed-vat-${Date.now()}-${Math.random()}.bundle`;
     logger = makeTestLogger();
   });
 
   afterEach(async () => {
     // Clean up the database file
-    try {
-      await unlink(databasePath);
-    } catch {
-      // Ignore errors if file doesn't exist
+    for (const path of [databasePath, doomedBundlePath]) {
+      try {
+        await unlink(path);
+      } catch {
+        // Ignore errors if file doesn't exist
+      }
     }
   });
 
@@ -46,6 +55,147 @@ describe('persistent storage', { timeout: 20_000 }, () => {
     },
   };
 
+  it('boots past a vat whose bundle vanished between incarnations', async () => {
+    // A vat outlives the code it was launched from: a bundle gets rebuilt to a
+    // new path, pruned, or recorded as an absolute path that did not survive
+    // relocation. Copy a bundle so this test owns one it can take away.
+    await copyFile(
+      fileURLToPath(getBundleSpec('persistence-counter-vat')),
+      doomedBundlePath,
+    );
+    const database1 = await makeSQLKernelDatabase({ dbFilename: databasePath });
+    const kernel1 = await makeKernel(
+      database1,
+      false,
+      logger.logger.subLogger({ tags: ['test'] }),
+    );
+    // Two subclusters, one vat each. The keeper's bundle stays where it is; the
+    // doomed one's does not.
+    const { rootKref: keeperRoot } =
+      await kernel1.launchSubcluster(testSubcluster);
+    await waitUntilQuiescent();
+    const { subclusterId: doomedSubcluster } = await kernel1.launchSubcluster({
+      bootstrap: 'counter',
+      vats: {
+        counter: {
+          bundleSpec: pathToFileURL(doomedBundlePath).toString(),
+          parameters: { name: 'Doomed' },
+        },
+      },
+    });
+    await waitUntilQuiescent();
+    await kernel1.stop();
+
+    await unlink(doomedBundlePath);
+
+    const database2 = await makeSQLKernelDatabase({ dbFilename: databasePath });
+    const kernel2 = await makeKernel(
+      database2,
+      false,
+      logger.logger.subLogger({ tags: ['test'] }),
+    );
+
+    // Booting at all is the claim. The doomed vat's bundle is fetched inside its
+    // own worker, so the failure arrives mid-boot from a worker that is already
+    // running; restoring every vat as one unit turned that into a kernel nobody
+    // could start. (`makeKernel` awaits `initializeAllVats`, so by the time it
+    // returns the skip has already happened.)
+    expect(kernel2.getVatIds()).toStrictEqual(['v1']);
+    // The keeper is untouched by its neighbour's loss, state and all.
+    expect(await runResume(kernel2, keeperRoot)).toBe(
+      'Counter incremented to: 2',
+    );
+
+    // The skipped vat is still a member of its subcluster, so tearing that
+    // subcluster down walks straight into a vat with no worker. It has to
+    // succeed: otherwise the operator's one remedy strands half-done, with no
+    // way to be rid of the vat short of discarding the store.
+    await kernel2.terminateSubcluster(doomedSubcluster);
+    expect(kernel2.getSubcluster(doomedSubcluster)).toBeUndefined();
+
+    // Nothing the teardown did disturbed the healthy neighbour.
+    expect(await runResume(kernel2, keeperRoot)).toBe(
+      'Counter incremented to: 3',
+    );
+    await kernel2.stop();
+
+    // Terminating it has to mean terminating it. Give the bundle back and boot
+    // again: a record left in the store outlives the subcluster that owned it,
+    // so the vat the operator was rid of comes back — running, with its durable
+    // state intact but its c-lists cleaned up underneath it, and belonging to
+    // nothing. `getVats` asks every vat for its subcluster, so from then on
+    // `getStatus` throws for every caller that asks.
+    await copyFile(
+      fileURLToPath(getBundleSpec('persistence-counter-vat')),
+      doomedBundlePath,
+    );
+    const database3 = await makeSQLKernelDatabase({ dbFilename: databasePath });
+    const kernel3 = await makeKernel(
+      database3,
+      false,
+      logger.logger.subLogger({ tags: ['test'] }),
+    );
+
+    expect(kernel3.getVatIds()).toStrictEqual(['v1']);
+    expect(await kernel3.getStatus()).toBeDefined();
+    await kernel3.stop();
+  });
+
+  it('restores a vat whose code becomes reachable again', async () => {
+    // The other half of retaining the record rather than pruning it: a bundle
+    // that comes back brings its vat back, with the state it left off with.
+    await copyFile(
+      fileURLToPath(getBundleSpec('persistence-counter-vat')),
+      doomedBundlePath,
+    );
+    const doomedCluster = {
+      bootstrap: 'counter',
+      vats: {
+        counter: {
+          bundleSpec: pathToFileURL(doomedBundlePath).toString(),
+          parameters: { name: 'Doomed' },
+        },
+      },
+    };
+    const database1 = await makeSQLKernelDatabase({ dbFilename: databasePath });
+    const kernel1 = await makeKernel(
+      database1,
+      false,
+      logger.logger.subLogger({ tags: ['test'] }),
+    );
+    const { rootKref } = await kernel1.launchSubcluster(doomedCluster);
+    await waitUntilQuiescent();
+    await kernel1.stop();
+
+    // Incarnation 2: the bundle is gone, so the vat is skipped.
+    const bundleContents = await readFile(doomedBundlePath);
+    await unlink(doomedBundlePath);
+    const database2 = await makeSQLKernelDatabase({ dbFilename: databasePath });
+    const kernel2 = await makeKernel(
+      database2,
+      false,
+      logger.logger.subLogger({ tags: ['test'] }),
+    );
+    expect(kernel2.getVatIds()).toStrictEqual([]);
+    await kernel2.stop();
+
+    // Incarnation 3: the bundle is back, and so is the vat.
+    await writeFile(doomedBundlePath, bundleContents);
+    const database3 = await makeSQLKernelDatabase({ dbFilename: databasePath });
+    const kernel3 = await makeKernel(
+      database3,
+      false,
+      logger.logger.subLogger({ tags: ['test'] }),
+    );
+    expect(kernel3.getVatIds()).toStrictEqual(['v1']);
+    // Count 1 came from the incarnation that ran `bootstrap`; picking up at 2
+    // shows the durable store survived the incarnation it sat out.
+    expect(await runResume(kernel3, rootKref)).toBe(
+      'Counter incremented to: 2',
+    );
+    await kernel3.stop();
+  });
+
   it('maintains state across kernel restarts', async () => {
     const database1 = await makeSQLKernelDatabase({ dbFilename: databasePath });
     const kernel1 = await makeKernel(

packages/ocap-kernel/CHANGELOG.md

@@ -48,6 +48,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- A persisted vat whose code can no longer be loaded no longer makes the whole kernel unbootable ([#1025](https://github.com/MetaMask/ocap-kernel/pull/1025))
+  - A vat outlives the code it was launched from: a bundle can be rebuilt to a new path, pruned, or recorded as an absolute path that did not survive relocation. Boot restored every persisted vat in one `Promise.all`, so one such vat rejected `Kernel.make` outright and every other subcluster was lost with it. Under the daemon that surfaced only as a startup timeout
+  - The failure is now confined to the vat that owns it: that vat is skipped, its leftover worker is reaped (a bundle is fetched inside the worker, so the worker is live by the time the load fails — one left running is the wedged process this failure mode is known by), and an error naming the vat, its subcluster, and its code source is logged. The rest of the kernel boots
+  - The skipped vat's persisted record is kept rather than pruned, so a vat whose code becomes reachable again is restored by a later boot, resuming from the durable state it left off with. Whether an unrestorable vat should instead take its subcluster down with it is left to the subcluster lifecycle ([#979](https://github.com/MetaMask/ocap-kernel/issues/979))
+- A `notify`, GC action, or `bringOutYourDead` addressed to an endpoint that is not running is skipped instead of killing the run loop ([#1025](https://github.com/MetaMask/ocap-kernel/pull/1025))
+  - These deliveries looked up their endpoint unguarded, so an endpoint named by persisted state but absent from the running kernel threw `VatNotFoundError` from inside the crank, which killed the run loop permanently. Because the crank was rolled back, the item was re-dequeued on the next boot and killed that one too. Reachable whenever ownership entries outlive their vat — a terminated vat awaiting cleanup, or a vat skipped at boot per the entry above
+  - Unlike a `send`, none of these has a caller to reject; `send` already tolerated a vanished endpoint by rejecting the caller with `ENDPOINT_UNREACHABLE`. A skipped `notify` still releases the reference it was holding, and no longer translates the resolution first — those translations import if needed, which would mint c-list entries and take references in an endpoint that will never be told and so can never release them
+  - An endpoint id that names neither a vat nor a remote still throws, since that is corrupt state rather than an endpoint that has gone away
+- `terminateVat` now retires a vat that is persisted but not running, instead of throwing `VatNotFoundError` ([#1025](https://github.com/MetaMask/ocap-kernel/pull/1025))
+  - Such a vat could not be terminated, and `terminateSubcluster` — which walks persisted membership — rejected part-way through, after deleting the system-subcluster mapping and before removing the subcluster record. A subcluster containing one could not be torn down at all
+  - Retiring one discards its persisted record and rejects the promises it was deciding, as stopping a running vat does. Marking it terminated is not enough on its own: the deferred cleanup that follows leaves the record that decides whether the next boot restores the vat, and states that its caller has already rejected those promises. Left half-done, a terminated vat came back at the next boot whose code was reachable — running, in a subcluster that no longer existed, which then failed every `getStatus` — and anything awaiting a result from it waited forever
+  - A vat that is neither running nor persisted still throws
 - A message delivered to a kernel-owned kref with no registered service now rejects the caller with `ENDPOINT_UNREACHABLE` instead of throwing, which escaped the crank and killed the run loop — turning one unreachable reference into a dead kernel ([#1007](https://github.com/MetaMask/ocap-kernel/pull/1007))
   - Reachable without any kernel bug: an anonymous kernel object hosts something that cannot outlive the process, such as an accepted socket connection, so a vat holding one across a restart or a message to one still queued from the previous incarnation lands here. Kernel objects are born with a `(1, 1)` refcount, so the init sweep cannot delete such an object and its `kernel` owner survives (see [#1006](https://github.com/MetaMask/ocap-kernel/issues/1006))
   - Matches what `KernelRouter` already does for a delivery whose endpoint has vanished. A message sent with no result promise has nobody to report to, so it is logged instead

packages/ocap-kernel/src/Kernel.test.ts

@@ -2,6 +2,7 @@ import { VatNotFoundError } from '@metamask/kernel-errors';
 import type { KernelDatabase } from '@metamask/kernel-store';
 import type { JsonRpcMessage } from '@metamask/kernel-utils';
 import { waitUntilQuiescent } from '@metamask/kernel-utils';
+import type { LogEntry } from '@metamask/logger';
 import { Logger } from '@metamask/logger';
 import type { DuplexStream } from '@metamask/streams';
 import type { Mocked, MockInstance } from 'vitest';
@@ -130,6 +131,42 @@ const makeSingleVatClusterConfig = (): ClusterConfig => ({
   },
 });
 
+const HEALTHY_BUNDLE = 'file:///bundles/present.bundle';
+const MISSING_BUNDLE = 'file:///bundles/gone.bundle';
+
+const makeBundleVatClusterConfig = (bundleSpec: string): ClusterConfig => ({
+  bootstrap: 'testVat',
+  vats: {
+    testVat: { bundleSpec },
+  },
+});
+
+/**
+ * Build a `launch` implementation standing in for a worker whose bundle fetch
+ * fails. `fetchBlob` hands back whatever `fs.readFile` rejected with, so the
+ * failure arrives as a Node errno object rather than a well-formed `Error`
+ * with a useful stack.
+ *
+ * @returns A `PlatformServices['launch']` implementation that rejects for the
+ * vat configured with `MISSING_BUNDLE` and succeeds for every other vat.
+ */
+const makeMissingBundleLaunch = () => {
+  return async (_vatId: VatId, vatConfig: VatConfig) => {
+    if ('bundleSpec' in vatConfig && vatConfig.bundleSpec === MISSING_BUNDLE) {
+      throw Object.assign(
+        new Error(
+          `ENOENT: no such file or directory, open '${MISSING_BUNDLE}'`,
+        ),
+        { code: 'ENOENT', errno: -2, syscall: 'open' },
+      );
+    }
+    return { end: vi.fn() } as unknown as DuplexStream<
+      JsonRpcMessage,
+      JsonRpcMessage
+    >;
+  };
+};
+
 const makeMockClusterConfig = (): ClusterConfig => ({
   bootstrap: 'alice',
   vats: {
@@ -247,6 +284,75 @@ describe('Kernel', () => {
       expect(makeVatHandleMock).toHaveBeenCalledOnce();
       expect(kernel2.getVatIds()).toStrictEqual(['v1']);
     });
+
+    it('boots when a persisted vat is no longer restorable', async () => {
+      const db = makeMapKernelDatabase();
+      const kernel1 = await Kernel.make(mockPlatformServices, db);
+      // Two subclusters, one vat each. The unrestorable vat is deliberately
+      // in a *different* subcluster from the healthy one: under every
+      // lifecycle policy in play (including bootstrap-vat death cascading to
+      // its own subcluster, per #979), a failure in one subcluster leaves
+      // another subcluster's vats running. That keeps this test a statement
+      // about the defect rather than about a chosen remedy.
+      await kernel1.launchSubcluster(
+        makeBundleVatClusterConfig(HEALTHY_BUNDLE),
+      );
+      await kernel1.launchSubcluster(
+        makeBundleVatClusterConfig(MISSING_BUNDLE),
+      );
+      expect(kernel1.getVatIds()).toStrictEqual(['v1', 'v2']);
+
+      // The bundle behind v2 goes away between incarnations — rebuilt to a new
+      // path, pruned, or an absolute path that did not survive relocation.
+      // `fetchBlob` rejects with a bare Node errno object when that happens.
+      launchWorkerMock.mockImplementation(makeMissingBundleLaunch());
+
+      const kernel2 = await Kernel.make(mockPlatformServices, db);
+
+      // Booting at all is the claim: the rejection used to propagate out of
+      // `initializeAllVats`, so `Kernel.make` rejected and the daemon died
+      // during init. The healthy vat comes up and the unrestorable one does
+      // not; whether it should instead take its own subcluster down with it is
+      // #979's to decide.
+      expect(kernel2.getVatIds()).toStrictEqual(['v1']);
+    });
+
+    it('names the unrestorable vat and its bundle when booting past it', async () => {
+      const db = makeMapKernelDatabase();
+      // Capture through a transport rather than by spying on the logger's
+      // methods. `subLogger` builds a *fresh* `Logger` that shares its parent's
+      // transports, so a sub-logger's output never passes through the parent's
+      // methods — and every kernel component, the vat manager included, logs
+      // through one.
+      const entries: LogEntry[] = [];
+      const logger = new Logger({
+        tags: ['test'],
+        transports: [(entry) => entries.push(entry)],
+      });
+      const kernel1 = await Kernel.make(mockPlatformServices, db, { logger });
+      await kernel1.launchSubcluster(
+        makeBundleVatClusterConfig(HEALTHY_BUNDLE),
+      );
+      await kernel1.launchSubcluster(
+        makeBundleVatClusterConfig(MISSING_BUNDLE),
+      );
+
+      launchWorkerMock.mockImplementation(makeMissingBundleLaunch());
+      entries.length = 0;
+
+      await Kernel.make(mockPlatformServices, db, { logger });
+
+      // Skipping a persisted vat silently would trade an unbootable kernel for
+      // a kernel that is quietly missing a vat. One entry has to carry both the
+      // vat and its bundle — satisfying this by logging them from two unrelated
+      // places would tell an operator nothing.
+      const reported = entries.filter(
+        ({ level, message }) =>
+          level === 'error' && String(message).includes('v2'),
+      );
+      expect(reported).toHaveLength(1);
+      expect(String(reported[0]?.message)).toContain(MISSING_BUNDLE);
+    });
   });
 
   describe('queueMessage()', () => {