mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 09:20:43 +00:00
fix(subagents): explain browser tool profile filtering
This commit is contained in:
@@ -353,6 +353,41 @@ describe("createOpenClawCodingTools", () => {
|
||||
expect(names.has("browser")).toBe(false);
|
||||
});
|
||||
|
||||
it("keeps browser out of coding-profile subagents unless profile-stage alsoAllow adds it", () => {
|
||||
const baseConfig = {
|
||||
browser: { enabled: true },
|
||||
plugins: { entries: { browser: { enabled: true } } },
|
||||
tools: { profile: "coding" },
|
||||
} as OpenClawConfig;
|
||||
const codingSubagent = createOpenClawCodingTools({
|
||||
sessionKey: "agent:main:subagent:test",
|
||||
config: baseConfig,
|
||||
});
|
||||
const codingNames = new Set(codingSubagent.map((tool) => tool.name));
|
||||
expect(codingNames.has("browser")).toBe(false);
|
||||
|
||||
const subagentAllowOnly = createOpenClawCodingTools({
|
||||
sessionKey: "agent:main:subagent:test",
|
||||
config: {
|
||||
...baseConfig,
|
||||
tools: {
|
||||
profile: "coding",
|
||||
subagents: { tools: { allow: ["browser"] } },
|
||||
},
|
||||
} as OpenClawConfig,
|
||||
});
|
||||
expect(subagentAllowOnly.some((tool) => tool.name === "browser")).toBe(false);
|
||||
|
||||
const profileStageAlsoAllow = createOpenClawCodingTools({
|
||||
sessionKey: "agent:main:subagent:test",
|
||||
config: {
|
||||
...baseConfig,
|
||||
tools: { profile: "coding", alsoAllow: ["browser"] },
|
||||
} as OpenClawConfig,
|
||||
});
|
||||
expect(profileStageAlsoAllow.some((tool) => tool.name === "browser")).toBe(true);
|
||||
});
|
||||
|
||||
it("can keep message available when a cron route needs it under the coding profile", () => {
|
||||
const codingTools = createOpenClawCodingTools({
|
||||
config: { tools: { profile: "coding" } },
|
||||
|
||||
@@ -30,6 +30,7 @@ const coreTools = [
|
||||
stubActionTool("sessions_spawn", ["spawn", "handoff"]),
|
||||
stubActionTool("subagents", ["list", "show"]),
|
||||
stubActionTool("session_status", ["get", "show"]),
|
||||
stubActionTool("browser", ["status", "snapshot"]),
|
||||
stubTool("tts"),
|
||||
stubTool("image_generate"),
|
||||
stubTool("video_generate"),
|
||||
|
||||
@@ -13,6 +13,7 @@ describe("tool-catalog", () => {
|
||||
expect(policy!.allow).toContain("music_generate");
|
||||
expect(policy!.allow).toContain("video_generate");
|
||||
expect(policy!.allow).toContain("update_plan");
|
||||
expect(policy!.allow).not.toContain("browser");
|
||||
});
|
||||
|
||||
it("includes bundle MCP tools in coding and messaging profile policies", () => {
|
||||
|
||||
@@ -262,6 +262,50 @@ describe("resolveEffectiveToolInventory", () => {
|
||||
expect(result.profile).toBe("coding");
|
||||
});
|
||||
|
||||
it("adds an actionable notice when configured browser is filtered by the tool profile", async () => {
|
||||
const { resolveEffectiveToolInventory } = await loadHarness({
|
||||
tools: [
|
||||
mockTool({ name: "web_fetch", label: "Web Fetch", description: "Fetch web content" }),
|
||||
],
|
||||
effectivePolicy: { profile: "coding" },
|
||||
});
|
||||
|
||||
const result = resolveEffectiveToolInventory({
|
||||
cfg: {
|
||||
browser: { enabled: true },
|
||||
plugins: { entries: { browser: { enabled: true } } },
|
||||
} as never,
|
||||
});
|
||||
|
||||
expect(result.notices).toEqual([
|
||||
{
|
||||
id: "browser-filtered-by-profile",
|
||||
severity: "info",
|
||||
message:
|
||||
'Browser is configured, but the current tool profile does not include the browser tool. Add tools.alsoAllow: ["browser"] or agents.list[].tools.alsoAllow: ["browser"]; tools.subagents.tools.allow alone cannot add it back after profile filtering.',
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("does not add a browser profile notice when browser is already available", async () => {
|
||||
const { resolveEffectiveToolInventory } = await loadHarness({
|
||||
tools: [
|
||||
mockTool({ name: "browser", label: "Browser", description: "Control browser" }),
|
||||
mockTool({ name: "web_fetch", label: "Web Fetch", description: "Fetch web content" }),
|
||||
],
|
||||
effectivePolicy: { profile: "coding" },
|
||||
});
|
||||
|
||||
const result = resolveEffectiveToolInventory({
|
||||
cfg: {
|
||||
browser: { enabled: true },
|
||||
plugins: { entries: { browser: { enabled: true } } },
|
||||
} as never,
|
||||
});
|
||||
|
||||
expect(result.notices).toBeUndefined();
|
||||
});
|
||||
|
||||
it("passes resolved model compat into effective tool creation", async () => {
|
||||
const createToolsMock = vi.fn<typeof createOpenClawCodingTools>(() => [
|
||||
mockTool({ name: "exec", label: "Exec", description: "Run shell commands" }),
|
||||
|
||||
@@ -12,7 +12,9 @@ import { createOpenClawCodingTools } from "./pi-tools.js";
|
||||
import { resolveEffectiveToolPolicy } from "./pi-tools.policy.js";
|
||||
import { summarizeToolDescriptionText } from "./tool-description-summary.js";
|
||||
import { resolveToolDisplay } from "./tool-display.js";
|
||||
import { normalizeToolName } from "./tool-policy.js";
|
||||
import type {
|
||||
EffectiveToolInventoryNotice,
|
||||
EffectiveToolInventoryEntry,
|
||||
EffectiveToolInventoryGroup,
|
||||
EffectiveToolInventoryResult,
|
||||
@@ -70,6 +72,82 @@ function groupLabel(source: EffectiveToolSource): string {
|
||||
}
|
||||
}
|
||||
|
||||
function listIncludesTool(list: string[] | undefined, toolName: string): boolean {
|
||||
if (!Array.isArray(list)) {
|
||||
return false;
|
||||
}
|
||||
const normalizedToolName = normalizeToolName(toolName);
|
||||
return list.some((entry) => normalizeToolName(entry) === normalizedToolName);
|
||||
}
|
||||
|
||||
function policyDeniesTool(policy: { deny?: string[] } | undefined, toolName: string): boolean {
|
||||
return (
|
||||
listIncludesTool(policy?.deny, toolName) ||
|
||||
listIncludesTool(policy?.deny, "group:ui") ||
|
||||
listIncludesTool(policy?.deny, "group:openclaw")
|
||||
);
|
||||
}
|
||||
|
||||
function hasExplicitBrowserIntent(cfg: OpenClawConfig): boolean {
|
||||
return cfg.browser?.enabled !== false && Boolean(cfg.browser || cfg.plugins?.entries?.browser);
|
||||
}
|
||||
|
||||
function buildToolInventoryNotices(params: {
|
||||
cfg: OpenClawConfig;
|
||||
profile: string;
|
||||
entries: EffectiveToolInventoryEntry[];
|
||||
effectivePolicy: ReturnType<typeof resolveEffectiveToolPolicy>;
|
||||
}): EffectiveToolInventoryNotice[] | undefined {
|
||||
const hasBrowserTool = params.entries.some((entry) => normalizeToolName(entry.id) === "browser");
|
||||
if (hasBrowserTool || !hasExplicitBrowserIntent(params.cfg)) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const browserDenied = [
|
||||
params.effectivePolicy.globalPolicy,
|
||||
params.effectivePolicy.globalProviderPolicy,
|
||||
params.effectivePolicy.agentPolicy,
|
||||
params.effectivePolicy.agentProviderPolicy,
|
||||
].some((policy) => policyDeniesTool(policy, "browser"));
|
||||
if (browserDenied) {
|
||||
return [
|
||||
{
|
||||
id: "browser-denied-by-policy",
|
||||
severity: "info",
|
||||
message:
|
||||
"Browser is configured, but this session does not expose the browser tool because tool policy denies it. Remove the browser deny entry to use browser automation.",
|
||||
},
|
||||
];
|
||||
}
|
||||
|
||||
if (params.profile !== "full") {
|
||||
return [
|
||||
{
|
||||
id: "browser-filtered-by-profile",
|
||||
severity: "info",
|
||||
message:
|
||||
'Browser is configured, but the current tool profile does not include the browser tool. Add tools.alsoAllow: ["browser"] or agents.list[].tools.alsoAllow: ["browser"]; tools.subagents.tools.allow alone cannot add it back after profile filtering.',
|
||||
},
|
||||
];
|
||||
}
|
||||
|
||||
if (
|
||||
Array.isArray(params.cfg.plugins?.allow) &&
|
||||
!listIncludesTool(params.cfg.plugins.allow, "browser")
|
||||
) {
|
||||
return [
|
||||
{
|
||||
id: "browser-plugin-not-allowed",
|
||||
severity: "warning",
|
||||
message:
|
||||
'Browser is configured, but plugins.allow does not include browser. Add "browser" to plugins.allow or remove the restrictive plugin allowlist.',
|
||||
},
|
||||
];
|
||||
}
|
||||
|
||||
return undefined;
|
||||
}
|
||||
|
||||
function disambiguateLabels(entries: EffectiveToolInventoryEntry[]): EffectiveToolInventoryEntry[] {
|
||||
const counts = new Map<string, number>();
|
||||
for (const entry of entries) {
|
||||
@@ -170,6 +248,7 @@ export function resolveEffectiveToolInventory(
|
||||
})
|
||||
.toSorted((a, b) => a.label.localeCompare(b.label)),
|
||||
);
|
||||
const notices = buildToolInventoryNotices({ cfg: params.cfg, profile, entries, effectivePolicy });
|
||||
const groupsBySource = new Map<EffectiveToolSource, EffectiveToolInventoryEntry[]>();
|
||||
for (const entry of entries) {
|
||||
const tools = groupsBySource.get(entry.source) ?? [];
|
||||
@@ -192,5 +271,5 @@ export function resolveEffectiveToolInventory(
|
||||
})
|
||||
.filter((group): group is EffectiveToolInventoryGroup => group !== null);
|
||||
|
||||
return { agentId, profile, groups };
|
||||
return { agentId, profile, groups, ...(notices ? { notices } : {}) };
|
||||
}
|
||||
|
||||
@@ -19,10 +19,17 @@ export type EffectiveToolInventoryGroup = {
|
||||
tools: EffectiveToolInventoryEntry[];
|
||||
};
|
||||
|
||||
export type EffectiveToolInventoryNotice = {
|
||||
id: string;
|
||||
severity: "info" | "warning";
|
||||
message: string;
|
||||
};
|
||||
|
||||
export type EffectiveToolInventoryResult = {
|
||||
agentId: string;
|
||||
profile: string;
|
||||
groups: EffectiveToolInventoryGroup[];
|
||||
notices?: EffectiveToolInventoryNotice[];
|
||||
};
|
||||
|
||||
export type ResolveEffectiveToolInventoryParams = {
|
||||
|
||||
@@ -72,6 +72,40 @@ describe("tools product copy", () => {
|
||||
expect(text).not.toContain("unavailable right now");
|
||||
});
|
||||
|
||||
it("renders effective tool inventory notices", () => {
|
||||
const text = buildToolsMessage({
|
||||
agentId: "main",
|
||||
profile: "coding",
|
||||
groups: [
|
||||
{
|
||||
id: "core",
|
||||
label: "Built-in tools",
|
||||
source: "core",
|
||||
tools: [
|
||||
{
|
||||
id: "web_fetch",
|
||||
label: "Web Fetch",
|
||||
description: "Fetch web content",
|
||||
rawDescription: "Fetch web content",
|
||||
source: "core",
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
notices: [
|
||||
{
|
||||
id: "browser-filtered-by-profile",
|
||||
severity: "info",
|
||||
message:
|
||||
'Browser is configured, but the current tool profile does not include the browser tool. Add tools.alsoAllow: ["browser"].',
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
expect(text).toContain("Notes");
|
||||
expect(text).toContain('Add tools.alsoAllow: ["browser"].');
|
||||
});
|
||||
|
||||
it("keeps detailed descriptions in verbose mode", () => {
|
||||
const text = buildToolsMessage(
|
||||
{
|
||||
|
||||
@@ -97,5 +97,11 @@ export function buildToolsMessage(
|
||||
} else {
|
||||
lines.push("", "Use /tools verbose for descriptions.");
|
||||
}
|
||||
if (result.notices?.length) {
|
||||
lines.push("", "Notes");
|
||||
for (const notice of result.notices) {
|
||||
lines.push(` ${notice.message}`);
|
||||
}
|
||||
}
|
||||
return lines.join("\n");
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user