Skip to content

Commit 02d7ad4

Browse files
obviyusobviyus
committed
fix(agents): skip core tools for plugin-only allowlists
1 parent 9714eb3 commit 02d7ad4

4 files changed

Lines changed: 338 additions & 169 deletions

File tree

‎src/agents/pi-embedded-runner/run/attempt.spawn-workspace.test-support.ts‎

Lines changed: 36 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ type AttemptSpawnWorkspaceHoisted = {
6262
ensureGlobalUndiciEnvProxyDispatcherMock: UnknownMock;
6363
ensureGlobalUndiciStreamTimeoutsMock: UnknownMock;
6464
buildEmbeddedMessageActionDiscoveryInputMock: UnknownMock;
65+
createOpenClawCodingToolsMock: UnknownMock;
6566
subscribeEmbeddedPiSessionMock: Mock<SubscribeEmbeddedPiSessionFn>;
6667
acquireSessionWriteLockMock: Mock<AcquireSessionWriteLockFn>;
6768
installToolResultContextGuardMock: UnknownMock;
@@ -121,6 +122,7 @@ const hoisted = vi.hoisted((): AttemptSpawnWorkspaceHoisted => {
121122
const ensureGlobalUndiciEnvProxyDispatcherMock = vi.fn();
122123
const ensureGlobalUndiciStreamTimeoutsMock = vi.fn();
123124
const buildEmbeddedMessageActionDiscoveryInputMock = vi.fn((params: unknown) => params);
125+
const createOpenClawCodingToolsMock = vi.fn(() => []);
124126
const installToolResultContextGuardMock = vi.fn(() => () => {});
125127
const installContextEngineLoopHookMock = vi.fn(() => () => {});
126128
const flushPendingToolResultsAfterIdleMock = vi.fn(async () => {});
@@ -170,6 +172,7 @@ const hoisted = vi.hoisted((): AttemptSpawnWorkspaceHoisted => {
170172
ensureGlobalUndiciEnvProxyDispatcherMock,
171173
ensureGlobalUndiciStreamTimeoutsMock,
172174
buildEmbeddedMessageActionDiscoveryInputMock,
175+
createOpenClawCodingToolsMock,
173176
subscribeEmbeddedPiSessionMock,
174177
acquireSessionWriteLockMock,
175178
installToolResultContextGuardMock,
@@ -427,26 +430,8 @@ vi.mock("../../cache-trace.js", () => ({
427430
}));
428431

429432
vi.mock("../../pi-tools.js", () => ({
430-
createOpenClawCodingTools: (options?: { workspaceDir?: string; spawnWorkspaceDir?: string }) => [
431-
{
432-
name: "sessions_spawn",
433-
execute: async (
434-
_callId: string,
435-
input: { task?: string },
436-
_session?: unknown,
437-
_abortSignal?: unknown,
438-
_ctx?: unknown,
439-
) =>
440-
await hoisted.spawnSubagentDirectMock(
441-
{
442-
task: input.task ?? "",
443-
},
444-
{
445-
workspaceDir: options?.spawnWorkspaceDir ?? options?.workspaceDir,
446-
},
447-
),
448-
},
449-
],
433+
createOpenClawCodingTools: (options?: { workspaceDir?: string; spawnWorkspaceDir?: string }) =>
434+
hoisted.createOpenClawCodingToolsMock(options),
450435
resolveToolLoopDetectionConfig: () => undefined,
451436
}));
452437

@@ -526,6 +511,9 @@ vi.mock("../../tool-call-id.js", async (importOriginal) => {
526511
});
527512

528513
vi.mock("../../tool-fs-policy.js", () => ({
514+
createToolFsPolicy: (params: { workspaceOnly?: boolean }) => ({
515+
workspaceOnly: params.workspaceOnly === true,
516+
}),
529517
resolveEffectiveToolFsWorkspaceOnly: () => false,
530518
}));
531519

@@ -767,6 +755,34 @@ export function resetEmbeddedAttemptHarness(
767755
hoisted.buildEmbeddedMessageActionDiscoveryInputMock
768756
.mockReset()
769757
.mockImplementation((params) => params);
758+
hoisted.createOpenClawCodingToolsMock.mockReset().mockImplementation((...args: unknown[]) => {
759+
const options = args[0] as
760+
| {
761+
workspaceDir?: string;
762+
spawnWorkspaceDir?: string;
763+
}
764+
| undefined;
765+
return [
766+
{
767+
name: "sessions_spawn",
768+
execute: async (
769+
_callId: string,
770+
input: { task?: string },
771+
_session?: unknown,
772+
_abortSignal?: unknown,
773+
_ctx?: unknown,
774+
) =>
775+
await hoisted.spawnSubagentDirectMock(
776+
{
777+
task: input.task ?? "",
778+
},
779+
{
780+
workspaceDir: options?.spawnWorkspaceDir ?? options?.workspaceDir,
781+
},
782+
),
783+
},
784+
];
785+
});
770786
hoisted.subscribeEmbeddedPiSessionMock
771787
.mockReset()
772788
.mockImplementation(() => createSubscriptionMock());

‎src/agents/pi-embedded-runner/run/attempt.tools-allow-regression.test.ts‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
import { afterEach, beforeEach, describe, expect, it } from "vitest";
2+
import {
3+
cleanupTempPaths,
4+
createContextEngineAttemptRunner,
5+
getHoisted,
6+
resetEmbeddedAttemptHarness,
7+
} from "./attempt.spawn-workspace.test-support.js";
8+
9+
describe("runEmbeddedAttempt toolsAllow startup cost", () => {
10+
const tempPaths: string[] = [];
11+
12+
beforeEach(() => {
13+
resetEmbeddedAttemptHarness();
14+
});
15+
16+
afterEach(async () => {
17+
await cleanupTempPaths(tempPaths);
18+
});
19+
20+
it("keeps plugin-only allowlists on the shared tool policy path", async () => {
21+
const hoisted = getHoisted();
22+
hoisted.createOpenClawCodingToolsMock.mockReturnValue([
23+
{
24+
name: "memory_search",
25+
description: "search memory",
26+
parameters: { type: "object", properties: {} },
27+
execute: async () => "ok",
28+
},
29+
{
30+
name: "plugin_extra",
31+
description: "extra plugin tool",
32+
parameters: { type: "object", properties: {} },
33+
execute: async () => "ok",
34+
},
35+
]);
36+
37+
await createContextEngineAttemptRunner({
38+
contextEngine: {
39+
assemble: async ({ messages }) => ({ messages, estimatedTokens: 1 }),
40+
},
41+
attemptOverrides: {
42+
toolsAllow: ["memory_search"],
43+
},
44+
sessionKey: "agent:main:main",
45+
tempPaths,
46+
});
47+
48+
expect(hoisted.createOpenClawCodingToolsMock).toHaveBeenCalledWith(
49+
expect.objectContaining({
50+
includeCoreTools: false,
51+
runtimeToolAllowlist: ["memory_search"],
52+
}),
53+
);
54+
const createSessionOptions = hoisted.createAgentSessionMock.mock.calls[0]?.[0] as
55+
| { customTools?: { name: string }[] }
56+
| undefined;
57+
expect(createSessionOptions?.customTools?.map((tool) => tool.name)).toEqual(["memory_search"]);
58+
});
59+
});

‎src/agents/pi-embedded-runner/run/attempt.ts‎

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -496,6 +496,54 @@ export function applyEmbeddedAttemptToolsAllow<T extends { name: string }>(
496496
return tools.filter((tool) => allowSet.has(normalizeToolName(tool.name)));
497497
}
498498

499+
const CORE_CODING_TOOL_ALLOWLIST_NAMES = new Set([
500+
"agents_list",
501+
"apply_patch",
502+
"bash",
503+
"canvas",
504+
"cron",
505+
"edit",
506+
"exec",
507+
"gateway",
508+
"heartbeat_response",
509+
"image",
510+
"image_generate",
511+
"message",
512+
"music_generate",
513+
"nodes",
514+
"pdf",
515+
"read",
516+
"session_status",
517+
"sessions_history",
518+
"sessions_list",
519+
"sessions_send",
520+
"sessions_spawn",
521+
"sessions_yield",
522+
"subagents",
523+
"tts",
524+
"update_plan",
525+
"video_generate",
526+
"web_fetch",
527+
"web_search",
528+
"write",
529+
]);
530+
531+
function shouldBuildCoreCodingToolsForAllowlist(toolsAllow?: string[]): boolean {
532+
if (!toolsAllow || toolsAllow.length === 0) {
533+
return true;
534+
}
535+
return toolsAllow.some((toolName) => {
536+
const normalized = normalizeToolName(toolName);
537+
return (
538+
normalized === "*" ||
539+
normalized.startsWith("group:") ||
540+
normalized === "bundle-mcp" ||
541+
normalized.includes(TOOL_NAME_SEPARATOR) ||
542+
CORE_CODING_TOOL_ALLOWLIST_NAMES.has(normalized)
543+
);
544+
});
545+
}
546+
499547
export function normalizeMessagesForLlmBoundary(messages: AgentMessage[]): AgentMessage[] {
500548
const normalized = stripToolResultDetails(normalizeAssistantReplayContent(messages));
501549
return stripRuntimeContextCustomMessages(normalized);
@@ -696,6 +744,10 @@ export async function runEmbeddedAttempt(
696744
config: params.config,
697745
agentId: params.agentId,
698746
});
747+
const effectiveFsWorkspaceOnly = resolveAttemptFsWorkspaceOnly({
748+
config: params.config,
749+
sessionAgentId,
750+
});
699751
prepStages.mark("workspace-sandbox");
700752

701753
let restoreSkillEnv: (() => void) | undefined;
@@ -833,6 +885,7 @@ export async function runEmbeddedAttempt(
833885
currentChannelId: params.currentChannelId,
834886
currentThreadTs: params.currentThreadTs,
835887
currentMessageId: params.currentMessageId,
888+
includeCoreTools: shouldBuildCoreCodingToolsForAllowlist(params.toolsAllow),
836889
replyToMode: params.replyToMode,
837890
hasRepliedRef: params.hasRepliedRef,
838891
modelHasVision: params.model.input?.includes("image") ?? false,
@@ -942,10 +995,6 @@ export async function runEmbeddedAttempt(
942995
config: params.config,
943996
agentId: params.agentId,
944997
});
945-
const effectiveFsWorkspaceOnly = resolveAttemptFsWorkspaceOnly({
946-
config: params.config,
947-
sessionAgentId,
948-
});
949998
// Track sessions_yield tool invocation (callback pattern, like clientToolCallDetected)
950999
let yieldDetected = false;
9511000
let yieldMessage: string | null = null;

0 commit comments

Comments
 (0)