Skip to content

Commit 79d2e56

Browse files
steipetesteipete
authored
refactor(agents): require one resolved capability profile per run (#99817)
applyFinalEffectiveToolPolicy and collectAttemptExplicitToolAllowlistSources took ~15 raw identity params each and silently re-resolved a second capability profile when callers forgot to pass one — with divergent session inputs (attempt allowlist sources resolved policy off the live session key while tool construction used the sandbox policy key). The profile param is now required, the fallback resolution stacks are deleted, and the attempt threads its single resolved profile into tool construction, the final bundled pass, and allowlist sources. Doctor and gateway tools.effective callers resolve their config-scope profile explicitly at the callsite. createOpenClawCodingTools keeps its optional self-resolve as public plugin-SDK convenience.
1 parent 23b878a commit 79d2e56

7 files changed

Lines changed: 95 additions & 176 deletions

File tree

‎src/agents/agent-bundle-mcp-tools.request-boundary.test.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
materializeBundleMcpToolsForRun,
77
} from "./agent-bundle-mcp-materialize.js";
88
import type { McpCatalogTool, SessionMcpRuntime } from "./agent-bundle-mcp-types.js";
9+
import { resolveConversationCapabilityProfile } from "./conversation-capability-profile.js";
910
import { applyFinalEffectiveToolPolicy } from "./embedded-agent-runner/effective-tool-policy.js";
1011
import { splitSdkTools } from "./embedded-agent-runner/tool-split.js";
1112

@@ -87,6 +88,7 @@ async function buildConfiguredMcpToolNamesAtRequestBoundary(params: {
8788
const filtered = applyFinalEffectiveToolPolicy({
8889
bundledTools: runtime.tools,
8990
config: params.cfg,
91+
conversationCapabilityProfile: resolveConversationCapabilityProfile({ config: params.cfg }),
9092
warn: () => {},
9193
});
9294
const { customTools } = splitSdkTools({ tools: filtered, sandboxEnabled: false });
@@ -172,9 +174,11 @@ describe("configured MCP tools reach the request boundary (#76063)", () => {
172174
toolNames: ["zeta_tool", "alpha_tool", "mu_tool"],
173175
}),
174176
});
177+
const cfg: OpenClawConfig = { tools: { profile: "coding" } };
175178
const filtered = applyFinalEffectiveToolPolicy({
176179
bundledTools: runtime.tools,
177-
config: { tools: { profile: "coding" } },
180+
config: cfg,
181+
conversationCapabilityProfile: resolveConversationCapabilityProfile({ config: cfg }),
178182
warn: () => {},
179183
});
180184
const { customTools } = splitSdkTools({ tools: filtered, sandboxEnabled: false });

‎src/agents/embedded-agent-runner/compact.ts‎

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -992,27 +992,9 @@ async function compactEmbeddedAgentSessionDirectOnce(
992992
const filteredBundledTools = applyFinalEffectiveToolPolicy({
993993
bundledTools: [...(bundleMcpRuntime?.tools ?? []), ...(bundleLspRuntime?.tools ?? [])],
994994
config: params.config,
995-
sandboxToolPolicy: sandbox?.tools,
996-
sessionKey: sandboxSessionKey,
997-
// Intentionally omit explicit agentId: the core tools just built with
998-
// createOpenClawCodingTools(...) also omit it, so both paths resolve
999-
// agentId the same way via resolveAgentIdFromSessionKey(sessionKey).
1000-
// Passing effectiveSkillAgentId here would diverge from the core-tool
1001-
// policy for legacy/non-agent session keys where the two sources fall
1002-
// back to different ids.
1003-
modelProvider: model.provider,
1004-
modelId,
1005-
messageProvider: resolvedMessageProvider,
1006-
agentAccountId: params.agentAccountId,
1007-
groupId: params.groupId,
1008-
groupChannel: params.groupChannel,
1009-
groupSpace: params.groupSpace,
1010-
spawnedBy: params.spawnedBy,
1011-
senderId: params.senderId,
1012-
senderName: params.senderName,
1013-
senderUsername: params.senderUsername,
1014-
senderE164: params.senderE164,
1015-
senderIsOwner: params.senderIsOwner,
995+
// The same profile constructed the core tool set above, so core and
996+
// bundled tools cannot disagree about policy inputs (agentId included:
997+
// both resolve it from the session key inside the profile).
1016998
conversationCapabilityProfile: runtimeCapabilityProfile,
1017999
warn: (message) => log.warn(message),
10181000
});

‎src/agents/embedded-agent-runner/effective-tool-policy.test.ts‎

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,12 @@ import fs from "node:fs";
33
import os from "node:os";
44
import path from "node:path";
55
import { describe, expect, it } from "vitest";
6+
import type { OpenClawConfig } from "../../config/types.openclaw.js";
67
import { setPluginToolMeta } from "../../plugins/tools.js";
8+
import {
9+
type ConversationCapabilityProfileParams,
10+
resolveConversationCapabilityProfile,
11+
} from "../conversation-capability-profile.js";
712
import type { AnyAgentTool } from "../tools/common.js";
813
import { applyFinalEffectiveToolPolicy } from "./effective-tool-policy.js";
914

@@ -19,9 +24,33 @@ function makeTool(name: string): AnyAgentTool {
1924
};
2025
}
2126

27+
// Mirrors the production composition: resolve the conversation capability
28+
// profile from server-verified inputs, then apply the final bundled pass.
29+
function applyFinalPolicy(
30+
params: {
31+
bundledTools: AnyAgentTool[];
32+
config?: OpenClawConfig;
33+
warn?: (message: string) => void;
34+
} & Pick<
35+
ConversationCapabilityProfileParams,
36+
"sessionKey" | "messageProvider" | "senderId" | "groupId" | "groupChannel"
37+
>,
38+
): AnyAgentTool[] {
39+
const { bundledTools, config, warn, ...profileParams } = params;
40+
return applyFinalEffectiveToolPolicy({
41+
bundledTools,
42+
config,
43+
conversationCapabilityProfile: resolveConversationCapabilityProfile({
44+
config,
45+
...profileParams,
46+
}),
47+
warn: warn ?? (() => {}),
48+
});
49+
}
50+
2251
describe("applyFinalEffectiveToolPolicy", () => {
2352
it("filters bundled tools through the configured allowlist", () => {
24-
const filtered = applyFinalEffectiveToolPolicy({
53+
const filtered = applyFinalPolicy({
2554
bundledTools: [makeTool("mcp__bundle__fs_delete"), makeTool("mcp__bundle__fs_read")],
2655
config: { tools: { allow: ["mcp__bundle__fs_read"] } },
2756
warn: () => {},
@@ -55,7 +84,7 @@ describe("applyFinalEffectiveToolPolicy", () => {
5584
"utf-8",
5685
);
5786

58-
const filtered = applyFinalEffectiveToolPolicy({
87+
const filtered = applyFinalPolicy({
5988
bundledTools: [makeTool("mcp__bundle__fs_delete"), makeTool("mcp__bundle__fs_read")],
6089
config: {
6190
session: {
@@ -96,7 +125,7 @@ describe("applyFinalEffectiveToolPolicy", () => {
96125
setPluginToolMeta(deniedTool, { pluginId: "bundle-mcp", optional: false });
97126
setPluginToolMeta(allowedTool, { pluginId: "bundle-mcp", optional: false });
98127

99-
const filtered = applyFinalEffectiveToolPolicy({
128+
const filtered = applyFinalPolicy({
100129
bundledTools: [deniedTool, allowedTool],
101130
config: {
102131
session: {
@@ -120,7 +149,7 @@ describe("applyFinalEffectiveToolPolicy", () => {
120149
it("applies channel-normalized per-sender policy to bundled tools", () => {
121150
// Teams normalizes to msteams in policy keys, which must happen before
122151
// sender-specific deny rules are applied.
123-
const filtered = applyFinalEffectiveToolPolicy({
152+
const filtered = applyFinalPolicy({
124153
bundledTools: [makeTool("mcp__bundle__exec"), makeTool("mcp__bundle__read")],
125154
config: {
126155
tools: {
@@ -138,7 +167,7 @@ describe("applyFinalEffectiveToolPolicy", () => {
138167
});
139168

140169
it("returns the empty array unchanged when there are no bundled tools", () => {
141-
const filtered = applyFinalEffectiveToolPolicy({
170+
const filtered = applyFinalPolicy({
142171
bundledTools: [],
143172
config: { tools: { allow: ["message"] } },
144173
warn: () => {},
@@ -149,7 +178,7 @@ describe("applyFinalEffectiveToolPolicy", () => {
149178

150179
it("drops caller-provided groupId when it disagrees with session-derived group context", () => {
151180
const warnings: string[] = [];
152-
applyFinalEffectiveToolPolicy({
181+
applyFinalPolicy({
153182
bundledTools: [makeTool("mcp__bundle__read")],
154183
// Session key encodes a concrete group (discord room 111); caller tries
155184
// to override with a different group id so a more permissive group
@@ -167,7 +196,7 @@ describe("applyFinalEffectiveToolPolicy", () => {
167196

168197
it("drops caller-provided groupId when session encodes no group context (fail-closed)", () => {
169198
const warnings: string[] = [];
170-
applyFinalEffectiveToolPolicy({
199+
applyFinalPolicy({
171200
bundledTools: [makeTool("mcp__bundle__read")],
172201
// Direct/non-group session key: no session-derived group ids. A caller
173202
// supplying a groupId here has no server-verified ground truth; it
@@ -185,7 +214,7 @@ describe("applyFinalEffectiveToolPolicy", () => {
185214

186215
it("leaves groupId untouched when caller did not supply one", () => {
187216
const warnings: string[] = [];
188-
applyFinalEffectiveToolPolicy({
217+
applyFinalPolicy({
189218
bundledTools: [makeTool("mcp__bundle__read")],
190219
sessionKey: "agent:alice:main",
191220
warn: (message) => warnings.push(message),
@@ -198,7 +227,7 @@ describe("applyFinalEffectiveToolPolicy", () => {
198227

199228
it("does not emit unknown-entry warnings for core tool allowlists in the bundled pass", () => {
200229
const warnings: string[] = [];
201-
applyFinalEffectiveToolPolicy({
230+
applyFinalPolicy({
202231
bundledTools: [makeTool("mcp__bundle__read")],
203232
// Core tool names like `read` and `exec` are not in the bundled-only
204233
// input here, but they are valid core tools resolved by the first
@@ -212,7 +241,7 @@ describe("applyFinalEffectiveToolPolicy", () => {
212241

213242
it("still warns on genuinely unknown entries in the bundled pass", () => {
214243
const warnings: string[] = [];
215-
applyFinalEffectiveToolPolicy({
244+
applyFinalPolicy({
216245
bundledTools: [makeTool("mcp__bundle__read")],
217246
config: { tools: { allow: ["mcp__bundle__read", "totally-made-up-tool"] } },
218247
warn: (message) => warnings.push(message),
@@ -225,7 +254,7 @@ describe("applyFinalEffectiveToolPolicy", () => {
225254
const mcpTool = makeTool("bundleProbe__bundle_probe");
226255
setPluginToolMeta(mcpTool, { pluginId: "bundle-mcp", optional: false });
227256

228-
const filtered = applyFinalEffectiveToolPolicy({
257+
const filtered = applyFinalPolicy({
229258
bundledTools: [mcpTool],
230259
config: { tools: { profile: "coding" } },
231260
warn: () => {},
@@ -238,7 +267,7 @@ describe("applyFinalEffectiveToolPolicy", () => {
238267
const mcpTool = makeTool("bundleProbe__bundle_probe");
239268
setPluginToolMeta(mcpTool, { pluginId: "bundle-mcp", optional: false });
240269

241-
const filtered = applyFinalEffectiveToolPolicy({
270+
const filtered = applyFinalPolicy({
242271
bundledTools: [mcpTool],
243272
config: { tools: { profile: "coding", deny: ["bundle-mcp"] } },
244273
warn: () => {},

‎src/agents/embedded-agent-runner/effective-tool-policy.ts‎

Lines changed: 11 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,7 @@
33
*/
44
import type { OpenClawConfig } from "../../config/types.openclaw.js";
55
import { getPluginToolMeta } from "../../plugins/tools.js";
6-
import {
7-
resolveConversationCapabilityProfile,
8-
type ResolvedConversationCapabilityProfile,
9-
} from "../conversation-capability-profile.js";
6+
import type { ResolvedConversationCapabilityProfile } from "../conversation-capability-profile.js";
107
import { buildDeclaredToolAllowlistContext } from "../tool-policy-declared-context.js";
118
import {
129
applyToolPolicyPipeline,
@@ -17,14 +14,12 @@ import { collectExplicitDenylist, mergeAlsoAllowPolicy } from "../tool-policy.js
1714
import type { AnyAgentTool } from "../tools/common.js";
1815

1916
/**
20-
* Identity inputs used by `resolveGroupToolPolicy` to look up channel/group
21-
* tool policy. These fields are an authorization signal (they can widen
22-
* bundled-tool availability via a group-scoped allowlist), so callers MUST
23-
* pass values derived from server-verified session metadata (session key,
24-
* inbound transport event), not from tool-call or model-controlled input.
25-
* The helper cross-checks caller-provided `groupId` against session-derived
26-
* group ids and drops the caller value when they disagree, but it cannot
27-
* detect drift on fields that have no session-bound counterpart.
17+
* The capability profile is an authorization signal (group/sender policies can
18+
* widen bundled-tool availability), so callers MUST resolve it from
19+
* server-verified session metadata (session key, inbound transport event),
20+
* never from tool-call or model-controlled input. Passing the same profile
21+
* that constructed the core tool set keeps this final bundled-tool pass and
22+
* tool construction from ever disagreeing about policy inputs.
2823
*/
2924
type FinalEffectiveToolPolicyParams = {
3025
// Tools appended to the core tool set after `createOpenClawCodingTools()`
@@ -34,23 +29,7 @@ type FinalEffectiveToolPolicyParams = {
3429
// metadata no longer survives core-tool wrapping/normalization.
3530
bundledTools: AnyAgentTool[];
3631
config?: OpenClawConfig;
37-
sandboxToolPolicy?: { allow?: string[]; deny?: string[] };
38-
sessionKey?: string;
39-
agentId?: string;
40-
modelProvider?: string;
41-
modelId?: string;
42-
messageProvider?: string;
43-
agentAccountId?: string | null;
44-
groupId?: string | null;
45-
groupChannel?: string | null;
46-
groupSpace?: string | null;
47-
spawnedBy?: string | null;
48-
senderId?: string | null;
49-
senderName?: string | null;
50-
senderUsername?: string | null;
51-
senderE164?: string | null;
52-
senderIsOwner?: boolean;
53-
conversationCapabilityProfile?: ResolvedConversationCapabilityProfile;
32+
conversationCapabilityProfile: ResolvedConversationCapabilityProfile;
5433
warn: (message: string) => void;
5534
toolPolicyAuditLogLevel?: "info" | "debug";
5635
};
@@ -61,27 +40,7 @@ export function applyFinalEffectiveToolPolicy(
6140
if (params.bundledTools.length === 0) {
6241
return params.bundledTools;
6342
}
64-
const capabilityProfile =
65-
params.conversationCapabilityProfile ??
66-
resolveConversationCapabilityProfile({
67-
config: params.config,
68-
sessionKey: params.sessionKey,
69-
agentId: params.agentId,
70-
agentAccountId: params.agentAccountId,
71-
messageProvider: params.messageProvider,
72-
groupId: params.groupId,
73-
groupChannel: params.groupChannel,
74-
groupSpace: params.groupSpace,
75-
spawnedBy: params.spawnedBy,
76-
senderId: params.senderId,
77-
senderName: params.senderName,
78-
senderUsername: params.senderUsername,
79-
senderE164: params.senderE164,
80-
senderIsOwner: params.senderIsOwner,
81-
modelProvider: params.modelProvider,
82-
modelId: params.modelId,
83-
sandboxToolPolicy: params.sandboxToolPolicy,
84-
});
43+
const capabilityProfile = params.conversationCapabilityProfile;
8544
const { trustedGroup } = capabilityProfile.policy;
8645
// Resolve here for warnings and to strip caller-only group metadata before
8746
// this pass; resolveGroupToolPolicy re-checks internally for all callers.
@@ -104,6 +63,7 @@ export function applyFinalEffectiveToolPolicy(
10463
providerProfileAlsoAllow,
10564
groupPolicy,
10665
senderPolicy,
66+
sandboxPolicy,
10767
subagentPolicy,
10868
inheritedToolPolicy,
10969
} = capabilityProfile.policy;
@@ -138,7 +98,7 @@ export function applyFinalEffectiveToolPolicy(
13898
senderPolicy,
13999
agentId,
140100
}),
141-
{ policy: params.sandboxToolPolicy, label: "sandbox tools.allow" },
101+
{ policy: sandboxPolicy, label: "sandbox tools.allow" },
142102
{ policy: subagentPolicy, label: "subagent tools.allow" },
143103
{ policy: inheritedToolPolicy, label: "inherited tools" },
144104
].map((step) => Object.assign({}, step, { suppressUnavailableCoreToolWarning: true }));

0 commit comments

Comments
 (0)