From fd71bc04ec2350fd6b0bc77ef684d6c2db275e9c Mon Sep 17 00:00:00 2001 From: Kim <150593189+KimGLee@users.noreply.github.com> Date: Sun, 5 Apr 2026 06:30:22 +0800 Subject: [PATCH] fix(skills): unify runtime inclusion and available_skills exposure policy (#60852) Merged via squash. Prepared head SHA: 2b48b3a4559696b1535298477d7aa487228d1ad5 Co-authored-by: KimGLee <150593189+KimGLee@users.noreply.github.com> Co-authored-by: altaywtf <9790196+altaywtf@users.noreply.github.com> Reviewed-by: @altaywtf --- ...skills.buildworkspaceskillsnapshot.test.ts | 6 +-- .../skills.loadworkspaceskillentries.test.ts | 20 ++++++++ .../skills.resolveskillspromptforrun.test.ts | 22 +++++++++ src/agents/skills.test.ts | 18 +++++++ src/agents/skills/compact-format.test.ts | 48 ++++++++++++++++--- src/agents/skills/skill-contract.ts | 12 ++--- src/agents/skills/types.ts | 7 +++ src/agents/skills/workspace.ts | 30 +++++++++--- 8 files changed, 140 insertions(+), 23 deletions(-) diff --git a/src/agents/skills.buildworkspaceskillsnapshot.test.ts b/src/agents/skills.buildworkspaceskillsnapshot.test.ts index 11d664b134b..67f1e9d4e21 100644 --- a/src/agents/skills.buildworkspaceskillsnapshot.test.ts +++ b/src/agents/skills.buildworkspaceskillsnapshot.test.ts @@ -104,10 +104,8 @@ describe("buildWorkspaceSkillSnapshot", () => { expect(snapshot.prompt).toContain("visible-skill"); expect(snapshot.prompt).not.toContain("hidden-skill"); - expect(snapshot.skills.map((skill) => skill.name).toSorted()).toEqual([ - "hidden-skill", - "visible-skill", - ]); + expect(snapshot.skills.map((skill) => skill.name)).toContain("hidden-skill"); + expect(snapshot.skills.map((skill) => skill.name)).toContain("visible-skill"); }); it("keeps prompt output aligned with buildWorkspaceSkillsPrompt", async () => { diff --git a/src/agents/skills.loadworkspaceskillentries.test.ts b/src/agents/skills.loadworkspaceskillentries.test.ts index b11fdf61d41..2a15f5fe2e4 100644 --- a/src/agents/skills.loadworkspaceskillentries.test.ts +++ b/src/agents/skills.loadworkspaceskillentries.test.ts @@ -149,6 +149,26 @@ describe("loadWorkspaceSkillEntries", () => { expect(entries.map((entry) => entry.skill.name)).toContain("fallback-name"); }); + it("marks disable-model-invocation skills as hidden in exposure metadata for newly loaded entries", async () => { + const workspaceDir = await createTempWorkspaceDir(); + await writeSkill({ + dir: path.join(workspaceDir, "skills", "hidden-skill"), + name: "hidden-skill", + description: "Hidden prompt entry", + frontmatterExtra: "disable-model-invocation: true", + }); + + const entries = loadWorkspaceSkillEntries(workspaceDir, { + managedSkillsDir: path.join(workspaceDir, ".managed"), + bundledSkillsDir: path.join(workspaceDir, ".bundled"), + }); + + const hiddenEntry = entries.find((entry) => entry.skill.name === "hidden-skill"); + + expect(hiddenEntry?.invocation?.disableModelInvocation).toBe(true); + expect(hiddenEntry?.exposure?.includeInAvailableSkillsPrompt).toBe(false); + }); + it("inherits agents.defaults.skills when an agent omits skills", async () => { const workspaceDir = await createTempWorkspaceDir(); await writeSkill({ diff --git a/src/agents/skills.resolveskillspromptforrun.test.ts b/src/agents/skills.resolveskillspromptforrun.test.ts index 77e9dd6265e..410aa1fbc9f 100644 --- a/src/agents/skills.resolveskillspromptforrun.test.ts +++ b/src/agents/skills.resolveskillspromptforrun.test.ts @@ -30,6 +30,27 @@ describe("resolveSkillsPromptForRun", () => { expect(prompt).toContain("/app/skills/demo-skill/SKILL.md"); }); + it("keeps legacy entries with disableModelInvocation hidden when exposure metadata is absent", () => { + const hidden: SkillEntry = { + skill: createFixtureSkill({ + name: "hidden-skill", + description: "Hidden", + filePath: "/app/skills/hidden-skill/SKILL.md", + baseDir: "/app/skills/hidden-skill", + source: "openclaw-workspace", + disableModelInvocation: true, + }), + frontmatter: {}, + }; + + const prompt = resolveSkillsPromptForRun({ + entries: [hidden], + workspaceDir: "/tmp/openclaw", + }); + + expect(prompt).not.toContain("/app/skills/hidden-skill/SKILL.md"); + }); + it("inherits agents.defaults.skills when rebuilding prompt for an agent", () => { const visible: SkillEntry = { skill: createFixtureSkill({ @@ -117,6 +138,7 @@ function createFixtureSkill(params: { filePath: string; baseDir: string; source: string; + disableModelInvocation?: boolean; }): SkillEntry["skill"] { return createCanonicalFixtureSkill(params); } diff --git a/src/agents/skills.test.ts b/src/agents/skills.test.ts index 8f1185fdd66..a4272b41596 100644 --- a/src/agents/skills.test.ts +++ b/src/agents/skills.test.ts @@ -332,6 +332,24 @@ describe("buildWorkspaceSkillsPrompt", () => { expect(prompt).toContain("Does demo things"); expect(prompt).toContain(path.join(skillDir, "SKILL.md")); }); + + it("omits disable-model-invocation skills from available_skills for freshly loaded entries", async () => { + const workspaceDir = await makeWorkspace(); + const skillDir = path.join(workspaceDir, "skills", "hidden-skill"); + + await writeSkill({ + dir: skillDir, + name: "hidden-skill", + description: "Hidden from the prompt", + frontmatterExtra: "disable-model-invocation: true", + }); + + const prompt = buildWorkspaceSkillsPrompt(workspaceDir, resolveTestSkillDirs(workspaceDir)); + + expect(prompt).not.toContain("hidden-skill"); + expect(prompt).not.toContain("Hidden from the prompt"); + expect(prompt).not.toContain(path.join(skillDir, "SKILL.md")); + }); }); describe("applySkillEnvOverrides", () => { diff --git a/src/agents/skills/compact-format.test.ts b/src/agents/skills/compact-format.test.ts index 5a4c0259c22..1d518469e0c 100644 --- a/src/agents/skills/compact-format.test.ts +++ b/src/agents/skills/compact-format.test.ts @@ -22,7 +22,15 @@ function makeSkill(name: string, desc = "A skill", filePath = `/skills/${name}/S } function makeEntry(skill: Skill): SkillEntry { - return { skill, frontmatter: {} }; + return { + skill, + frontmatter: {}, + exposure: { + includeInRuntimeRegistry: true, + includeInAvailableSkillsPrompt: true, + userInvocable: true, + }, + }; } function buildPrompt( @@ -43,16 +51,21 @@ function buildPrompt( } describe("formatSkillsCompact", () => { - it("keeps the full-format XML output aligned with the upstream formatter", () => { - const hidden: Skill = { ...makeSkill("hidden"), disableModelInvocation: true }; + it("keeps the full-format XML output aligned with the upstream formatter for visible skills", () => { const skills = [ makeSkill("weather", "Get weather & forecasts"), makeSkill("notes", "Summarize notes", "/tmp/notes/SKILL.md"), - hidden, ]; expect(formatSkillsForPrompt(skills)).toBe(upstreamFormatSkillsForPrompt(skills)); }); + it("renders all passed skills in the full formatter without reapplying visibility policy", () => { + const hidden: Skill = { ...makeSkill("hidden"), disableModelInvocation: true }; + const out = formatSkillsForPrompt([makeSkill("visible"), hidden]); + expect(out).toContain("visible"); + expect(out).toContain("hidden"); + }); + it("returns empty string for no skills", () => { expect(formatSkillsCompact([])).toBe(""); }); @@ -65,11 +78,11 @@ describe("formatSkillsCompact", () => { expect(out).not.toContain(""); }); - it("filters out disableModelInvocation skills", () => { + it("renders all passed skills without reapplying visibility policy", () => { const hidden: Skill = { ...makeSkill("hidden"), disableModelInvocation: true }; const out = formatSkillsCompact([makeSkill("visible"), hidden]); expect(out).toContain("visible"); - expect(out).not.toContain("hidden"); + expect(out).toContain("hidden"); }); it("escapes XML special characters", () => { @@ -87,6 +100,29 @@ describe("formatSkillsCompact", () => { }); describe("applySkillsPromptLimits (via buildWorkspaceSkillsPrompt)", () => { + it("respects explicit exposure metadata before compact formatting", () => { + const hidden = makeEntry({ ...makeSkill("hidden"), disableModelInvocation: true }); + hidden.exposure = { + includeInRuntimeRegistry: true, + includeInAvailableSkillsPrompt: false, + userInvocable: true, + }; + + const prompt = buildWorkspaceSkillsPrompt("/fake", { + entries: [makeEntry(makeSkill("visible")), hidden], + config: { + skills: { + limits: { + maxSkillsPromptChars: 4_000, + }, + }, + } satisfies OpenClawConfig, + }); + + expect(prompt).toContain("visible"); + expect(prompt).not.toContain("hidden"); + }); + it("tier 1: uses full format when under budget", () => { const skills = [makeSkill("weather", "Get weather data")]; const prompt = buildPrompt(skills, { maxChars: 50_000 }); diff --git a/src/agents/skills/skill-contract.ts b/src/agents/skills/skill-contract.ts index a3d1b91dcfd..1662ed8aa5f 100644 --- a/src/agents/skills/skill-contract.ts +++ b/src/agents/skills/skill-contract.ts @@ -36,13 +36,13 @@ function escapeXml(str: string): string { } /** - * Keep this formatter byte-for-byte aligned with the upstream Agent Skills XML - * layout so we can avoid importing the full pi-coding-agent package root on the - * cold skills path. + * Keep this formatter's XML layout byte-for-byte aligned with the upstream + * Agent Skills formatter so we can avoid importing the full pi-coding-agent + * package root on the cold skills path. Visibility policy is applied upstream + * before calling this helper. */ export function formatSkillsForPrompt(skills: Skill[]): string { - const visibleSkills = skills.filter((skill) => !skill.disableModelInvocation); - if (visibleSkills.length === 0) { + if (skills.length === 0) { return ""; } const lines = [ @@ -52,7 +52,7 @@ export function formatSkillsForPrompt(skills: Skill[]): string { "", "", ]; - for (const skill of visibleSkills) { + for (const skill of skills) { lines.push(" "); lines.push(` ${escapeXml(skill.name)}`); lines.push(` ${escapeXml(skill.description)}`); diff --git a/src/agents/skills/types.ts b/src/agents/skills/types.ts index e4f9b0fe78e..e7013166eb6 100644 --- a/src/agents/skills/types.ts +++ b/src/agents/skills/types.ts @@ -67,11 +67,18 @@ export type SkillsInstallPreferences = { export type ParsedSkillFrontmatter = Record; +export type SkillExposure = { + includeInRuntimeRegistry: boolean; + includeInAvailableSkillsPrompt: boolean; + userInvocable: boolean; +}; + export type SkillEntry = { skill: Skill; frontmatter: ParsedSkillFrontmatter; metadata?: OpenClawSkillMetadata; invocation?: SkillInvocationPolicy; + exposure?: SkillExposure; }; export type SkillEligibilityContext = { diff --git a/src/agents/skills/workspace.ts b/src/agents/skills/workspace.ts index 95d62ff8dee..57ea27ba466 100644 --- a/src/agents/skills/workspace.ts +++ b/src/agents/skills/workspace.ts @@ -45,6 +45,16 @@ function compactSkillPaths(skills: Skill[]): Skill[] { })); } +function isSkillVisibleInAvailableSkillsPrompt(entry: SkillEntry): boolean { + if (entry.exposure) { + return entry.exposure.includeInAvailableSkillsPrompt !== false; + } + if (entry.invocation) { + return entry.invocation.disableModelInvocation !== true; + } + return entry.skill.disableModelInvocation !== true; +} + function filterSkillEntries( entries: SkillEntry[], config?: OpenClawConfig, @@ -469,11 +479,20 @@ function loadSkillEntries( filePath: skill.filePath, maxBytes: limits.maxSkillFileBytes, }) ?? ({} as ParsedSkillFrontmatter); + const invocation = resolveSkillInvocationPolicy(frontmatter); return { skill, frontmatter, metadata: resolveOpenClawMetadata(frontmatter), - invocation: resolveSkillInvocationPolicy(frontmatter), + invocation, + exposure: { + includeInRuntimeRegistry: true, + // Freshly loaded entries preserve the documented disable-model-invocation + // contract, while legacy entries without exposure metadata still use the + // fallback in isSkillVisibleInAvailableSkillsPrompt(). + includeInAvailableSkillsPrompt: invocation.disableModelInvocation !== true, + userInvocable: invocation.userInvocable !== false, + }, }; }); return skillEntries; @@ -494,8 +513,7 @@ function escapeXml(str: string): string { * preserving awareness of all skills before resorting to dropping. */ export function formatSkillsCompact(skills: Skill[]): string { - const visible = skills.filter((s) => !s.disableModelInvocation); - if (visible.length === 0) return ""; + if (skills.length === 0) return ""; const lines = [ "\n\nThe following skills provide specialized instructions for specific tasks.", "Use the read tool to load a skill's file when the task matches its name.", @@ -503,7 +521,7 @@ export function formatSkillsCompact(skills: Skill[]): string { "", "", ]; - for (const skill of visible) { + for (const skill of skills) { lines.push(" "); lines.push(` ${escapeXml(skill.name)}`); lines.push(` ${escapeXml(skill.filePath)}`); @@ -629,9 +647,7 @@ function resolveWorkspaceSkillPromptState( effectiveSkillFilter, opts?.eligibility, ); - const promptEntries = eligible.filter( - (entry) => entry.invocation?.disableModelInvocation !== true, - ); + const promptEntries = eligible.filter((entry) => isSkillVisibleInAvailableSkillsPrompt(entry)); const remoteNote = opts?.eligibility?.remote?.note?.trim(); const resolvedSkills = promptEntries.map((entry) => entry.skill); // Derive prompt-facing skills with compacted paths (e.g. ~/...) once.