mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-25 20:05:46 -06:00
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 <patrick.a.erichsen@gmail.com>
This commit is contained in:
@@ -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<string, unknown> = {};
|
||||
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,
|
||||
|
||||
@@ -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<string, string> = {
|
||||
"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<GeminiCliBackendConfig["output"]>;
|
||||
@@ -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<NonNullable<CliBackendPlugin["resolveExecutionArgs"]>>[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(
|
||||
|
||||
@@ -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<string, unknown>;
|
||||
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<string, unknown>;
|
||||
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-"));
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user