From 77997d8e74ae63b280cd5c217bacd99b474f3dc7 Mon Sep 17 00:00:00 2001 From: LYY Date: Mon, 18 May 2026 16:25:04 +0800 Subject: [PATCH] fix(skill-loader): support unambiguous short skill names --- .../skill-content.test.ts | 104 ++++++++++++++++++ .../skill-template-resolver.ts | 14 +-- src/tools/delegate-task/tools.test.ts | 85 +++++++++++++- 3 files changed, 193 insertions(+), 10 deletions(-) diff --git a/src/features/opencode-skill-loader/skill-content.test.ts b/src/features/opencode-skill-loader/skill-content.test.ts index dedf74413..d82d012a5 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") + // Also create the exact match by placing it at dir root + const dir = join(testConfigDir, "skills") + mkdirSync(dir, { recursive: true }) + writeFileSync(join(dir, "debugging.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 dir = join(testConfigDir, "skills") + mkdirSync(dir, { recursive: true }) + writeFileSync(join(dir, "debugging.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..8ba3a6c50 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({ @@ -56,7 +56,7 @@ 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) @@ -73,16 +73,12 @@ 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") { diff --git a/src/tools/delegate-task/tools.test.ts b/src/tools/delegate-task/tools.test.ts index 35dde5055..92aeb6e09 100644 --- a/src/tools/delegate-task/tools.test.ts +++ b/src/tools/delegate-task/tools.test.ts @@ -3208,7 +3208,90 @@ 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 without reporting not found", async () => { + // given: a nested discovered skill under a temp config dir + 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 () => ({}) } + 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: async () => ({ data: {} }), + promptAsync: async () => ({ data: {} }), + 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: should NOT report "Skills not found" + expect(result).not.toContain("Skills not found") + }) + }) + + describe("buildSystemContent", () => { test("returns undefined when no skills and no category promptAppend", () => { // given const { buildSystemContent } = require("./tools")