fix: generate skill/slashcommand descriptions synchronously when pre-provided (#1087)
* fix: generate skill/slashcommand tool descriptions synchronously when pre-provided When skills are passed via options (pre-resolved), build the tool description synchronously instead of fire-and-forget async. This eliminates the race condition where the description getter returns the bare prefix before the async cache-warming microtask completes. Fixes #1039 * chore: changes by sisyphus-dev-ai --------- Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
This commit is contained in:
parent
0aa8f486af
commit
208af055ef
@ -57,6 +57,45 @@ const mockContext = {
|
|||||||
abort: new AbortController().signal,
|
abort: new AbortController().signal,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
describe("skill tool - synchronous description", () => {
|
||||||
|
it("includes available_skills immediately when skills are pre-provided", () => {
|
||||||
|
// #given
|
||||||
|
const loadedSkills = [createMockSkill("test-skill")]
|
||||||
|
|
||||||
|
// #when
|
||||||
|
const tool = createSkillTool({ skills: loadedSkills })
|
||||||
|
|
||||||
|
// #then
|
||||||
|
expect(tool.description).toContain("<available_skills>")
|
||||||
|
expect(tool.description).toContain("test-skill")
|
||||||
|
})
|
||||||
|
|
||||||
|
it("includes all pre-provided skills in available_skills immediately", () => {
|
||||||
|
// #given
|
||||||
|
const loadedSkills = [
|
||||||
|
createMockSkill("playwright"),
|
||||||
|
createMockSkill("frontend-ui-ux"),
|
||||||
|
createMockSkill("git-master"),
|
||||||
|
]
|
||||||
|
|
||||||
|
// #when
|
||||||
|
const tool = createSkillTool({ skills: loadedSkills })
|
||||||
|
|
||||||
|
// #then
|
||||||
|
expect(tool.description).toContain("playwright")
|
||||||
|
expect(tool.description).toContain("frontend-ui-ux")
|
||||||
|
expect(tool.description).toContain("git-master")
|
||||||
|
})
|
||||||
|
|
||||||
|
it("shows no-skills message immediately when empty skills are pre-provided", () => {
|
||||||
|
// #given / #when
|
||||||
|
const tool = createSkillTool({ skills: [] })
|
||||||
|
|
||||||
|
// #then
|
||||||
|
expect(tool.description).toContain("No skills are currently available")
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
describe("skill tool - agent restriction", () => {
|
describe("skill tool - agent restriction", () => {
|
||||||
it("allows skill without agent restriction to any agent", async () => {
|
it("allows skill without agent restriction to any agent", async () => {
|
||||||
// #given
|
// #given
|
||||||
|
|||||||
@ -147,7 +147,14 @@ export function createSkillTool(options: SkillLoadOptions = {}): ToolDefinition
|
|||||||
return cachedDescription
|
return cachedDescription
|
||||||
}
|
}
|
||||||
|
|
||||||
getDescription()
|
if (options.skills) {
|
||||||
|
const skillInfos = options.skills.map(loadedSkillToInfo)
|
||||||
|
cachedDescription = skillInfos.length === 0
|
||||||
|
? TOOL_DESCRIPTION_NO_SKILLS
|
||||||
|
: TOOL_DESCRIPTION_PREFIX + formatSkillsXml(skillInfos)
|
||||||
|
} else {
|
||||||
|
getDescription()
|
||||||
|
}
|
||||||
|
|
||||||
return tool({
|
return tool({
|
||||||
get description() {
|
get description() {
|
||||||
|
|||||||
76
src/tools/slashcommand/tools.test.ts
Normal file
76
src/tools/slashcommand/tools.test.ts
Normal file
@ -0,0 +1,76 @@
|
|||||||
|
import { describe, it, expect } from "bun:test"
|
||||||
|
import { createSlashcommandTool } from "./tools"
|
||||||
|
import type { CommandInfo } from "./types"
|
||||||
|
import type { LoadedSkill } from "../../features/opencode-skill-loader"
|
||||||
|
|
||||||
|
function createMockCommand(name: string, description = ""): CommandInfo {
|
||||||
|
return {
|
||||||
|
name,
|
||||||
|
metadata: {
|
||||||
|
name,
|
||||||
|
description: description || `Test command ${name}`,
|
||||||
|
},
|
||||||
|
scope: "builtin",
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function createMockSkill(name: string, description = ""): LoadedSkill {
|
||||||
|
return {
|
||||||
|
name,
|
||||||
|
path: `/test/skills/${name}/SKILL.md`,
|
||||||
|
resolvedPath: `/test/skills/${name}`,
|
||||||
|
definition: {
|
||||||
|
name,
|
||||||
|
description: description || `Test skill ${name}`,
|
||||||
|
template: "Test template",
|
||||||
|
},
|
||||||
|
scope: "opencode-project",
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("slashcommand tool - synchronous description", () => {
|
||||||
|
it("includes available_skills immediately when commands and skills are pre-provided", () => {
|
||||||
|
// #given
|
||||||
|
const commands = [createMockCommand("commit", "Create a git commit")]
|
||||||
|
const skills = [createMockSkill("playwright", "Browser automation via Playwright MCP")]
|
||||||
|
|
||||||
|
// #when
|
||||||
|
const tool = createSlashcommandTool({ commands, skills })
|
||||||
|
|
||||||
|
// #then
|
||||||
|
expect(tool.description).toContain("<available_skills>")
|
||||||
|
expect(tool.description).toContain("commit")
|
||||||
|
expect(tool.description).toContain("playwright")
|
||||||
|
})
|
||||||
|
|
||||||
|
it("includes all pre-provided commands and skills in description immediately", () => {
|
||||||
|
// #given
|
||||||
|
const commands = [
|
||||||
|
createMockCommand("commit", "Git commit"),
|
||||||
|
createMockCommand("plan", "Create plan"),
|
||||||
|
]
|
||||||
|
const skills = [
|
||||||
|
createMockSkill("playwright", "Browser automation"),
|
||||||
|
createMockSkill("frontend-ui-ux", "Frontend design"),
|
||||||
|
createMockSkill("git-master", "Git operations"),
|
||||||
|
]
|
||||||
|
|
||||||
|
// #when
|
||||||
|
const tool = createSlashcommandTool({ commands, skills })
|
||||||
|
|
||||||
|
// #then
|
||||||
|
expect(tool.description).toContain("commit")
|
||||||
|
expect(tool.description).toContain("plan")
|
||||||
|
expect(tool.description).toContain("playwright")
|
||||||
|
expect(tool.description).toContain("frontend-ui-ux")
|
||||||
|
expect(tool.description).toContain("git-master")
|
||||||
|
})
|
||||||
|
|
||||||
|
it("shows prefix-only description when both commands and skills are empty", () => {
|
||||||
|
// #given / #when
|
||||||
|
const tool = createSlashcommandTool({ commands: [], skills: [] })
|
||||||
|
|
||||||
|
// #then - even with no items, description should be built synchronously (not just prefix)
|
||||||
|
expect(tool.description).toContain("Load a skill")
|
||||||
|
})
|
||||||
|
})
|
||||||
@ -210,8 +210,12 @@ export function createSlashcommandTool(options: SlashcommandToolOptions = {}): T
|
|||||||
return cachedDescription
|
return cachedDescription
|
||||||
}
|
}
|
||||||
|
|
||||||
// Pre-warm the cache immediately
|
if (options.commands !== undefined && options.skills !== undefined) {
|
||||||
buildDescription()
|
const allItems = [...options.commands, ...options.skills.map(skillToCommandInfo)]
|
||||||
|
cachedDescription = buildDescriptionFromItems(allItems)
|
||||||
|
} else {
|
||||||
|
buildDescription()
|
||||||
|
}
|
||||||
|
|
||||||
return tool({
|
return tool({
|
||||||
get description() {
|
get description() {
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user