mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-05 22:32:12 +00:00
fix(skills): unify runtime inclusion and available_skills exposure policy (#60852)
Merged via squash.
Prepared head SHA: 2b48b3a455
Co-authored-by: KimGLee <150593189+KimGLee@users.noreply.github.com>
Co-authored-by: altaywtf <9790196+altaywtf@users.noreply.github.com>
Reviewed-by: @altaywtf
This commit is contained in:
@@ -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 () => {
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -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 <data> & 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("<description>");
|
||||
});
|
||||
|
||||
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 });
|
||||
|
||||
@@ -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 {
|
||||
"",
|
||||
"<available_skills>",
|
||||
];
|
||||
for (const skill of visibleSkills) {
|
||||
for (const skill of skills) {
|
||||
lines.push(" <skill>");
|
||||
lines.push(` <name>${escapeXml(skill.name)}</name>`);
|
||||
lines.push(` <description>${escapeXml(skill.description)}</description>`);
|
||||
|
||||
@@ -67,11 +67,18 @@ export type SkillsInstallPreferences = {
|
||||
|
||||
export type ParsedSkillFrontmatter = Record<string, string>;
|
||||
|
||||
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 = {
|
||||
|
||||
@@ -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 {
|
||||
"",
|
||||
"<available_skills>",
|
||||
];
|
||||
for (const skill of visible) {
|
||||
for (const skill of skills) {
|
||||
lines.push(" <skill>");
|
||||
lines.push(` <name>${escapeXml(skill.name)}</name>`);
|
||||
lines.push(` <location>${escapeXml(skill.filePath)}</location>`);
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user