fix: accept "tools": "" in scenario files like --tools ""

README.md

@@ -126,6 +126,13 @@ runs need headroom. Set `maxBudgetUsd` to `3` or more for artifact scenarios.
 A run that hits the cap fails with an explicit
 `claude hit the $N max budget` error rather than a silent bad sample.
 
+**Tools:** `--tools <list|default|''>` (or `"tools"` in a scenario file, with
+a per-case override) sets the tools each run gets: a comma-separated list,
+`default`, or `""` for none. `"tools": ""` in a scenario file means the same
+as `--tools ""`. When unset, text mode gets no tools and artifact mode gets
+`default`. A per-case `"tools"` wins over the top-level value and over
+`--tools`.
+
 **Turn cap:** `--max-turns <n>` (or `"maxTurns"` in a scenario file, with a
 per-case override) caps agentic turns per run on the claude-p runner. Unlike
 a budget abort, a turn-capped run is a *measured outcome*: it is recorded and

SPEC.md

@@ -69,6 +69,11 @@ used as a substitute for sandboxing.
 The runner captures final output text, `total_cost_usd`, turn count, duration,
 and model usage keys.
 
+The `--tools` value comes from the per-case `tools` field, then `--tools` or
+the top-level `tools` field, then the mode default (`""` for text mode,
+`default` for artifact mode or install delivery). An explicit `""` at any
+level means no tools and is never treated as unset.
+
 A `maxTurns` cap (scenario field, per-case override, or `--max-turns`) is
 passed through as `--max-turns`. A run that stops at the cap (result subtype
 `error_max_turns`, on any exit code) is returned as a normal result flagged

src/engine/config.ts

@@ -210,7 +210,7 @@ export function loadCompareConfig(
     maxBudgetUsd: overrides.maxBudgetUsd ?? numberValue(raw.maxBudgetUsd, 1),
     maxTurns: overrides.maxTurns ?? optionalNumber(raw.maxTurns, "maxTurns"),
     mode: overrides.mode ?? modeValue(raw.mode, undefined),
-    tools: overrides.tools ?? stringValue(raw.tools, undefined),
+    tools: overrides.tools ?? toolsValue(raw.tools, "tools"),
     addDirs: (overrides.addDirs ?? stringArray(raw.addDirs, "addDirs")).map((dir) => resolveFrom(baseDir, dir)),
     sandboxRoot,
     sandboxSeed,
@@ -238,7 +238,7 @@ function normalizeCase(baseDir: string, raw: RawCase, index: number): EvalCaseCo
     seed: optionalPath(baseDir, stringValue(raw.seed, undefined)),
     addDirs: stringArray(raw.addDirs, `${name}.addDirs`).map((dir) => resolveFrom(baseDir, dir)),
     mode: modeValue(raw.mode, undefined),
-    tools: stringValue(raw.tools, undefined),
+    tools: toolsValue(raw.tools, `${name}.tools`),
     maxTurns: optionalNumber(raw.maxTurns, `${name}.maxTurns`),
   };
 }
@@ -462,6 +462,15 @@ function modeValue(value: unknown, fallback: RunMode | undefined): RunMode | und
   throw new Error(`mode must be "text" or "artifact"`);
 }
 
+/** Accepts "" like the --tools "" flag: an explicit empty list disables tools. */
+function toolsValue(value: unknown, label: string): string | undefined {
+  if (value === undefined) return undefined;
+  if (typeof value !== "string") {
+    throw new Error(`${label} must be a string: a comma-separated tool list, "default", or "" for no tools`);
+  }
+  return value;
+}
+
 function stringValue(value: unknown, fallback: string): string;
 function stringValue(value: unknown, fallback: string | undefined): string | undefined;
 function stringValue(value: unknown, fallback: string | undefined): string | undefined {

test/config.test.ts

@@ -2,7 +2,9 @@ import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { expect, test } from "bun:test";
-import { loadCompareConfig } from "../src/engine/config";
+import { runCompare } from "../src/engine/compare";
+import { loadCompareConfig, type CompareConfig } from "../src/engine/config";
+import type { Runner, RunnerRunOptions } from "../src/types";
 
 test("loadCompareConfig normalizes paths and rejects zero case runs", () => {
   const dir = mkdtempSync(join(tmpdir(), "promptdiff-config-test-"));
@@ -322,3 +324,95 @@ test("loadCompareConfig accepts an explicit empty baselineSkills but still requi
     rmSync(dir, { recursive: true, force: true });
   }
 });
+
+function toolsRecordingRunner(sandboxTools: boolean, toolsSeen: string[]): Runner {
+  return {
+    name: sandboxTools ? "claude-p" : "openai",
+    capabilities: { sandboxTools, skillRegistry: sandboxTools, images: false, streamEvents: false },
+    async run(options: RunnerRunOptions) {
+      toolsSeen.push(options.tools);
+      return { output: "ok", costUsd: 0, turns: 1, durationMs: 1, models: [options.model], raw: {} };
+    },
+  };
+}
+
+async function toolsSeenBy(config: CompareConfig, sandboxTools: boolean): Promise<string[]> {
+  const toolsSeen: string[] = [];
+  const runner = toolsRecordingRunner(sandboxTools, toolsSeen);
+  await runCompare({ config, runners: { baseline: runner, proposed: runner } });
+  return toolsSeen;
+}
+
+test('scenario "tools": "" means no tools, the same as --tools ""', async () => {
+  const dir = mkdtempSync(join(tmpdir(), "promptdiff-config-empty-tools-"));
+  try {
+    writeFileSync(join(dir, "agent.md"), "Agent", "utf8");
+    writeFileSync(join(dir, "baseline.md"), "Baseline", "utf8");
+    writeFileSync(join(dir, "proposed.md"), "Proposed", "utf8");
+    const base = {
+      agent: "./agent.md",
+      baselineSkills: ["./baseline.md"],
+      proposedSkills: ["./proposed.md"],
+      model: "gpt-4o-mini",
+      runner: "openai",
+      runs: 1,
+      sandbox: { root: "./runs" },
+    };
+    const textCase = { name: "t", prompt: "p", grader: { type: "text", contains: ["ok"] } };
+
+    // The shape from issue #37: top-level text mode with an explicit empty tools list.
+    writeFileSync(join(dir, "top-level.json"), JSON.stringify({ ...base, mode: "text", tools: "", scenarios: [textCase] }), "utf8");
+    writeFileSync(join(dir, "no-tools-key.json"), JSON.stringify({ ...base, mode: "text", scenarios: [textCase] }), "utf8");
+    const fromJson = loadCompareConfig(join(dir, "top-level.json"));
+    const fromFlag = loadCompareConfig(join(dir, "no-tools-key.json"), { tools: "" });
+    expect(fromJson.tools).toBe("");
+    expect(fromJson.tools).toBe(fromFlag.tools!);
+    // Runs on a text-only runner with no tools, exactly as the flag does.
+    expect(await toolsSeenBy(fromJson, false)).toEqual(["", ""]);
+    expect(await toolsSeenBy(fromFlag, false)).toEqual(["", ""]);
+
+    // "" must not read as unset: artifact mode would otherwise fall back to "default".
+    writeFileSync(
+      join(dir, "artifact.json"),
+      JSON.stringify({ ...base, runner: "claude-p", tools: "", scenarios: [{ ...textCase, mode: "artifact" }] }),
+      "utf8",
+    );
+    expect(await toolsSeenBy(loadCompareConfig(join(dir, "artifact.json")), true)).toEqual(["", ""]);
+
+    // A per-scenario "" overrides a top-level tools list, so the text-only runner accepts it.
+    writeFileSync(
+      join(dir, "per-case.json"),
+      JSON.stringify({ ...base, tools: "Bash,Read", scenarios: [{ ...textCase, tools: "" }] }),
+      "utf8",
+    );
+    const perCase = loadCompareConfig(join(dir, "per-case.json"));
+    expect(perCase.cases[0]?.tools).toBe("");
+    expect(await toolsSeenBy(perCase, false)).toEqual(["", ""]);
+  } finally {
+    rmSync(dir, { recursive: true, force: true });
+  }
+});
+
+test("a non-string tools value fails with a message naming the field", () => {
+  const dir = mkdtempSync(join(tmpdir(), "promptdiff-config-bad-tools-"));
+  try {
+    writeFileSync(join(dir, "agent.md"), "Agent", "utf8");
+    writeFileSync(join(dir, "baseline.md"), "Baseline", "utf8");
+    writeFileSync(join(dir, "proposed.md"), "Proposed", "utf8");
+    const base = {
+      agent: "./agent.md",
+      baselineSkills: ["./baseline.md"],
+      proposedSkills: ["./proposed.md"],
+      model: "sonnet",
+    };
+    const textCase = { name: "t", prompt: "p", grader: { type: "text", contains: ["ok"] } };
+
+    writeFileSync(join(dir, "top-level.json"), JSON.stringify({ ...base, tools: ["Bash"], scenarios: [textCase] }), "utf8");
+    expect(() => loadCompareConfig(join(dir, "top-level.json"))).toThrow(/^tools must be a string/);
+
+    writeFileSync(join(dir, "per-case.json"), JSON.stringify({ ...base, scenarios: [{ ...textCase, tools: false }] }), "utf8");
+    expect(() => loadCompareConfig(join(dir, "per-case.json"))).toThrow(/^t\.tools must be a string/);
+  } finally {
+    rmSync(dir, { recursive: true, force: true });
+  }
+});

test/delivery.test.ts

@@ -163,9 +163,19 @@ test("scenario files parse delivery and reject install with disabled tools", ()
     writeFileSync(defaultPath, JSON.stringify(base), "utf8");
     expect(loadCompareConfig(defaultPath).delivery).toBe("inline");
 
-    // The JSON parser already rejects a literal empty tools string; the CLI
-    // --tools "" override is the path that can smuggle it in.
+    // tools "" disables tools whether it comes from the scenario file or the
+    // --tools "" override, and install delivery rejects both.
     expect(() => loadCompareConfig(okPath, { tools: "" })).toThrow(/tools enabled/);
+    const noToolsPath = join(dir, "no-tools.json");
+    writeFileSync(noToolsPath, JSON.stringify({ ...base, delivery: "install", tools: "" }), "utf8");
+    expect(() => loadCompareConfig(noToolsPath)).toThrow(/tools enabled/);
+    const caseNoToolsPath = join(dir, "case-no-tools.json");
+    writeFileSync(
+      caseNoToolsPath,
+      JSON.stringify({ ...base, delivery: "install", scenarios: [{ ...base.scenarios[0], tools: "" }] }),
+      "utf8",
+    );
+    expect(() => loadCompareConfig(caseNoToolsPath)).toThrow(/tools enabled/);
 
     const badRegexPath = join(dir, "bad-regex.json");
     writeFileSync(