From f4058528e5db44636778f4ff16d54dd006e1d76b Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 27 Jul 2026 16:51:20 -0400 Subject: [PATCH] refactor(agents): centralize message-tool and direct-visibility predicates (#114720) * refactor(agents): centralize message-tool and direct-visibility predicates * refactor(agents): unexport classifyTool and drop redundant union --- .../codex/src/app-server/dynamic-tools.ts | 3 +++ src/agents/agent-tools.ts | 5 ++--- .../run/attempt-tool-base-prepare.ts | 5 +++-- .../run/attempt-tool-catalog.ts | 5 +---- src/agents/harness/tool-surface-bridge.ts | 4 ++-- src/agents/local-model-lean.ts | 3 ++- src/agents/tool-search-catalog.ts | 21 ++++++++++++++++++- src/agents/tool-search-directory.ts | 20 +++++------------- src/agents/tool-search.test.ts | 5 ++++- src/agents/tool-search.ts | 16 ++------------ src/auto-reply/source-reply-delivery-mode.ts | 15 +++++++++++++ 11 files changed, 59 insertions(+), 43 deletions(-) create mode 100644 src/auto-reply/source-reply-delivery-mode.ts diff --git a/extensions/codex/src/app-server/dynamic-tools.ts b/extensions/codex/src/app-server/dynamic-tools.ts index 1122cc9bc2b2..8af6bec716c3 100644 --- a/extensions/codex/src/app-server/dynamic-tools.ts +++ b/extensions/codex/src/app-server/dynamic-tools.ts @@ -377,6 +377,9 @@ const CODEX_OPENCLAW_DYNAMIC_TOOL_NAMESPACE = "openclaw"; // Keep OpenClaw control-path tools directly callable even when Codex tool_search // is unavailable or resolves a connector-only universe. Developer instructions // still steer normal Codex subagents to native spawn_agent. +// sessions_yield is normally routed by its catalogMode "direct-only" before +// this set is consulted; the name entry stays as the metadata-independent +// contract that control-path tools remain directly callable. const ALWAYS_DIRECT_DYNAMIC_TOOL_NAMES = new Set([ "agents_list", "sessions_spawn", diff --git a/src/agents/agent-tools.ts b/src/agents/agent-tools.ts index 139d6f99fdd1..579214645b5d 100644 --- a/src/agents/agent-tools.ts +++ b/src/agents/agent-tools.ts @@ -9,6 +9,7 @@ import type { TaskSuggestionDeliveryMode, } from "../auto-reply/get-reply-options.types.js"; import { HEARTBEAT_RESPONSE_TOOL_NAME } from "../auto-reply/heartbeat-tool-response.js"; +import { messageToolOwnsVisibleReply } from "../auto-reply/source-reply-delivery-mode.js"; import type { ChatType } from "../channels/chat-type.js"; import type { InboundEventKind } from "../channels/inbound-event/kind.js"; import type { ModelCompatConfig } from "../config/types.models.js"; @@ -560,9 +561,7 @@ function createOpenClawCodingToolsInternal(options?: OpenClawCodingToolsOptions) sourceReplyDeliveryMode: options?.sourceReplyDeliveryMode, }); const runtimeProfileAlsoAllow = [ - ...(options?.forceMessageTool || options?.sourceReplyDeliveryMode === "message_tool_only" - ? ["message"] - : []), + ...(options && messageToolOwnsVisibleReply(options) ? ["message"] : []), ...(runtimeToolAllowlistIncludesMessage ? ["message"] : []), ...(forceHeartbeatTool ? [HEARTBEAT_RESPONSE_TOOL_NAME] : []), ...toolSearchControlAllowlist, diff --git a/src/agents/embedded-agent-runner/run/attempt-tool-base-prepare.ts b/src/agents/embedded-agent-runner/run/attempt-tool-base-prepare.ts index 947872a390ff..d2d85ab883ac 100644 --- a/src/agents/embedded-agent-runner/run/attempt-tool-base-prepare.ts +++ b/src/agents/embedded-agent-runner/run/attempt-tool-base-prepare.ts @@ -1,3 +1,4 @@ +import { messageToolOwnsVisibleReply } from "../../../auto-reply/source-reply-delivery-mode.js"; import type { DiagnosticTraceContext } from "../../../infra/diagnostic-trace-context.js"; import { extractModelCompat } from "../../../plugins/provider-model-compat.js"; import { getPluginToolMeta } from "../../../plugins/tools.js"; @@ -57,8 +58,7 @@ export function prepareEmbeddedAttemptToolBase(params: { toolSearchCatalogExecutor: ToolSearchCatalogToolExecutor; }) { const { attempt } = params; - const forceDirectMessageTool = - attempt.forceMessageTool === true || attempt.sourceReplyDeliveryMode === "message_tool_only"; + const forceDirectMessageTool = messageToolOwnsVisibleReply(attempt); const toolsAllowWithForcedRuntimeTools = mergeForcedEmbeddedAttemptToolsAllow( attempt.toolsAllow, { @@ -337,6 +337,7 @@ export function prepareEmbeddedAttemptToolBase(params: { computerContextEpoch, cronCreatorToolAllowlist, effectiveToolsAllow, + forceDirectMessageTool, inheritedToolAllowlist, localModelLeanEnabled, localModelLeanPreserveToolNames, diff --git a/src/agents/embedded-agent-runner/run/attempt-tool-catalog.ts b/src/agents/embedded-agent-runner/run/attempt-tool-catalog.ts index 658870165168..c873927e3638 100644 --- a/src/agents/embedded-agent-runner/run/attempt-tool-catalog.ts +++ b/src/agents/embedded-agent-runner/run/attempt-tool-catalog.ts @@ -93,10 +93,7 @@ export function prepareEmbeddedAttemptToolCatalog(input: { : []; // When the message tool is the only reply path it must stay directly visible // in every search mode; a hidden delivery tool can leave the run mute. - const requiredDirectToolNames = - attempt.forceMessageTool === true || attempt.sourceReplyDeliveryMode === "message_tool_only" - ? ["message"] - : []; + const requiredDirectToolNames = preparedToolBase.forceDirectMessageTool ? ["message"] : []; const toolSearch = codeModeControlsEnabledForRun ? applyCodeModeCatalog({ tools: [...codeModeTools, ...effectiveTools], diff --git a/src/agents/harness/tool-surface-bridge.ts b/src/agents/harness/tool-surface-bridge.ts index 8bd0c8531d63..20085fb73a90 100644 --- a/src/agents/harness/tool-surface-bridge.ts +++ b/src/agents/harness/tool-surface-bridge.ts @@ -1,3 +1,4 @@ +import { messageToolOwnsVisibleReply } from "../../auto-reply/source-reply-delivery-mode.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; import type { HookContext } from "../agent-tools.before-tool-call.js"; import { getActiveAgentRingZeroTools } from "../agent-tools.ring-zero-context.js"; @@ -79,8 +80,7 @@ export function createAgentHarnessToolSurfaceRuntime(params: { sourceReplyDeliveryMode?: string; toolsAllow?: readonly string[]; }): AgentHarnessToolSurfaceRuntime { - const forceDirectMessageTool = - params.forceMessageTool === true || params.sourceReplyDeliveryMode === "message_tool_only"; + const forceDirectMessageTool = messageToolOwnsVisibleReply(params); const codeModeConfig = resolveCodeModeConfig(params.config, params.agentId); const toolSearchRuntimeConfig = resolveAgentToolSearchRuntimeConfig({ config: params.config, diff --git a/src/agents/local-model-lean.ts b/src/agents/local-model-lean.ts index ea4cf230e293..24dde446c798 100644 --- a/src/agents/local-model-lean.ts +++ b/src/agents/local-model-lean.ts @@ -3,6 +3,7 @@ * Removes high-latency or channel-dependent tools for local models while * preserving explicitly required delivery tools. */ +import { messageToolOwnsVisibleReply } from "../auto-reply/source-reply-delivery-mode.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { normalizeAgentId, parseAgentSessionKey } from "../routing/session-key.js"; import { resolveAgentConfig, resolveDefaultAgentId } from "./agent-scope-config.js"; @@ -44,7 +45,7 @@ export function resolveLocalModelLeanPreserveToolNames(params?: { sourceReplyDeliveryMode?: string; }): string[] { const names = [...(params?.toolNames ?? [])]; - if (params?.forceMessageTool || params?.sourceReplyDeliveryMode === "message_tool_only") { + if (params && messageToolOwnsVisibleReply(params)) { names.push("message"); } return [...new Set(names)]; diff --git a/src/agents/tool-search-catalog.ts b/src/agents/tool-search-catalog.ts index 5ce56e2acf48..75d888eb0868 100644 --- a/src/agents/tool-search-catalog.ts +++ b/src/agents/tool-search-catalog.ts @@ -6,6 +6,7 @@ import { rewrapToolWithBeforeToolCallHook, wrapToolWithBeforeToolCallHook, } from "./agent-tools.before-tool-call.js"; +import { isCoreCodingSurfaceToolName } from "./core-tool-factory-descriptors.js"; import type { ToolDefinition } from "./sessions/index.js"; import { compactToolInputHint, compactToolOutputHint } from "./tool-schema-hints.js"; import { @@ -223,7 +224,7 @@ function rememberReusableCatalog(key: string | undefined, catalog: ToolSearchCat } } -export function classifyTool(tool: CatalogTool): { +function classifyTool(tool: CatalogTool): { source: CatalogSource; sourceName?: string; mcp?: PluginToolMcpMeta; @@ -285,6 +286,24 @@ function shouldCatalogTool(tool: AnyAgentTool): boolean { return !TOOL_SEARCH_CONTROL_TOOL_NAMES.has(tool.name) && tool.catalogMode !== "direct-only"; } +/** + * Core file/shell primitives and caller-required names (e.g. message when it is + * the only reply path) stay visible while remaining searchable. Both must + * resolve to trusted OpenClaw tools: an MCP lookalike must never become a + * direct delivery or core-coding tool. + */ +export function isDirectVisibleCatalogTool( + tool: AnyAgentTool, + directToolNames: ReadonlySet, +): boolean { + const classified = classifyTool(tool); + return ( + classified.source === "openclaw" && + (directToolNames.has(tool.name) || + (isCoreCodingSurfaceToolName(tool.name) && classified.sourceName === "core")) + ); +} + export function registerHeadlessToolSearchCatalog(params: { catalogRef: ToolSearchCatalogRef; tools: readonly AnyAgentTool[]; diff --git a/src/agents/tool-search-directory.ts b/src/agents/tool-search-directory.ts index bb50fb42e051..80cb9d6f41ca 100644 --- a/src/agents/tool-search-directory.ts +++ b/src/agents/tool-search-directory.ts @@ -1,10 +1,9 @@ import { normalizeStringEntries } from "@openclaw/normalization-core/string-normalization"; import { truncateUtf16Safe } from "@openclaw/normalization-core/utf16-slice"; -import { isCoreCodingSurfaceToolName } from "./core-tool-factory-descriptors.js"; import { applyToolCatalogCompaction, - classifyTool, collectUniqueCatalogToolNames, + isDirectVisibleCatalogTool, resolveCatalog, visibleCatalogEntries, } from "./tool-search-catalog.js"; @@ -63,19 +62,10 @@ export function applyToolSchemaDirectoryCatalog(params: { ...params, enabled: config.enabled, isVisibleControlTool: (tool) => TOOL_SCHEMA_DIRECTORY_CONTROL_TOOL_NAMES.has(tool.name), - // Required names must resolve to trusted OpenClaw tools; an MCP lookalike - // must never become a direct delivery or core-coding tool. - isVisibleCatalogTool: (tool) => { - if (!uniqueCatalogToolNames.has(tool.name)) { - return false; - } - const classified = classifyTool(tool); - return ( - classified.source === "openclaw" && - (directToolNames.has(tool.name) || - (isCoreCodingSurfaceToolName(tool.name) && classified.sourceName === "core")) - ); - }, + // The unique-name gate defers any cross-source name collision before the + // shared trust check runs. + isVisibleCatalogTool: (tool) => + uniqueCatalogToolNames.has(tool.name) && isDirectVisibleCatalogTool(tool, directToolNames), }); } diff --git a/src/agents/tool-search.test.ts b/src/agents/tool-search.test.ts index ee1bc46109b0..e695b05a41bb 100644 --- a/src/agents/tool-search.test.ts +++ b/src/agents/tool-search.test.ts @@ -2389,7 +2389,10 @@ describe("Tool Search", () => { const config = { tools: { - toolSearch: { enabled: true, mode: "code", codeTimeoutMs: 100 }, + // Generous timeout: the child process must have started the bridged call + // before the deadline fires, or the abort assertion races process spawn + // latency under machine load. + toolSearch: { enabled: true, mode: "code", codeTimeoutMs: 1500 }, }, } as never; applyToolSearchCatalog({ diff --git a/src/agents/tool-search.ts b/src/agents/tool-search.ts index 2dfd3b449cb6..6b15dc0b1369 100644 --- a/src/agents/tool-search.ts +++ b/src/agents/tool-search.ts @@ -3,13 +3,12 @@ import { normalizeStringEntries } from "@openclaw/normalization-core/string-norm import { Type } from "typebox"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import type { HookContext } from "./agent-tools.before-tool-call.js"; -import { isCoreCodingSurfaceToolName } from "./core-tool-factory-descriptors.js"; import type { AgentToolResult, AgentToolUpdateCallback } from "./runtime/index.js"; import type { ToolDefinition } from "./sessions/index.js"; import { addClientToolsToToolCatalog, applyToolCatalogCompaction, - classifyTool, + isDirectVisibleCatalogTool, reusableCatalogSnapshots, resolveCatalog, sessionCatalogs, @@ -112,18 +111,7 @@ export function applyToolSearchCatalog(params: { isVisibleControlTool: (tool) => TOOL_SEARCH_CONTROL_TOOL_NAMES.has(tool.name) && shouldExposeControlTool(tool.name, config.mode), - // Core file/shell primitives and caller-required names (e.g. message when it - // is the only reply path) stay visible while remaining searchable. Required - // names must resolve to trusted OpenClaw tools; an MCP lookalike must never - // become a direct delivery or core-coding tool. - isVisibleCatalogTool: (tool) => { - const classified = classifyTool(tool); - return ( - classified.source === "openclaw" && - (directToolNames.has(tool.name) || - (isCoreCodingSurfaceToolName(tool.name) && classified.sourceName === "core")) - ); - }, + isVisibleCatalogTool: (tool) => isDirectVisibleCatalogTool(tool, directToolNames), }); } diff --git a/src/auto-reply/source-reply-delivery-mode.ts b/src/auto-reply/source-reply-delivery-mode.ts new file mode 100644 index 000000000000..ff4d36125f6a --- /dev/null +++ b/src/auto-reply/source-reply-delivery-mode.ts @@ -0,0 +1,15 @@ +/** Canonical predicate for message-tool-owned visible replies. */ + +/** + * True when the visible source reply must flow through the message tool, either + * because the run forces it or because the delivery mode is message_tool_only. + * Consumers use this to keep the message tool visible/preserved: hiding the only + * reply path leaves the run mute. The mode is accepted as plain string because + * harness callers carry it untyped; only "message_tool_only" is meaningful here. + */ +export function messageToolOwnsVisibleReply(params: { + forceMessageTool?: boolean; + sourceReplyDeliveryMode?: string; +}): boolean { + return params.forceMessageTool === true || params.sourceReplyDeliveryMode === "message_tool_only"; +}