fix: address PR review findings for Vite v8 migration

- Export and add unit tests for removeDynamicImportsPlugin (16 tests)
- Add this.warn() when dynamic imports cannot be replaced
- Replace definite-assignment mutex with makePromiseKit
- Move exports: {} after endowment spreads to prevent accidental override
- Fix inaccurate comments (remove self, clarify ESM interop trigger)
- Add bundle-loader endowment tests for exports, globalThis, global

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

Author: sirtimid

packages/kernel-utils/src/vite-plugins/bundle-vat.ts

@@ -1,3 +1,4 @@
+import { makePromiseKit } from '@endo/promise-kit';
 import type {
   OutputChunk,
   Plugin as RolldownPlugin,
@@ -21,14 +22,18 @@ export type { VatBundle } from '../vat-bundle.ts';
  * dynamic imports from the code so that Rolldown accepts the IIFE format.
  *
  * Pattern transformed:
- *   `import('specifier')` → `Promise.resolve({})`
+ *   `import('specifier')` / `import("specifier")` / `` import(`specifier`) ``
+ *   → `Promise.resolve({})`
+ *
+ * Only replaces calls with literal string specifiers. Dynamic expressions
+ * like `import(variable)` are left untouched.
  *
  * @returns A Rolldown plugin.
  */
-function removeDynamicImportsPlugin(): RolldownPlugin {
+export function removeDynamicImportsPlugin(): RolldownPlugin {
   return {
     name: 'ocap-kernel:remove-dynamic-imports',
-    transform(code) {
+    transform(code, id) {
       if (!/\bimport\s*\(/u.test(code)) {
         return null;
       }
@@ -39,6 +44,10 @@ function removeDynamicImportsPlugin(): RolldownPlugin {
       );
 
       if (transformed === code) {
+        this.warn(
+          `Module "${id}" contains dynamic import() expressions that could not ` +
+            `be replaced (e.g. computed specifiers). Rolldown may reject IIFE output.`,
+        );
         return null;
       }
 
@@ -66,10 +75,8 @@ export async function bundleVat(sourcePath: string): Promise<VatBundle> {
   // Wait for any in-flight build to finish before starting ours, then
   // register a slot so the next caller waits for us.
   const prevQueue = buildQueue;
-  let releaseLock!: () => void;
-  buildQueue = new Promise<void>((resolve) => {
-    releaseLock = resolve;
-  });
+  const { promise, resolve: releaseLock } = makePromiseKit<void>();
+  buildQueue = promise;
 
   let result: Awaited<ReturnType<typeof build>>;
   try {

packages/kernel-utils/src/vite-plugins/index.ts

@@ -2,7 +2,7 @@ import type { Plugin } from 'vite';
 
 import { bundleVat } from './bundle-vat.ts';
 
-export { bundleVat } from './bundle-vat.ts';
+export { bundleVat, removeDynamicImportsPlugin } from './bundle-vat.ts';
 export type { VatBundle } from './bundle-vat.ts';
 
 type VatEntry = {

packages/kernel-utils/src/vite-plugins/remove-dynamic-imports-plugin.test.ts

@@ -0,0 +1,79 @@
+import { describe, it, expect, vi } from 'vitest';
+
+import { removeDynamicImportsPlugin } from './bundle-vat.ts';
+
+type TransformFn = (
+  code: string,
+  id: string,
+) => { code: string; map: null } | null;
+
+describe('removeDynamicImportsPlugin', () => {
+  const plugin = removeDynamicImportsPlugin();
+  const transform = (plugin.transform as TransformFn).bind({
+    warn: vi.fn(),
+  });
+
+  it.each([
+    ['single-quoted import', `import('module')`, `Promise.resolve({})`],
+    ['double-quoted import', `import("module")`, `Promise.resolve({})`],
+    ['backtick-quoted import', 'import(`module`)', `Promise.resolve({})`],
+    [
+      'import with whitespace before paren',
+      `import ('module')`,
+      `Promise.resolve({})`,
+    ],
+    [
+      'import with whitespace around specifier',
+      `import(  'module'  )`,
+      `Promise.resolve({})`,
+    ],
+    [
+      'multiple dynamic imports in one file',
+      `const a = import('foo');\nconst b = import("bar");`,
+      `const a = Promise.resolve({});\nconst b = Promise.resolve({});`,
+    ],
+    [
+      'dynamic import surrounded by other code',
+      `const x = 1;\nawait import('lazy-util');\nconst y = 2;`,
+      `const x = 1;\nawait Promise.resolve({});\nconst y = 2;`,
+    ],
+  ])('replaces %s', (_name, code, expected) => {
+    const result = transform(code, 'test.ts');
+    expect(result).toStrictEqual({ code: expected, map: null });
+  });
+
+  it.each([
+    ['no import at all', 'const x = 1;'],
+    ['ESM import statement', `import foo from 'bar';`],
+    ['ESM named import', `import { foo } from 'bar';`],
+    ['empty code', ''],
+  ])('returns null for %s (fast-path miss)', (_name, code) => {
+    expect(transform(code, 'test.ts')).toBeNull();
+  });
+
+  it.each([
+    ['computed specifier (variable)', `import(variable)`],
+    ['computed specifier (expression)', `import(getPath())`],
+    ['computed specifier (concatenation)', `import('./locales/' + lang)`],
+  ])('returns null for %s (no literal match)', (_name, code) => {
+    expect(transform(code, 'test.ts')).toBeNull();
+  });
+
+  it('does not match reimport or similar substrings', () => {
+    const code = `reimport('foo')`;
+    expect(transform(code, 'test.ts')).toBeNull();
+  });
+
+  it('warns when fast-path matches but replacement regex does not', () => {
+    const warn = vi.fn();
+    const boundTransform = (plugin.transform as TransformFn).bind({
+      warn,
+    });
+
+    boundTransform(`import(variable)`, 'test.ts');
+
+    expect(warn).toHaveBeenCalledWith(
+      expect.stringContaining('could not be replaced'),
+    );
+  });
+});

packages/ocap-kernel/src/vats/bundle-loader.test.ts

@@ -85,5 +85,32 @@ describe('loadBundle', () => {
       });
       expect(result).toStrictEqual({ inescapableValue: 'test' });
     });
+
+    it('provides exports object to compartment', () => {
+      const content = JSON.stringify({
+        moduleFormat: 'iife',
+        code: 'Object.defineProperty(exports, Symbol.toStringTag, { value: "Module" }); var __vatExports__ = { ok: true };',
+      });
+      const result = loadBundle(content);
+      expect(result).toStrictEqual({ ok: true });
+    });
+
+    it('provides globalThis on compartment global', () => {
+      const content = JSON.stringify({
+        moduleFormat: 'iife',
+        code: 'var __vatExports__ = { hasGlobalThis: typeof globalThis === "object" && globalThis !== undefined };',
+      });
+      const result = loadBundle(content);
+      expect(result).toStrictEqual({ hasGlobalThis: true });
+    });
+
+    it('provides global on compartment global', () => {
+      const content = JSON.stringify({
+        moduleFormat: 'iife',
+        code: 'var __vatExports__ = { hasGlobal: typeof global === "object" && global !== undefined };',
+      });
+      const result = loadBundle(content);
+      expect(result).toStrictEqual({ hasGlobal: true });
+    });
   });
 });

packages/ocap-kernel/src/vats/bundle-loader.ts

@@ -38,19 +38,18 @@ export function loadBundle(
   const compartment = new Compartment({
     // SES globals that may be used by bundled code
     harden: globalThis.harden,
-    // Rolldown adds `Object.defineProperty(exports, Symbol.toStringTag, ...)` to
-    // IIFE bundles for modules with no named exports. Provide an empty object so
-    // that call does not throw in the Compartment.
-    exports: {},
     ...endowments,
     ...inescapableGlobalProperties,
+    // Rolldown adds `Object.defineProperty(exports, Symbol.toStringTag, ...)` to
+    // IIFE bundles as part of ESM interop. This MUST come after endowment spreads
+    // to ensure it is not accidentally overridden.
+    exports: {},
   });
   // Rolldown-generated CJS helpers use `var localThis = globalThis`, and
   // some bundled libraries (e.g. lodash) detect the global via
-  // `typeof global == "object" && global` or `Function("return this")()`.
-  // None of these work in a SES Compartment out of the box:
-  //   - `global` / `self` are not compartment bindings
-  //   - `Function("return this")()` returns `undefined` in strict mode
+  // `typeof global == "object" && global`.
+  // Neither `globalThis` nor `global` are available in a SES Compartment by
+  // default — only hardened intrinsics are exposed as named bindings.
   // Inject these properties on the compartment's own global so that both
   // patterns resolve to the compartment's global (which has all the intrinsics).
   const cg = compartment.globalThis as Record<string, unknown>;