From 14d43ad93cfefed3a76e15d2b8172ff446ec2ab2 Mon Sep 17 00:00:00 2001 From: Jacqueline Henriksen Date: Tue, 18 Aug 2026 15:06:28 -0700 Subject: [PATCH] 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> --- ui/src/e2e/agent-config-save.e2e.test.ts | 86 +++++++++++++++++++ ui/src/i18n/locales/en.ts | 1 + ui/src/lib/agents/display.test.ts | 48 +++++++++++ ui/src/lib/agents/display.ts | 21 ++++- ui/src/pages/agents/agents-page.ts | 14 +-- ui/src/pages/agents/panels-overview.test.ts | 33 +++++++ ui/src/pages/agents/panels-overview.ts | 3 +- .../panels-tools-skills.browser.test.ts | 76 +++++++++++++++- ui/src/pages/agents/panels-tools-skills.ts | 26 +++--- ui/src/pages/agents/skills.test.ts | 2 +- ui/src/pages/agents/skills.ts | 11 ++- 11 files changed, 294 insertions(+), 27 deletions(-) create mode 100644 ui/src/pages/agents/panels-overview.test.ts diff --git a/ui/src/e2e/agent-config-save.e2e.test.ts b/ui/src/e2e/agent-config-save.e2e.test.ts index 085e26297ef7..139a5c85263d 100644 --- a/ui/src/e2e/agent-config-save.e2e.test.ts +++ b/ui/src/e2e/agent-config-save.e2e.test.ts @@ -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" }); + }, + ); + }); }); diff --git a/ui/src/i18n/locales/en.ts b/ui/src/i18n/locales/en.ts index 86139dec61ae..b54f3266d3c2 100644 --- a/ui/src/i18n/locales/en.ts +++ b/ui/src/i18n/locales/en.ts @@ -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", diff --git a/ui/src/lib/agents/display.test.ts b/ui/src/lib/agents/display.test.ts index 734247376fed..2b5817caa03a 100644 --- a/ui/src/lib/agents/display.test.ts +++ b/ui/src/lib/agents/display.test.ts @@ -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( { diff --git a/ui/src/lib/agents/display.ts b/ui/src/lib/agents/display.ts index 889f8b4c7c60..3ff06e392f22 100644 --- a/ui/src/lib/agents/display.ts +++ b/ui/src/lib/agents/display.ts @@ -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 }; + defaults?: { + workspace?: string; + model?: unknown; + models?: Record; + skills?: string[]; + }; entries?: Record; }; tools?: { @@ -323,6 +329,17 @@ export function resolveAgentConfig(config: Record | null, agent }; } +/** Resolves the effective skill allowlist, including inherited agent defaults. */ +export function resolveAgentSkillsFilter(config: Record | 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, diff --git a/ui/src/pages/agents/agents-page.ts b/ui/src/pages/agents/agents-page.ts index 5dc04cee0da4..ef756a66071a 100644 --- a/ui/src/pages/agents/agents-page.ts +++ b/ui/src/pages/agents/agents-page.ts @@ -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()); diff --git a/ui/src/pages/agents/panels-overview.test.ts b/ui/src/pages/agents/panels-overview.test.ts new file mode 100644 index 000000000000..750f0312f56e --- /dev/null +++ b/ui/src/pages/agents/panels-overview.test.ts @@ -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"); +}); diff --git a/ui/src/pages/agents/panels-overview.ts b/ui/src/pages/agents/panels-overview.ts index a056904f7412..36892e42c75d 100644 --- a/ui/src/pages/agents/panels-overview.ts +++ b/ui/src/pages/agents/panels-overview.ts @@ -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 ?? "-"; diff --git a/ui/src/pages/agents/panels-tools-skills.browser.test.ts b/ui/src/pages/agents/panels-tools-skills.browser.test.ts index f977d735c017..c007bb871be5 100644 --- a/ui/src/pages/agents/panels-tools-skills.browser.test.ts +++ b/ui/src/pages/agents/panels-tools-skills.browser.test.ts @@ -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(".agent-skill-row wa-switch")).map( + (toggle) => (toggle as HTMLElement & { checked: boolean }).checked, + ), + ).toEqual([true, false]); + const buttons = Array.from(container.querySelectorAll("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("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 () => { diff --git a/ui/src/pages/agents/panels-tools-skills.ts b/ui/src/pages/agents/panels-tools-skills.ts index 0437505ece4d..765a193abfc1 100644 --- a/ui/src/pages/agents/panels-tools-skills.ts +++ b/ui/src/pages/agents/panels-tools-skills.ts @@ -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`
${t("agents.skillsPanel.loadConfig")}
` : nothing} ${usingAllowlist - ? html`
${t("agents.skillsPanel.customAllowlist")}
` + ? html`
+ ${t( + inheritedAllowlist + ? "agents.skillsPanel.inheritedAllowlist" + : "agents.skillsPanel.customAllowlist", + )} +
` : html`
${t("agents.skillsPanel.allEnabled")}
`} ${!reportReady && !params.loading ? html`
${t("agents.skillsPanel.loadAgent")}
` @@ -964,13 +975,6 @@ export function renderAgentSkills(params: { description: html`${t("agents.skillsPanel.subtitle")} ${totalCount > 0 ? html`${enabledCount}/${totalCount}` : nothing}`, actions: html` -