feat: filter agent-restricted skills from prompts and tool description
Skills with an `agent` frontmatter field are intended for a specific agent. Previously they still appeared in: - every agent's system prompt (via `buildAvailableSkills`) - the `skill` tool's `<available_items>` description visible to all agents This wasted tokens and could mislead agents into attempting calls that would be rejected at execution time. Changes: - `buildAvailableSkills`: new optional `agentName` parameter; when provided, skills whose `definition.agent` does not match are excluded - `builtin-agents.ts`: pass per-agent name to `buildAvailableSkills` for sisyphus, hephaestus, and atlas, so each agent's prompt only lists the skills it is allowed to use - `createSkillTool` (`tools.ts`): exclude agent-restricted skills from both the eager and lazy description builds, keeping the shared tool description free of skills the current agent cannot access Execution-time enforcement (throwing on mismatch) is unchanged; this change adds the earlier, description-level visibility gate. Tests: new `available-skills.test.ts` (5 cases) + 3 new cases in `tools.test.ts` covering the description-filter and execute paths. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -100,8 +100,6 @@ export async function createBuiltinAgents(
|
|||||||
description: categories?.[name]?.description ?? CATEGORY_DESCRIPTIONS[name] ?? "General tasks",
|
description: categories?.[name]?.description ?? CATEGORY_DESCRIPTIONS[name] ?? "General tasks",
|
||||||
}))
|
}))
|
||||||
|
|
||||||
const availableSkills = buildAvailableSkills(discoveredSkills, browserProvider, disabledSkills, teamModeEnabled)
|
|
||||||
|
|
||||||
// Collect general agents first (for availableAgents), but don't add to result yet
|
// Collect general agents first (for availableAgents), but don't add to result yet
|
||||||
const { pendingAgentConfigs, availableAgents } = collectPendingBuiltinAgents({
|
const { pendingAgentConfigs, availableAgents } = collectPendingBuiltinAgents({
|
||||||
agentSources,
|
agentSources,
|
||||||
@@ -129,7 +127,7 @@ export async function createBuiltinAgents(
|
|||||||
systemDefaultModel,
|
systemDefaultModel,
|
||||||
isFirstRunNoCache,
|
isFirstRunNoCache,
|
||||||
availableAgents,
|
availableAgents,
|
||||||
availableSkills,
|
availableSkills: buildAvailableSkills(discoveredSkills, browserProvider, disabledSkills, teamModeEnabled, "sisyphus"),
|
||||||
availableCategories,
|
availableCategories,
|
||||||
mergedCategories,
|
mergedCategories,
|
||||||
directory,
|
directory,
|
||||||
@@ -148,7 +146,7 @@ export async function createBuiltinAgents(
|
|||||||
systemDefaultModel,
|
systemDefaultModel,
|
||||||
isFirstRunNoCache,
|
isFirstRunNoCache,
|
||||||
availableAgents,
|
availableAgents,
|
||||||
availableSkills,
|
availableSkills: buildAvailableSkills(discoveredSkills, browserProvider, disabledSkills, teamModeEnabled, "hephaestus"),
|
||||||
availableCategories,
|
availableCategories,
|
||||||
mergedCategories,
|
mergedCategories,
|
||||||
directory,
|
directory,
|
||||||
@@ -171,7 +169,7 @@ export async function createBuiltinAgents(
|
|||||||
availableModels,
|
availableModels,
|
||||||
systemDefaultModel,
|
systemDefaultModel,
|
||||||
availableAgents,
|
availableAgents,
|
||||||
availableSkills,
|
availableSkills: buildAvailableSkills(discoveredSkills, browserProvider, disabledSkills, teamModeEnabled, "atlas"),
|
||||||
mergedCategories,
|
mergedCategories,
|
||||||
directory,
|
directory,
|
||||||
userCategories: categories,
|
userCategories: categories,
|
||||||
|
|||||||
@@ -1,9 +1,24 @@
|
|||||||
import { describe, expect, test } from "bun:test"
|
/// <reference types="bun-types" />
|
||||||
|
import { describe, expect, it, test } from "bun:test"
|
||||||
|
import type { LoadedSkill } from "../../features/opencode-skill-loader/types"
|
||||||
import { buildAvailableSkills } from "./available-skills"
|
import { buildAvailableSkills } from "./available-skills"
|
||||||
|
|
||||||
type DiscoveredSkills = Parameters<typeof buildAvailableSkills>[0]
|
type DiscoveredSkills = Parameters<typeof buildAvailableSkills>[0]
|
||||||
|
|
||||||
|
function makeSkill(name: string, agent?: string): LoadedSkill {
|
||||||
|
return {
|
||||||
|
name,
|
||||||
|
resolvedPath: `/test/skills/${name}`,
|
||||||
|
definition: {
|
||||||
|
name,
|
||||||
|
description: `Skill ${name}`,
|
||||||
|
template: "",
|
||||||
|
agent,
|
||||||
|
},
|
||||||
|
scope: "user",
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
describe("buildAvailableSkills", () => {
|
describe("buildAvailableSkills", () => {
|
||||||
test("includes team-mode when team mode is enabled", () => {
|
test("includes team-mode when team mode is enabled", () => {
|
||||||
// given
|
// given
|
||||||
@@ -27,3 +42,67 @@ describe("buildAvailableSkills", () => {
|
|||||||
expect(availableSkills.some((skill) => skill.name === "team-mode")).toBe(false)
|
expect(availableSkills.some((skill) => skill.name === "team-mode")).toBe(false)
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
describe("buildAvailableSkills - agentName filtering", () => {
|
||||||
|
it("includes agent-restricted skill when agentName is not provided (backward compat)", () => {
|
||||||
|
// given
|
||||||
|
const skills = [makeSkill("oracle-only", "oracle")]
|
||||||
|
|
||||||
|
// when
|
||||||
|
const result = buildAvailableSkills(skills, undefined, undefined, undefined, undefined)
|
||||||
|
|
||||||
|
// then: no agentName → no filtering, skill is included
|
||||||
|
expect(result.map((s) => s.name)).toContain("oracle-only")
|
||||||
|
})
|
||||||
|
|
||||||
|
it("includes skill when agentName matches the skill's agent field", () => {
|
||||||
|
// given
|
||||||
|
const skills = [makeSkill("sisyphus-only", "sisyphus")]
|
||||||
|
|
||||||
|
// when
|
||||||
|
const result = buildAvailableSkills(skills, undefined, undefined, undefined, "sisyphus")
|
||||||
|
|
||||||
|
// then: matching agent → included
|
||||||
|
expect(result.map((s) => s.name)).toContain("sisyphus-only")
|
||||||
|
})
|
||||||
|
|
||||||
|
it("excludes skill when agentName does not match the skill's agent field", () => {
|
||||||
|
// given
|
||||||
|
const skills = [makeSkill("sisyphus-only", "sisyphus")]
|
||||||
|
|
||||||
|
// when
|
||||||
|
const result = buildAvailableSkills(skills, undefined, undefined, undefined, "oracle")
|
||||||
|
|
||||||
|
// then: wrong agent → excluded
|
||||||
|
expect(result.map((s) => s.name)).not.toContain("sisyphus-only")
|
||||||
|
})
|
||||||
|
|
||||||
|
it("includes skill with no agent field regardless of agentName", () => {
|
||||||
|
// given
|
||||||
|
const skills = [makeSkill("public-skill")]
|
||||||
|
|
||||||
|
// when
|
||||||
|
const result = buildAvailableSkills(skills, undefined, undefined, undefined, "sisyphus")
|
||||||
|
|
||||||
|
// then: no agent restriction → always included
|
||||||
|
expect(result.map((s) => s.name)).toContain("public-skill")
|
||||||
|
})
|
||||||
|
|
||||||
|
it("filters per-agent while keeping public skills", () => {
|
||||||
|
// given
|
||||||
|
const skills = [
|
||||||
|
makeSkill("public-skill"),
|
||||||
|
makeSkill("sisyphus-only", "sisyphus"),
|
||||||
|
makeSkill("oracle-only", "oracle"),
|
||||||
|
]
|
||||||
|
|
||||||
|
// when
|
||||||
|
const result = buildAvailableSkills(skills, undefined, undefined, undefined, "sisyphus")
|
||||||
|
|
||||||
|
// then
|
||||||
|
const names = result.map((s) => s.name)
|
||||||
|
expect(names).toContain("public-skill")
|
||||||
|
expect(names).toContain("sisyphus-only")
|
||||||
|
expect(names).not.toContain("oracle-only")
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|||||||
@@ -14,6 +14,7 @@ export function buildAvailableSkills(
|
|||||||
browserProvider?: BrowserAutomationProvider,
|
browserProvider?: BrowserAutomationProvider,
|
||||||
disabledSkills?: Set<string>,
|
disabledSkills?: Set<string>,
|
||||||
teamModeEnabled?: boolean,
|
teamModeEnabled?: boolean,
|
||||||
|
agentName?: string,
|
||||||
): AvailableSkill[] {
|
): AvailableSkill[] {
|
||||||
const builtinSkills = createBuiltinSkills({ browserProvider, disabledSkills, teamModeEnabled })
|
const builtinSkills = createBuiltinSkills({ browserProvider, disabledSkills, teamModeEnabled })
|
||||||
const builtinSkillNames = new Set(builtinSkills.map(s => s.name))
|
const builtinSkillNames = new Set(builtinSkills.map(s => s.name))
|
||||||
@@ -25,7 +26,13 @@ export function buildAvailableSkills(
|
|||||||
}))
|
}))
|
||||||
|
|
||||||
const discoveredAvailable: AvailableSkill[] = discoveredSkills
|
const discoveredAvailable: AvailableSkill[] = discoveredSkills
|
||||||
.filter(s => !builtinSkillNames.has(s.name) && !disabledSkills?.has(s.name))
|
.filter(s => {
|
||||||
|
if (builtinSkillNames.has(s.name) || disabledSkills?.has(s.name)) return false
|
||||||
|
// If the skill declares an agent restriction and we know the current agent,
|
||||||
|
// exclude skills that don't belong to this agent.
|
||||||
|
if (agentName && s.definition.agent && s.definition.agent !== agentName) return false
|
||||||
|
return true
|
||||||
|
})
|
||||||
.map((skill) => ({
|
.map((skill) => ({
|
||||||
name: skill.name,
|
name: skill.name,
|
||||||
description: skill.definition.description ?? "",
|
description: skill.definition.description ?? "",
|
||||||
|
|||||||
@@ -64,13 +64,18 @@ export function createSkillTool(options: SkillLoadOptions = {}): ToolDefinition
|
|||||||
if (!force && cachedDescription) return cachedDescription
|
if (!force && cachedDescription) return cachedDescription
|
||||||
const skills = await getSkills()
|
const skills = await getSkills()
|
||||||
const commands = getCommands()
|
const commands = getCommands()
|
||||||
const skillInfos = skills.map(loadedSkillToInfo)
|
// Exclude agent-restricted skills from the description: they must not be
|
||||||
|
// visible to agents that are not their designated owner. The execute-time
|
||||||
|
// check already enforces the restriction at call time.
|
||||||
|
const publicSkills = skills.filter((s) => !s.definition.agent)
|
||||||
|
const skillInfos = publicSkills.map(loadedSkillToInfo)
|
||||||
cachedDescription = formatCombinedDescription(skillInfos, commands)
|
cachedDescription = formatCombinedDescription(skillInfos, commands)
|
||||||
return cachedDescription
|
return cachedDescription
|
||||||
}
|
}
|
||||||
|
|
||||||
if (options.skills !== undefined) {
|
if (options.skills !== undefined) {
|
||||||
const skillInfos = options.skills.map(loadedSkillToInfo)
|
const publicSkills = options.skills.filter((s) => !s.definition.agent)
|
||||||
|
const skillInfos = publicSkills.map(loadedSkillToInfo)
|
||||||
const commandsForDescription = options.commands ?? []
|
const commandsForDescription = options.commands ?? []
|
||||||
let needsAsyncRefresh = false
|
let needsAsyncRefresh = false
|
||||||
|
|
||||||
|
|||||||
@@ -636,6 +636,50 @@ describe("skill tool - dynamic discovery", () => {
|
|||||||
expect(result).not.toContain("SHOULD_BE_OVERRIDDEN")
|
expect(result).not.toContain("SHOULD_BE_OVERRIDDEN")
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
describe("skill tool - agent-restricted skill visibility in description", () => {
|
||||||
|
it("excludes agent-restricted skill from description <available_items>", () => {
|
||||||
|
// given: a skill restricted to oracle, and a public skill
|
||||||
|
const loadedSkills = [
|
||||||
|
createMockSkill("public-skill"),
|
||||||
|
createMockSkill("oracle-only-skill", { agent: "oracle" }),
|
||||||
|
]
|
||||||
|
|
||||||
|
// when: tool is created with these skills (as tool-registry would inject them)
|
||||||
|
const tool = createSkillTool({ skills: loadedSkills })
|
||||||
|
|
||||||
|
// then: oracle-only skill must NOT appear in the description
|
||||||
|
expect(tool.description).toContain("public-skill")
|
||||||
|
expect(tool.description).not.toContain("oracle-only-skill")
|
||||||
|
})
|
||||||
|
|
||||||
|
it("includes public skill (no agent field) in description regardless of context", () => {
|
||||||
|
// given
|
||||||
|
const loadedSkills = [createMockSkill("public-skill")]
|
||||||
|
|
||||||
|
// when
|
||||||
|
const tool = createSkillTool({ skills: loadedSkills })
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(tool.description).toContain("public-skill")
|
||||||
|
})
|
||||||
|
|
||||||
|
it("execute still works for agent-restricted skill when called with correct agent context", async () => {
|
||||||
|
// given: tool created WITHOUT the restricted skill in description list,
|
||||||
|
// but the full skill list is available for execute via getSkills()
|
||||||
|
// (simulating what tool-registry does: description uses filtered list,
|
||||||
|
// but execute discovers from disk / full list)
|
||||||
|
const restrictedSkill = createMockSkill("oracle-only-skill", { agent: "oracle" })
|
||||||
|
const tool = createSkillTool({ skills: [restrictedSkill] })
|
||||||
|
const oracleContext = { ...mockContext, agent: "oracle" }
|
||||||
|
|
||||||
|
// when: oracle agent explicitly calls the skill
|
||||||
|
const result = await tool.execute({ name: "oracle-only-skill" }, oracleContext)
|
||||||
|
|
||||||
|
// then: execution succeeds
|
||||||
|
expect(result).toContain("oracle-only-skill")
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
describe("skill tool - dynamic description cache invalidation", () => {
|
describe("skill tool - dynamic description cache invalidation", () => {
|
||||||
it("keeps description available after execute misses a skill", async () => {
|
it("keeps description available after execute misses a skill", async () => {
|
||||||
// given
|
// given
|
||||||
|
|||||||
Reference in New Issue
Block a user