mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-25 11:55:47 -06:00
fix(ui): reflect inherited agent skill allowlists (#124429)
* fix(ui): honor inherited agent skill filters * test(ui): cover inherited agent skill allowlists * test(ui): publish real gateway skill proof * chore(proof): remove generated UI evidence * fix(ui): show inherited skills in agent context * fix(ui): show inherited skills in agent context * docs(proof): record rebased gateway UI verification * docs(proof): capture current-head agent skills UI * docs(proof): bind agent skills capture to final head * chore(ui): remove PR-only proof artifacts * fix(ui): preserve list-form agent skill overrides * fix(ui): preserve list-form agent mutations * fix(ui): use canonical list skill replace path * docs(ui): attach inherited skills gateway proof * chore(proof): keep visual evidence out of source tree * fix(ui): remove misleading enable-all action * test(ui): reflect removed enable-all action * fix(ui): describe skill reset as inherited defaults * fix(ui): keep skill reset on canonical roster * fix(ui): reset list-roster skill overrides * fix(ui): satisfy inherited skills proof checks * fix(ui): keep agent skill reset on canonical entries * test(ui): focus inherited skill regression proof --------- Co-authored-by: RoboClaw <309084314+roboclaw-bot@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
4be131107f
commit
14d43ad93c
@@ -131,4 +131,90 @@ suite.define(() => {
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it("stages skill changes from the inherited allowlist", async () => {
|
||||
await suite.withPage(
|
||||
{
|
||||
locale: "en-US",
|
||||
serviceWorkers: "block",
|
||||
viewport: { height: 900, width: 1280 },
|
||||
},
|
||||
async ({ page }) => {
|
||||
const config = {
|
||||
agents: {
|
||||
defaults: { skills: ["github"] },
|
||||
entries: { main: { default: true } },
|
||||
},
|
||||
};
|
||||
const skill = (name: string, blockedByAgentFilter: boolean) => ({
|
||||
name,
|
||||
description: `${name} skill`,
|
||||
source: "openclaw-managed",
|
||||
bundled: false,
|
||||
filePath: `/tmp/skills/${name}/SKILL.md`,
|
||||
baseDir: `/tmp/skills/${name}`,
|
||||
skillKey: name,
|
||||
always: false,
|
||||
disabled: false,
|
||||
blockedByAllowlist: false,
|
||||
blockedByAgentFilter,
|
||||
eligible: true,
|
||||
requirements: { bins: [], anyBins: [], env: [], config: [], os: [] },
|
||||
missing: { bins: [], anyBins: [], env: [], config: [], os: [] },
|
||||
configChecks: [],
|
||||
install: [],
|
||||
});
|
||||
const gateway = await installMockGateway(page, {
|
||||
assistantName: "Main agent",
|
||||
defaultAgentId: "main",
|
||||
methodResponses: {
|
||||
"agents.list": {
|
||||
agents: [{ id: "main", name: "Main agent" }],
|
||||
defaultId: "main",
|
||||
mainKey: "main",
|
||||
scope: "agent",
|
||||
},
|
||||
"config.get": {
|
||||
config,
|
||||
sourceConfig: config,
|
||||
runtimeConfig: config,
|
||||
hash: "agent-config-hash-1",
|
||||
issues: [],
|
||||
raw: JSON.stringify(config),
|
||||
valid: true,
|
||||
},
|
||||
"skills.status": {
|
||||
agentId: "main",
|
||||
agentSkillFilter: ["github"],
|
||||
workspaceDir: "/tmp/workspace",
|
||||
managedSkillsDir: "/tmp/skills",
|
||||
skills: [skill("github", false), skill("weather", true)],
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const response = await page.goto(`${suite.server.baseUrl}settings/agents/main/skills`);
|
||||
expect(response?.status()).toBe(200);
|
||||
await gateway.waitForRequest("config.get");
|
||||
await gateway.waitForRequest("skills.status");
|
||||
|
||||
await gateway.deferNext("config.set");
|
||||
await page
|
||||
.locator(".agent-skill-row", { hasText: "github skill" })
|
||||
.locator("wa-switch")
|
||||
.click();
|
||||
|
||||
const request = await gateway.waitForRequest("config.set");
|
||||
const params = requireRecord(request.params);
|
||||
expect(JSON.parse(String(params.raw))).toEqual({
|
||||
agents: {
|
||||
defaults: { skills: ["github"] },
|
||||
entries: { main: { default: true, skills: [] } },
|
||||
},
|
||||
});
|
||||
expect(params.baseHash).toBe("agent-config-hash-1");
|
||||
await gateway.resolveDeferred("config.set", { hash: "agent-config-hash-2" });
|
||||
},
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1310,6 +1310,7 @@ export const en: TranslationMap = {
|
||||
subtitle: "Per-agent skill allowlist and workspace skills.",
|
||||
loadConfig: "Load the gateway config to set per-agent skills.",
|
||||
customAllowlist: "This agent uses a custom skill allowlist.",
|
||||
inheritedAllowlist: "This agent inherits the default skill allowlist.",
|
||||
allEnabled: "All skills are enabled. Disabling any skill will create a per-agent allowlist.",
|
||||
loadAgent: "Load skills for this agent to view workspace-specific entries.",
|
||||
filter: "Filter",
|
||||
|
||||
@@ -16,6 +16,7 @@ import {
|
||||
listSelectableAgents,
|
||||
normalizeAgentLabel,
|
||||
normalizeAgentTargetLabel,
|
||||
resolveAgentSkillsFilter,
|
||||
resolveEffectiveModelFallbacks,
|
||||
resolveToolProfileOptions,
|
||||
resolveToolSections,
|
||||
@@ -373,6 +374,36 @@ describe("resolveChatAvatarRenderUrl", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveAgentSkillsFilter", () => {
|
||||
it("inherits the default filter when the agent has no override", () => {
|
||||
expect(
|
||||
resolveAgentSkillsFilter(
|
||||
{
|
||||
agents: {
|
||||
defaults: { skills: [" github ", "weather"] },
|
||||
entries: { main: { default: true } },
|
||||
},
|
||||
},
|
||||
"main",
|
||||
),
|
||||
).toEqual(["github", "weather"]);
|
||||
});
|
||||
|
||||
it("prefers an explicit empty agent filter over inherited defaults", () => {
|
||||
expect(
|
||||
resolveAgentSkillsFilter(
|
||||
{
|
||||
agents: {
|
||||
defaults: { skills: ["github"] },
|
||||
entries: { main: { skills: [] } },
|
||||
},
|
||||
},
|
||||
"main",
|
||||
),
|
||||
).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("buildAgentContext", () => {
|
||||
it("falls back to agent payload workspace/model when config form is unavailable", () => {
|
||||
const context = buildAgentContext(
|
||||
@@ -421,6 +452,23 @@ describe("buildAgentContext", () => {
|
||||
expect(context.model).toBe("openai/gpt-5.5 (+1 fallback)");
|
||||
});
|
||||
|
||||
it("shows inherited skill filters in the agent context", () => {
|
||||
const context = buildAgentContext(
|
||||
{ id: "main" },
|
||||
{
|
||||
agents: {
|
||||
defaults: { skills: ["github", "weather"] },
|
||||
entries: { main: { default: true } },
|
||||
},
|
||||
},
|
||||
null,
|
||||
"main",
|
||||
null,
|
||||
);
|
||||
|
||||
expect(context.skillsLabel).toBe("2 selected");
|
||||
});
|
||||
|
||||
it("prefers per-agent configured identity over runtime global identity in agent panels", () => {
|
||||
const context = buildAgentContext(
|
||||
{
|
||||
|
||||
@@ -4,6 +4,7 @@ import {
|
||||
normalizeLowercaseStringOrEmpty,
|
||||
normalizeOptionalString,
|
||||
} from "@openclaw/normalization-core/string-coerce";
|
||||
import { normalizeStringEntries } from "@openclaw/normalization-core/string-normalization";
|
||||
import {
|
||||
expandToolGroups,
|
||||
normalizeToolPolicyName,
|
||||
@@ -228,7 +229,12 @@ type AgentConfigEntry = {
|
||||
|
||||
type ConfigSnapshot = {
|
||||
agents?: {
|
||||
defaults?: { workspace?: string; model?: unknown; models?: Record<string, { alias?: string }> };
|
||||
defaults?: {
|
||||
workspace?: string;
|
||||
model?: unknown;
|
||||
models?: Record<string, { alias?: string }>;
|
||||
skills?: string[];
|
||||
};
|
||||
entries?: Record<string, AgentConfigEntry>;
|
||||
};
|
||||
tools?: {
|
||||
@@ -323,6 +329,17 @@ export function resolveAgentConfig(config: Record<string, unknown> | null, agent
|
||||
};
|
||||
}
|
||||
|
||||
/** Resolves the effective skill allowlist, including inherited agent defaults. */
|
||||
export function resolveAgentSkillsFilter(config: Record<string, unknown> | null, agentId: string) {
|
||||
const resolved = resolveAgentConfig(config, agentId);
|
||||
if (Array.isArray(resolved.entry?.skills)) {
|
||||
return normalizeStringEntries(resolved.entry.skills);
|
||||
}
|
||||
return Array.isArray(resolved.defaults?.skills)
|
||||
? normalizeStringEntries(resolved.defaults.skills)
|
||||
: undefined;
|
||||
}
|
||||
|
||||
export type AgentContext = {
|
||||
workspace: string;
|
||||
model: string;
|
||||
@@ -364,7 +381,7 @@ export function buildAgentContext(
|
||||
const identityAvatar = resolveAgentAvatarUrl(agent, agentIdentity)
|
||||
? "custom"
|
||||
: (resolveAgentTextAvatar(agent, agentIdentity) ?? "—");
|
||||
const skillFilter = Array.isArray(config.entry?.skills) ? config.entry?.skills : null;
|
||||
const skillFilter = resolveAgentSkillsFilter(configForm, agent.id);
|
||||
const skillCount = skillFilter?.length ?? null;
|
||||
return {
|
||||
workspace,
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
import { consume } from "@lit/context";
|
||||
import { normalizeStringEntries } from "@openclaw/normalization-core/string-normalization";
|
||||
import { html, type PropertyValues } from "lit";
|
||||
import { property, state } from "lit/decorators.js";
|
||||
import type { GatewayBrowserClient } from "../../api/gateway.ts";
|
||||
@@ -18,7 +17,7 @@ import { resolveControlUiAuthToken } from "../../app/control-ui-auth.ts";
|
||||
import { renderDocsLink } from "../../components/settings-ui.ts";
|
||||
import { renderSettingsWorkspace } from "../../components/settings-workspace.ts";
|
||||
import { t } from "../../i18n/index.ts";
|
||||
import { selectableAgentsList } from "../../lib/agents/display.ts";
|
||||
import { resolveAgentSkillsFilter, selectableAgentsList } from "../../lib/agents/display.ts";
|
||||
import {
|
||||
loadToolsCatalog,
|
||||
loadToolsEffective,
|
||||
@@ -1114,9 +1113,14 @@ class AgentsPage
|
||||
if (!target || !skillName.trim()) {
|
||||
return;
|
||||
}
|
||||
const base = Array.isArray(target.entry.skills)
|
||||
? normalizeStringEntries(target.entry.skills)
|
||||
: (this.agentSkillsReport?.skills?.map((skill) => skill.name).filter(Boolean) ?? []);
|
||||
const base =
|
||||
resolveAgentSkillsFilter(
|
||||
currentConfigObject(this.context.runtimeConfig.state),
|
||||
agentId,
|
||||
) ??
|
||||
this.agentSkillsReport?.agentSkillFilter ??
|
||||
this.agentSkillsReport?.skills?.map((skill) => skill.name).filter(Boolean) ??
|
||||
[];
|
||||
const next = new Set(base);
|
||||
if (enabled) {
|
||||
next.add(skillName.trim());
|
||||
|
||||
@@ -0,0 +1,33 @@
|
||||
// Control UI tests cover the agents overview context display.
|
||||
import { render } from "lit";
|
||||
import { expect, it } from "vitest";
|
||||
import { createAgentViewTestProps as createProps } from "./agents-view.test-helpers.ts";
|
||||
import { renderAgents } from "./view.ts";
|
||||
|
||||
it("shows inherited skills in the Agent Context overview", () => {
|
||||
const container = document.createElement("div");
|
||||
render(
|
||||
renderAgents(
|
||||
createProps({
|
||||
config: {
|
||||
form: {
|
||||
agents: {
|
||||
defaults: { skills: ["github", "weather"] },
|
||||
entries: { beta: {} },
|
||||
},
|
||||
},
|
||||
loading: false,
|
||||
saving: false,
|
||||
dirty: false,
|
||||
error: null,
|
||||
},
|
||||
}),
|
||||
),
|
||||
container,
|
||||
);
|
||||
|
||||
const skillsFilterRow = Array.from(container.querySelectorAll("dt")).find(
|
||||
(term) => term.textContent?.trim() === "Skills Filter",
|
||||
)?.nextElementSibling;
|
||||
expect(skillsFilterRow?.textContent?.trim()).toBe("2 selected");
|
||||
});
|
||||
@@ -16,6 +16,7 @@ import {
|
||||
buildModelOptions,
|
||||
normalizeModelValue,
|
||||
resolveAgentConfig,
|
||||
resolveAgentSkillsFilter,
|
||||
resolveAgentRuntimeLabel,
|
||||
resolveAgentTextAvatar,
|
||||
resolveEffectiveModelFallbacks,
|
||||
@@ -103,7 +104,7 @@ export function renderAgentOverview(params: {
|
||||
resolveEffectiveModelFallbacks(config.entry?.model, config.defaults?.model) ??
|
||||
(configForm ? null : resolveModelFallbacks(agentModel));
|
||||
const fallbackChips = modelFallbacks ?? [];
|
||||
const skillFilter = Array.isArray(config.entry?.skills) ? config.entry?.skills : null;
|
||||
const skillFilter = resolveAgentSkillsFilter(configForm, agent.id);
|
||||
const skillCount = skillFilter?.length ?? null;
|
||||
const disabled = !params.canUpdateConfig || !configForm || configLoading || configSaving;
|
||||
const thinkingDefault = agent.thinkingDefault ?? "-";
|
||||
|
||||
@@ -504,6 +504,77 @@ describe("agents tools panel (browser)", () => {
|
||||
});
|
||||
|
||||
describe("agents skills panel (browser)", () => {
|
||||
it("reflects an inherited default skill allowlist", async () => {
|
||||
const container = document.createElement("div");
|
||||
const skill = (name: string, blockedByAgentFilter: boolean): SkillStatusEntry => ({
|
||||
name,
|
||||
description: `${name} skill`,
|
||||
source: "openclaw-managed",
|
||||
bundled: false,
|
||||
filePath: `/tmp/skills/${name}/SKILL.md`,
|
||||
baseDir: `/tmp/skills/${name}`,
|
||||
skillKey: name,
|
||||
always: false,
|
||||
disabled: false,
|
||||
blockedByAllowlist: false,
|
||||
blockedByAgentFilter,
|
||||
eligible: true,
|
||||
requirements: { bins: [], anyBins: [], env: [], config: [], os: [] },
|
||||
missing: { bins: [], anyBins: [], env: [], config: [], os: [] },
|
||||
configChecks: [],
|
||||
install: [],
|
||||
});
|
||||
|
||||
render(
|
||||
renderAgentSkills({
|
||||
agentId: "main",
|
||||
canPatchConfig: true,
|
||||
canUpdateConfig: true,
|
||||
report: {
|
||||
workspaceDir: "/tmp/workspace",
|
||||
managedSkillsDir: "/tmp/skills",
|
||||
agentId: "main",
|
||||
agentSkillFilter: ["github"],
|
||||
skills: [skill("github", false), skill("weather", true)],
|
||||
},
|
||||
loading: false,
|
||||
error: null,
|
||||
activeAgentId: "main",
|
||||
configForm: {
|
||||
agents: {
|
||||
defaults: { skills: ["github"] },
|
||||
entries: { main: { default: true } },
|
||||
},
|
||||
},
|
||||
configLoading: false,
|
||||
configSaving: false,
|
||||
configDirty: false,
|
||||
filter: "",
|
||||
onFilterChange: () => undefined,
|
||||
onRefresh: () => undefined,
|
||||
onToggle: () => undefined,
|
||||
onClear: () => undefined,
|
||||
onDisableAll: () => undefined,
|
||||
onConfigReload: () => undefined,
|
||||
onConfigSave: () => undefined,
|
||||
}),
|
||||
container,
|
||||
);
|
||||
await Promise.resolve();
|
||||
|
||||
expect(container.querySelector(".callout.info")?.textContent).toContain(
|
||||
"inherits the default skill allowlist",
|
||||
);
|
||||
expect(
|
||||
Array.from(container.querySelectorAll<HTMLElement>(".agent-skill-row wa-switch")).map(
|
||||
(toggle) => (toggle as HTMLElement & { checked: boolean }).checked,
|
||||
),
|
||||
).toEqual([true, false]);
|
||||
const buttons = Array.from(container.querySelectorAll<HTMLButtonElement>("button"));
|
||||
expect(buttons[0]?.disabled).toBe(false);
|
||||
expect(buttons[1]?.disabled).toBe(true);
|
||||
});
|
||||
|
||||
it("gates allowlist clearing separately from staged config edits", async () => {
|
||||
const container = document.createElement("div");
|
||||
render(
|
||||
@@ -537,9 +608,8 @@ describe("agents skills panel (browser)", () => {
|
||||
await Promise.resolve();
|
||||
|
||||
const buttons = Array.from(container.querySelectorAll<HTMLButtonElement>("button"));
|
||||
expect(buttons[0]?.disabled).toBe(true);
|
||||
expect(buttons[1]?.disabled).toBe(false);
|
||||
expect(buttons[2]?.disabled).toBe(true);
|
||||
expect(buttons[0]?.disabled).toBe(false);
|
||||
expect(buttons[1]?.disabled).toBe(true);
|
||||
});
|
||||
|
||||
it("explains an unsatisfied one-of binary requirement", async () => {
|
||||
|
||||
@@ -28,6 +28,7 @@ import {
|
||||
isAllowedByPolicy,
|
||||
matchesList,
|
||||
resolveAgentConfig,
|
||||
resolveAgentSkillsFilter,
|
||||
resolveToolProfileOptions,
|
||||
resolveToolProfile,
|
||||
resolveToolSections,
|
||||
@@ -922,12 +923,16 @@ export function renderAgentSkills(params: {
|
||||
!params.configLoading &&
|
||||
!params.configSaving;
|
||||
const config = resolveAgentConfig(params.configForm, params.agentId);
|
||||
const allowlist = Array.isArray(config.entry?.skills) ? config.entry?.skills : undefined;
|
||||
const allowSet = new Set(normalizeStringEntries(allowlist ?? []));
|
||||
const explicitAllowlist = Array.isArray(config.entry?.skills)
|
||||
? normalizeStringEntries(config.entry.skills)
|
||||
: undefined;
|
||||
const allowlist = resolveAgentSkillsFilter(params.configForm, params.agentId);
|
||||
const allowSet = new Set(allowlist ?? []);
|
||||
const usingAllowlist = allowlist !== undefined;
|
||||
const inheritedAllowlist = explicitAllowlist === undefined && usingAllowlist;
|
||||
const canClear =
|
||||
params.canPatchConfig &&
|
||||
usingAllowlist &&
|
||||
explicitAllowlist !== undefined &&
|
||||
Boolean(params.configForm) &&
|
||||
!params.configLoading &&
|
||||
!params.configSaving;
|
||||
@@ -952,7 +957,13 @@ export function renderAgentSkills(params: {
|
||||
? html`<div class="callout info">${t("agents.skillsPanel.loadConfig")}</div>`
|
||||
: nothing}
|
||||
${usingAllowlist
|
||||
? html`<div class="callout info">${t("agents.skillsPanel.customAllowlist")}</div>`
|
||||
? html`<div class="callout info">
|
||||
${t(
|
||||
inheritedAllowlist
|
||||
? "agents.skillsPanel.inheritedAllowlist"
|
||||
: "agents.skillsPanel.customAllowlist",
|
||||
)}
|
||||
</div>`
|
||||
: html`<div class="callout info">${t("agents.skillsPanel.allEnabled")}</div>`}
|
||||
${!reportReady && !params.loading
|
||||
? html`<div class="callout info">${t("agents.skillsPanel.loadAgent")}</div>`
|
||||
@@ -964,13 +975,6 @@ export function renderAgentSkills(params: {
|
||||
description: html`${t("agents.skillsPanel.subtitle")}
|
||||
${totalCount > 0 ? html`<span class="mono">${enabledCount}/${totalCount}</span>` : nothing}`,
|
||||
actions: html`
|
||||
<button
|
||||
class="btn btn--sm"
|
||||
?disabled=${!canClear}
|
||||
@click=${() => params.onClear(params.agentId)}
|
||||
>
|
||||
${t("agentTools.enableAll")}
|
||||
</button>
|
||||
<button
|
||||
class="btn btn--sm"
|
||||
?disabled=${!editable}
|
||||
|
||||
@@ -23,7 +23,7 @@ describe("clearAgentSkillFilter", () => {
|
||||
},
|
||||
},
|
||||
},
|
||||
note: "Enable all agent skills",
|
||||
note: "Reset agent skills to inherited defaults",
|
||||
replacePaths: ["agents.entries.Research.skills"],
|
||||
canDispatch: expect.any(Function),
|
||||
});
|
||||
|
||||
@@ -54,17 +54,20 @@ export async function clearAgentSkillFilter(
|
||||
if (!target || !Array.isArray(target.entry.skills) || !canDispatch()) {
|
||||
return false;
|
||||
}
|
||||
const authoredAgentId = target.path[2];
|
||||
const targetKey = target.path[2];
|
||||
if (typeof targetKey !== "string") {
|
||||
return false;
|
||||
}
|
||||
return runtimeConfig.patch({
|
||||
raw: {
|
||||
agents: {
|
||||
entries: {
|
||||
[authoredAgentId]: { skills: null },
|
||||
[targetKey]: { skills: null },
|
||||
},
|
||||
},
|
||||
},
|
||||
note: "Enable all agent skills",
|
||||
replacePaths: [`agents.entries.${authoredAgentId}.skills`],
|
||||
note: "Reset agent skills to inherited defaults",
|
||||
replacePaths: [`agents.entries.${targetKey}.skills`],
|
||||
canDispatch,
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user