From 9c8531979201ecd1bf85ad0e69c4e80deba303a7 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 21 Aug 2026 17:47:17 -0700 Subject: [PATCH] fix(agents): stop recommending unavailable session tools (#127648) * fix(agents): stop recommending unavailable session tools * fix(agents): preserve authorized tool guidance placement * fix(agents): keep spawn recovery guidance executable --- ...t-tools.deferred-followup-guidance.test.ts | 219 ++++++++++++++++-- src/agents/agent-tools.deferred-followup.ts | 68 +++++- ...subagents.sessions-spawn.allowlist.test.ts | 8 +- src/agents/spawn-plan.ts | 4 +- .../subagents/spawn/subagent-spawn-request.ts | 4 +- ...ent-spawn.mode-session-diagnostics.test.ts | 6 +- src/agents/tool-description-presets.test.ts | 5 +- src/agents/tool-description-presets.ts | 8 +- src/agents/tools/conversation-tools.ts | 2 +- src/agents/tools/sessions-spawn-tool.test.ts | 58 ++++- src/agents/tools/sessions-spawn-tool.ts | 4 +- src/agents/tools/sessions-spawn-visible.ts | 4 +- 12 files changed, 343 insertions(+), 47 deletions(-) diff --git a/src/agents/agent-tools.deferred-followup-guidance.test.ts b/src/agents/agent-tools.deferred-followup-guidance.test.ts index 7fd68d9ee6a5..2a8acbaea796 100644 --- a/src/agents/agent-tools.deferred-followup-guidance.test.ts +++ b/src/agents/agent-tools.deferred-followup-guidance.test.ts @@ -5,6 +5,12 @@ import { getPluginToolMeta, setPluginToolMeta } from "../plugins/tools.js"; import { applyToolAvailabilityDescriptions } from "./agent-tools.deferred-followup.js"; import type { AnyAgentTool } from "./agent-tools.types.js"; import { getChannelAgentToolMeta, setChannelAgentToolMeta } from "./channel-tool-metadata.js"; +import { + describeSessionsSearchTool, + describeSessionsSendTool, + describeSessionsSpawnTool, +} from "./tool-description-presets.js"; +import { createConversationsSendTool } from "./tools/conversation-tools.js"; function findToolDescription(toolName: string, includeCron: boolean) { const tools = applyToolAvailabilityDescriptions([ @@ -46,25 +52,55 @@ describe("createOpenClawCodingTools availability guidance", () => { ); }); - it("preserves ownership metadata when replacing process descriptions", () => { - const processTool = { - name: "process", - description: "plugin process", - } as AnyAgentTool; - setPluginToolMeta(processTool, { pluginId: "example", optional: false }); - setChannelAgentToolMeta(processTool as never, { channelId: "example-channel" }); + it.each([ + { name: "process", description: "plugin process", available: [] }, + { + name: "sessions_send", + description: describeSessionsSendTool(), + available: ["conversations_list", "conversations_send"], + }, + { + name: "sessions_search", + description: describeSessionsSearchTool(), + available: ["sessions_history"], + }, + { + name: "sessions_spawn", + description: describeSessionsSpawnTool(), + available: ["agents_list"], + }, + { + name: "conversations_send", + description: createConversationsSendTool().description, + available: ["conversations_list"], + }, + ])( + "preserves ownership metadata when replacing $name descriptions", + ({ name, description, available }) => { + const originalTool = { + name, + description, + } as AnyAgentTool; + setPluginToolMeta(originalTool, { pluginId: "example", optional: false }); + setChannelAgentToolMeta(originalTool as never, { channelId: "example-channel" }); - const [updated] = applyToolAvailabilityDescriptions([processTool]); + const [updated] = applyToolAvailabilityDescriptions([ + originalTool, + ...available.map( + (toolName) => ({ name: toolName, description: "available" }) as AnyAgentTool, + ), + ]); - expect(updated).not.toBe(processTool); - expect(getPluginToolMeta(expectDefined(updated, "updated test invariant"))).toEqual({ - pluginId: "example", - optional: false, - }); - expect(getChannelAgentToolMeta(updated as never)).toEqual({ - channelId: "example-channel", - }); - }); + expect(updated).not.toBe(originalTool); + expect(getPluginToolMeta(expectDefined(updated, "updated test invariant"))).toEqual({ + pluginId: "example", + optional: false, + }); + expect(getChannelAgentToolMeta(updated as never)).toEqual({ + channelId: "example-channel", + }); + }, + ); it("mentions sessions_spawn only when it survives tool filtering", () => { const withoutSpawn = applyToolAvailabilityDescriptions([ @@ -83,4 +119,153 @@ describe("createOpenClawCodingTools availability guidance", () => { expect(tool.description).toContain("sessions_spawn"); } }); + + it.each([ + { + name: "sessions_send", + description: describeSessionsSendTool(), + unavailable: ["conversations_list", "conversations_send", "conversations_turn"], + }, + { + name: "sessions_search", + description: describeSessionsSearchTool(), + unavailable: ["sessions_history"], + }, + { + name: "sessions_spawn", + description: describeSessionsSpawnTool({ swarmEnabled: true }), + unavailable: ["agents_list", "agents_wait", "subagents", "sessions_history"], + }, + { + name: "conversations_send", + description: createConversationsSendTool().description, + unavailable: ["conversations_list"], + }, + ])( + "does not advertise unavailable follow-up tools from $name", + ({ name, description, unavailable }) => { + const [tool] = applyToolAvailabilityDescriptions([{ name, description } as AnyAgentTool]); + + for (const unavailableTool of unavailable) { + expect(tool?.description).not.toContain(unavailableTool); + } + }, + ); + + it.each([ + { available: [], expected: [] }, + { available: ["conversations_list"], expected: [] }, + { available: ["conversations_send"], expected: [] }, + { + available: ["conversations_list", "conversations_send"], + expected: ["conversations_list", "conversations_send"], + }, + { + available: ["conversations_list", "conversations_turn"], + expected: ["conversations_list", "conversations_turn"], + }, + { + available: ["conversations_list", "conversations_send", "conversations_turn"], + expected: ["conversations_list", "conversations_send", "conversations_turn"], + }, + ])("describes only executable conversation routes: $available", ({ available, expected }) => { + const [tool] = applyToolAvailabilityDescriptions([ + { name: "sessions_send", description: describeSessionsSendTool() }, + ...available.map((name) => ({ name, description: "available" })), + ] as AnyAgentTool[]); + + for (const name of ["conversations_list", "conversations_send", "conversations_turn"]) { + expect(tool?.description.includes(name)).toBe(expected.includes(name)); + } + }); + + it("preserves the existing fully authorized session-send description byte for byte", () => { + const [tool] = applyToolAvailabilityDescriptions([ + { name: "sessions_send", description: describeSessionsSendTool() }, + { name: "conversations_list", description: "list" }, + { name: "conversations_send", description: "send" }, + { name: "conversations_turn", description: "turn" }, + ] as AnyAgentTool[]); + + expect(tool?.description).toBe( + [ + "Run a visible session on this Gateway by sessionKey/label, or a configured local agent by agentId; sessionKey wins redundant label.", + "A session identifies model context, not an external address; its reply may still announce through established delivery context.", + "For an exact external destination, use `conversations_list` plus `conversations_send`/`conversations_turn`.", + 'Thread chats rejected: target parent channel. Missing configured-agent main created. Waits for reply when available; status "no_reply" is terminal, so do not wait for an announcement.', + "watch:true: notice arrives when others later change target session.", + ].join(" "), + ); + }); + + it("keeps authorized history guidance and the prepared session URL", () => { + const sessionLinkBase = "https://gateway.example/control"; + const [tool] = applyToolAvailabilityDescriptions([ + { + name: "sessions_search", + description: describeSessionsSearchTool({ sessionLinkBase }), + }, + { name: "sessions_history", description: "history" }, + ] as AnyAgentTool[]); + + expect(tool?.description).toContain("sessions_history"); + expect(tool?.description).toContain(`${sessionLinkBase}/chat/`); + expect(tool?.description.indexOf("Follow up with sessions_history")).toBeLessThan( + tool?.description.indexOf("When pointing the user at a session") ?? Infinity, + ); + }); + + it("restores conversation lookup guidance only when lookup is authorized", () => { + const [tool] = applyToolAvailabilityDescriptions([ + createConversationsSendTool(), + { name: "conversations_list", description: "lookup" } as AnyAgentTool, + ]); + + expect(tool?.description).toBe( + "Send directly through a conversationRef from conversations_list. This performs channel delivery; it does not run the local agent in the backing session.", + ); + }); + + it("keeps only authorized spawn follow-ups without losing prepared runtime facts", () => { + const [tool] = applyToolAvailabilityDescriptions([ + { + name: "sessions_spawn", + description: describeSessionsSpawnTool({ + acpAvailable: false, + threadAvailable: true, + sessionToolsVisibility: "self", + swarmEnabled: true, + }), + }, + { name: "agents_list", description: "agent lookup" }, + { name: "sessions_history", description: "history" }, + ] as AnyAgentTool[]); + + expect(tool?.description).toContain("configured agent (see agents_list);"); + expect(tool?.description).toContain("sessions_history"); + expect(tool?.description).not.toContain("agents_wait"); + expect(tool?.description).not.toContain("subagents"); + expect(tool?.description).toContain("persistent/thread-bound"); + expect(tool?.description).toContain("(self: current session only)"); + expect(tool?.description).not.toContain('runtime="acp"'); + }); + + it("preserves original inline spawn guidance when every follow-up remains available", () => { + const [tool] = applyToolAvailabilityDescriptions([ + { + name: "sessions_spawn", + description: describeSessionsSpawnTool({ acpAvailable: false, swarmEnabled: true }), + }, + ...["agents_list", "agents_wait", "subagents", "sessions_history"].map((name) => ({ + name, + description: "available", + })), + ] as AnyAgentTool[]); + + expect(tool?.description).toContain("configured agent (see agents_list);"); + expect(tool?.description).toContain("`groupId` groups a batch; await with agents_wait."); + expect(tool?.description).toContain( + "No spawn for quick lookup/single read. Check spawns via `subagents`/`sessions_history`. After spawn,", + ); + }); }); diff --git a/src/agents/agent-tools.deferred-followup.ts b/src/agents/agent-tools.deferred-followup.ts index 80030081d326..a7cd18b68f6d 100644 --- a/src/agents/agent-tools.deferred-followup.ts +++ b/src/agents/agent-tools.deferred-followup.ts @@ -10,14 +10,75 @@ function replaceDescription(tool: AnyAgentTool, description: string): AnyAgentTo return copyAgentToolMetadata(tool, updated); } +const SESSION_TOOL_FOLLOWUPS = [ + [ + "sessions_search", + "sessions_history", + "Search your own past sessions for matching user and assistant text.", + "Search your own past sessions for matching user and assistant text. Follow up with sessions_history using a returned sessionKey, sessionId, and messageId for neighboring context.", + ], + [ + "conversations_send", + "conversations_list", + "through a conversationRef.", + "through a conversationRef from conversations_list.", + ], + ["sessions_spawn", "agents_list", "configured agent;", "configured agent (see agents_list);"], + [ + "sessions_spawn", + "agents_wait", + "`groupId` groups a batch.", + "`groupId` groups a batch; await with agents_wait.", + ], +] as const; + +function describeAvailableSessionTool( + tool: AnyAgentTool, + availableTools: ReadonlySet, +): string { + let description = tool.description; + // Preserve byte-stable default prompt placement while gating every named sibling. + for (const [sourceTool, requiredTool, original, expanded] of SESSION_TOOL_FOLLOWUPS) { + if (sourceTool === tool.name && availableTools.has(requiredTool)) { + description = description.replace(original, expanded); + } + } + if (tool.name === "sessions_send") { + const deliveryTools = ["conversations_send", "conversations_turn"].filter((name) => + availableTools.has(name), + ); + if (availableTools.has("conversations_list") && deliveryTools.length > 0) { + const guidance = `For an exact external destination, use \`conversations_list\` plus ${deliveryTools.map((name) => `\`${name}\``).join("/")}.`; + description = description.replace( + " Thread chats rejected:", + ` ${guidance} Thread chats rejected:`, + ); + } + } + if (tool.name === "sessions_spawn") { + const statusTools = ["subagents", "sessions_history"].filter((name) => + availableTools.has(name), + ); + if (statusTools.length > 0) { + const guidance = statusTools.map((name) => `\`${name}\``).join("/"); + description = description.replace( + "No spawn for quick lookup/single read.", + `No spawn for quick lookup/single read. Check spawns via ${guidance}.`, + ); + } + } + return description; +} + /** Return tools with cross-tool guidance adjusted for the tools that survived filtering. */ export function applyToolAvailabilityDescriptions( tools: AnyAgentTool[], params?: { agentId?: string }, ): AnyAgentTool[] { + const availableTools = new Set(tools.map((tool) => tool.name)); const hasCronTool = tools.some((tool) => isAutomationsToolName(tool.name)); - const hasProcessTool = tools.some((tool) => tool.name === "process"); - const hasSessionsSpawnTool = tools.some((tool) => tool.name === "sessions_spawn"); + const hasProcessTool = availableTools.has("process"); + const hasSessionsSpawnTool = availableTools.has("sessions_spawn"); return tools.map((tool) => { if (tool.name === "exec") { return replaceDescription( @@ -34,6 +95,7 @@ export function applyToolAvailabilityDescriptions( if (tool.name === "agents_wait") { return replaceDescription(tool, describeAgentsWaitTool(hasSessionsSpawnTool)); } - return tool; + const description = describeAvailableSessionTool(tool, availableTools); + return description === tool.description ? tool : replaceDescription(tool, description); }); } diff --git a/src/agents/openclaw-tools.subagents.sessions-spawn.allowlist.test.ts b/src/agents/openclaw-tools.subagents.sessions-spawn.allowlist.test.ts index c8ab4f27bbea..0673fec0314e 100644 --- a/src/agents/openclaw-tools.subagents.sessions-spawn.allowlist.test.ts +++ b/src/agents/openclaw-tools.subagents.sessions-spawn.allowlist.test.ts @@ -221,6 +221,7 @@ describe("subagent spawn allowlist + sandbox guards", () => { const result = await spawn({}); expectStatus(result, "forbidden"); expect(result.error ?? "").toContain("sessions_spawn requires explicit agentId"); + expect(result.error ?? "").not.toContain("agents_list"); expect(hoisted.callGatewayMock).not.toHaveBeenCalled(); }); @@ -253,19 +254,16 @@ describe("subagent spawn allowlist + sandbox guards", () => { name: "rejects malformed agentId strings before any gateway work", agentId: "Agent not found: xyz", extraAgents: [{ id: "research" }], - mentionsAgentsList: true, }, { name: "rejects agentId containing path separators", agentId: "../../../etc/passwd", extraAgents: [], - mentionsAgentsList: false, }, { name: "rejects agentId exceeding 64 characters", agentId: "a".repeat(65), extraAgents: [], - mentionsAgentsList: false, }, ])("$name", async (row) => { setConfig({ @@ -276,9 +274,7 @@ describe("subagent spawn allowlist + sandbox guards", () => { const result = await spawn({ agentId: row.agentId }); expectStatus(result, "error"); expect(result.error ?? "").toContain("Invalid agentId"); - if (row.mentionsAgentsList) { - expect(result.error ?? "").toContain("agents_list"); - } + expect(result.error ?? "").not.toContain("agents_list"); expect(hoisted.callGatewayMock).not.toHaveBeenCalled(); }); diff --git a/src/agents/spawn-plan.ts b/src/agents/spawn-plan.ts index 3b988a6b58bc..3eedd85cdf32 100644 --- a/src/agents/spawn-plan.ts +++ b/src/agents/spawn-plan.ts @@ -154,7 +154,7 @@ function buildThreadBindingUnavailableError(kind: SpawnBackendKind, mode: SpawnM return ( 'sessions_spawn(mode="session") is only available on channels that expose thread bindings (e.g. Discord threads, Slack threads, Telegram forum topics). ' + "This request is not running on a channel that can bind a subagent thread. " + - 'Use mode="run" for one-shot subagent work, or sessions_send(sessionKey=...) to keep talking to a persistent session without thread binding.' + 'Use mode="run" for one-shot subagent work.' ); } return ( @@ -354,7 +354,7 @@ export function resolveSpawnAdmission(params: { return { ok: false, error: - "sessions_spawn requires explicit agentId when requireAgentId is configured. Use agents_list to see allowed agent ids.", + "sessions_spawn requires explicit agentId when requireAgentId is configured. Provide an allowed configured agentId.", }; } const targetPolicy = resolveSubagentTargetPolicy({ diff --git a/src/agents/subagents/spawn/subagent-spawn-request.ts b/src/agents/subagents/spawn/subagent-spawn-request.ts index d5937b28f60c..36e38978c01d 100644 --- a/src/agents/subagents/spawn/subagent-spawn-request.ts +++ b/src/agents/subagents/spawn/subagent-spawn-request.ts @@ -91,7 +91,7 @@ export function resolveSubagentSpawnRequest( if (requestedAgentId && !isValidAgentId(requestedAgentId)) { return rejectSubagentSpawnRequest( "error", - `Invalid agentId "${requestedAgentId}". Agent IDs must match [a-z0-9][a-z0-9_-]{0,63}. Use agents_list to discover valid targets.`, + `Invalid agentId "${requestedAgentId}". Agent IDs must match [a-z0-9][a-z0-9_-]{0,63}.`, ); } const requestThreadBinding = params.thread === true; @@ -109,7 +109,7 @@ export function resolveSubagentSpawnRequest( return rejectSubagentSpawnRequest( "error", 'sessions_spawn(mode="session") requires thread=true so the subagent can stay bound to a channel thread. ' + - 'Retry with { mode: "session", thread: true } on a channel that supports threads, use mode="run" for one-shot work, or use sessions_send(sessionKey=...) to keep talking to a persistent session without thread binding.', + 'Retry with { mode: "session", thread: true } on a channel that supports threads, or use mode="run" for one-shot work.', ); } const cleanup = diff --git a/src/agents/subagents/spawn/subagent-spawn.mode-session-diagnostics.test.ts b/src/agents/subagents/spawn/subagent-spawn.mode-session-diagnostics.test.ts index 4aec4c3791dc..66b1099b3c0f 100644 --- a/src/agents/subagents/spawn/subagent-spawn.mode-session-diagnostics.test.ts +++ b/src/agents/subagents/spawn/subagent-spawn.mode-session-diagnostics.test.ts @@ -38,7 +38,7 @@ describe('spawnSubagentDirect mode="session" diagnostics (#67400)', () => { if (result.status === "error") { expect(result.error).toContain("thread: true"); expect(result.error).toContain('mode="run"'); - expect(result.error).toContain("sessions_send"); + expect(result.error).not.toContain("sessions_send"); } }); @@ -60,7 +60,7 @@ describe('spawnSubagentDirect mode="session" diagnostics (#67400)', () => { if (result.status === "error") { expect(result.error).toContain("not running on a channel"); expect(result.error).toContain('mode="run"'); - expect(result.error).toContain("sessions_send"); + expect(result.error).not.toContain("sessions_send"); } }); }); @@ -96,7 +96,7 @@ describe('spawnSubagentDirect mode="session" with thread binding-capable channel if (result.status === "error") { expect(result.error).toContain("thread: true"); expect(result.error).toContain('mode="run"'); - expect(result.error).toContain("sessions_send"); + expect(result.error).not.toContain("sessions_send"); } }); }); diff --git a/src/agents/tool-description-presets.test.ts b/src/agents/tool-description-presets.test.ts index e26874e66043..cd57c4489297 100644 --- a/src/agents/tool-description-presets.test.ts +++ b/src/agents/tool-description-presets.test.ts @@ -26,8 +26,7 @@ const SESSION_DESCRIPTIONS = [ { tool: "sessions_search", describe: describeSessionsSearchTool, - original: - "Search your own past sessions for matching user and assistant text. Follow up with sessions_history using a returned sessionKey, sessionId, and messageId for neighboring context.", + original: "Search your own past sessions for matching user and assistant text.", }, ] as const; @@ -48,7 +47,7 @@ describe("sessions_send tool description", () => { expect(SESSIONS_SEND_TOOL_DISPLAY_SUMMARY).toContain("same-Gateway"); expect(describeSessionsSendTool()).toContain("on this Gateway"); expect(describeSessionsSendTool()).toContain("not an external address"); - expect(describeSessionsSendTool()).toContain("`conversations_send`/`conversations_turn`"); + expect(describeSessionsSendTool()).not.toContain("conversations_"); expect(describeSessionsSendTool()).toContain("reply may still announce"); }); }); diff --git a/src/agents/tool-description-presets.ts b/src/agents/tool-description-presets.ts index 2188c09bd016..c0af230be5a9 100644 --- a/src/agents/tool-description-presets.ts +++ b/src/agents/tool-description-presets.ts @@ -86,7 +86,6 @@ export function describeSessionsHistoryTool(options?: SessionLinkDescriptionOpti export function describeSessionsSearchTool(options?: SessionLinkDescriptionOptions): string { return [ "Search your own past sessions for matching user and assistant text.", - "Follow up with sessions_history using a returned sessionKey, sessionId, and messageId for neighboring context.", ...(options?.sessionLinkBase ? [describeSessionLinkRule(options.sessionLinkBase)] : []), ].join(" "); } @@ -96,7 +95,6 @@ export function describeSessionsSendTool(): string { return [ "Run a visible session on this Gateway by sessionKey/label, or a configured local agent by agentId; sessionKey wins redundant label.", "A session identifies model context, not an external address; its reply may still announce through established delivery context.", - "For an exact external destination, use `conversations_list` plus `conversations_send`/`conversations_turn`.", 'Thread chats rejected: target parent channel. Missing configured-agent main created. Waits for reply when available; status "no_reply" is terminal, so do not wait for an announcement.', "watch:true: notice arrives when others later change target session.", ].join(" "); @@ -131,12 +129,12 @@ export function describeSessionsSpawnTool(options?: { options?.threadAvailable ? '`mode="run"` one-shot; `mode="session"` persistent/thread-bound only on supporting requester channel.' : '`mode="run"` one-shot background.', - "`agentId` targets a configured agent (see agents_list); `model` overrides its model; `cleanup` delete|keep hidden child session; `sandbox` inherit|require.", + "`agentId` targets a configured agent; `model` overrides its model; `cleanup` delete|keep hidden child session; `sandbox` inherit|require.", '`visible=true`: durable visible session. Default for coding, multi-step work, or results user may revisit/steer/keep — not only when a thread is requested. Shows in web UI sidebar; works without UI: completion announces back, progress checkable. `category` explicitly groups it; omission or an empty string leaves it ungrouped. Subagent only; omit `mode` (no `mode="run"`), `thread`, `thinking`, `lightContext`, `attachments`, `attachAs`; inherits the caller tool-policy ceiling; may check out a git worktree via `worktree`/`worktreeName`/`worktreeBaseRef`. When its accepted result includes `sessionUrl`, channel acknowledgements put the session URL on the first line and `Owner: