fix(agents): name the export and the empty value the sandbox refused

packages/agents/src/capabilities/args.ts

@@ -126,6 +126,12 @@ function missingRequiredArgs(
 /**
  * Name the call, the keys it is missing, and — when it passed something — what
  * it passed instead, which is usually the whole diagnosis.
+ *
+ * A key that is present but null/undefined has to be called out, or the report
+ * contradicts itself: `edit_storyboard({storyboard_id, ops})` with an unset
+ * variable read "missing required argument storyboard_id. Got: storyboard_id,
+ * ops", which sends the caller hunting for a spelling that was already right
+ * instead of at the value that never got computed.
  */
 function missingArgsMessage(
   spec: CapabilityArgSpec,
@@ -136,8 +142,16 @@ function missingArgsMessage(
   const got = Object.keys(args);
   const gotPart =
     got.length > 0 ? ` Got: ${got.join(", ")}.` : " Got no arguments.";
+  const empty = missing.filter((key) => key in args);
+  const emptyPart =
+    empty.length === 0
+      ? ""
+      : ` ${empty.join(", ")} ${empty.length === 1 ? "was" : "were"} ` +
+        `passed as null/undefined — the key is right, the value is missing.`;
   return (
-    `${spec.name}: missing required ${plural} ${missing.join(", ")}.` + gotPart
+    `${spec.name}: missing required ${plural} ${missing.join(", ")}.` +
+    gotPart +
+    emptyPart
   );
 }
 

packages/agents/src/codeact/capability-modules.ts

@@ -20,7 +20,12 @@ import {
   sandboxCapabilitySpecifier,
   SANDBOX_CAPABILITY_PACK
 } from "@nodetool-ai/protocol";
-import { parseCodeBody, staticImportSpecifiers } from "@nodetool-ai/node-sdk";
+import {
+  parseCodeBody,
+  staticImportBindings,
+  staticImportSpecifiers
+} from "@nodetool-ai/node-sdk";
+import type { CodeBodyStatement } from "@nodetool-ai/node-sdk";
 
 import {
   capabilityModuleSpecs,
@@ -143,6 +148,9 @@ export async function mountCapabilityModules(
     sessionNames.set(name, new Set(added));
   }
 
+  const unknown = unknownImportedExports(parsed.statements, exportsByModule);
+  if (unknown !== undefined) return { ok: false, error: unknown };
+
   const facades = new Map(
     unique.map((name) => [
       sandboxCapabilitySpecifier(name),
@@ -182,3 +190,51 @@ export async function mountCapabilityModules(
   };
   return { ok: true, mount: { facades, call } };
 }
+
+/**
+ * A capability module exports its wire names verbatim — `generate_image`, not
+ * `generateImage` — so the spelling a model reaches for by habit resolves to
+ * nothing. Left to the guest that surfaces as QuickJS's "Could not find export
+ * 'generateImage'", which names neither the module's exports nor the one it
+ * meant; the model then guesses again. Refuse the action here instead, with
+ * the export list and the near match.
+ *
+ * Returns the refusal, or `undefined` when every imported name exists.
+ */
+function unknownImportedExports(
+  statements: readonly CodeBodyStatement[],
+  exportsByModule: ReadonlyMap<string, readonly string[]>
+): string | undefined {
+  for (const binding of staticImportBindings(statements)) {
+    const module = sandboxCapabilityModuleName(binding.specifier);
+    const available =
+      module === undefined ? undefined : exportsByModule.get(module);
+    if (available === undefined) continue;
+    const missing = binding.named.filter((name) => !available.includes(name));
+    if (missing.length === 0) continue;
+    const names = missing.map((name) => `"${name}"`).join(", ");
+    const suggestions = missing
+      .map((name) => nearMatchSentence(name, available))
+      .filter((sentence) => sentence.length > 0)
+      .join(" ");
+    return (
+      `The action imports ${names} from "${binding.specifier}", which that ` +
+      `module does not export. ${suggestions}Capability modules export the ` +
+      `wire name verbatim, in snake_case. "${binding.specifier}" exports: ` +
+      `${available.join(", ")}.`
+    );
+  }
+  return undefined;
+}
+
+/** "Did you mean …" for the one name that differs only in casing or `_`. */
+function nearMatchSentence(
+  wanted: string,
+  available: readonly string[]
+): string {
+  const flatten = (name: string): string =>
+    name.replace(/_/g, "").toLowerCase();
+  const target = flatten(wanted);
+  const match = available.find((name) => flatten(name) === target);
+  return match === undefined ? "" : `Did you mean "${match}" for "${wanted}"? `;
+}

packages/agents/src/js-sandbox-worker/interpreter.ts

@@ -616,7 +616,8 @@ export function createGuestModuleHost(
   const resolve = (baseName: string, requested: string): string => {
     if (locked) {
       return deny(
-        `dynamic import() is not allowed in the sandbox (requested "${requested}")`
+        `dynamic import() is not allowed in the sandbox (requested "${requested}")` +
+          ` — declare it as a static \`import\` at the top of the code instead`
       );
     }
     if (requested === SANDBOX_HOST_BRIDGE_SPECIFIER) {

packages/agents/tests/capabilities-args.test.ts

@@ -153,6 +153,20 @@ describe("coerceCapabilityArgs argument checking", () => {
     ).toThrow(/save_asset: missing required argument name/);
   });
 
+  it("says a required key was passed empty rather than reading as self-contradictory", () => {
+    expect(() =>
+      coerceCapabilityArgs(SAVE_ASSET, [{ name: undefined, source: "s" }])
+    ).toThrow(
+      /Got: name, source\. name was passed as null\/undefined — the key is right, the value is missing\./
+    );
+  });
+
+  it("says nothing about empty values when the key is simply absent", () => {
+    expect(() =>
+      coerceCapabilityArgs(SAVE_ASSET, [{ source: "s" }])
+    ).toThrow(/save_asset: missing required argument name\. Got: source\.$/);
+  });
+
   it("leaves a call with no arguments to the implementation", () => {
     expect(coerceCapabilityArgs(SAVE_ASSET, [])).toEqual({});
   });

packages/agents/tests/capability-module-exports.test.ts

@@ -0,0 +1,144 @@
+/**
+ * A capability module exports its wire name verbatim — snake_case — so
+ * `import { generateImage }` resolves to nothing, and QuickJS reports that as
+ * "Could not find export", naming neither the module's exports nor the one the
+ * caller meant. These pin the refusal that replaces it.
+ *
+ * No real wire name is spelled here: the registry cases derive the names they
+ * assert on, so the coverage sync cannot read this file as exercising a
+ * capability it only mentions.
+ */
+
+import { describe, expect, it } from "vitest";
+import type { ProcessingContext } from "@nodetool-ai/runtime";
+
+import { capabilityModuleSpecs } from "../src/capabilities/dispatcher.js";
+import { UNGATED, createCapabilityRun } from "../src/capabilities/index.js";
+import {
+  mountCapabilityModules,
+  SESSION_CAPABILITY_MODULE
+} from "../src/codeact/capability-modules.js";
+
+/** `do_a_thing` → `doAThing`, the spelling a model reaches for by habit. */
+function camelize(wireName: string): string {
+  return wireName.replace(/_([a-z])/g, (_, ch: string) => ch.toUpperCase());
+}
+
+/**
+ * A session graft is the one path that serves modules without a
+ * `CapabilityRun`, so it exercises the check with no host to build — and its
+ * export names are this test's own, not the platform's.
+ */
+const session = [
+  {
+    module: SESSION_CAPABILITY_MODULE,
+    exports: ["do_a_thing", "find_a_thing"],
+    call: async () => ({})
+  }
+];
+
+const SESSION_SPECIFIER = `@nodetool-ai/sandbox-nodetool/${SESSION_CAPABILITY_MODULE}`;
+
+const mount = (code: string) =>
+  mountCapabilityModules(code, undefined, { session });
+
+describe("mountCapabilityModules — imported export names", () => {
+  it("serves an import that names a real export", async () => {
+    const result = await mount(
+      `import { do_a_thing } from "${SESSION_SPECIFIER}";`
+    );
+    expect(result.ok).toBe(true);
+  });
+
+  it("refuses a camelCase spelling and names the wire form", async () => {
+    const result = await mount(
+      `import { doAThing } from "${SESSION_SPECIFIER}";`
+    );
+    expect(result.ok).toBe(false);
+    if (result.ok) return;
+    expect(result.error).toContain("does not export");
+    expect(result.error).toContain('Did you mean "do_a_thing" for "doAThing"?');
+    expect(result.error).toContain("do_a_thing, find_a_thing");
+  });
+
+  it("lists the module's exports when nothing is close", async () => {
+    const result = await mount(`import { pick } from "${SESSION_SPECIFIER}";`);
+    expect(result.ok).toBe(false);
+    if (result.ok) return;
+    expect(result.error).not.toContain("Did you mean");
+    expect(result.error).toContain("exports: do_a_thing, find_a_thing.");
+  });
+
+  it("reports every unknown name from one import at once", async () => {
+    const result = await mount(
+      `import { doAThing, pick } from "${SESSION_SPECIFIER}";`
+    );
+    expect(result.ok).toBe(false);
+    if (result.ok) return;
+    expect(result.error).toContain('"doAThing", "pick"');
+  });
+
+  it("leaves a default import alone — it names no export", async () => {
+    const result = await mount(`import all from "${SESSION_SPECIFIER}";`);
+    expect(result.ok).toBe(true);
+  });
+
+  it("still refuses an unknown module before checking names", async () => {
+    const result = await mount(
+      `import { anything } from "@nodetool-ai/sandbox-nodetool/not-a-module";`
+    );
+    expect(result.ok).toBe(false);
+    if (result.ok) return;
+    expect(result.error).toContain("is not a NodeTool capability module");
+  });
+});
+
+describe("mountCapabilityModules — the registry's own modules", () => {
+  const run = createCapabilityRun({
+    context: { userId: "user-exports" } as unknown as ProcessingContext,
+    gate: UNGATED
+  });
+
+  /** A module's first multi-word export, and the camelCase miss for it. */
+  async function wireName(module: string): Promise<string> {
+    const [spec] = await capabilityModuleSpecs([module]);
+    const name = spec?.exports.find((exported) => exported.includes("_"));
+    if (name === undefined) {
+      throw new Error(`${module} declares no snake_case export to test with`);
+    }
+    return name;
+  }
+
+  it("serves the wire name the media module declares", async () => {
+    const name = await wireName("media");
+    const result = await mountCapabilityModules(
+      `import { ${name} } from "@nodetool-ai/sandbox-nodetool/media";`,
+      run
+    );
+    expect(result.ok).toBe(true);
+  });
+
+  it("refuses the camelCase miss and points back at the wire name", async () => {
+    const name = await wireName("media");
+    const result = await mountCapabilityModules(
+      `import { ${camelize(name)} } from "@nodetool-ai/sandbox-nodetool/media";`,
+      run
+    );
+    expect(result.ok).toBe(false);
+    if (result.ok) return;
+    expect(result.error).toContain(
+      `Did you mean "${name}" for "${camelize(name)}"?`
+    );
+  });
+
+  it("refuses a name no module declares and lists what models exports", async () => {
+    const name = await wireName("models");
+    const result = await mountCapabilityModules(
+      'import { pick } from "@nodetool-ai/sandbox-nodetool/models";',
+      run
+    );
+    expect(result.ok).toBe(false);
+    if (result.ok) return;
+    expect(result.error).toContain(name);
+  });
+});

packages/node-sdk/src/code-analysis.ts

@@ -118,6 +118,49 @@ export function staticImportSpecifiers(
   return specifiers;
 }
 
+/** One static import declaration's specifier and the names it pulls off it. */
+export interface StaticImportBinding {
+  readonly specifier: string;
+  /**
+   * Names in `import { a, b as c }` — the exported names, not the local
+   * aliases, so they can be checked against what the module declares.
+   * A default or namespace import contributes nothing: it names no export.
+   */
+  readonly named: readonly string[];
+}
+
+/**
+ * Specifier plus imported names for every top-level static `import`, in source
+ * order.
+ *
+ * {@link staticImportSpecifiers} answers which modules a body wants;
+ * this answers which of their exports it wants, so a loader can refuse a
+ * misspelled name with the module's own export list instead of leaving the
+ * guest to report "Could not find export".
+ */
+export function staticImportBindings(
+  statements: readonly CodeBodyStatement[]
+): StaticImportBinding[] {
+  const bindings: StaticImportBinding[] = [];
+  for (const statement of statements) {
+    if (statement.type !== "ImportDeclaration") continue;
+    const source = statement.source;
+    if (!isString(source.value)) continue;
+    const named: string[] = [];
+    for (const specifier of statement.specifiers) {
+      if (specifier.type !== "ImportSpecifier") continue;
+      const imported = specifier.imported;
+      if (imported.type === "Identifier") {
+        named.push(imported.name);
+      } else if (isString(imported.value)) {
+        named.push(imported.value);
+      }
+    }
+    bindings.push({ specifier: source.value, named });
+  }
+  return bindings;
+}
+
 /** Module access the loader refuses outright, anywhere in the body. */
 export interface DynamicModuleAccess {
   /** `import(...)` — the loader denies every dynamic resolution. */

packages/node-sdk/tests/static-import-bindings.test.ts

@@ -0,0 +1,51 @@
+import { describe, expect, it } from "vitest";
+
+import { parseCodeBody, staticImportBindings } from "../src/code-analysis.js";
+
+function bindings(code: string) {
+  const parsed = parseCodeBody(code);
+  if ("error" in parsed) throw new Error(parsed.error);
+  return staticImportBindings(parsed.statements);
+}
+
+describe("staticImportBindings", () => {
+  it("reports the exported names a body pulls off each module", () => {
+    expect(
+      bindings(
+        'import { generate_image, find_model } from "@nodetool-ai/sandbox-nodetool/media";'
+      )
+    ).toEqual([
+      {
+        specifier: "@nodetool-ai/sandbox-nodetool/media",
+        named: ["generate_image", "find_model"]
+      }
+    ]);
+  });
+
+  it("reports the exported name, not the local alias", () => {
+    expect(bindings('import { parse as p } from "@acme/x";')).toEqual([
+      { specifier: "@acme/x", named: ["parse"] }
+    ]);
+  });
+
+  it("reports no names for a default or namespace import", () => {
+    expect(
+      bindings('import media from "@a/b";\nimport * as all from "@c/d";')
+    ).toEqual([
+      { specifier: "@a/b", named: [] },
+      { specifier: "@c/d", named: [] }
+    ]);
+  });
+
+  it("reports a string-literal import name", () => {
+    expect(bindings('import { "a-b" as ab } from "@a/b";')).toEqual([
+      { specifier: "@a/b", named: ["a-b"] }
+    ]);
+  });
+
+  it("ignores a side-effect import's absent name list and dynamic import", () => {
+    expect(bindings('import "@a/b";\nconst x = () => import("@c/d");')).toEqual(
+      [{ specifier: "@a/b", named: [] }]
+    );
+  });
+});