From 1427c3a78d80550e8edb26570e32856448edc9e9 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sun, 26 Apr 2026 23:11:42 -0700 Subject: [PATCH] fix(sessions_spawn): tolerate ACP-only fields for subagent runtime (#72331) --- CHANGELOG.md | 1 + docs/tools/subagents.md | 6 +++ src/agents/tools/sessions-spawn-tool.test.ts | 39 +++++++++++++++----- src/agents/tools/sessions-spawn-tool.ts | 21 ++--------- 4 files changed, 40 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae2b66e4b34..601b6c5791c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,7 @@ Docs: https://docs.openclaw.ai - Diagnostics/OTEL: capture privacy-safe model-call request payload bytes, streamed response bytes, first-response latency, and total duration in diagnostic events, plugin hooks, stability snapshots, and OTEL model-call spans/metrics without logging raw model content. Fixes #33832. Thanks @wwh830. - Logging: write validated diagnostic trace context as top-level `traceId`, `spanId`, `parentSpanId`, and `traceFlags` fields in file-log JSONL records so traced requests and model calls are easier to correlate in log processors. Refs #40353. Thanks @liangruochong44-ui. - Logging/sessions: apply configured redaction patterns to persisted session transcript text and accept escaped character classes in safe custom redaction regexes, so transcript JSONL no longer keeps matching sensitive text in the clear. Fixes #42982. Thanks @panpan0000. +- Agents/sessions: let `sessions_spawn runtime="subagent"` ignore ACP-only `streamTo` and `resumeSessionId` fields while keeping ACP passthrough and documenting `streamTo` as ACP-only. Fixes #43556 and #63120; covers #56326, #61724, #64714, and #67248. Thanks @skernelx, @damselem, @Br1an67, @Mintalix, @IsaacAPerez, @vvitovec, and @Sanjays2402. - Providers/Ollama: honor `/api/show` capabilities when registering local models so non-tool Ollama models no longer receive the agent tool surface, and keep native Ollama thinking opt-in instead of enabling it by default. Fixes #64710 and duplicate #65343. Thanks @yuan-b, @netherby, @xilopaint, and @Diyforfun2026. - Image tool/media: honor `tools.media.image.timeoutSeconds` and matching per-model image timeouts in explicit image analysis, including the MiniMax VLM fallback path, so slow local vision models are not capped by hardcoded 30s/60s aborts. Fixes #67889; supersedes #67929. Thanks @AllenT22 and @alchip. - Providers/Ollama: read larger custom Modelfile `PARAMETER num_ctx` values from `/api/show` so auto-discovered Ollama models with expanded context no longer stay pinned to the base model context. Fixes #68344. Thanks @neeravmakwana. diff --git a/docs/tools/subagents.md b/docs/tools/subagents.md index 0d7a97aa990..72819b503d9 100644 --- a/docs/tools/subagents.md +++ b/docs/tools/subagents.md @@ -140,6 +140,12 @@ chat channel. `acp` is only for external ACP harnesses (`claude`, `droid`, `gemini`, `opencode`, or explicitly requested Codex ACP/acpx) and for `agents.list[]` entries whose `runtime.type` is `acp`. + + ACP-only. Resumes an existing ACP harness session when `runtime: "acp"`; ignored for native sub-agent spawns. + + + ACP-only. Streams ACP run output to the parent session when `runtime: "acp"`; omit for native sub-agent spawns. + Override the sub-agent model. Invalid values are skipped and the sub-agent runs on the default model with a warning in the tool result. diff --git a/src/agents/tools/sessions-spawn-tool.test.ts b/src/agents/tools/sessions-spawn-tool.test.ts index 7f43549b3f6..954aa42dca2 100644 --- a/src/agents/tools/sessions-spawn-tool.test.ts +++ b/src/agents/tools/sessions-spawn-tool.test.ts @@ -74,7 +74,7 @@ describe("sessions_spawn tool", () => { properties?: { runtime?: { enum?: string[] }; resumeSessionId?: unknown; - streamTo?: unknown; + streamTo?: { description?: string }; }; }; @@ -94,7 +94,7 @@ describe("sessions_spawn tool", () => { properties?: { runtime?: { enum?: string[] }; resumeSessionId?: unknown; - streamTo?: unknown; + streamTo?: { description?: string }; }; }; @@ -103,6 +103,7 @@ describe("sessions_spawn tool", () => { expect(schema.properties?.runtime?.enum).toEqual(["subagent", "acp"]); expect(schema.properties?.resumeSessionId).toBeDefined(); expect(schema.properties?.streamTo).toBeDefined(); + expect(schema.properties?.streamTo?.description).toContain('Requires runtime="acp"'); }); it("hides ACP runtime affordances when the ACP backend is unhealthy", () => { @@ -489,7 +490,7 @@ describe("sessions_spawn tool", () => { ); }); - it("rejects resumeSessionId without runtime=acp", async () => { + it("ignores resumeSessionId without runtime=acp", async () => { const tool = createSessionsSpawnTool({ agentSessionKey: "agent:main:main", }); @@ -499,8 +500,20 @@ describe("sessions_spawn tool", () => { resumeSessionId: "7f4a78e0-f6be-43fe-855c-c1c4fd229bc4", }); - expect(JSON.stringify(result)).toContain("resumeSessionId is only supported for runtime=acp"); - expect(hoisted.spawnSubagentDirectMock).not.toHaveBeenCalled(); + expect(result.details).toMatchObject({ + status: "accepted", + childSessionKey: "agent:main:subagent:1", + runId: "run-subagent", + }); + expect(hoisted.spawnSubagentDirectMock).toHaveBeenCalledWith( + expect.objectContaining({ + task: "resume prior work", + }), + expect.any(Object), + ); + expect(hoisted.spawnSubagentDirectMock.mock.calls[0]?.[0]).not.toHaveProperty( + "resumeSessionId", + ); expect(hoisted.spawnAcpDirectMock).not.toHaveBeenCalled(); }); @@ -529,7 +542,7 @@ describe("sessions_spawn tool", () => { expect(hoisted.spawnSubagentDirectMock).not.toHaveBeenCalled(); }); - it('rejects streamTo when runtime is not "acp"', async () => { + it('ignores streamTo when runtime is not "acp"', async () => { const tool = createSessionsSpawnTool({ agentSessionKey: "agent:main:main", }); @@ -541,12 +554,18 @@ describe("sessions_spawn tool", () => { }); expect(result.details).toMatchObject({ - status: "error", + status: "accepted", + childSessionKey: "agent:main:subagent:1", + runId: "run-subagent", }); - const details = result.details as { error?: string }; - expect(details.error).toContain("streamTo is only supported for runtime=acp"); expect(hoisted.spawnAcpDirectMock).not.toHaveBeenCalled(); - expect(hoisted.spawnSubagentDirectMock).not.toHaveBeenCalled(); + expect(hoisted.spawnSubagentDirectMock).toHaveBeenCalledWith( + expect.objectContaining({ + task: "analyze file", + }), + expect.any(Object), + ); + expect(hoisted.spawnSubagentDirectMock.mock.calls[0]?.[0]).not.toHaveProperty("streamTo"); }); it("keeps attachment content schema unconstrained for llama.cpp grammar safety", () => { diff --git a/src/agents/tools/sessions-spawn-tool.ts b/src/agents/tools/sessions-spawn-tool.ts index 7b98cc32843..ed95ca2d49d 100644 --- a/src/agents/tools/sessions-spawn-tool.ts +++ b/src/agents/tools/sessions-spawn-tool.ts @@ -157,7 +157,10 @@ function createSessionsSpawnToolSchema(params: { acpAvailable: boolean }) { 'Resume an existing agent session by its ID (e.g. a Codex session UUID from ~/.codex/sessions/). Requires runtime="acp". The agent replays conversation history via session/load instead of starting fresh.', }), ), - streamTo: optionalStringEnum(SESSIONS_SPAWN_ACP_STREAM_TARGETS), + streamTo: optionalStringEnum(SESSIONS_SPAWN_ACP_STREAM_TARGETS, { + description: + 'Stream ACP run output to the parent session. Requires runtime="acp"; omit for runtime="subagent".', + }), } : {}), }; @@ -261,22 +264,6 @@ export function createSessionsSpawnTool( }>) : undefined; - if (streamTo && runtime !== "acp") { - return jsonResult({ - status: "error", - error: `streamTo is only supported for runtime=acp; got runtime=${runtime}`, - ...roleContext, - }); - } - - if (resumeSessionId && runtime !== "acp") { - return jsonResult({ - status: "error", - error: `resumeSessionId is only supported for runtime=acp; got runtime=${runtime}`, - ...roleContext, - }); - } - if (runtime === "acp") { const { isSpawnAcpAcceptedResult, spawnAcpDirect } = await loadAcpSpawnModule(); if (Array.isArray(attachments) && attachments.length > 0) {