From 33dae97b812ca13f411318d41f80ea06d62ecf48 Mon Sep 17 00:00:00 2001 From: Vito Cappello Date: Tue, 28 Jul 2026 22:07:02 -0400 Subject: [PATCH] fix(agents): restore subagent completion delivery on CLI runtimes (#115422) * fix(agents): deliver tool-free CLI completions * fix(agents): enforce empty Gemini CLI tool caps * refactor(agents): remove ineffective Gemini admin settings * fix(agents): block inherited Gemini MCP servers * fix(agents): enforce Gemini MCP cap via argv --------- Co-authored-by: VACInc <3279061+VACInc@users.noreply.github.com> Co-authored-by: Patrick Erichsen --- extensions/google/cli-backend-auth.runtime.ts | 33 +++-- extensions/google/cli-backend.ts | 40 ++++++ extensions/google/setup-api.test.ts | 130 +++++++++++++++++- src/agents/cli-runner/prepare.test.ts | 93 ++++++++++++- src/agents/cli-runner/prepare.ts | 26 ++-- 5 files changed, 295 insertions(+), 27 deletions(-) diff --git a/extensions/google/cli-backend-auth.runtime.ts b/extensions/google/cli-backend-auth.runtime.ts index 5d1be69d74be..8a3fd09a4f4e 100644 --- a/extensions/google/cli-backend-auth.runtime.ts +++ b/extensions/google/cli-backend-auth.runtime.ts @@ -266,8 +266,21 @@ function applyGeminiCliToolAvailability( throw new Error("Gemini CLI cannot expose backend-native tools in an exact restricted run."); } const mcpServers = isRecord(base.mcpServers) ? { ...base.mcpServers } : {}; - if (!isRecord(mcpServers.openclaw)) { - throw new Error("Gemini CLI exact tool availability requires the OpenClaw MCP server."); + // A fully empty cap must not require the loopback server: tool-free handoffs + // intentionally suppress that runtime before backend preparation. + const exposesOpenClawTools = availability.openClaw.length > 0; + let restrictedMcpServers: Record = {}; + if (exposesOpenClawTools) { + const openClawMcpServer = mcpServers.openclaw; + if (!isRecord(openClawMcpServer)) { + throw new Error("Gemini CLI exact tool availability requires the OpenClaw MCP server."); + } + restrictedMcpServers = { + openclaw: { + ...openClawMcpServer, + includeTools: [...availability.openClaw], + }, + }; } const tools = isRecord(base.tools) ? { ...base.tools } : {}; // `tools.allowed` has higher policy priority than the `tools.core` default @@ -281,6 +294,9 @@ function applyGeminiCliToolAvailability( } = tools; const mcp = isRecord(base.mcp) ? { ...base.mcp } : {}; const { serverCommand: _serverCommand, ...nonAuthorityMcpSettings } = mcp; + // Gemini treats an empty MCP allowlist as unrestricted. Use a per-run name + // that no inherited server can know when this run must expose no MCP tools. + const allowedMcpServers = exposesOpenClawTools ? ["openclaw"] : [crypto.randomUUID()]; const experimental = isRecord(base.experimental) ? { ...base.experimental } : {}; const agents = isRecord(base.agents) ? { ...base.agents } : {}; const agentOverrides = isRecord(agents.overrides) ? { ...agents.overrides } : {}; @@ -290,17 +306,16 @@ function applyGeminiCliToolAvailability( ...base, tools: { ...nonAuthorityToolSettings, - core: ["mcp_openclaw_*"], + core: exposesOpenClawTools ? ["mcp_openclaw_*"] : [], discoveryCommand: "", callCommand: "", }, - mcp: { ...nonAuthorityMcpSettings, allowed: ["openclaw"], serverCommand: "" }, - mcpServers: { - openclaw: { - ...mcpServers.openclaw, - includeTools: [...availability.openClaw], - }, + mcp: { + ...nonAuthorityMcpSettings, + allowed: allowedMcpServers, + serverCommand: "", }, + mcpServers: restrictedMcpServers, experimental: { ...experimental, enableAgents: false }, agents: { ...agents, diff --git a/extensions/google/cli-backend.ts b/extensions/google/cli-backend.ts index 12692536446d..00d26f9e56e5 100644 --- a/extensions/google/cli-backend.ts +++ b/extensions/google/cli-backend.ts @@ -1,3 +1,4 @@ +import crypto from "node:crypto"; import type { CliBackendPlugin } from "openclaw/plugin-sdk/cli-backend"; import { CLI_FRESH_WATCHDOG_DEFAULTS, @@ -10,6 +11,7 @@ const GEMINI_MODEL_ALIASES: Record = { "flash-lite": "gemini-3.1-flash-lite", }; const GEMINI_CLI_DEFAULT_MODEL_REF = "google-gemini-cli/gemini-3-flash-preview"; +const GEMINI_ALLOWED_MCP_SERVERS_ARG = "--allowed-mcp-server-names"; type GeminiCliBackendConfig = CliBackendPlugin["config"]; type GeminiCliOutputMode = NonNullable; @@ -55,6 +57,43 @@ function normalizeGeminiCliBackendConfig(config: GeminiCliBackendConfig): Gemini }; } +function isGeminiAllowedMcpServersArg(arg: string): boolean { + const [name] = arg.split("=", 1); + if (!name?.startsWith("--")) { + return false; + } + return name.slice(2).replaceAll(/[-_]/g, "").toLowerCase() === "allowedmcpservernames"; +} + +function resolveGeminiCliExecutionArgs( + ctx: Parameters>[0], +): readonly string[] { + if (!ctx.toolAvailability) { + return ctx.baseArgs; + } + const terminatorIndex = ctx.baseArgs.indexOf("--"); + const optionArgs = terminatorIndex === -1 ? ctx.baseArgs : ctx.baseArgs.slice(0, terminatorIndex); + const positionalArgs = terminatorIndex === -1 ? [] : ctx.baseArgs.slice(terminatorIndex); + const args: string[] = []; + for (let index = 0; index < optionArgs.length; index += 1) { + const arg = optionArgs[index]; + if (arg && isGeminiAllowedMcpServersArg(arg)) { + if (!arg.includes("=")) { + index += 1; + } + continue; + } + if (arg !== undefined) { + args.push(arg); + } + } + + // Gemini intersects file-based allowlists, where an empty intersection means + // unrestricted. The argv override bypasses that merge and prevents MCP startup. + const allowedServer = ctx.toolAvailability.openClaw.length > 0 ? "openclaw" : crypto.randomUUID(); + return [...args, GEMINI_ALLOWED_MCP_SERVERS_ARG, allowedServer, ...positionalArgs]; +} + export function buildGoogleGeminiCliBackend(): CliBackendPlugin { return { id: "google-gemini-cli", @@ -81,6 +120,7 @@ export function buildGoogleGeminiCliBackend(): CliBackendPlugin { toolAvailabilityEnforcement: "prepare-execution", authEpochMode: "profile-only", normalizeConfig: normalizeGeminiCliBackendConfig, + resolveExecutionArgs: resolveGeminiCliExecutionArgs, prepareExecution: async (ctx) => { const { prepareGeminiCliExecution } = await import("./cli-backend-auth.runtime.js"); return await prepareGeminiCliExecution( diff --git a/extensions/google/setup-api.test.ts b/extensions/google/setup-api.test.ts index d5bf7ecbdaec..3164410a2e6f 100644 --- a/extensions/google/setup-api.test.ts +++ b/extensions/google/setup-api.test.ts @@ -152,6 +152,57 @@ describe("google gemini cli backend config", () => { expect(backend.toolAvailabilityEnforcement).toBe("prepare-execution"); }); + it("enforces exact MCP server availability with a per-run argv override", () => { + const backend = buildGoogleGeminiCliBackend(); + const baseContext = { + workspaceDir: "/tmp/openclaw-gemini-test", + provider: "google-gemini-cli", + modelId: "gemini-3.1-pro-preview", + useResume: false, + baseArgs: [ + "--allowedMcpServerNames", + "camel-hostile", + "--allowed_mcp_server_names=snake-hostile", + "--allowed-mcp-server-names", + "hostile", + "--allowed-mcp-server-names=also-hostile", + "--prompt", + "{prompt}", + "--", + "--allowed-mcp-server-names", + "positional-hostile", + ], + }; + + const restrictedArgs = backend.resolveExecutionArgs?.({ + ...baseContext, + toolAvailability: { native: [], openClaw: ["read"], mcp: ["read"] }, + }); + expect(restrictedArgs).toEqual([ + "--prompt", + "{prompt}", + "--allowed-mcp-server-names", + "openclaw", + "--", + "--allowed-mcp-server-names", + "positional-hostile", + ]); + + const emptyArgs = backend.resolveExecutionArgs?.({ + ...baseContext, + toolAvailability: { native: [], openClaw: [], mcp: [] }, + }); + expect(emptyArgs?.slice(0, -4)).toEqual(["--prompt", "{prompt}", "--allowed-mcp-server-names"]); + expect(emptyArgs?.at(-4)).toMatch( + /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/, + ); + expect(emptyArgs?.slice(-3)).toEqual([ + "--", + "--allowed-mcp-server-names", + "positional-hostile", + ]); + }); + it("keeps legacy json output overrides on the json parser", () => { const backend = buildGoogleGeminiCliBackend(); const normalized = backend.normalizeConfig?.({ @@ -272,17 +323,28 @@ describe("google gemini cli backend auth bridge", () => { skills?: Record; security?: { auth?: { selectedType?: string } }; }; - expect(settings.tools?.core).toEqual(["mcp_openclaw_*"]); + expect(settings.tools?.core).toEqual(allowed.length > 0 ? ["mcp_openclaw_*"] : []); expect(settings.tools).not.toHaveProperty("allowed"); expect(settings.tools?.discoveryCommand).toBe(""); expect(settings.tools?.callCommand).toBe(""); - expect(settings.mcp?.allowed).toEqual(["openclaw"]); + if (allowed.length > 0) { + expect(settings.mcp?.allowed).toEqual(["openclaw"]); + } else { + expect(settings.mcp?.allowed).toHaveLength(1); + expect(settings.mcp?.allowed?.[0]).toMatch( + /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/, + ); + } expect(settings.mcp?.serverCommand).toBe(""); - expect(settings.mcpServers?.openclaw).toMatchObject({ - url: "http://127.0.0.1:23119/mcp", - headers: { authorization: "Bearer loopback-token" }, - includeTools: [...allowed], - }); + if (allowed.length > 0) { + expect(settings.mcpServers?.openclaw).toMatchObject({ + url: "http://127.0.0.1:23119/mcp", + headers: { authorization: "Bearer loopback-token" }, + includeTools: [...allowed], + }); + } else { + expect(settings.mcpServers).toEqual({}); + } expect(settings.mcpServers?.hostile).toBeUndefined(); expect(settings.experimental?.enableAgents).toBe(false); expect(settings.agents?.overrides?.codebase_investigator).toEqual({ @@ -322,6 +384,60 @@ describe("google gemini cli backend auth bridge", () => { }); }); + it("enforces an exact empty tool cap without an OpenClaw MCP server", async () => { + await withTempDir("openclaw-test-workspace-", async (workspaceDir) => { + const inheritedSettingsPath = path.join(workspaceDir, "system-settings.json"); + await fs.writeFile( + inheritedSettingsPath, + JSON.stringify({ + tools: { core: ["run_shell_command"], allowed: ["*"] }, + mcp: { allowed: ["hostile"] }, + mcpServers: { + openclaw: { command: "inherited-openclaw-server" }, + hostile: { command: "hostile-server" }, + }, + experimental: { enableAgents: true }, + hooksConfig: { enabled: true }, + skills: { enabled: true }, + }), + "utf8", + ); + + const prepared = await buildGoogleGeminiCliBackend().prepareExecution?.({ + workspaceDir, + provider: "google-gemini-cli", + modelId: "gemini-3.1-pro-preview", + env: { GEMINI_CLI_SYSTEM_SETTINGS_PATH: inheritedSettingsPath }, + toolAvailability: { native: [], openClaw: [], mcp: [] }, + }); + try { + expect(prepared?.toolAvailabilityEnforced).toBe(true); + await stageGeminiPreparedExecution(prepared); + const systemSettingsPath = prepared?.env?.GEMINI_CLI_SYSTEM_SETTINGS_PATH; + const settings = JSON.parse(await fs.readFile(systemSettingsPath ?? "", "utf8")) as { + tools?: { core?: string[] }; + mcp?: { allowed?: string[] }; + mcpServers?: Record; + experimental?: { enableAgents?: boolean }; + hooksConfig?: { enabled?: boolean }; + skills?: { enabled?: boolean }; + }; + expect(settings.tools?.core).toEqual([]); + expect(settings.tools).not.toHaveProperty("allowed"); + expect(settings.mcp?.allowed).toHaveLength(1); + expect(settings.mcp?.allowed?.[0]).toMatch( + /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/, + ); + expect(settings.mcpServers).toEqual({}); + expect(settings.experimental?.enableAgents).toBe(false); + expect(settings.hooksConfig?.enabled).toBe(false); + expect(settings.skills?.enabled).toBe(false); + } finally { + await prepared?.cleanup?.(); + } + }); + }); + it("materializes selected OpenClaw OAuth credentials into a persistent profile-scoped Gemini CLI home", async () => { const backend = buildGoogleGeminiCliBackend(); const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-test-workspace-")); diff --git a/src/agents/cli-runner/prepare.test.ts b/src/agents/cli-runner/prepare.test.ts index 5c489f1fb36a..37698169c996 100644 --- a/src/agents/cli-runner/prepare.test.ts +++ b/src/agents/cli-runner/prepare.test.ts @@ -2857,7 +2857,72 @@ describe("prepareCliRunContext", () => { ); }); - it("requires prepared-execution backends to acknowledge exact enforcement and cleans up", async () => { + it("translates disableTools into an exact empty cap for selectable backends", async () => { + const resolveExecutionArgs = vi.fn((context: { baseArgs: readonly string[] }) => [ + ...context.baseArgs, + ]); + const getActiveMcpLoopbackRuntime = vi.fn(() => ({ + port: 31783, + ownerToken: "loopback-owner-token", + nonOwnerToken: "loopback-non-owner-token", + })); + setCliRunnerPrepareTestDeps({ getActiveMcpLoopbackRuntime }); + setRawCliBackendForPrepareTest({ + id: "selectable-cli", + pluginId: "selectable-plugin", + bundleMcp: true, + bundleMcpMode: "claude-config-file", + nativeToolMode: "selectable", + toolAvailabilityEnforcement: "execution-args", + resolveExecutionArgs, + config: { + command: "selectable-cli", + args: ["--print"], + output: "jsonl", + input: "stdin", + sessionMode: "existing", + }, + }); + + const context = await fixture.prepare({ + provider: "selectable-cli", + disableTools: true, + }); + + expect(context.params.cliToolAvailability).toEqual({ native: [], openClaw: [] }); + expect(getActiveMcpLoopbackRuntime).not.toHaveBeenCalled(); + }); + + it("lets disableTools override a selectable backend toolsAllow projection", async () => { + const resolveExecutionArgs = vi.fn((context: { baseArgs: readonly string[] }) => [ + ...context.baseArgs, + ]); + setRawCliBackendForPrepareTest({ + id: "selectable-cli", + pluginId: "selectable-plugin", + bundleMcp: false, + nativeToolMode: "selectable", + toolAvailabilityEnforcement: "execution-args", + resolveExecutionArgs, + config: { + command: "selectable-cli", + args: ["--print"], + output: "jsonl", + input: "stdin", + sessionMode: "existing", + }, + }); + + const context = await fixture.prepare({ + provider: "selectable-cli", + disableTools: true, + toolsAllow: ["write"], + }); + + expect(context.params.cliToolAvailability).toEqual({ native: [], openClaw: [] }); + }); + + it("requires prepared-execution backends to enforce the derived disabled-tools cap", async () => { const cleanup = vi.fn(async () => {}); const prepareExecution = vi.fn(async () => ({ cleanup })); setRawCliBackendForPrepareTest({ @@ -2879,7 +2944,7 @@ describe("prepareCliRunContext", () => { await expect( fixture.prepare({ provider: "settings-cli", - cliToolAvailability: { native: [], openClaw: [] }, + disableTools: true, }), ).rejects.toThrow( "did not enforce exact per-run tool availability during execution preparation", @@ -2890,6 +2955,30 @@ describe("prepareCliRunContext", () => { expect(cleanup).toHaveBeenCalledOnce(); }); + it("still rejects disableTools when a selectable backend cannot enforce an exact cap", async () => { + setRawCliBackendForPrepareTest({ + id: "selectable-cli", + pluginId: "selectable-plugin", + nativeToolMode: "selectable", + config: { + command: "selectable-cli", + args: ["--print"], + output: "jsonl", + input: "stdin", + sessionMode: "existing", + }, + }); + + await expect( + fixture.prepare({ + provider: "selectable-cli", + disableTools: true, + }), + ).rejects.toThrow( + "CLI backend selectable-cli cannot run with tools disabled because it exposes native tools", + ); + }); + it("accepts a positive prepared-execution enforcement acknowledgement", async () => { const prepareExecution = vi.fn(async () => ({ toolAvailabilityEnforced: true as const })); setRawCliBackendForPrepareTest({ diff --git a/src/agents/cli-runner/prepare.ts b/src/agents/cli-runner/prepare.ts index 6695fc2777c7..0d4d4b59ef67 100644 --- a/src/agents/cli-runner/prepare.ts +++ b/src/agents/cli-runner/prepare.ts @@ -378,6 +378,12 @@ export async function prepareCliRunContext( if (!backendResolved) { throw new Error(`Unknown CLI backend: ${params.provider}`); } + const canEnforceExactToolAvailability = + backendResolved.nativeToolMode === "selectable" && + ((backendResolved.toolAvailabilityEnforcement === "execution-args" && + backendResolved.resolveExecutionArgs !== undefined) || + (backendResolved.toolAvailabilityEnforcement === "prepare-execution" && + backendResolved.prepareExecution !== undefined)); let runtimeToolsAllowPolicy: string[] | undefined; if (params.toolsAllow !== undefined) { if (params.cliToolAvailability !== undefined) { @@ -412,6 +418,16 @@ export async function prepareCliRunContext( }; } } + if (params.disableTools === true && !isSideQuestion && canEnforceExactToolAvailability) { + // Selectable backends need the exact empty cap as well as the generic flag; + // otherwise their native tools remain selectable and the run must fail closed. + runtimeToolsAllowPolicy = undefined; + params = { + ...params, + toolsAllow: undefined, + cliToolAvailability: { native: [], openClaw: [] }, + }; + } const internalParams = params as RunCliAgentPrepareParams; const nodeClaudePlacement = resolveNodeClaudePlacement({ backendId: backendResolved.id, @@ -429,15 +445,7 @@ export async function prepareCliRunContext( }, }; } - if ( - params.cliToolAvailability !== undefined && - (backendResolved.nativeToolMode !== "selectable" || - !backendResolved.toolAvailabilityEnforcement || - (backendResolved.toolAvailabilityEnforcement === "execution-args" && - !backendResolved.resolveExecutionArgs) || - (backendResolved.toolAvailabilityEnforcement === "prepare-execution" && - !backendResolved.prepareExecution)) - ) { + if (params.cliToolAvailability !== undefined && !canEnforceExactToolAvailability) { // Cron persists this verbatim and failure alerts truncate at 200 characters, // so keep the upgrade recovery and fail-closed outcome compact. throw new Error(