fix(skill): support YAML array format for allowed-tools field (#1163)
Fixes #1021 The allowed-tools field in skill frontmatter now supports both formats: - Space-separated string: 'allowed-tools: Read Write Edit Bash' - YAML array: 'allowed-tools: [Read, Write, Edit, Bash]' - Multi-line YAML array format also works Previously, skills using YAML array format would silently fail to parse, causing them to not appear in the <available_skills> list. Changes: - Updated parseAllowedTools() in loader.ts, async-loader.ts, and merger.ts to handle both string and string[] types - Updated SkillMetadata type to accept string | string[] for allowed-tools - Added 4 test cases covering all allowed-tools formats
This commit is contained in:
@@ -128,8 +128,15 @@ $ARGUMENTS
|
||||
}
|
||||
}
|
||||
|
||||
function parseAllowedTools(allowedTools: string | undefined): string[] | undefined {
|
||||
function parseAllowedTools(allowedTools: string | string[] | undefined): string[] | undefined {
|
||||
if (!allowedTools) return undefined
|
||||
|
||||
// Handle YAML array format: already parsed as string[]
|
||||
if (Array.isArray(allowedTools)) {
|
||||
return allowedTools.map(t => t.trim()).filter(Boolean)
|
||||
}
|
||||
|
||||
// Handle space-separated string format: "Read Write Edit Bash"
|
||||
return allowedTools.split(/\s+/).filter(Boolean)
|
||||
}
|
||||
|
||||
|
||||
@@ -268,6 +268,123 @@ Skill body.
|
||||
} finally {
|
||||
process.chdir(originalCwd)
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
describe("allowed-tools parsing", () => {
|
||||
it("parses space-separated allowed-tools string", async () => {
|
||||
// #given
|
||||
const skillContent = `---
|
||||
name: space-separated-tools
|
||||
description: Skill with space-separated allowed-tools
|
||||
allowed-tools: Read Write Edit Bash
|
||||
---
|
||||
Skill body.
|
||||
`
|
||||
createTestSkill("space-separated-tools", skillContent)
|
||||
|
||||
// #when
|
||||
const { discoverSkills } = await import("./loader")
|
||||
const originalCwd = process.cwd()
|
||||
process.chdir(TEST_DIR)
|
||||
|
||||
try {
|
||||
const skills = await discoverSkills({ includeClaudeCodePaths: false })
|
||||
const skill = skills.find(s => s.name === "space-separated-tools")
|
||||
|
||||
// #then
|
||||
expect(skill).toBeDefined()
|
||||
expect(skill?.allowedTools).toEqual(["Read", "Write", "Edit", "Bash"])
|
||||
} finally {
|
||||
process.chdir(originalCwd)
|
||||
}
|
||||
})
|
||||
|
||||
it("parses YAML inline array allowed-tools", async () => {
|
||||
// #given
|
||||
const skillContent = `---
|
||||
name: yaml-inline-array
|
||||
description: Skill with YAML inline array allowed-tools
|
||||
allowed-tools: [Read, Write, Edit, Bash]
|
||||
---
|
||||
Skill body.
|
||||
`
|
||||
createTestSkill("yaml-inline-array", skillContent)
|
||||
|
||||
// #when
|
||||
const { discoverSkills } = await import("./loader")
|
||||
const originalCwd = process.cwd()
|
||||
process.chdir(TEST_DIR)
|
||||
|
||||
try {
|
||||
const skills = await discoverSkills({ includeClaudeCodePaths: false })
|
||||
const skill = skills.find(s => s.name === "yaml-inline-array")
|
||||
|
||||
// #then
|
||||
expect(skill).toBeDefined()
|
||||
expect(skill?.allowedTools).toEqual(["Read", "Write", "Edit", "Bash"])
|
||||
} finally {
|
||||
process.chdir(originalCwd)
|
||||
}
|
||||
})
|
||||
|
||||
it("parses YAML multi-line array allowed-tools", async () => {
|
||||
// #given
|
||||
const skillContent = `---
|
||||
name: yaml-multiline-array
|
||||
description: Skill with YAML multi-line array allowed-tools
|
||||
allowed-tools:
|
||||
- Read
|
||||
- Write
|
||||
- Edit
|
||||
- Bash
|
||||
---
|
||||
Skill body.
|
||||
`
|
||||
createTestSkill("yaml-multiline-array", skillContent)
|
||||
|
||||
// #when
|
||||
const { discoverSkills } = await import("./loader")
|
||||
const originalCwd = process.cwd()
|
||||
process.chdir(TEST_DIR)
|
||||
|
||||
try {
|
||||
const skills = await discoverSkills({ includeClaudeCodePaths: false })
|
||||
const skill = skills.find(s => s.name === "yaml-multiline-array")
|
||||
|
||||
// #then
|
||||
expect(skill).toBeDefined()
|
||||
expect(skill?.allowedTools).toEqual(["Read", "Write", "Edit", "Bash"])
|
||||
} finally {
|
||||
process.chdir(originalCwd)
|
||||
}
|
||||
})
|
||||
|
||||
it("returns undefined for skill without allowed-tools", async () => {
|
||||
// #given
|
||||
const skillContent = `---
|
||||
name: no-allowed-tools
|
||||
description: Skill without allowed-tools field
|
||||
---
|
||||
Skill body.
|
||||
`
|
||||
createTestSkill("no-allowed-tools", skillContent)
|
||||
|
||||
// #when
|
||||
const { discoverSkills } = await import("./loader")
|
||||
const originalCwd = process.cwd()
|
||||
process.chdir(TEST_DIR)
|
||||
|
||||
try {
|
||||
const skills = await discoverSkills({ includeClaudeCodePaths: false })
|
||||
const skill = skills.find(s => s.name === "no-allowed-tools")
|
||||
|
||||
// #then
|
||||
expect(skill).toBeDefined()
|
||||
expect(skill?.allowedTools).toBeUndefined()
|
||||
} finally {
|
||||
process.chdir(originalCwd)
|
||||
}
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -50,8 +50,15 @@ async function loadMcpJsonFromDir(skillDir: string): Promise<SkillMcpConfig | un
|
||||
return undefined
|
||||
}
|
||||
|
||||
function parseAllowedTools(allowedTools: string | undefined): string[] | undefined {
|
||||
function parseAllowedTools(allowedTools: string | string[] | undefined): string[] | undefined {
|
||||
if (!allowedTools) return undefined
|
||||
|
||||
// Handle YAML array format: already parsed as string[]
|
||||
if (Array.isArray(allowedTools)) {
|
||||
return allowedTools.map(t => t.trim()).filter(Boolean)
|
||||
}
|
||||
|
||||
// Handle space-separated string format: "Read Write Edit Bash"
|
||||
return allowedTools.split(/\s+/).filter(Boolean)
|
||||
}
|
||||
|
||||
|
||||
@@ -9,6 +9,14 @@ import { parseFrontmatter } from "../../shared/frontmatter"
|
||||
import { sanitizeModelField } from "../../shared/model-sanitizer"
|
||||
import { deepMerge } from "../../shared/deep-merge"
|
||||
|
||||
function parseAllowedToolsFromMetadata(allowedTools: string | string[] | undefined): string[] | undefined {
|
||||
if (!allowedTools) return undefined
|
||||
if (Array.isArray(allowedTools)) {
|
||||
return allowedTools.map(t => t.trim()).filter(Boolean)
|
||||
}
|
||||
return allowedTools.split(/\s+/).filter(Boolean)
|
||||
}
|
||||
|
||||
const SCOPE_PRIORITY: Record<SkillScope, number> = {
|
||||
builtin: 1,
|
||||
config: 2,
|
||||
@@ -119,7 +127,7 @@ $ARGUMENTS
|
||||
}
|
||||
|
||||
const allowedTools = entry["allowed-tools"] ||
|
||||
(fileMetadata["allowed-tools"] ? fileMetadata["allowed-tools"].split(/\s+/).filter(Boolean) : undefined)
|
||||
(fileMetadata["allowed-tools"] ? parseAllowedToolsFromMetadata(fileMetadata["allowed-tools"]) : undefined)
|
||||
|
||||
return {
|
||||
name,
|
||||
|
||||
@@ -13,7 +13,7 @@ export interface SkillMetadata {
|
||||
license?: string
|
||||
compatibility?: string
|
||||
metadata?: Record<string, string>
|
||||
"allowed-tools"?: string
|
||||
"allowed-tools"?: string | string[]
|
||||
mcp?: SkillMcpConfig
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user