diff --git a/src/features/opencode-skill-loader/skill-content.test.ts b/src/features/opencode-skill-loader/skill-content.test.ts index dedf74413..1df3528c5 100644 --- a/src/features/opencode-skill-loader/skill-content.test.ts +++ b/src/features/opencode-skill-loader/skill-content.test.ts @@ -3,6 +3,7 @@ import { describe, it, expect, beforeEach, afterEach } from "bun:test" import { join } from "node:path" import { tmpdir } from "node:os" +import { mkdirSync, writeFileSync } from "node:fs" import { clearSkillCache, resolveSkillContent, @@ -11,6 +12,13 @@ import { resolveMultipleSkillsAsync, } from "./skill-content" +function createNestedSkill(baseDir: string, namespace: string, name: string, content: string): void { + const dir = join(baseDir, "skills", namespace, name) + mkdirSync(dir, { recursive: true }) + const yaml = `---\nname: ${name}\ndescription: ${namespace}/${name} skill\n---\n${content}` + writeFileSync(join(dir, "SKILL.md"), yaml) +} + let originalEnv: Record let testConfigDir: string @@ -185,6 +193,58 @@ describe("resolveSkillContentAsync", () => { // then: returns null expect(result).toBeNull() }) + + it("resolves nested skill by unique short name async", async () => { + // given: a discovered nested skill superpowers/systematic-debugging + createNestedSkill(testConfigDir, "superpowers", "systematic-debugging", "Short name test content") + + // when: resolving by short name + const result = await resolveSkillContentAsync("systematic-debugging") + + // then: finds the nested skill + expect(result).not.toBeNull() + expect(result).toContain("Short name test content") + }) + + it("returns null for ambiguous short name async", async () => { + // given: two skills with same short name in different namespaces + createNestedSkill(testConfigDir, "superpowers", "debugging", "superpowers content") + createNestedSkill(testConfigDir, "utils", "debugging", "utils content") + + // when: resolving by ambiguous short name + const result = await resolveSkillContentAsync("debugging") + + // then: ambiguous => null + expect(result).toBeNull() + }) + + it("prefers exact match over short name match async", async () => { + // given: an exact skill name "debugging" and a nested "superpowers/debugging" + createNestedSkill(testConfigDir, "superpowers", "debugging", "nested debugging") + // Exact match as a non-namespaced dir with SKILL.md + const exactDir = join(testConfigDir, "skills", "debugging") + mkdirSync(exactDir, { recursive: true }) + writeFileSync(join(exactDir, "SKILL.md"), "---\nname: debugging\ndescription: exact debugging\n---\nexact match content") + + // when: resolving by name "debugging" + const result = await resolveSkillContentAsync("debugging") + + // then: prefers exact match over the nested one + expect(result).not.toBeNull() + expect(result).toContain("exact match content") + }) + + it("is case-insensitive for short name matching async", async () => { + // given: a nested skill with lowercase name + createNestedSkill(testConfigDir, "superpowers", "systematic-debugging", "case insensitive match") + + // when: resolving by uppercase short name + const result = await resolveSkillContentAsync("Systematic-Debugging") + + // then: finds it case-insensitively + expect(result).not.toBeNull() + expect(result).toContain("case insensitive match") + }) }) describe("resolveMultipleSkillsAsync", () => { @@ -377,6 +437,50 @@ describe("resolveMultipleSkillsAsync", () => { expect(result.resolved.size).toBe(0) expect(result.notFound).toEqual([]) }) + + it("resolves nested skill by unique short name in mixed batch", async () => { + // given: nested skill and builtin skill + createNestedSkill(testConfigDir, "superpowers", "systematic-debugging", "short name resolved") + + // when: mixing short name with full builtin name + const result = await resolveMultipleSkillsAsync(["systematic-debugging", "playwright"]) + + // then: both resolved + expect(result.resolved.size).toBe(2) + expect(result.notFound).toEqual([]) + expect(result.resolved.get("systematic-debugging")).toContain("short name resolved") + expect(result.resolved.get("playwright")).toContain("Playwright Browser Automation") + }) + + it("does not resolve ambiguous short name in batch", async () => { + // given: two skills with same short name + createNestedSkill(testConfigDir, "superpowers", "debugging", "sp content") + createNestedSkill(testConfigDir, "utils", "debugging", "utils content") + + // when: resolving ambiguous short name with builtin + const result = await resolveMultipleSkillsAsync(["debugging", "playwright"]) + + // then: debugging not found, playwright resolved + expect(result.resolved.size).toBe(1) + expect(result.resolved.has("playwright")).toBe(true) + expect(result.notFound).toContain("debugging") + }) + + it("prefers exact match over short name in batch", async () => { + // given: an exact skill and a nested skill with same base name + const exactDir = join(testConfigDir, "skills", "debugging") + mkdirSync(exactDir, { recursive: true }) + writeFileSync(join(exactDir, "SKILL.md"), "---\nname: debugging\ndescription: exact debugging\n---\nexact match content") + createNestedSkill(testConfigDir, "superpowers", "debugging", "nested content") + + // when: resolving "debugging" in batch + const result = await resolveMultipleSkillsAsync(["debugging", "playwright"]) + + // then: exact match wins + expect(result.resolved.size).toBe(2) + expect(result.notFound).toEqual([]) + expect(result.resolved.get("debugging")).toContain("exact match content") + }) }) describe("resolveSkillContent with browserProvider", () => { diff --git a/src/features/opencode-skill-loader/skill-template-resolver.ts b/src/features/opencode-skill-loader/skill-template-resolver.ts index 0a9b31f18..54c8912ca 100644 --- a/src/features/opencode-skill-loader/skill-template-resolver.ts +++ b/src/features/opencode-skill-loader/skill-template-resolver.ts @@ -1,9 +1,9 @@ +import { matchSkillByName } from "../../tools/skill/skill-matcher" import { createBuiltinSkills } from "../builtin-skills/skills" -import type { LoadedSkill } from "./types" -import type { SkillResolutionOptions } from "./skill-resolution-options" import { injectGitMasterConfig } from "./git-master-template-injection" -import { getAllSkills } from "./skill-discovery" import { extractSkillTemplate } from "./loaded-skill-template-extractor" +import { getAllSkills } from "./skill-discovery" +import type { SkillResolutionOptions } from "./skill-resolution-options" export function resolveSkillContent(skillName: string, options?: SkillResolutionOptions): string | null { const skills = createBuiltinSkills({ @@ -14,7 +14,7 @@ export function resolveSkillContent(skillName: string, options?: SkillResolution const skill = skills.find((builtinSkill) => builtinSkill.name === skillName) if (!skill) return null - if (skillName === "git-master") { + if (skill.name === "git-master") { return injectGitMasterConfig(skill.template, options?.gitMasterConfig) } @@ -30,18 +30,18 @@ export function resolveMultipleSkills( disabledSkills: options?.disabledSkills, teamModeEnabled: options?.teamModeEnabled, }) - const skillMap = new Map(skills.map((skill) => [skill.name, skill.template])) + const skillMap = new Map(skills.map((skill) => [skill.name, skill])) const resolved = new Map() const notFound: string[] = [] for (const name of skillNames) { - const template = skillMap.get(name) - if (template) { - if (name === "git-master") { - resolved.set(name, injectGitMasterConfig(template, options?.gitMasterConfig)) + const match = skillMap.get(name) + if (match) { + if (match.name === "git-master") { + resolved.set(name, injectGitMasterConfig(match.template, options?.gitMasterConfig)) } else { - resolved.set(name, template) + resolved.set(name, match.template) } } else { notFound.push(name) @@ -56,12 +56,12 @@ export async function resolveSkillContentAsync( options?: SkillResolutionOptions ): Promise { const allSkills = await getAllSkills(options) - const skill = allSkills.find((loadedSkill) => loadedSkill.name === skillName) + const skill = matchSkillByName(allSkills, skillName) if (!skill) return null const template = await extractSkillTemplate(skill) - if (skillName === "git-master") { + if (skill.name === "git-master") { return injectGitMasterConfig(template, options?.gitMasterConfig) } @@ -73,19 +73,15 @@ export async function resolveMultipleSkillsAsync( options?: SkillResolutionOptions ): Promise<{ resolved: Map; notFound: string[] }> { const allSkills = await getAllSkills(options) - const skillMap = new Map() - for (const skill of allSkills) { - skillMap.set(skill.name, skill) - } const resolved = new Map() const notFound: string[] = [] for (const name of skillNames) { - const skill = skillMap.get(name) + const skill = matchSkillByName(allSkills, name) if (skill) { const template = await extractSkillTemplate(skill) - if (name === "git-master") { + if (skill.name === "git-master") { resolved.set(name, injectGitMasterConfig(template, options?.gitMasterConfig)) } else { resolved.set(name, template) diff --git a/src/tools/delegate-task/tools.test.ts b/src/tools/delegate-task/tools.test.ts index 5cfa7936e..fb493535c 100644 --- a/src/tools/delegate-task/tools.test.ts +++ b/src/tools/delegate-task/tools.test.ts @@ -3281,7 +3281,102 @@ describe("sisyphus-task", () => { }) }) - describe("buildSystemContent", () => { + describe("delegate task with short skill name", () => { + let envCleanup: Record + + beforeEach(() => { + envCleanup = { + CLAUDE_CONFIG_DIR: process.env.CLAUDE_CONFIG_DIR, + OPENCODE_CONFIG_DIR: process.env.OPENCODE_CONFIG_DIR, + } + }) + + afterEach(() => { + for (const [key, value] of Object.entries(envCleanup)) { + if (value !== undefined) { + process.env[key] = value + } else { + delete process.env[key] + } + } + }) + + test("resolves short named discovered skill and flows content into prompt", async () => { + // given: a nested discovered skill under a temp config dir + // (intentionally verifies the full integration path: delegate-task -> skill-resolver -> + // resolveMultipleSkillsAsync -> matchSkillByName, not just unit-testing the resolver) + const { join } = require("node:path") + const { tmpdir } = require("node:os") + const { mkdirSync, writeFileSync } = require("node:fs") + const unique = `delegate-shortname-${Date.now()}-${Math.random().toString(16).slice(2)}` + const testConfigDir = join(tmpdir(), unique) + process.env.CLAUDE_CONFIG_DIR = testConfigDir + process.env.OPENCODE_CONFIG_DIR = testConfigDir + const skillDir = join(testConfigDir, "skills", "superpowers", "systematic-debugging") + mkdirSync(skillDir, { recursive: true }) + writeFileSync( + join(skillDir, "SKILL.md"), + "---\nname: systematic-debugging\ndescription: Nested debug skill\n---\nDebug instructions" + ) + clearSkillCache() + + const { createDelegateTask } = require("./tools") + const mockManager = { launch: async () => ({}) } + + let promptBody: any + const promptMock = async (input: any) => { + promptBody = input.body + return { data: {} } + } + + const mockClient = { + app: { agents: async () => ({ data: [] }) }, + config: { get: async () => ({ data: { model: SYSTEM_DEFAULT_MODEL } }) }, + session: { + get: async () => ({ data: { directory: "/project" } }), + create: async () => ({ data: { id: "ses_shortname_test" } }), + prompt: promptMock, + promptAsync: promptMock, + messages: async () => ({ + data: [{ info: { role: "assistant" }, parts: [{ type: "text", text: "Done" }] }], + }), + status: async () => ({ data: {} }), + }, + } + + const tool = createDelegateTask({ + manager: mockManager, + client: mockClient, + }) + + const toolContext = { + sessionID: "parent-session", + messageID: "parent-message", + agent: "sisyphus", + abort: new AbortController().signal, + } + + // when: using short name in load_skills + const result = await tool.execute( + { + description: "Test short name resolution", + prompt: "Do something", + category: "ultrabrain", + run_in_background: false, + load_skills: ["systematic-debugging"], + }, + toolContext + ) + + // then: must NOT return "Skills not found" (failing means short name wasn't resolved) + expect(result).not.toContain("Skills not found") + // and the resolved skill content must have been injected into the prompt body + expect(promptBody).toBeDefined() + expect(promptBody.system).toContain("Debug instructions") + }) + }) + + describe("buildSystemContent", () => { test("returns undefined when no skills and no category promptAppend", () => { // given const { buildSystemContent } = require("./tools")