luna/oh-my-opencode
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #4146 from LYY/fix/skill-shortname-fallback

Browse Source 
fix: support unambiguous short skill names in delegated skill loading
This commit is contained in:
YeonGyu-Kim
2026-05-20 23:42:44 +09:00
committed by GitHub
parent 2fd627144a 47fced7473
commit 3e9c3f8ada
3 changed files with 214 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/features/opencode-skill-loader/skill-content.test.ts
+104
View File
@@ -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<string, string | undefined>
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", () => {
src/features/opencode-skill-loader/skill-template-resolver.ts
+14 -18
View File
@@ -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<string, string>()
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<string | null> {
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<string, string>; notFound: string[] }> {
const allSkills = await getAllSkills(options)
const skillMap = new Map<string, LoadedSkill>()
for (const skill of allSkills) {
skillMap.set(skill.name, skill)
}
const resolved = new Map<string, string>()
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)
src/tools/delegate-task/tools.test.ts
+96 -1
View File
@@ -3281,7 +3281,102 @@ describe("sisyphus-task", () => {
})
})
describe("buildSystemContent", () => {
describe("delegate task with short skill name", () => {
let envCleanup: Record<string, string | undefined>
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")