fix(code-execution): codeExecution.enabled takes only true or false

changelog.d/473-code-execution-enabled.fixed.md

@@ -0,0 +1 @@
+- `codeExecution.enabled` takes `true` or `false`, or can be left out, and `null` counts as left out (#473). `create()` refuses any other value, and a `codeExecution` that isn't an object, naming the value's kind, not the value. The switch was read by truthiness, so `"enabled": "false"`, as a JSON config or a `${VAR}` substitution can write it, gave agents the `code_execution` tool and a python interpreter on the host, and a bare `"codeExecution": true` was read as off. Only `true` turns it on now. As before, a configuration that isn't on has its other settings left unchecked. A host that passes `codeExecution` on from its own configuration without checking it now gets this refusal from `create()` where such a value used to start.

src/code-execution/tool-definition.ts

@@ -9,6 +9,7 @@
  */
 
 import type { ToolDefinition } from '../types/index.js';
+import { kindOf } from '../mcpl/transport.js';
 import { formatLimit, MAX_TIMER_MS } from './py-runner.js';
 
 export const CODE_EXECUTION_TOOL_NAME = 'code_execution';
@@ -106,15 +107,41 @@ export function scriptTimeLimits(opts?: {
   const defaultMs = Math.min(MAX_TIMER_MS, opts?.scriptTimeoutMs ?? 600_000);
   return {
     defaultMs,
-    // A ceiling below the default is refused when the framework is created
-    // (validateCodeExecutionConfig); never below the default here either.
+    // A ceiling below the default is refused when a framework with code
+    // execution on is created (validateCodeExecutionConfig); never below the
+    // default here either.
     maxMs: Math.min(MAX_TIMER_MS, Math.max(defaultMs, opts?.maxScriptTimeoutMs ?? defaultMs)),
     backgroundMaxMs: Math.min(MAX_TIMER_MS, opts?.backgroundMaxLifetimeMs ?? 86_400_000),
   };
 }
 
-/** Refuse a configuration whose time limits contradict each other. */
-export function validateCodeExecutionConfig(cfg: { scriptTimeoutMs?: number; maxScriptTimeoutMs?: number }): void {
+/**
+ * Refuse a codeExecution that isn't an object, or whose switch is set to
+ * anything but true or false, and, once it's on, one whose time limits
+ * contradict each other. A JSON config, or a `${VAR}` substituted as a string,
+ * can carry a quoted "false" where the type says boolean, and only `true` turns
+ * code execution on, so the quotes would hide what the value does; a bare
+ * `codeExecution: true` would be read as off. A null switch counts as not set,
+ * as a left-out one does. The refusal names the value's kind, not the value.
+ * A configuration that isn't on has nothing else read.
+ */
+export function validateCodeExecutionConfig(cfg: {
+  enabled?: unknown;
+  scriptTimeoutMs?: number;
+  maxScriptTimeoutMs?: number;
+}): void {
+  if (cfg === null || typeof cfg !== 'object' || Array.isArray(cfg)) {
+    throw new Error(`codeExecution must be an object, such as { enabled: true }, not ${cfg === null ? 'null' : kindOf(cfg)}`);
+  }
+  if (cfg.enabled !== undefined && cfg.enabled !== null && typeof cfg.enabled !== 'boolean') {
+    const wanted = typeof cfg.enabled === 'string' ? 'true or false, unquoted,' : 'true or false,';
+    throw new Error(
+      `codeExecution.enabled must be ${wanted} not ${kindOf(cfg.enabled)}: ` +
+        'true gives agents the code_execution tool, which runs their scripts in a python interpreter on the host, ' +
+        'and false or leaving it out doesn\'t',
+    );
+  }
+  if (cfg.enabled !== true) return;
   const defaultMs = cfg.scriptTimeoutMs ?? 600_000;
   if (cfg.maxScriptTimeoutMs !== undefined && !(cfg.maxScriptTimeoutMs >= defaultMs)) {
     throw new Error(

src/framework.ts

@@ -2300,7 +2300,7 @@ export class AgentFramework {
    */
   static async create(config: FrameworkConfig): Promise<AgentFramework> {
     // Before anything opens or starts: a refused config must leave nothing behind.
-    if (config.codeExecution?.enabled) validateCodeExecutionConfig(config.codeExecution);
+    if (config.codeExecution != null) validateCodeExecutionConfig(config.codeExecution);
 
     // Create or use existing store
     let store: JsStore;
@@ -2636,7 +2636,7 @@ export class AgentFramework {
 
     // Client-side programmatic tool calling (code_execution). Config is only
     // retained when enabled — everything downstream gates on the field.
-    framework.codeExecutionConfig = config.codeExecution?.enabled ? config.codeExecution : null;
+    framework.codeExecutionConfig = config.codeExecution?.enabled === true ? config.codeExecution : null;
 
     framework.toolClassOverrides = normalizeClassTable(config.toolClassOverrides, 'toolClassOverrides');
     framework.hostToolClasses = [

src/mcpl/transport.ts

@@ -520,7 +520,7 @@ export function assertServerConfigBooleans(config: McplServerConfig): void {
 
 /** The kind of a value that isn't undefined, null or a boolean, as a refusal
  *  names it in place of the value. */
-function kindOf(value: unknown): string {
+export function kindOf(value: unknown): string {
   if (Array.isArray(value)) return 'an array';
   if (typeof value !== 'object') return `a ${typeof value}`;
   const prototype = Object.getPrototypeOf(value) as { constructor?: { name?: string } } | null;

src/types/framework.ts

@@ -41,7 +41,9 @@ export interface ProcessLoggingConfig {
  * the agent already has broader host access through its tools).
  */
 export interface CodeExecutionConfig {
-  /** Master switch — the tool is only synthesized when true. */
+  /** Master switch — only `true` synthesizes the tool. `create()` refuses a
+   *  value that isn't a boolean, a quoted "false" included; `null` or leaving
+   *  it out counts as off. */
   enabled: boolean;
   /** Python interpreter to spawn (default: 'python3' from PATH). */
   pythonPath?: string;
@@ -210,7 +212,8 @@ export interface FrameworkConfig {
 
   /**
    * Client-side programmatic tool calling (`code_execution` tool). Omitted or
-   * `enabled: false` → tool absent, no python is ever spawned.
+   * `enabled: false` → tool absent, no python is ever spawned. A value that
+   * isn't an object is refused at `create()`.
    */
   codeExecution?: CodeExecutionConfig;
 

test/code-execution.test.ts

@@ -1302,6 +1302,77 @@ describe('code_execution per-call time limit (time_limit_ms)', () => {
     }
   });
 
+  const createWith = (storePath: string, codeExecution: unknown) =>
+    AgentFramework.create({
+      storePath,
+      membrane: new MockMembrane().asMembrane(),
+      agents: [],
+      modules: [],
+      syncIntervalMs: 0,
+      codeExecution: codeExecution as never,
+    });
+
+  /** create() refuses each config with exactly its message, before anything
+   *  opens: no store is left at storePath. */
+  async function assertRefused(cases: ReadonlyArray<readonly [unknown, string]>): Promise<void> {
+    const { tempDir, storePath } = tempStorePath('pytc-refused-');
+    try {
+      for (const [codeExecution, message] of cases) {
+        await assert.rejects(createWith(storePath, codeExecution), (error: Error) => {
+          assert.equal(error.message, message);
+          return true;
+        });
+        assert.equal(existsSync(storePath), false, 'the refused config opened no store');
+      }
+    } finally {
+      rmSync(tempDir, { recursive: true, force: true });
+    }
+  }
+
+  it('create() refuses a codeExecution.enabled that is not true or false, a quoted "false" included', async () => {
+    // A quoted "false" is truthy, so it turned code execution on, and 0 is
+    // falsy, so it was read as off without a word. Each message names the
+    // value's kind, never the value, since a substituted value can be a
+    // credential.
+    const whatTrueDoes = 'true gives agents the code_execution tool, which runs their scripts in a python interpreter on the host, '
+      + 'and false or leaving it out doesn\'t';
+    await assertRefused([
+      [{ enabled: 'false' }, `codeExecution.enabled must be true or false, unquoted, not a string: ${whatTrueDoes}`],
+      [{ enabled: 'sk-secret-token' }, `codeExecution.enabled must be true or false, unquoted, not a string: ${whatTrueDoes}`],
+      [{ enabled: 0 }, `codeExecution.enabled must be true or false, not a number: ${whatTrueDoes}`],
+      [{ enabled: { on: true } }, `codeExecution.enabled must be true or false, not an object: ${whatTrueDoes}`],
+    ]);
+  });
+
+  it('create() refuses a codeExecution that is not an object, which would read as off', async () => {
+    await assertRefused([
+      [true, 'codeExecution must be an object, such as { enabled: true }, not a boolean'],
+      ['true', 'codeExecution must be an object, such as { enabled: true }, not a string'],
+      [[], 'codeExecution must be an object, such as { enabled: true }, not an array'],
+    ]);
+  });
+
+  it('a null or left-out codeExecution.enabled counts as off, and a configuration that is off has nothing else checked', async () => {
+    // Contradictory limits are refused only once code execution is on.
+    const contradictory = { scriptTimeoutMs: 600_000, maxScriptTimeoutMs: 60_000 };
+    for (const codeExecution of [
+      null,
+      { enabled: null, ...contradictory },
+      { enabled: false, ...contradictory },
+      { ...contradictory },
+    ]) {
+      const { tempDir, storePath } = tempStorePath('pytc-enabled-off-');
+      let framework: AgentFramework | undefined;
+      try {
+        framework = await createWith(storePath, codeExecution);
+        assert.equal(framework.getAllTools().some((tool) => tool.name === 'code_execution'), false);
+      } finally {
+        await framework?.stop();
+        rmSync(tempDir, { recursive: true, force: true });
+      }
+    }
+  });
+
   async function withFramework(
     codeExecution: Record<string, unknown>,
     body: (framework: AgentFramework) => Promise<void>,