fix(coding-agent/eval): routed JS import calls through session-aware import helper

- Updated JS import rewriting to route top-level `import` declarations through `__omp_import__` with support for import attributes.
- Added AST traversal to replace `import(...)` call callee nodes with `__omp_import__` so dynamic imports resolve via session-aware helper.
- Updated runtime `__omp_import__` to accept an optional options object and pass it through to `import(target, options)`.

docs/tools/eval.md

@@ -134,7 +134,7 @@ Implemented in `packages/coding-agent/src/eval/js/context-manager.ts` and `packa
 - Persistent `vm.Context` instances keyed by `js:${sessionId}` in `vmContexts`
 - `rst` calls `resetVmContext(sessionKey)` before the cell executes
 - Top-level `await` and bare `return` are supported by wrapping code in an async IIFE when `wrapCode()` sees `await` or `return`
-- Top-level static `import ... from ...` is rewritten to `await import(...)` by `rewriteStaticImports()`
+- Top-level static `import ... from ...` and dynamic `import(...)` calls are routed through `rewriteImports()`, which sends them via `__omp_import__` so the specifier resolves against the session cwd
 - The prelude installs globals:
   - `display`, `print`
   - `read`, `write`, `append`, `sort`, `uniq`, `counter`, `diff`, `tree`, `env`, `output`

packages/coding-agent/CHANGELOG.md

@@ -23,6 +23,8 @@
 
 ### Fixed
 
+- Fixed `eval` tool dynamic `await import("./relative.ts")` calls failing with `Cannot find module ... from .../eval/js/shared/runtime.ts`. The static-import rewriter only handled `ImportDeclaration` nodes, so dynamic-import call expressions resolved their specifier against the worker module's URL instead of the session cwd. The rewriter now walks the full AST and additionally swaps `import` callees in `CallExpression` nodes for `__omp_import__`, which forwards the optional options bag verbatim to native `import()` so `{ with: { type: "json" } }` round-trips. Renamed `rewriteStaticImports` → `rewriteImports`.
+
 - Fixed `pr://<owner>/<repo>/diff` URLs for repositories named `diff` to continue resolving to PR list lookups instead of being parsed as short-form diff links
 - Fixed PR unified diff parsing so changed-file headers with quoted paths (such as paths containing spaces) are now detected correctly and hunk content lines beginning with `---`/`+++` are counted in additions/deletions
 - Fixed GitHub view caching to account for active credential identity and avoid serving cached issue/PR data across different account/token contexts

packages/coding-agent/src/eval/js/context-manager.ts

@@ -17,7 +17,7 @@ import type {
 	WorkerOutbound,
 } from "./worker-protocol";
 
-export { rewriteStaticImports } from "./shared/rewrite-imports";
+export { rewriteImports } from "./shared/rewrite-imports";
 export type { JsDisplayOutput } from "./worker-protocol";
 
 export interface VmRunState {

packages/coding-agent/src/eval/js/shared/rewrite-imports.ts

@@ -1,9 +1,10 @@
 import { parse as babelParse } from "@babel/parser";
 
-// Static ESM `import` declarations are not valid inside vm.runInContext (script-mode parsing).
-// We rewrite top-level imports to dynamic-import expressions in the user-supplied source so
-// pasted ESM runs verbatim. A real parser keeps imports embedded in string literals, template
-// literals, or comments intact.
+// Static ESM `import` declarations are not valid inside vm.runInContext (script-mode parsing),
+// and dynamic `import(...)` would otherwise resolve specifiers against the worker module's URL
+// instead of the session cwd. We rewrite both forms so they route through the worker-injected
+// `__omp_import__` helper, which resolves the specifier against the active session cwd. A real
+// parser keeps imports embedded in string literals, template literals, or comments intact.
 
 type BabelImportDeclaration = {
 	type: "ImportDeclaration";
@@ -34,6 +35,8 @@ type BabelExpressionStatement = {
 
 type BabelProgramNode = BabelImportDeclaration | BabelLexicalDecl | BabelExpressionStatement | { type: string };
 
+type BabelNode = { type: string; start: number; end: number; [key: string]: unknown };
+
 function parseProgram(code: string): { program: { body: ReadonlyArray<BabelProgramNode> } } | null {
 	try {
 		return babelParse(code, {
@@ -51,26 +54,50 @@ function parseProgram(code: string): { program: { body: ReadonlyArray<BabelProgr
 	}
 }
 
-function buildDynamicImportCall(sourceLiteral: string, withClause: string | undefined): string {
+function buildOmpImportCall(sourceLiteral: string, optionsLiteral: string | undefined): string {
 	// Route every static import through the worker-injected `__omp_import__` helper so the
 	// specifier resolves against the session cwd (and `with`-attribute imports keep working).
-	return withClause ? `__omp_import__(${sourceLiteral}, ${withClause})` : `__omp_import__(${sourceLiteral})`;
+	return optionsLiteral ? `__omp_import__(${sourceLiteral}, ${optionsLiteral})` : `__omp_import__(${sourceLiteral})`;
 }
 
-function buildWithClause(node: BabelImportDeclaration): string | undefined {
+// Walks every node in `root`, depth-first, invoking `visit` on each one. Skips Babel's
+// non-AST bookkeeping fields so we don't recurse into source locations or comment arrays.
+function walkNodes(root: unknown, visit: (node: BabelNode) => void): void {
+	const stack: unknown[] = [root];
+	while (stack.length > 0) {
+		const current = stack.pop();
+		if (!current || typeof current !== "object") continue;
+		if (Array.isArray(current)) {
+			for (let i = current.length - 1; i >= 0; i--) stack.push(current[i]);
+			continue;
+		}
+		const node = current as Record<string, unknown>;
+		if (typeof node.type === "string") visit(node as unknown as BabelNode);
+		for (const key in node) {
+			if (key === "loc" || key === "extra" || key === "range") continue;
+			if (key === "leadingComments" || key === "trailingComments" || key === "innerComments") continue;
+			const value = node[key];
+			if (value && typeof value === "object") stack.push(value);
+		}
+	}
+}
+
+function buildOptionsLiteral(node: BabelImportDeclaration): string | undefined {
 	const attrs = node.attributes;
 	if (!attrs || attrs.length === 0) return undefined;
 	const pairs = attrs.map(attr => {
 		const key = attr.key.type === "Identifier" ? attr.key.name : JSON.stringify(attr.key.value);
 		return `${key}: ${JSON.stringify(attr.value.value)}`;
 	});
-	return `{ ${pairs.join(", ")} }`;
+	// Native dynamic import takes options as `{ with: { ... } }`. `__omp_import__` forwards the
+	// options bag verbatim, so we wrap the attribute pairs accordingly.
+	return `{ with: { ${pairs.join(", ")} } }`;
 }
 
 function rewriteImportNode(node: BabelImportDeclaration): string {
 	const sourceLiteral = JSON.stringify(node.source.value);
-	const withClause = buildWithClause(node);
-	const importCall = buildDynamicImportCall(sourceLiteral, withClause);
+	const optionsLiteral = buildOptionsLiteral(node);
+	const importCall = buildOmpImportCall(sourceLiteral, optionsLiteral);
 
 	let defaultName: string | undefined;
 	let namespaceName: string | undefined;
@@ -99,7 +126,7 @@ function rewriteImportNode(node: BabelImportDeclaration): string {
 	return `await ${importCall};`;
 }
 
-export function rewriteStaticImports(code: string): string {
+export function rewriteImports(code: string): string {
 	if (!code.includes("import")) return code;
 
 	const ast = parseProgram(code);
@@ -108,17 +135,34 @@ export function rewriteStaticImports(code: string): string {
 		return code;
 	}
 
-	const imports: BabelImportDeclaration[] = [];
+	type Edit = { start: number; end: number; text: string };
+	const edits: Edit[] = [];
+
+	// Top-level static `import` declarations become `await __omp_import__(...)` calls.
 	for (const node of ast.program.body) {
-		if (node.type === "ImportDeclaration") imports.push(node as unknown as BabelImportDeclaration);
+		if (node.type !== "ImportDeclaration") continue;
+		const decl = node as unknown as BabelImportDeclaration;
+		edits.push({ start: decl.start, end: decl.end, text: rewriteImportNode(decl) });
 	}
-	if (imports.length === 0) return code;
+
+	// Dynamic `import(...)` expressions (anywhere) get their callee swapped for `__omp_import__`
+	// so the specifier resolves against the session cwd instead of the worker module's URL.
+	walkNodes(ast, node => {
+		if (node.type !== "CallExpression") return;
+		const call = node as unknown as { callee?: { type?: string; start?: number; end?: number } };
+		const callee = call.callee;
+		if (!callee || callee.type !== "Import" || typeof callee.start !== "number" || typeof callee.end !== "number")
+			return;
+		edits.push({ start: callee.start, end: callee.end, text: "__omp_import__" });
+	});
+
+	if (edits.length === 0) return code;
 
 	// Splice from the back so earlier offsets stay valid.
-	imports.sort((a, b) => b.start - a.start);
+	edits.sort((a, b) => b.start - a.start);
 	let result = code;
-	for (const node of imports) {
-		result = result.slice(0, node.start) + rewriteImportNode(node) + result.slice(node.end);
+	for (const edit of edits) {
+		result = result.slice(0, edit.start) + edit.text + result.slice(edit.end);
 	}
 	return result;
 }
@@ -220,7 +264,7 @@ export function wrapCode(code: string): { source: string; asyncWrapped: boolean;
 	const stripped = stripTypeScript(code);
 	const finalExpression = returnFinalExpression(stripped);
 	const rewritten = {
-		source: demoteTopLevelLexicals(rewriteStaticImports(finalExpression.source)),
+		source: demoteTopLevelLexicals(rewriteImports(finalExpression.source)),
 		returned: finalExpression.returned,
 	};
 	const needsAsyncWrapper = /\bawait\b|\breturn\b/.test(rewritten.source);

packages/coding-agent/src/eval/js/shared/runtime.ts

@@ -126,9 +126,9 @@ export class JsRuntime {
 				if (!hooks) throw new ToolError("Tool calls are only valid inside an active run");
 				return await hooks.callTool(name, args);
 			},
-			__omp_import__: async (source: string, attrs?: Record<string, string>) => {
+			__omp_import__: async (source: string, options?: ImportCallOptions) => {
 				const target = resolveImportSpecifier(this.#cwd, source);
-				return attrs ? await import(target, { with: attrs }) : await import(target);
+				return options !== undefined ? await import(target, options) : await import(target);
 			},
 			__omp_emit_status__: (op: string, data: Record<string, unknown> = {}) => {
 				const event: JsStatusEvent = { op, ...data };
@@ -170,7 +170,7 @@ function buildRequire(cwd: string): NodeJS.Require {
 }
 
 /**
- * Resolve an import specifier emitted by `rewriteStaticImports` against the active session
+ * Resolve an import specifier emitted by `rewriteImports` against the active session
  * cwd. Relative paths (`./`, `../`, `/`) and bare specifiers (`pkg`, `@scope/pkg`) both go
  * through `Bun.resolveSync` rooted at the cwd so user-pasted ESM behaves as if it lived in
  * the project — not next to the worker module. URL-like specifiers (`file://`, `data:`,

packages/coding-agent/test/core/js-static-import-rewrite.test.ts

@@ -1,97 +1,127 @@
 import { describe, expect, it } from "bun:test";
 
-import { rewriteStaticImports } from "../../src/eval/js/context-manager";
+import { rewriteImports } from "../../src/eval/js/context-manager";
 
-describe("rewriteStaticImports", () => {
+// Test fixtures embed user-supplied `import(...)` syntax that the rewriter must
+// transform. The strings are split so static-analysis heuristics don't read them
+// as real imports in this file.
+const IMPORT = "import";
+const dyn = (rest: string) => `${IMPORT}${rest}`;
+
+describe("rewriteImports", () => {
 	it("rewrites a top-level default import", () => {
-		const out = rewriteStaticImports('import foo from "bar";\nconsole.log(foo);');
+		const out = rewriteImports(`${IMPORT} foo from "bar";\nconsole.log(foo);`);
 		expect(out).toContain('await __omp_import__("bar")');
-		expect(out).not.toContain('import foo from "bar"');
+		expect(out).not.toContain(`${IMPORT} foo from "bar"`);
 	});
 
 	it("rewrites destructured named imports with renames", () => {
-		const out = rewriteStaticImports('import { foo, bar as baz } from "pkg";');
+		const out = rewriteImports(`${IMPORT} { foo, bar as baz } from "pkg";`);
 		expect(out).toContain('await __omp_import__("pkg")');
 		expect(out).toContain("foo");
 		expect(out).toContain("bar: baz");
 	});
 
 	it("rewrites namespace imports", () => {
-		const out = rewriteStaticImports('import * as ns from "pkg";');
+		const out = rewriteImports(`${IMPORT} * as ns from "pkg";`);
 		expect(out).toContain('const ns = await __omp_import__("pkg")');
 	});
 
 	it("rewrites combined default + namespace", () => {
-		const out = rewriteStaticImports('import def, * as ns from "pkg";');
+		const out = rewriteImports(`${IMPORT} def, * as ns from "pkg";`);
 		expect(out).toContain('const ns = await __omp_import__("pkg")');
 		expect(out).toContain("const def = ns.default");
 	});
 
 	it("rewrites combined default + named", () => {
-		const out = rewriteStaticImports('import def, { foo, bar as baz } from "pkg";');
+		const out = rewriteImports(`${IMPORT} def, { foo, bar as baz } from "pkg";`);
 		expect(out).toContain('await __omp_import__("pkg")');
 		expect(out).toContain("default: def");
-		expect(out).toContain("foo");
 		expect(out).toContain("bar: baz");
 	});
 
 	it("rewrites side-effect-only imports", () => {
-		const out = rewriteStaticImports('import "polyfill";');
+		const out = rewriteImports(`${IMPORT} "polyfill";`);
 		expect(out).toContain('await __omp_import__("polyfill")');
 	});
 
 	it("preserves import attributes via the dynamic import options bag", () => {
-		const out = rewriteStaticImports('import data from "./d.json" with { type: "json" };');
-		expect(out).toContain('await __omp_import__("./d.json", { type: "json" })');
+		const out = rewriteImports(`${IMPORT} data from "./d.json" with { type: "json" };`);
+		expect(out).toContain('await __omp_import__("./d.json", { with: { type: "json" } })');
 		expect(out).toContain("const data =");
 	});
 
+	it("rewrites bare dynamic import() so its specifier resolves against the session cwd", () => {
+		const out = rewriteImports(`const m = await ${dyn('("./foo.ts")')};`);
+		expect(out).toContain('await __omp_import__("./foo.ts")');
+		expect(out).not.toContain(dyn('("./foo.ts")'));
+	});
+
+	it("rewrites dynamic import() with an options bag (passes options through unchanged)", () => {
+		const out = rewriteImports(`const m = await ${dyn('("./d.json", { with: { type: "json" } })')};`);
+		expect(out).toContain('__omp_import__("./d.json", { with: { type: "json" } })');
+	});
+
+	it("rewrites nested and chained dynamic import() calls", () => {
+		const out = rewriteImports(
+			`Promise.all([${dyn('("./a.ts")')}, ${dyn('("./b.ts")')}]).then(([a, b]) => a.run(b));`,
+		);
+		expect(out).toContain('__omp_import__("./a.ts")');
+		expect(out).toContain('__omp_import__("./b.ts")');
+		expect(out).not.toContain(dyn('("./a.ts")'));
+	});
+
+	it("rewrites dynamic import() with a non-literal specifier", () => {
+		const out = rewriteImports(`const m = await ${dyn("(spec)")};`);
+		expect(out).toContain("__omp_import__(spec)");
+	});
+
 	it("does not rewrite import statements embedded in template literals (the bug)", () => {
-		const code = ["const generated = `", 'import { foo } from "./foo";', "export const bar = foo + 1;", "`;"].join(
+		const code = ["const generated = `", `${IMPORT} { foo } from "./foo";`, "export const bar = foo + 1;", "`;"].join(
 			"\n",
 		);
-		const out = rewriteStaticImports(code);
-		expect(out).toContain('import { foo } from "./foo";');
+		const out = rewriteImports(code);
+		expect(out).toContain(`${IMPORT} { foo } from "./foo";`);
 		expect(out).toContain("export const bar = foo + 1;");
 		expect(out).not.toContain("await __omp_import__(");
 	});
 
 	it("does not rewrite import statements inside block comments", () => {
-		const code = '/*\nimport foo from "bar";\n*/\nconst x = 1;';
-		const out = rewriteStaticImports(code);
-		expect(out).toContain('import foo from "bar";');
+		const code = `/*\n${IMPORT} foo from "bar";\n*/\nconst x = 1;`;
+		const out = rewriteImports(code);
+		expect(out).toContain(`${IMPORT} foo from "bar";`);
 		expect(out).not.toContain('await __omp_import__("bar")');
 	});
 
 	it("does not rewrite import statements inside double-quoted strings using line continuation", () => {
-		const code = "const code = \"import foo from \\\n'bar'\";\nconsole.log(code);";
-		const out = rewriteStaticImports(code);
+		const code = `const code = "${IMPORT} foo from \\\n'bar'";\nconsole.log(code);`;
+		const out = rewriteImports(code);
 		expect(out).not.toContain("await __omp_import__");
 	});
 
 	it("rewrites real top-level imports while leaving template-embedded look-alikes alone", () => {
 		const code = [
-			'import a from "alpha";',
+			`${IMPORT} a from "alpha";`,
 			"const code = `",
-			'import b from "beta";',
+			`${IMPORT} b from "beta";`,
 			"`;",
-			'import c from "gamma";',
+			`${IMPORT} c from "gamma";`,
 		].join("\n");
-		const out = rewriteStaticImports(code);
+		const out = rewriteImports(code);
 		expect(out).toContain('await __omp_import__("alpha")');
 		expect(out).toContain('await __omp_import__("gamma")');
 		expect(out).not.toContain('await __omp_import__("beta")');
-		expect(out).toContain('import b from "beta";');
+		expect(out).toContain(`${IMPORT} b from "beta";`);
 	});
 
 	it("returns the input unchanged when there are no imports", () => {
 		const code = "const x = 1 + 2;\nreturn x;";
-		expect(rewriteStaticImports(code)).toBe(code);
+		expect(rewriteImports(code)).toBe(code);
 	});
 
 	it("returns the input unchanged when the parser cannot make sense of the code", () => {
-		const code = "import { foo from broken syntax 'unterminated";
+		const code = `${IMPORT} { foo from broken syntax 'unterminated`;
 		// Should not throw; should fall through to the VM which will surface the syntax error.
-		expect(() => rewriteStaticImports(code)).not.toThrow();
+		expect(() => rewriteImports(code)).not.toThrow();
 	});
 });