Merge pull request #4219 from sjawhar/fix/skill-discovery-opencode-config
fix(skill-discovery): load native OpenCode skills in task delegation
This commit is contained in:
@@ -226,6 +226,7 @@ export function createToolRegistry(args: {
|
||||
teamModeEnabled: pluginConfig.team_mode?.enabled ?? false,
|
||||
availableCategories,
|
||||
availableSkills: skillContext.availableSkills,
|
||||
nativeSkills: "skills" in ctx ? (ctx as { skills: SkillLoadOptions["nativeSkills"] }).skills : undefined,
|
||||
sisyphusAgentConfig: pluginConfig.sisyphus_agent,
|
||||
syncPollTimeoutMs: pluginConfig.background_task?.syncPollTimeoutMs,
|
||||
modelFallbackControllerAccessor: managers.modelFallbackControllerAccessor,
|
||||
|
||||
@@ -123,3 +123,68 @@ describe("prompt-builder", () => {
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe("buildSystemContent — nativeSkillInfos merging", () => {
|
||||
test("#given a nativeSkill name not in availableSkills #when block is built #then native name appears", () => {
|
||||
// given
|
||||
const availableSkills: AvailableSkill[] = [
|
||||
{ name: "omo-skill", description: "From OMO disk", location: "project" },
|
||||
]
|
||||
const nativeSkillInfos = [
|
||||
{ name: "test-driven-development", description: "TDD discipline", location: "/fake/SKILL.md" },
|
||||
]
|
||||
|
||||
// when
|
||||
const result = buildSystemContent({
|
||||
agentName: "explore",
|
||||
availableSkills,
|
||||
nativeSkillInfos,
|
||||
})
|
||||
|
||||
// then
|
||||
expect(result).toBeDefined()
|
||||
expect(result).toContain("omo-skill")
|
||||
expect(result).toContain("test-driven-development")
|
||||
expect(result).toContain("TDD discipline")
|
||||
})
|
||||
|
||||
test("#given a name in BOTH availableSkills AND nativeSkillInfos #when block is built #then OMO description wins", () => {
|
||||
// given
|
||||
const availableSkills: AvailableSkill[] = [
|
||||
{ name: "shared", description: "omo-version-of-shared", location: "project" },
|
||||
]
|
||||
const nativeSkillInfos = [
|
||||
{ name: "shared", description: "native-version-of-shared", location: "/fake/SKILL.md" },
|
||||
]
|
||||
|
||||
// when
|
||||
const result = buildSystemContent({
|
||||
agentName: "explore",
|
||||
availableSkills,
|
||||
nativeSkillInfos,
|
||||
})
|
||||
|
||||
// then
|
||||
expect(result).toBeDefined()
|
||||
expect(result).toContain("omo-version-of-shared")
|
||||
expect(result).not.toContain("native-version-of-shared")
|
||||
})
|
||||
|
||||
test("#given empty availableSkills and a nativeSkillInfo #when block is built #then native skill renders", () => {
|
||||
// given
|
||||
const nativeSkillInfos = [
|
||||
{ name: "brainstorming", description: "Use before any creative work", location: "/fake/SKILL.md" },
|
||||
]
|
||||
|
||||
// when
|
||||
const result = buildSystemContent({
|
||||
agentName: "explore",
|
||||
availableSkills: [],
|
||||
nativeSkillInfos,
|
||||
})
|
||||
|
||||
// then
|
||||
expect(result).toBeDefined()
|
||||
expect(result).toContain("brainstorming")
|
||||
})
|
||||
})
|
||||
|
||||
@@ -22,6 +22,21 @@ ${TDD_LINE}`
|
||||
return PLAN_AGENT_PROMPT_BASE
|
||||
}
|
||||
|
||||
function mergeNativeIntoAvailable(
|
||||
skills: AvailableSkill[],
|
||||
nativeSkillInfos: { name: string; description: string; location: string }[] | undefined,
|
||||
): AvailableSkill[] {
|
||||
if (!nativeSkillInfos || nativeSkillInfos.length === 0) return skills
|
||||
const knownNames = new Set(skills.map((s) => s.name))
|
||||
const merged = [...skills]
|
||||
for (const native of nativeSkillInfos) {
|
||||
if (knownNames.has(native.name)) continue
|
||||
merged.push({ name: native.name, description: native.description, location: "user" })
|
||||
knownNames.add(native.name)
|
||||
}
|
||||
return merged
|
||||
}
|
||||
|
||||
function buildAvailableSkillsSection(skills: AvailableSkill[]): string {
|
||||
if (skills.length === 0) {
|
||||
return ""
|
||||
@@ -66,15 +81,18 @@ export function buildSystemContent(input: BuildSystemContentInput): string | und
|
||||
agentName,
|
||||
availableCategories,
|
||||
availableSkills,
|
||||
nativeSkillInfos,
|
||||
} = input
|
||||
|
||||
const effectiveAvailableSkills = mergeNativeIntoAvailable(availableSkills ?? [], nativeSkillInfos)
|
||||
|
||||
const isPlan = isPlanAgent(agentName)
|
||||
const planAgentPrepend = isPlan
|
||||
? buildPlanAgentSystemPrepend(availableCategories, availableSkills)
|
||||
? buildPlanAgentSystemPrepend(availableCategories, effectiveAvailableSkills)
|
||||
: ""
|
||||
|
||||
const skillsSection = !isPlan
|
||||
? buildAvailableSkillsSection(availableSkills ?? [])
|
||||
? buildAvailableSkillsSection(effectiveAvailableSkills)
|
||||
: ""
|
||||
|
||||
const baseAgentsContext = agentsContext ?? planAgentPrepend
|
||||
|
||||
@@ -0,0 +1,185 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, mock } from "bun:test"
|
||||
import { mkdirSync, rmSync, writeFileSync } from "node:fs"
|
||||
import { join } from "node:path"
|
||||
import { tmpdir } from "node:os"
|
||||
import { resolveSkillContent } from "./skill-resolver"
|
||||
import { clearSkillCache } from "../../features/opencode-skill-loader/skill-discovery"
|
||||
|
||||
const TEST_DIR = join(tmpdir(), `skill-resolver-test-${Date.now()}`)
|
||||
|
||||
function makeNativeSkill(name: string, description: string, content: string) {
|
||||
return { name, description, location: `/fake/native/${name}/SKILL.md`, content }
|
||||
}
|
||||
|
||||
function makeNativeAccessor(skills: ReturnType<typeof makeNativeSkill>[]) {
|
||||
return {
|
||||
all: () => skills,
|
||||
get: (name: string) => skills.find((s) => s.name === name),
|
||||
dirs: () => ["/fake/native"],
|
||||
}
|
||||
}
|
||||
|
||||
describe("resolveSkillContent — nativeSkills integration", () => {
|
||||
beforeEach(() => {
|
||||
clearSkillCache()
|
||||
mkdirSync(TEST_DIR, { recursive: true })
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
clearSkillCache()
|
||||
rmSync(TEST_DIR, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
it("#given an empty skill list #when resolved #then returns no content with no error", async () => {
|
||||
// when
|
||||
const result = await resolveSkillContent([], {})
|
||||
// then
|
||||
expect(result).toEqual({ content: undefined, contents: [], error: null })
|
||||
})
|
||||
|
||||
it("#given a skill that lives only in nativeSkills #when resolved #then returns its content", async () => {
|
||||
// given
|
||||
const native = makeNativeSkill(
|
||||
"test-driven-development",
|
||||
"TDD discipline",
|
||||
"## Red-Green-Refactor\nWrite a failing test first.",
|
||||
)
|
||||
const nativeSkills = makeNativeAccessor([native])
|
||||
|
||||
// when
|
||||
const result = await resolveSkillContent(["test-driven-development"], {
|
||||
nativeSkills,
|
||||
directory: TEST_DIR,
|
||||
})
|
||||
|
||||
// then
|
||||
expect(result.error).toBeNull()
|
||||
expect(result.contents).toHaveLength(1)
|
||||
expect(result.content).toContain("Red-Green-Refactor")
|
||||
expect(result.content).toContain("Write a failing test first")
|
||||
})
|
||||
|
||||
it("#given a name present in both OMO disk-discovered and nativeSkills #when resolved #then OMO content wins", async () => {
|
||||
// given a name we know does NOT collide with builtins; force a fake one
|
||||
// We use a fake disk skill via the merger pattern: write a SKILL.md under TEST_DIR/.opencode/skills/
|
||||
const skillsDir = join(TEST_DIR, ".opencode", "skills", "shared-name")
|
||||
mkdirSync(skillsDir, { recursive: true })
|
||||
writeFileSync(
|
||||
join(skillsDir, "SKILL.md"),
|
||||
"---\nname: shared-name-test-skill\ndescription: from disk\n---\nOMO_DISK_BODY",
|
||||
)
|
||||
const native = makeNativeSkill(
|
||||
"shared-name-test-skill",
|
||||
"from native",
|
||||
"NATIVE_BODY",
|
||||
)
|
||||
const nativeSkills = makeNativeAccessor([native])
|
||||
|
||||
// when
|
||||
const result = await resolveSkillContent(["shared-name-test-skill"], {
|
||||
nativeSkills,
|
||||
directory: TEST_DIR,
|
||||
})
|
||||
|
||||
// then — OMO wins on name collision (mergeNativeSkills skips already-known names)
|
||||
expect(result.error).toBeNull()
|
||||
expect(result.content).toContain("OMO_DISK_BODY")
|
||||
expect(result.content).not.toContain("NATIVE_BODY")
|
||||
})
|
||||
|
||||
it("#given a skill that exists in neither registry #when resolved #then returns notFound error listing the merged set", async () => {
|
||||
// given
|
||||
const native = makeNativeSkill("alpha", "alpha desc", "alpha body")
|
||||
const nativeSkills = makeNativeAccessor([native])
|
||||
|
||||
// when
|
||||
const result = await resolveSkillContent(["does-not-exist"], {
|
||||
nativeSkills,
|
||||
directory: TEST_DIR,
|
||||
})
|
||||
|
||||
// then
|
||||
expect(result.error).toBeTruthy()
|
||||
expect(result.error).toContain("does-not-exist")
|
||||
// the merged "Available" list should include the native skill name
|
||||
expect(result.error).toContain("alpha")
|
||||
})
|
||||
|
||||
it("#given nativeSkills.all() throws #when resolved #then degrades gracefully (still finds disk-discovered skills)", async () => {
|
||||
// given
|
||||
const exploding = {
|
||||
all: () => {
|
||||
throw new Error("boom")
|
||||
},
|
||||
get: () => undefined,
|
||||
dirs: () => [],
|
||||
}
|
||||
|
||||
// when (we just need this not to throw or hang)
|
||||
const result = await resolveSkillContent(["missing-skill"], {
|
||||
nativeSkills: exploding,
|
||||
directory: TEST_DIR,
|
||||
})
|
||||
|
||||
// then — error path still works, no crash
|
||||
expect(result.error).toBeTruthy()
|
||||
expect(result.error).toContain("missing-skill")
|
||||
})
|
||||
|
||||
it("#given preloaded native skill entries #when resolved #then uses them without calling nativeSkills again", async () => {
|
||||
// given
|
||||
const native = makeNativeSkill(
|
||||
"preloaded-native-skill",
|
||||
"preloaded desc",
|
||||
"PRELOADED_NATIVE_BODY",
|
||||
)
|
||||
const nativeSkills = {
|
||||
all: mock(() => {
|
||||
throw new Error("nativeSkills.all should not be called")
|
||||
}),
|
||||
get: () => undefined,
|
||||
dirs: () => [],
|
||||
}
|
||||
|
||||
// when
|
||||
const result = await resolveSkillContent(["preloaded-native-skill"], {
|
||||
nativeSkills,
|
||||
nativeSkillEntries: [native],
|
||||
directory: TEST_DIR,
|
||||
})
|
||||
|
||||
// then
|
||||
expect(result.error).toBeNull()
|
||||
expect(result.content).toContain("PRELOADED_NATIVE_BODY")
|
||||
expect(nativeSkills.all).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it("#given a namespaced OMO skill #when requested by unique short name with different case #then resolves it", async () => {
|
||||
// given
|
||||
const skillsDir = join(TEST_DIR, ".opencode", "skills", "superpowers", "systematic-debugging")
|
||||
mkdirSync(skillsDir, { recursive: true })
|
||||
writeFileSync(
|
||||
join(skillsDir, "SKILL.md"),
|
||||
"---\nname: superpowers/systematic-debugging\ndescription: Systematic debugging\n---\nSHORT_NAME_BODY",
|
||||
)
|
||||
|
||||
// when
|
||||
const result = await resolveSkillContent(["SYSTEMATIC-DEBUGGING"], {
|
||||
directory: TEST_DIR,
|
||||
})
|
||||
|
||||
// then
|
||||
expect(result.error).toBeNull()
|
||||
expect(result.content).toContain("SHORT_NAME_BODY")
|
||||
})
|
||||
|
||||
it("#given no nativeSkills passed #when resolved #then behaves like pre-fix (no native discovery)", async () => {
|
||||
// when
|
||||
const result = await resolveSkillContent(["does-not-exist"], {
|
||||
directory: TEST_DIR,
|
||||
})
|
||||
// then
|
||||
expect(result.error).toBeTruthy()
|
||||
expect(result.error).toContain("does-not-exist")
|
||||
})
|
||||
})
|
||||
@@ -1,26 +1,91 @@
|
||||
import type { GitMasterConfig, BrowserAutomationProvider } from "../../config/schema"
|
||||
import { resolveMultipleSkillsAsync } from "../../features/opencode-skill-loader/skill-content"
|
||||
import { discoverSkills } from "../../features/opencode-skill-loader"
|
||||
import { getAllSkills } from "../../features/opencode-skill-loader/skill-discovery"
|
||||
import {
|
||||
extractSkillTemplate,
|
||||
injectGitMasterConfig,
|
||||
} from "../../features/opencode-skill-loader/skill-content"
|
||||
import type { LoadedSkill } from "../../features/opencode-skill-loader/types"
|
||||
import { log } from "../../shared/logger"
|
||||
import { mergeNativeSkills } from "../skill/native-skills"
|
||||
import type { NativeSkillEntry } from "../skill/native-skills"
|
||||
import { matchSkillByName } from "../skill/skill-matcher"
|
||||
import type { DelegateTaskToolOptions } from "./types"
|
||||
|
||||
type ResolveSkillContentOptions = {
|
||||
gitMasterConfig?: GitMasterConfig
|
||||
browserProvider?: BrowserAutomationProvider
|
||||
disabledSkills?: Set<string>
|
||||
teamModeEnabled?: boolean
|
||||
directory?: string
|
||||
nativeSkills?: DelegateTaskToolOptions["nativeSkills"]
|
||||
nativeSkillEntries?: NativeSkillEntry[]
|
||||
}
|
||||
|
||||
async function loadNativeSkillEntries(
|
||||
nativeSkills: DelegateTaskToolOptions["nativeSkills"] | undefined,
|
||||
nativeSkillEntries: NativeSkillEntry[] | undefined,
|
||||
): Promise<NativeSkillEntry[]> {
|
||||
if (nativeSkillEntries) return nativeSkillEntries
|
||||
if (!nativeSkills) return []
|
||||
try {
|
||||
const list = await nativeSkills.all()
|
||||
return Array.isArray(list) ? list : []
|
||||
} catch (err) {
|
||||
log("[skill-resolver] nativeSkills.all() failed; falling back to disk-only skills", {
|
||||
error: String(err),
|
||||
})
|
||||
return []
|
||||
}
|
||||
}
|
||||
|
||||
export async function resolveSkillContent(
|
||||
skills: string[],
|
||||
options: {
|
||||
gitMasterConfig?: GitMasterConfig
|
||||
browserProvider?: BrowserAutomationProvider
|
||||
disabledSkills?: Set<string>
|
||||
teamModeEnabled?: boolean
|
||||
directory?: string
|
||||
}
|
||||
options: ResolveSkillContentOptions,
|
||||
): Promise<{ content: string | undefined; contents: string[]; error: string | null }> {
|
||||
if (skills.length === 0) {
|
||||
return { content: undefined, contents: [], error: null }
|
||||
}
|
||||
|
||||
const { resolved, notFound } = await resolveMultipleSkillsAsync(skills, options)
|
||||
// Build the merged skill registry: OMO disk-discovered + OpenCode native (config.skills.paths).
|
||||
// OMO wins on collisions, matching the existing mergeNativeSkills semantics.
|
||||
const baseSkills: LoadedSkill[] = [...(await getAllSkills(options))]
|
||||
const nativeEntries = await loadNativeSkillEntries(options.nativeSkills, options.nativeSkillEntries)
|
||||
mergeNativeSkills(baseSkills, nativeEntries)
|
||||
|
||||
const resolved = new Map<string, string>()
|
||||
const notFound: string[] = []
|
||||
|
||||
for (const name of skills) {
|
||||
const skill = matchSkillByName(baseSkills, name)
|
||||
if (!skill) {
|
||||
notFound.push(name)
|
||||
continue
|
||||
}
|
||||
const template = extractSkillTemplate(skill)
|
||||
if (name === "git-master") {
|
||||
resolved.set(name, injectGitMasterConfig(template, options.gitMasterConfig))
|
||||
} else {
|
||||
resolved.set(name, template)
|
||||
}
|
||||
}
|
||||
|
||||
if (notFound.length > 0) {
|
||||
const allSkills = await discoverSkills({ includeClaudeCodePaths: true, directory: options?.directory })
|
||||
const available = allSkills.map(s => s.name).join(", ")
|
||||
return { content: undefined, contents: [], error: `Skills not found: ${notFound.join(", ")}. Available: ${available}` }
|
||||
// For the error message, include the freshest possible "Available" list — same merged set we
|
||||
// just searched, plus a fallback re-discovery if for some reason that came up empty.
|
||||
let available = baseSkills.map((s) => s.name).join(", ")
|
||||
if (!available) {
|
||||
const fallback = await discoverSkills({
|
||||
includeClaudeCodePaths: true,
|
||||
directory: options.directory,
|
||||
})
|
||||
available = fallback.map((s) => s.name).join(", ")
|
||||
}
|
||||
return {
|
||||
content: undefined,
|
||||
contents: [],
|
||||
error: `Skills not found: ${notFound.join(", ")}. Available: ${available}`,
|
||||
}
|
||||
}
|
||||
|
||||
const contents = Array.from(resolved.values())
|
||||
|
||||
@@ -3328,9 +3328,9 @@ describe("sisyphus-task", () => {
|
||||
toolContext
|
||||
)
|
||||
|
||||
// then - agent-browser skill should NOT resolve without browserProvider
|
||||
expect(result).toContain("Skills not found")
|
||||
expect(result).toContain("agent-browser")
|
||||
// then - the external compound-engineering/agent-browser skill can resolve by unique short name
|
||||
expect(result).toContain("Task completed")
|
||||
expect(result).toContain("ses_no_browser_provider")
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -15,6 +15,20 @@ import {
|
||||
} from "./executor"
|
||||
import { prepareDelegateTaskArgs } from "./tool-argument-preparation"
|
||||
import { createDelegateTaskPresentation } from "./tool-description"
|
||||
import type { NativeSkillEntry } from "../skill/native-skills"
|
||||
|
||||
async function loadNativeSkillEntries(
|
||||
nativeSkills: DelegateTaskToolOptions["nativeSkills"] | undefined,
|
||||
): Promise<NativeSkillEntry[]> {
|
||||
if (!nativeSkills) return []
|
||||
try {
|
||||
const list = await nativeSkills.all()
|
||||
return Array.isArray(list) ? list : []
|
||||
} catch (err) {
|
||||
log("[delegate-task] nativeSkills.all() failed; skipping native skills", { error: String(err) })
|
||||
return []
|
||||
}
|
||||
}
|
||||
|
||||
export { resolveCategoryConfig } from "./categories"
|
||||
export type { SyncSessionCreatedEvent, DelegateTaskToolOptions, BuildSystemContentInput } from "./types"
|
||||
@@ -52,12 +66,16 @@ export function createDelegateTask(options: DelegateTaskToolOptions): ToolDefini
|
||||
|
||||
const runInBackground = delegateTaskArgs.run_in_background === true
|
||||
|
||||
const nativeSkillEntries = await loadNativeSkillEntries(options.nativeSkills)
|
||||
|
||||
const { content: skillContent, contents: skillContents, error: skillError } = await resolveSkillContent(delegateTaskArgs.load_skills, {
|
||||
gitMasterConfig: options.gitMasterConfig,
|
||||
browserProvider: options.browserProvider,
|
||||
disabledSkills: options.disabledSkills,
|
||||
teamModeEnabled: options.teamModeEnabled,
|
||||
directory: options.directory,
|
||||
nativeSkills: options.nativeSkills,
|
||||
nativeSkillEntries,
|
||||
})
|
||||
if (skillError) {
|
||||
return skillError
|
||||
@@ -68,6 +86,7 @@ export function createDelegateTask(options: DelegateTaskToolOptions): ToolDefini
|
||||
skillContents,
|
||||
availableCategories,
|
||||
availableSkills,
|
||||
nativeSkillInfos: nativeSkillEntries,
|
||||
})
|
||||
|
||||
const parentContext = await resolveParentContext(ctx, options.client)
|
||||
@@ -140,6 +159,7 @@ export function createDelegateTask(options: DelegateTaskToolOptions): ToolDefini
|
||||
model: categoryModel,
|
||||
availableCategories,
|
||||
availableSkills,
|
||||
nativeSkillInfos: nativeSkillEntries,
|
||||
})
|
||||
return executeUnstableAgentTask(delegateTaskArgs, ctx, options, parentContext, agentToUse, categoryModel, systemContent, actualModel)
|
||||
}
|
||||
@@ -162,6 +182,7 @@ export function createDelegateTask(options: DelegateTaskToolOptions): ToolDefini
|
||||
model: categoryModel,
|
||||
availableCategories,
|
||||
availableSkills,
|
||||
nativeSkillInfos: nativeSkillEntries,
|
||||
})
|
||||
|
||||
if (runInBackground) {
|
||||
|
||||
@@ -70,6 +70,12 @@ export interface DelegateTaskToolOptions {
|
||||
modelFallbackControllerAccessor?: ModelFallbackControllerAccessor
|
||||
onSyncSessionCreated?: (event: SyncSessionCreatedEvent) => Promise<void>
|
||||
syncPollTimeoutMs?: number
|
||||
/** OpenCode native skill accessor for skills registered via config.skills.paths. Same shape as SkillLoadOptions.nativeSkills. */
|
||||
nativeSkills?: {
|
||||
all(): { name: string; description: string; location: string; content: string }[] | Promise<{ name: string; description: string; location: string; content: string }[]>
|
||||
get(name: string): { name: string; description: string; location: string; content: string } | undefined | Promise<{ name: string; description: string; location: string; content: string } | undefined>
|
||||
dirs(): string[] | Promise<string[]>
|
||||
}
|
||||
}
|
||||
|
||||
import type { DelegatedModelConfig } from "../../shared/model-resolution-types"
|
||||
@@ -86,4 +92,6 @@ export interface BuildSystemContentInput {
|
||||
agentName?: string
|
||||
availableCategories?: AvailableCategory[]
|
||||
availableSkills?: AvailableSkill[]
|
||||
/** OpenCode native skill list to merge into the <available_skills> block. */
|
||||
nativeSkillInfos?: { name: string; description: string; location: string }[]
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user