From 6a7b76e119caa401e9a9c304e612ef0d9453f93c Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 25 Apr 2026 21:23:13 +0100 Subject: [PATCH] fix(acp): guard sessions_spawn runtime targets --- CHANGELOG.md | 3 + docs/tools/acp-agents.md | 7 ++ docs/tools/subagents.md | 3 +- .../src/bot.create-telegram-bot.test.ts | 2 + src/agents/acp-spawn.test.ts | 83 +++++++++++++++++++ src/agents/acp-spawn.ts | 31 ++++++- src/agents/tool-description-presets.ts | 1 + 7 files changed, 128 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06a0814ec05..5b13830baa5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,9 @@ Docs: https://docs.openclaw.ai - Plugins/doctor: keep bundled plugin runtime-dependency repairs inside the managed OpenClaw stage even when user npm prefix/global config points npm at `$HOME/node_modules`. Fixes #71730. +- ACP/sessions_spawn: reject normal OpenClaw config agent ids when callers + explicitly request `runtime="acp"`, while allowing agents configured with + `runtime.type="acp"` to resolve to their ACP harness id. Fixes #63914. - Plugins/Voice Call: treat missing provider credentials as setup-incomplete during Gateway startup and log the missing keys as a warning instead of a runtime startup error, while keeping explicit command/tool errors when used. Thanks diff --git a/docs/tools/acp-agents.md b/docs/tools/acp-agents.md index 478f602d609..4034f5d8443 100644 --- a/docs/tools/acp-agents.md +++ b/docs/tools/acp-agents.md @@ -78,6 +78,13 @@ Natural-language triggers that should route to the ACP runtime: OpenClaw picks `runtime: "acp"`, resolves the harness `agentId`, binds to the current conversation or thread when supported, and routes follow-ups to that session until close/expiry. Codex only follows this path when ACP is explicit or the requested background runtime still needs ACP. +For `sessions_spawn`, `runtime: "acp"` targets ACP harness ids such as `codex`, +`claude`, `gemini`, or `opencode`. Do not pass a normal OpenClaw config agent +id from `agents_list` unless that entry is explicitly configured with +`agents.list[].runtime.type="acp"`; otherwise use the default sub-agent runtime. +When an OpenClaw agent is configured with `runtime.type="acp"`, OpenClaw uses +`runtime.acp.agent` as the underlying harness id. + ## ACP versus sub-agents Use ACP when you want an external harness runtime. Use native Codex app-server for Codex conversation binding/control. Use sub-agents when you want OpenClaw-native delegated runs. diff --git a/docs/tools/subagents.md b/docs/tools/subagents.md index 2626aec1353..520871aeaa8 100644 --- a/docs/tools/subagents.md +++ b/docs/tools/subagents.md @@ -60,7 +60,7 @@ transcript path on disk when you need the raw full transcript. - `--model` and `--thinking` override defaults for that specific run. - Use `info`/`log` to inspect details and output after completion. - `/subagents spawn` is one-shot mode (`mode: "run"`). For persistent thread-bound sessions, use `sessions_spawn` with `thread: true` and `mode: "session"`. -- For ACP harness sessions (Codex, Claude Code, Gemini CLI), use `sessions_spawn` with `runtime: "acp"` and see [ACP Agents](/tools/acp-agents), especially the [ACP delivery model](/tools/acp-agents#delivery-model) when debugging completions or agent-to-agent loops. +- For ACP harness sessions (Codex, Claude Code, Gemini CLI, OpenCode), use `sessions_spawn` with `runtime: "acp"` and see [ACP Agents](/tools/acp-agents), especially the [ACP delivery model](/tools/acp-agents#delivery-model) when debugging completions or agent-to-agent loops. `runtime: "acp"` expects an external ACP harness id, or an `agents.list[]` entry with `runtime.type="acp"`; use the default sub-agent runtime for normal OpenClaw config agents from `agents_list`. Primary goals: @@ -103,6 +103,7 @@ Tool params: - `task` (required) - `label?` (optional) - `agentId?` (optional; spawn under another agent id if allowed) +- `runtime?` (`subagent|acp`, default `subagent`; `acp` is only for external ACP harnesses such as `codex`, `claude`, `gemini`, or `opencode`, or for `agents.list[]` entries whose `runtime.type` is `acp`) - `model?` (optional; overrides the sub-agent model; invalid values are skipped and the sub-agent runs on the default model with a warning in the tool result) - `thinking?` (optional; overrides thinking level for the sub-agent run) - `runTimeoutSeconds?` (defaults to `agents.defaults.subagents.runTimeoutSeconds` when set, otherwise `0`; when set, the sub-agent run is aborted after N seconds) diff --git a/extensions/telegram/src/bot.create-telegram-bot.test.ts b/extensions/telegram/src/bot.create-telegram-bot.test.ts index d4053daccfd..fd4959867a3 100644 --- a/extensions/telegram/src/bot.create-telegram-bot.test.ts +++ b/extensions/telegram/src/bot.create-telegram-bot.test.ts @@ -1954,6 +1954,8 @@ describe("createTelegramBot", () => { expect(replySpy).toHaveBeenCalledTimes(1); const payload = replySpy.mock.calls[0][0]; expect(payload.SessionKey).toContain(testCase.expectedSessionKeyFragment); + expect(payload.BodyForAgent).toBe(testCase.text); + expect(payload.BodyForAgent).not.toContain("t.me/c/"); } }); diff --git a/src/agents/acp-spawn.test.ts b/src/agents/acp-spawn.test.ts index 86c7f636e2c..a573fc54363 100644 --- a/src/agents/acp-spawn.test.ts +++ b/src/agents/acp-spawn.test.ts @@ -742,6 +742,89 @@ describe("spawnAcpDirect", () => { ); }); + it("rejects OpenClaw config agent ids when runtime=acp targets a native agent", async () => { + replaceSpawnConfig({ + ...createDefaultSpawnConfig(), + acp: { + enabled: true, + backend: "acpx", + allowedAgents: ["codex"], + }, + agents: { + list: [{ id: "pleres" }], + defaults: { + subagents: { + allowAgents: ["*"], + maxSpawnDepth: 2, + }, + }, + }, + }); + + const result = await spawnAcpDirect( + { + task: "Investigate flaky tests", + agentId: "pleres", + }, + { + agentSessionKey: "agent:main:main", + }, + ); + + expect(result).toMatchObject({ + status: "error", + errorCode: "runtime_agent_mismatch", + }); + expect(result).toHaveProperty("error", expect.stringContaining("OpenClaw config agent")); + expect(hoisted.initializeSessionMock).not.toHaveBeenCalled(); + expect(hoisted.callGatewayMock).not.toHaveBeenCalledWith( + expect.objectContaining({ method: "agent" }), + ); + }); + + it("maps OpenClaw ACP runtime agent aliases to their configured harness id", async () => { + replaceSpawnConfig({ + ...createDefaultSpawnConfig(), + agents: { + list: [ + { + id: "reviewer", + runtime: { + type: "acp", + acp: { + agent: "codex", + }, + }, + }, + ], + defaults: { + subagents: { + allowAgents: ["codex"], + maxSpawnDepth: 2, + }, + }, + }, + }); + + const result = await spawnAcpDirect( + { + task: "Investigate flaky tests", + agentId: "reviewer", + }, + { + agentSessionKey: "agent:main:main", + }, + ); + + expectAcceptedSpawn(result); + expect(hoisted.initializeSessionMock).toHaveBeenCalledWith( + expect.objectContaining({ + agent: "codex", + sessionKey: expect.stringMatching(/^agent:codex:acp:/), + }), + ); + }); + it("inherits subagent envelope fields onto ACP children", async () => { replaceSpawnConfig({ ...hoisted.state.cfg, diff --git a/src/agents/acp-spawn.ts b/src/agents/acp-spawn.ts index de9bcb35a9b..f7b753ac34d 100644 --- a/src/agents/acp-spawn.ts +++ b/src/agents/acp-spawn.ts @@ -128,6 +128,7 @@ export const ACP_SPAWN_ERROR_CODES = [ "subagent_policy", "thread_required", "target_agent_required", + "runtime_agent_mismatch", "agent_forbidden", "cwd_resolution_failed", "thread_binding_invalid", @@ -395,6 +396,24 @@ function resolveTargetAcpAgentId(params: { }): { ok: true; agentId: string } | { ok: false; error: string } { const requested = normalizeOptionalAgentId(params.requestedAgentId); if (requested) { + const configuredAgent = params.cfg.agents?.list?.find( + (agent) => normalizeOptionalAgentId(agent.id) === requested, + ); + if (configuredAgent?.runtime?.type === "acp") { + return { + ok: true, + agentId: normalizeOptionalAgentId(configuredAgent.runtime.acp?.agent) ?? requested, + }; + } + if (configuredAgent && !isExplicitlyAllowedAcpAgent(params.cfg, requested)) { + return { + ok: false, + error: + `agentId "${requested}" is an OpenClaw config agent, not an ACP harness. ` + + 'Use runtime="subagent" or omit runtime for OpenClaw config agents. ' + + 'Use runtime="acp" only with external ACP harness ids such as codex, claude, gemini, or opencode, or configure agents.list[].runtime.type="acp" with runtime.acp.agent.', + }; + } return { ok: true, agentId: requested }; } @@ -410,6 +429,13 @@ function resolveTargetAcpAgentId(params: { }; } +function isExplicitlyAllowedAcpAgent(cfg: OpenClawConfig, agentId: string): boolean { + return (cfg.acp?.allowedAgents ?? []).some((entry) => { + const normalized = normalizeOptionalAgentId(entry); + return normalized === "*" || normalized === agentId; + }); +} + function normalizeOptionalAgentId(value: string | undefined | null): string | undefined { const trimmed = normalizeOptionalString(value) ?? ""; if (!trimmed) { @@ -1091,7 +1117,10 @@ export async function spawnAcpDirect( if (!targetAgentResult.ok) { return createAcpSpawnFailure({ status: "error", - errorCode: "target_agent_required", + errorCode: + params.agentId && normalizeOptionalAgentId(params.agentId) + ? "runtime_agent_mismatch" + : "target_agent_required", error: targetAgentResult.error, }); } diff --git a/src/agents/tool-description-presets.ts b/src/agents/tool-description-presets.ts index 0cf455f4b61..b59095f53ed 100644 --- a/src/agents/tool-description-presets.ts +++ b/src/agents/tool-description-presets.ts @@ -36,6 +36,7 @@ export function describeSessionsSpawnTool(): string { 'Spawn a clean isolated session by default with `runtime="subagent"` or `runtime="acp"`.', '`mode="run"` is one-shot and `mode="session"` is persistent or thread-bound.', "Subagents inherit the parent workspace directory automatically.", + '`runtime="acp"` is for external ACP harness ids such as codex, claude, gemini, or opencode, or agents configured with `agents.list[].runtime.type="acp"`.', 'For native subagents only, set `context="fork"` when the child needs the current transcript context; otherwise omit it or use `context="isolated"`.', "Use this when the work should happen in a fresh child session instead of the current one.", ].join(" ");