Proxy Codex exec_command and update_plan onto Corbits tools

Author: TheGreatAxios

docs/IMPLEMENTATION.md

@@ -160,7 +160,7 @@ Sixteen packages under `src/agent/directors/<id>/` register in `DIRECTOR_REGISTR
 4. `directorProfiles()` is the spawn catalog (`default-agents.ts`) — closed set minus skywalker; plugin agent profiles still load and can override by id.
 5. Primary chat role is Skywalker: `buildChatRole()` → `createSkywalkerSystemPrompt()`. Product mutation tools are stripped from the primary toolset and from CORE/CATALOG ads (`PRIMARY_DENIED_PRODUCT_TOOLS`) — never-implement is structural for path tools. Residual: `run_shell` stays on primary; MCP tools loaded later are not re-stripped by that deny list; optional `writePaths` (when a profile sets it) only gate path-keyed product tools.
 
-   **Codex `apply_patch` proxy.** When the active provider is Codex (`isCodexProviderName`), `createAgentToolset` and `runSubAgent` mount an `apply_patch` stringTool from `createCodexToolProxies` that parses the Codex envelope and forwards each op through the posix `ToolRunner` (`write_file` / `delete_file` / `read_file`) so permission plugins still apply. Primary still denies `apply_patch` via `PRIMARY_DENIED_PRODUCT_TOOLS`; implement and docs leaf allowlists (`IMPLEMENT_TOOLS` / `DOCS_TOOLS`) include it so Codex workers keep the proxy after the capability filter. `CORE_TOOL_NAMES` does not list it.
+   **Codex tool proxies.** When the active provider is Codex (`isCodexProviderName`), `createAgentToolset` and `runSubAgent` mount `apply_patch`, `shell`, and `update_plan` stringTools from `createCodexToolProxies`, all forwarding through the same posix `ToolRunner` seam (`runTool`) so permission plugins still apply. `apply_patch` parses the Codex envelope and forwards each op (`write_file` / `delete_file` / `read_file`). `shell` — the native Codex name is `shell`, not `exec_command`, per the pinned base-instructions text quoted in `codex-responses-adapter.ts`'s bridge message — normalizes Codex's `command` (string or `["bash","-lc",script]`-style argv array), `workdir`, and `timeout_ms` onto `run_shell`'s `{command, cwd?, timeout?}` and is gated by `allowShellFromCapabilities` (mirrors `allowDeleteFromCapabilities` against `run_shell`). `update_plan` maps Codex's `plan: [{step, status}]` onto `manage_tasks(action: "create")`; `pending`/`in_progress`/`completed` map to `todo`/`doing`/`done` — `manage_tasks`'s `cancelled` status has no Codex equivalent and is never produced by this proxy. Primary still denies `apply_patch` via `PRIMARY_DENIED_PRODUCT_TOOLS` (`shell` and `update_plan` are not product-mutation tools, same classification as `run_shell` and `manage_tasks`, so they are not stripped); implement and docs leaf allowlists (`IMPLEMENT_TOOLS` / `DOCS_TOOLS`) include `apply_patch` so Codex workers keep the proxy after the capability filter. `CORE_TOOL_NAMES` does not list it.
 6. Shipped directors omit `writePaths`. The optional field is still enforced in the permission gate via ALS identity (`identity-context.ts` + `write-path-policy.ts`) when a plugin/custom profile sets it.
 7. Spawn effort: pin > package `modelRole` default (`defaultEffortForDirector`; intern=low; plan/review/orchestrator=high; implement/explore/docs/test=medium) > orchestrator/worker binary > parent inheritance. Optional skills are listed in the identity header for awareness; workers do not mount `use_skill` (guidance is baked into package system prompts). Primary mounts `use_skill` for its own skill list.
 

src/agent/codex-tool-mount.test.ts

@@ -1,14 +1,24 @@
 /**
- * Mount coverage for Codex apply_patch proxies: primary deny, allowlists,
- * and implement-shaped capability filter retention.
+ * Mount coverage for Codex tool proxies (apply_patch, shell, update_plan):
+ * primary deny, allowlists, and implement-shaped capability filter retention.
+ *
+ * runSubAgent has no standalone toolset-factory export to import directly (the
+ * mount is inline in runSubAgent's tool-assembly), so the subagent mount path
+ * is covered here via the same allowDeleteFromCapabilities /
+ * allowShellFromCapabilities calls runSubAgent makes against a leaf
+ * capability filter, feeding createCodexToolProxies exactly as run.ts does.
  */
 import { mkdtempSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { afterEach, describe, expect, test, spyOn } from "bun:test";
 import * as posixModule from "@intx/tools-posix";
 
-import { createCodexToolProxies } from "./codex-tool-proxies.js";
+import {
+  allowDeleteFromCapabilities,
+  allowShellFromCapabilities,
+  createCodexToolProxies,
+} from "./codex-tool-proxies.js";
 import { IMPLEMENT_TOOLS, DOCS_TOOLS } from "./directors/tool-sets.js";
 import { CORE_TOOL_NAMES, PRIMARY_DENIED_PRODUCT_TOOLS } from "./tool-search.js";
 
@@ -39,6 +49,8 @@ describe("Codex apply_patch mount", () => {
     });
     const names = toolset.dynamicRunner.currentDefinitions().map((d) => d.name);
     expect(names).not.toContain("apply_patch");
+    expect(names).not.toContain("shell");
+    expect(names).not.toContain("update_plan");
     await toolset.dispose();
   });
 
@@ -65,6 +77,11 @@ describe("Codex apply_patch mount", () => {
     const names = toolset.dynamicRunner.currentDefinitions().map((d) => d.name);
     expect(names).not.toContain("apply_patch");
     expect(PRIMARY_DENIED_PRODUCT_TOOLS).toContain("apply_patch");
+    // shell / update_plan are not product-mutation tools (same classification
+    // as run_shell / manage_tasks), so PRIMARY_DENIED_PRODUCT_TOOLS does not
+    // strip them — they stay mounted on primary, mirroring run_shell.
+    expect(names).toContain("shell");
+    expect(names).toContain("update_plan");
     await toolset.dispose();
   });
 
@@ -79,14 +96,51 @@ describe("Codex apply_patch mount", () => {
       isCodex: true,
       runTool: async () => ({ content: "ok" }),
     });
-    expect(proxies.map((t) => t.definition.name)).toEqual(["apply_patch"]);
+    expect(proxies.map((t) => t.definition.name)).toEqual([
+      "apply_patch",
+      "shell",
+      "update_plan",
+    ]);
 
     const allow = new Set<string>(IMPLEMENT_TOOLS);
     const kept = proxies.filter((t) => allow.has(t.definition.name));
-    expect(kept.map((t) => t.definition.name)).toContain("apply_patch");
+    expect(kept.map((t) => t.definition.name)).toEqual(["apply_patch", "shell", "update_plan"]);
 
     const docsAllow = new Set<string>(DOCS_TOOLS);
     const docsKept = proxies.filter((t) => docsAllow.has(t.definition.name));
-    expect(docsKept.map((t) => t.definition.name)).toContain("apply_patch");
+    expect(docsKept.map((t) => t.definition.name)).toEqual(["apply_patch", "update_plan"]);
+  });
+
+  test("runSubAgent-shaped mount: docs capability filter denies shell, keeps update_plan", () => {
+    // Mirrors run.ts: allowDelete / allowShell are derived from the leaf
+    // capability filter before createCodexToolProxies runs. update_plan is
+    // never gated by it (manage_tasks is unconditionally mounted for every
+    // sub-agent), so it stays regardless of the allowlist shape.
+    const docsCapabilities = { mode: "allow" as const, tools: DOCS_TOOLS };
+    const proxies = createCodexToolProxies({
+      isCodex: true,
+      runTool: async () => ({ content: "ok" }),
+      allowDelete: allowDeleteFromCapabilities(docsCapabilities),
+      allowShell: allowShellFromCapabilities(docsCapabilities),
+    });
+    expect(proxies.map((t) => t.definition.name)).toEqual([
+      "apply_patch",
+      "shell",
+      "update_plan",
+    ]);
+
+    const docsAllow = new Set<string>(DOCS_TOOLS);
+    const docsKept = proxies.filter((t) => docsAllow.has(t.definition.name));
+    expect(docsKept.map((t) => t.definition.name)).toEqual(["apply_patch", "update_plan"]);
+  });
+
+  test("non-Codex runSubAgent-shaped mount produces no proxies at all", () => {
+    const proxies = createCodexToolProxies({
+      isCodex: false,
+      runTool: async () => ({ content: "ok" }),
+      allowDelete: allowDeleteFromCapabilities({ mode: "allow", tools: IMPLEMENT_TOOLS }),
+      allowShell: allowShellFromCapabilities({ mode: "allow", tools: IMPLEMENT_TOOLS }),
+    });
+    expect(proxies).toEqual([]);
   });
 });

src/agent/codex-tool-proxies.test.ts

@@ -4,6 +4,7 @@ import type { AgentTool } from "@intx/agent";
 
 import {
   allowDeleteFromCapabilities,
+  allowShellFromCapabilities,
   createCodexToolProxies,
   type CodexRunTool,
 } from "./codex-tool-proxies.js";
@@ -38,6 +39,12 @@ function makeRecorder(initial: Record<string, string> = {}): {
       files.delete(path);
       return { content: `Deleted file: ${path}` };
     }
+    if (name === "run_shell") {
+      return { content: `ran: ${JSON.stringify(args)}` };
+    }
+    if (name === "manage_tasks") {
+      return { content: "Tasks updated." };
+    }
     return { content: `unknown tool: ${name}`, isError: true };
   };
   return { calls, files, runTool };
@@ -51,6 +58,11 @@ async function invokeApplyPatch(tools: AgentTool[], input: string) {
   );
 }
 
+async function invokeTool(tools: AgentTool[], name: string, args: Record<string, unknown>) {
+  const runner = createToolRunner(tools);
+  return runner.run({ id: "call-1", name, arguments: args }, new AbortController().signal);
+}
+
 describe("createCodexToolProxies", () => {
   test("returns [] when not Codex", () => {
     const tools = createCodexToolProxies({
@@ -60,14 +72,13 @@ describe("createCodexToolProxies", () => {
     expect(tools).toEqual([]);
   });
 
-  test("returns apply_patch stringTool when Codex", () => {
+  test("returns apply_patch, shell, update_plan stringTools when Codex", () => {
     const tools = createCodexToolProxies({
       isCodex: true,
       runTool: async () => ({ content: "unused" }),
     });
-    expect(tools).toHaveLength(1);
-    expect(tools[0]!.definition.name).toBe("apply_patch");
-    expect(tools[0]!.kind).toBe("string");
+    expect(tools.map((t) => t.definition.name)).toEqual(["apply_patch", "shell", "update_plan"]);
+    expect(tools.every((t) => t.kind === "string")).toBe(true);
     expect(tools[0]!.definition.inputSchema).toMatchObject({
       required: ["input"],
     });
@@ -312,6 +323,117 @@ print("Hello, world!")
   });
 });
 
+describe("shell proxy", () => {
+  test("string command forwards to run_shell", async () => {
+    const { calls, runTool } = makeRecorder();
+    const tools = createCodexToolProxies({ isCodex: true, runTool });
+    const result = await invokeTool(tools, "shell", { command: "ls -la" });
+    expect(result.isError).toBeFalsy();
+    expect(calls).toEqual([{ name: "run_shell", args: { command: "ls -la" } }]);
+  });
+
+  test("bash -lc argv triple unwraps to the script", async () => {
+    const { calls, runTool } = makeRecorder();
+    const tools = createCodexToolProxies({ isCodex: true, runTool });
+    await invokeTool(tools, "shell", { command: ["bash", "-lc", "echo 'hi there'"] });
+    expect(calls).toEqual([{ name: "run_shell", args: { command: "echo 'hi there'" } }]);
+  });
+
+  test("other argv arrays are shell-quoted and joined", async () => {
+    const { calls, runTool } = makeRecorder();
+    const tools = createCodexToolProxies({ isCodex: true, runTool });
+    await invokeTool(tools, "shell", { command: ["echo", "hello world"] });
+    expect(calls).toEqual([{ name: "run_shell", args: { command: "echo 'hello world'" } }]);
+  });
+
+  test("workdir and timeout_ms translate to cwd and timeout", async () => {
+    const { calls, runTool } = makeRecorder();
+    const tools = createCodexToolProxies({ isCodex: true, runTool });
+    await invokeTool(tools, "shell", {
+      command: "pwd",
+      workdir: "/tmp/work",
+      timeout_ms: 5000,
+    });
+    expect(calls).toEqual([
+      { name: "run_shell", args: { command: "pwd", cwd: "/tmp/work", timeout: 5000 } },
+    ]);
+  });
+
+  test("missing command surfaces as tool error", async () => {
+    const { calls, runTool } = makeRecorder();
+    const tools = createCodexToolProxies({ isCodex: true, runTool });
+    const result = await invokeTool(tools, "shell", {});
+    expect(result.isError).toBe(true);
+    expect(result.content).toMatch(/command/);
+    expect(calls).toEqual([]);
+  });
+
+  test("allowShell false refuses without calling run_shell", async () => {
+    const { calls, runTool } = makeRecorder();
+    const tools = createCodexToolProxies({ isCodex: true, runTool, allowShell: false });
+    const result = await invokeTool(tools, "shell", { command: "ls" });
+    expect(result.isError).toBe(true);
+    expect(result.content).toMatch(/not allowed/);
+    expect(calls).toEqual([]);
+  });
+
+  test("run_shell isError propagates as tool error", async () => {
+    const runTool: CodexRunTool = async () => ({ content: "boom", isError: true });
+    const tools = createCodexToolProxies({ isCodex: true, runTool });
+    const result = await invokeTool(tools, "shell", { command: "ls" });
+    expect(result.isError).toBe(true);
+    expect(result.content).toMatch(/boom/);
+  });
+});
+
+describe("update_plan proxy", () => {
+  test("maps plan steps onto manage_tasks(action=create)", async () => {
+    const { calls, runTool } = makeRecorder();
+    const tools = createCodexToolProxies({ isCodex: true, runTool });
+    const result = await invokeTool(tools, "update_plan", {
+      explanation: "getting started",
+      plan: [
+        { step: "Read the file", status: "completed" },
+        { step: "Write the fix", status: "in_progress" },
+        { step: "Run tests", status: "pending" },
+      ],
+    });
+    expect(result.isError).toBeFalsy();
+    expect(calls).toEqual([
+      {
+        name: "manage_tasks",
+        args: {
+          action: "create",
+          tasks: [
+            { id: "p1", title: "Read the file", status: "done" },
+            { id: "p2", title: "Write the fix", status: "doing" },
+            { id: "p3", title: "Run tests", status: "todo" },
+          ],
+        },
+      },
+    ]);
+  });
+
+  test("malformed plan surfaces as tool error", async () => {
+    const { calls, runTool } = makeRecorder();
+    const tools = createCodexToolProxies({ isCodex: true, runTool });
+    const result = await invokeTool(tools, "update_plan", {
+      plan: [{ step: "no status here" }],
+    });
+    expect(result.isError).toBe(true);
+    expect(result.content).toMatch(/plan/);
+    expect(calls).toEqual([]);
+  });
+
+  test("missing plan surfaces as tool error", async () => {
+    const { calls, runTool } = makeRecorder();
+    const tools = createCodexToolProxies({ isCodex: true, runTool });
+    const result = await invokeTool(tools, "update_plan", {});
+    expect(result.isError).toBe(true);
+    expect(calls).toEqual([]);
+  });
+});
+
 describe("allowDeleteFromCapabilities", () => {
   test("docs allowlist (no delete_file) → false; implement → true", () => {
     expect(
@@ -329,3 +451,17 @@ describe("allowDeleteFromCapabilities", () => {
     ).toBe(false);
   });
 });
+
+describe("allowShellFromCapabilities", () => {
+  test("docs allowlist (no run_shell) → false; implement → true", () => {
+    expect(allowShellFromCapabilities({ mode: "allow", tools: DOCS_TOOLS })).toBe(false);
+    expect(allowShellFromCapabilities({ mode: "allow", tools: IMPLEMENT_TOOLS })).toBe(true);
+    expect(allowShellFromCapabilities(undefined)).toBe(true);
+    expect(
+      allowShellFromCapabilities({ mode: "exclude", tools: ["delete_file"] }),
+    ).toBe(true);
+    expect(
+      allowShellFromCapabilities({ mode: "exclude", tools: ["run_shell"] }),
+    ).toBe(false);
+  });
+});