Merge pull request #4141 from code-yeongyu/fix/3396-config-skills-paths-discovery
fix: discover skills from host config.skills.paths set by other plugins
This commit is contained in:
@@ -122,4 +122,50 @@ describe("applyAgentConfig .agents skills", () => {
|
|||||||
expect(discoveredSkills.map(skill => skill.name)).toContain("project-agent-skill")
|
expect(discoveredSkills.map(skill => skill.name)).toContain("project-agent-skill")
|
||||||
expect(discoveredSkills.map(skill => skill.name)).toContain("global-agent-skill")
|
expect(discoveredSkills.map(skill => skill.name)).toContain("global-agent-skill")
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test("discovers skills from host config.skills.paths set by other plugins", async () => {
|
||||||
|
// given - second call to discoverConfigSourceSkills returns host config skills
|
||||||
|
discoverConfigSourceSkillsSpy
|
||||||
|
.mockResolvedValueOnce([])
|
||||||
|
.mockResolvedValueOnce([
|
||||||
|
{
|
||||||
|
name: "host-config-skill",
|
||||||
|
definition: { name: "host-config-skill", template: "host-template" },
|
||||||
|
scope: "config",
|
||||||
|
},
|
||||||
|
])
|
||||||
|
|
||||||
|
// when
|
||||||
|
await applyAgentConfig({
|
||||||
|
config: {
|
||||||
|
model: "anthropic/claude-opus-4-6",
|
||||||
|
agent: {},
|
||||||
|
skills: { paths: ["/host/skills"] },
|
||||||
|
},
|
||||||
|
pluginConfig: createPluginConfig(),
|
||||||
|
ctx: { directory: "/tmp/project" },
|
||||||
|
pluginComponents: createPluginComponents(),
|
||||||
|
})
|
||||||
|
|
||||||
|
// then
|
||||||
|
const discoveredSkills = createBuiltinAgentsSpy.mock.calls[0]?.[6] as Array<{ name: string }>
|
||||||
|
expect(discoveredSkills.map(skill => skill.name)).toContain("host-config-skill")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("calls discoverConfigSourceSkills twice when host config has skills", async () => {
|
||||||
|
// when
|
||||||
|
await applyAgentConfig({
|
||||||
|
config: {
|
||||||
|
model: "anthropic/claude-opus-4-6",
|
||||||
|
agent: {},
|
||||||
|
skills: { paths: ["/host/skills"] },
|
||||||
|
},
|
||||||
|
pluginConfig: createPluginConfig(),
|
||||||
|
ctx: { directory: "/tmp/project" },
|
||||||
|
pluginComponents: createPluginComponents(),
|
||||||
|
})
|
||||||
|
|
||||||
|
// then - called twice: once for pluginConfig.skills, once for host config.skills
|
||||||
|
expect(discoverConfigSourceSkillsSpy).toHaveBeenCalledTimes(2)
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -36,6 +36,7 @@ import {
|
|||||||
} from "./agent-override-protection";
|
} from "./agent-override-protection";
|
||||||
import { buildPrometheusAgentConfig } from "./prometheus-agent-config-builder";
|
import { buildPrometheusAgentConfig } from "./prometheus-agent-config-builder";
|
||||||
import { buildPlanDemoteConfig } from "./plan-model-inheritance";
|
import { buildPlanDemoteConfig } from "./plan-model-inheritance";
|
||||||
|
import { adaptHostSkillConfig } from "../shared/host-skill-config";
|
||||||
|
|
||||||
type AgentConfigRecord = Record<string, Record<string, unknown> | undefined> & {
|
type AgentConfigRecord = Record<string, Record<string, unknown> | undefined> & {
|
||||||
build?: Record<string, unknown>;
|
build?: Record<string, unknown>;
|
||||||
@@ -62,8 +63,10 @@ export async function applyAgentConfig(params: {
|
|||||||
) as typeof params.pluginConfig.disabled_agents;
|
) as typeof params.pluginConfig.disabled_agents;
|
||||||
|
|
||||||
const includeClaudeSkillsForAwareness = params.pluginConfig.claude_code?.skills ?? true;
|
const includeClaudeSkillsForAwareness = params.pluginConfig.claude_code?.skills ?? true;
|
||||||
|
const hostSkillConfig = adaptHostSkillConfig(params.config.skills);
|
||||||
const [
|
const [
|
||||||
discoveredConfigSourceSkills,
|
discoveredConfigSourceSkills,
|
||||||
|
discoveredHostConfigSkills,
|
||||||
discoveredUserSkills,
|
discoveredUserSkills,
|
||||||
discoveredProjectSkills,
|
discoveredProjectSkills,
|
||||||
discoveredProjectAgentsSkills,
|
discoveredProjectAgentsSkills,
|
||||||
@@ -75,6 +78,10 @@ export async function applyAgentConfig(params: {
|
|||||||
config: params.pluginConfig.skills,
|
config: params.pluginConfig.skills,
|
||||||
configDir: params.ctx.directory,
|
configDir: params.ctx.directory,
|
||||||
}),
|
}),
|
||||||
|
discoverConfigSourceSkills({
|
||||||
|
config: hostSkillConfig,
|
||||||
|
configDir: params.ctx.directory,
|
||||||
|
}),
|
||||||
includeClaudeSkillsForAwareness ? discoverUserClaudeSkills() : Promise.resolve([]),
|
includeClaudeSkillsForAwareness ? discoverUserClaudeSkills() : Promise.resolve([]),
|
||||||
includeClaudeSkillsForAwareness
|
includeClaudeSkillsForAwareness
|
||||||
? discoverProjectClaudeSkills(params.ctx.directory)
|
? discoverProjectClaudeSkills(params.ctx.directory)
|
||||||
@@ -89,6 +96,7 @@ export async function applyAgentConfig(params: {
|
|||||||
|
|
||||||
const allDiscoveredSkills = [
|
const allDiscoveredSkills = [
|
||||||
...discoveredConfigSourceSkills,
|
...discoveredConfigSourceSkills,
|
||||||
|
...discoveredHostConfigSkills,
|
||||||
...discoveredOpencodeProjectSkills,
|
...discoveredOpencodeProjectSkills,
|
||||||
...discoveredProjectSkills,
|
...discoveredProjectSkills,
|
||||||
...discoveredProjectAgentsSkills,
|
...discoveredProjectAgentsSkills,
|
||||||
|
|||||||
@@ -157,4 +157,37 @@ describe("applyCommandConfig", () => {
|
|||||||
const commandConfig = config.command as Record<string, { agent?: string }>;
|
const commandConfig = config.command as Record<string, { agent?: string }>;
|
||||||
expect(commandConfig["start-work"]?.agent).toBe(getAgentListDisplayName("atlas"));
|
expect(commandConfig["start-work"]?.agent).toBe(getAgentListDisplayName("atlas"));
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("includes host config skills declared in config.skills.paths by other plugins", async () => {
|
||||||
|
// given - second call to discoverConfigSourceSkills returns host config skills
|
||||||
|
discoverConfigSourceSkillsSpy
|
||||||
|
.mockResolvedValueOnce([])
|
||||||
|
.mockResolvedValueOnce([
|
||||||
|
{
|
||||||
|
name: "host-config-skill",
|
||||||
|
definition: {
|
||||||
|
name: "host-config-skill",
|
||||||
|
description: "Host config skill",
|
||||||
|
template: "template",
|
||||||
|
},
|
||||||
|
scope: "config",
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
const config: Record<string, unknown> = {
|
||||||
|
command: {},
|
||||||
|
skills: { paths: ["/host/skills"] },
|
||||||
|
};
|
||||||
|
|
||||||
|
// when
|
||||||
|
await applyCommandConfig({
|
||||||
|
config,
|
||||||
|
pluginConfig: createPluginConfig(),
|
||||||
|
ctx: { directory: "/tmp" },
|
||||||
|
pluginComponents: createPluginComponents(),
|
||||||
|
});
|
||||||
|
|
||||||
|
// then
|
||||||
|
const commandConfig = config.command as Record<string, { description?: string }>;
|
||||||
|
expect(commandConfig["host-config-skill"]?.description).toContain("Host config skill");
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -26,6 +26,7 @@ import {
|
|||||||
log,
|
log,
|
||||||
} from "../shared";
|
} from "../shared";
|
||||||
import type { PluginComponents } from "./plugin-components-loader";
|
import type { PluginComponents } from "./plugin-components-loader";
|
||||||
|
import { adaptHostSkillConfig } from "../shared/host-skill-config";
|
||||||
|
|
||||||
export async function applyCommandConfig(params: {
|
export async function applyCommandConfig(params: {
|
||||||
config: Record<string, unknown>;
|
config: Record<string, unknown>;
|
||||||
@@ -47,8 +48,10 @@ export async function applyCommandConfig(params: {
|
|||||||
log(getSkillPluginConflictWarning(externalSkillPlugin.pluginName));
|
log(getSkillPluginConflictWarning(externalSkillPlugin.pluginName));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const hostSkillConfig = adaptHostSkillConfig(params.config.skills);
|
||||||
const [
|
const [
|
||||||
configSourceSkills,
|
configSourceSkills,
|
||||||
|
hostConfigSkills,
|
||||||
userCommands,
|
userCommands,
|
||||||
projectCommands,
|
projectCommands,
|
||||||
opencodeGlobalCommands,
|
opencodeGlobalCommands,
|
||||||
@@ -64,6 +67,10 @@ export async function applyCommandConfig(params: {
|
|||||||
config: params.pluginConfig.skills,
|
config: params.pluginConfig.skills,
|
||||||
configDir: params.ctx.directory,
|
configDir: params.ctx.directory,
|
||||||
}),
|
}),
|
||||||
|
discoverConfigSourceSkills({
|
||||||
|
config: hostSkillConfig,
|
||||||
|
configDir: params.ctx.directory,
|
||||||
|
}),
|
||||||
includeClaudeCommands ? loadUserCommands() : Promise.resolve({}),
|
includeClaudeCommands ? loadUserCommands() : Promise.resolve({}),
|
||||||
includeClaudeCommands ? loadProjectCommands(params.ctx.directory) : Promise.resolve({}),
|
includeClaudeCommands ? loadProjectCommands(params.ctx.directory) : Promise.resolve({}),
|
||||||
loadOpencodeGlobalCommands(),
|
loadOpencodeGlobalCommands(),
|
||||||
@@ -79,6 +86,7 @@ export async function applyCommandConfig(params: {
|
|||||||
params.config.command = {
|
params.config.command = {
|
||||||
...builtinCommands,
|
...builtinCommands,
|
||||||
...skillsToCommandDefinitionRecord(configSourceSkills),
|
...skillsToCommandDefinitionRecord(configSourceSkills),
|
||||||
|
...skillsToCommandDefinitionRecord(hostConfigSkills),
|
||||||
...userCommands,
|
...userCommands,
|
||||||
...userSkills,
|
...userSkills,
|
||||||
...globalAgentsSkills,
|
...globalAgentsSkills,
|
||||||
|
|||||||
@@ -0,0 +1,80 @@
|
|||||||
|
import { describe, expect, test } from "bun:test"
|
||||||
|
|
||||||
|
import { adaptHostSkillConfig } from "./host-skill-config"
|
||||||
|
|
||||||
|
describe("adaptHostSkillConfig", () => {
|
||||||
|
test("converts paths and urls into SkillsConfig sources", () => {
|
||||||
|
// given
|
||||||
|
const hostConfig = {
|
||||||
|
paths: ["/host/skills", "/other/skills"],
|
||||||
|
urls: ["https://example.com/skills/"],
|
||||||
|
}
|
||||||
|
|
||||||
|
// when
|
||||||
|
const result = adaptHostSkillConfig(hostConfig)
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(result).toEqual({
|
||||||
|
sources: ["/host/skills", "/other/skills", "https://example.com/skills/"],
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
test("filters blank and whitespace-only entries", () => {
|
||||||
|
// given
|
||||||
|
const hostConfig = {
|
||||||
|
paths: ["", " ", "/real/skills"],
|
||||||
|
urls: ["\n", "https://example.com/skills/"],
|
||||||
|
}
|
||||||
|
|
||||||
|
// when
|
||||||
|
const result = adaptHostSkillConfig(hostConfig)
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(result).toEqual({
|
||||||
|
sources: ["/real/skills", "https://example.com/skills/"],
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
test("returns undefined when no usable sources remain", () => {
|
||||||
|
// when
|
||||||
|
const result = adaptHostSkillConfig({ paths: ["", " "], urls: ["\t"] })
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(result).toBeUndefined()
|
||||||
|
})
|
||||||
|
|
||||||
|
test("returns undefined for null input", () => {
|
||||||
|
expect(adaptHostSkillConfig(null)).toBeUndefined()
|
||||||
|
})
|
||||||
|
|
||||||
|
test("returns undefined for undefined input", () => {
|
||||||
|
expect(adaptHostSkillConfig(undefined)).toBeUndefined()
|
||||||
|
})
|
||||||
|
|
||||||
|
test("returns undefined for non-object input", () => {
|
||||||
|
expect(adaptHostSkillConfig("string")).toBeUndefined()
|
||||||
|
})
|
||||||
|
|
||||||
|
test("handles missing paths or urls gracefully", () => {
|
||||||
|
// when - only paths
|
||||||
|
const pathsOnly = adaptHostSkillConfig({ paths: ["/skills"] })
|
||||||
|
expect(pathsOnly).toEqual({ sources: ["/skills"] })
|
||||||
|
|
||||||
|
// when - only urls
|
||||||
|
const urlsOnly = adaptHostSkillConfig({ urls: ["https://example.com/skills/"] })
|
||||||
|
expect(urlsOnly).toEqual({ sources: ["https://example.com/skills/"] })
|
||||||
|
})
|
||||||
|
|
||||||
|
test("ignores non-string array elements", () => {
|
||||||
|
// given
|
||||||
|
const hostConfig = {
|
||||||
|
paths: ["/valid", 42, null, true, "/also-valid"],
|
||||||
|
}
|
||||||
|
|
||||||
|
// when
|
||||||
|
const result = adaptHostSkillConfig(hostConfig)
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(result).toEqual({ sources: ["/valid", "/also-valid"] })
|
||||||
|
})
|
||||||
|
})
|
||||||
@@ -0,0 +1,28 @@
|
|||||||
|
import type { SkillsConfig } from "../config/schema/skills"
|
||||||
|
|
||||||
|
type HostSkillConfig = {
|
||||||
|
paths?: unknown
|
||||||
|
urls?: unknown
|
||||||
|
}
|
||||||
|
|
||||||
|
function toStringArray(value: unknown): string[] {
|
||||||
|
if (!Array.isArray(value)) return []
|
||||||
|
return value
|
||||||
|
.filter((item): item is string => typeof item === "string")
|
||||||
|
.map((item) => item.trim())
|
||||||
|
.filter((item) => item.length > 0)
|
||||||
|
}
|
||||||
|
|
||||||
|
export function adaptHostSkillConfig(value: unknown): SkillsConfig | undefined {
|
||||||
|
if (!value || typeof value !== "object") return undefined
|
||||||
|
|
||||||
|
const hostSkillConfig = value as HostSkillConfig
|
||||||
|
const sources = [
|
||||||
|
...toStringArray(hostSkillConfig.paths),
|
||||||
|
...toStringArray(hostSkillConfig.urls),
|
||||||
|
]
|
||||||
|
|
||||||
|
if (sources.length === 0) return undefined
|
||||||
|
|
||||||
|
return { sources } as SkillsConfig
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user