Skip to content

Commit c2c1746

Browse files
committed
fix: implement disabled skills functionality in skill resolution
1 parent a5db86e commit c2c1746

File tree

8 files changed

+100
-45
lines changed

8 files changed

+100
-45
lines changed

bun.lock

Lines changed: 14 additions & 14 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/agents/utils.test.ts

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ describe("createBuiltinAgents with model overrides", () => {
99
// #given - no overrides, using systemDefaultModel
1010

1111
// #when
12-
const agents = await createBuiltinAgents([], {}, undefined, TEST_DEFAULT_MODEL)
12+
const agents = await createBuiltinAgents([], {}, undefined, TEST_DEFAULT_MODEL, undefined, undefined, [], undefined, undefined)
1313

1414
// #then
1515
expect(agents.sisyphus.model).toBe("anthropic/claude-opus-4-5")
@@ -24,7 +24,7 @@ describe("createBuiltinAgents with model overrides", () => {
2424
}
2525

2626
// #when
27-
const agents = await createBuiltinAgents([], overrides, undefined, TEST_DEFAULT_MODEL)
27+
const agents = await createBuiltinAgents([], overrides, undefined, TEST_DEFAULT_MODEL, undefined, undefined, [], undefined, undefined)
2828

2929
// #then
3030
expect(agents.sisyphus.model).toBe("github-copilot/gpt-5.2")
@@ -37,7 +37,7 @@ describe("createBuiltinAgents with model overrides", () => {
3737
const systemDefaultModel = "anthropic/claude-opus-4-5"
3838

3939
// #when
40-
const agents = await createBuiltinAgents([], {}, undefined, systemDefaultModel)
40+
const agents = await createBuiltinAgents([], {}, undefined, systemDefaultModel, undefined, undefined, [], undefined, undefined)
4141

4242
// #then - falls back to system default when no availability match
4343
expect(agents.sisyphus.model).toBe("anthropic/claude-opus-4-5")
@@ -49,7 +49,7 @@ describe("createBuiltinAgents with model overrides", () => {
4949
// #given - no available models simulates CI without model cache
5050

5151
// #when
52-
const agents = await createBuiltinAgents([], {}, undefined, TEST_DEFAULT_MODEL)
52+
const agents = await createBuiltinAgents([], {}, undefined, TEST_DEFAULT_MODEL, undefined, undefined, [], undefined, undefined)
5353

5454
// #then - uses first fallback entry (openai/gpt-5.2) instead of system default
5555
expect(agents.oracle.model).toBe("openai/gpt-5.2")
@@ -65,7 +65,7 @@ describe("createBuiltinAgents with model overrides", () => {
6565
}
6666

6767
// #when
68-
const agents = await createBuiltinAgents([], overrides, undefined, TEST_DEFAULT_MODEL)
68+
const agents = await createBuiltinAgents([], overrides, undefined, TEST_DEFAULT_MODEL, undefined, undefined, [], undefined, undefined)
6969

7070
// #then
7171
expect(agents.oracle.model).toBe("openai/gpt-5.2")
@@ -81,7 +81,7 @@ describe("createBuiltinAgents with model overrides", () => {
8181
}
8282

8383
// #when
84-
const agents = await createBuiltinAgents([], overrides, undefined, TEST_DEFAULT_MODEL)
84+
const agents = await createBuiltinAgents([], overrides, undefined, TEST_DEFAULT_MODEL, undefined, undefined, [], undefined, undefined)
8585

8686
// #then
8787
expect(agents.oracle.model).toBe("anthropic/claude-sonnet-4")
@@ -97,12 +97,25 @@ describe("createBuiltinAgents with model overrides", () => {
9797
}
9898

9999
// #when
100-
const agents = await createBuiltinAgents([], overrides, undefined, TEST_DEFAULT_MODEL)
100+
const agents = await createBuiltinAgents([], overrides, undefined, TEST_DEFAULT_MODEL, undefined, undefined, [], undefined, undefined)
101101

102102
// #then
103103
expect(agents.sisyphus.model).toBe("github-copilot/gpt-5.2")
104104
expect(agents.sisyphus.temperature).toBe(0.5)
105105
})
106+
107+
test("createBuiltinAgents excludes disabled skills from availableSkills", async () => {
108+
// #given
109+
const disabledSkills = new Set(["playwright"])
110+
111+
// #when
112+
const agents = await createBuiltinAgents([], {}, undefined, TEST_DEFAULT_MODEL, undefined, undefined, [], undefined, disabledSkills)
113+
114+
// #then
115+
expect(agents.sisyphus.prompt).not.toContain("playwright")
116+
expect(agents.sisyphus.prompt).toContain("frontend-ui-ux")
117+
expect(agents.sisyphus.prompt).toContain("git-master")
118+
})
106119
})
107120

108121
describe("buildAgent with category and skills", () => {

src/agents/utils.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,8 @@ export async function createBuiltinAgents(
146146
categories?: CategoriesConfig,
147147
gitMasterConfig?: GitMasterConfig,
148148
discoveredSkills: LoadedSkill[] = [],
149-
client?: any
149+
client?: any,
150+
disabledSkills?: Set<string>
150151
): Promise<Record<string, AgentConfig>> {
151152
if (!systemDefaultModel) {
152153
throw new Error("createBuiltinAgents requires systemDefaultModel")
@@ -167,7 +168,7 @@ export async function createBuiltinAgents(
167168
description: categories?.[name]?.description ?? CATEGORY_DESCRIPTIONS[name] ?? "General tasks",
168169
}))
169170

170-
const builtinSkills = createBuiltinSkills()
171+
const builtinSkills = createBuiltinSkills({ disabledSkills })
171172
const builtinSkillNames = new Set(builtinSkills.map(s => s.name))
172173

173174
const builtinAvailable: AvailableSkill[] = builtinSkills.map((skill) => ({

src/features/builtin-skills/skills.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1198,6 +1198,10 @@ POTENTIAL ACTIONS:
11981198
- Bisect without proper good/bad boundaries -> Wasted time`,
11991199
}
12001200

1201-
export function createBuiltinSkills(): BuiltinSkill[] {
1202-
return [playwrightSkill, frontendUiUxSkill, gitMasterSkill]
1201+
export function createBuiltinSkills(options?: { disabledSkills?: Set<string> }): BuiltinSkill[] {
1202+
const skills = [playwrightSkill, frontendUiUxSkill, gitMasterSkill]
1203+
if (!options?.disabledSkills) {
1204+
return skills
1205+
}
1206+
return skills.filter((skill) => !options.disabledSkills!.has(skill.name))
12031207
}

src/features/opencode-skill-loader/skill-content.test.ts

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,12 @@ describe("resolveSkillContent", () => {
3333
expect(result).toBeNull()
3434
})
3535

36-
it("should return null for empty string", () => {
37-
// #given: builtin skills
38-
// #when: resolving content for empty string
39-
const result = resolveSkillContent("")
36+
it("should return null for disabled skill", () => {
37+
// #given: frontend-ui-ux skill disabled
38+
const options = { disabledSkills: new Set(["frontend-ui-ux"]) }
39+
40+
// #when: resolving content for disabled skill
41+
const result = resolveSkillContent("frontend-ui-ux", options)
4042

4143
// #then: returns null
4244
expect(result).toBeNull()
@@ -96,6 +98,20 @@ describe("resolveMultipleSkills", () => {
9698
expect(result.notFound).toEqual(["skill-one", "skill-two", "skill-three"])
9799
})
98100

101+
it("should treat disabled skills as not found", () => {
102+
// #given: frontend-ui-ux disabled, playwright not disabled
103+
const skillNames = ["frontend-ui-ux", "playwright"]
104+
const options = { disabledSkills: new Set(["frontend-ui-ux"]) }
105+
106+
// #when: resolving multiple skills with disabled one
107+
const result = resolveMultipleSkills(skillNames, options)
108+
109+
// #then: frontend-ui-ux in notFound, playwright resolved
110+
expect(result.resolved.size).toBe(1)
111+
expect(result.resolved.has("playwright")).toBe(true)
112+
expect(result.notFound).toEqual(["frontend-ui-ux"])
113+
})
114+
99115
it("should preserve skill order in resolved map", () => {
100116
// #given: list of skill names in specific order
101117
const skillNames = ["playwright", "frontend-ui-ux"]
@@ -122,10 +138,12 @@ describe("resolveSkillContentAsync", () => {
122138
expect(result).toContain("Role: Designer-Turned-Developer")
123139
})
124140

125-
it("should return null for non-existent skill", async () => {
126-
// #given: non-existent skill name
127-
// #when: resolving content async
128-
const result = await resolveSkillContentAsync("definitely-not-a-skill-12345")
141+
it("should return null for disabled skill", async () => {
142+
// #given: frontend-ui-ux disabled
143+
const options = { disabledSkills: new Set(["frontend-ui-ux"]) }
144+
145+
// #when: resolving content async for disabled skill
146+
const result = await resolveSkillContentAsync("frontend-ui-ux", options)
129147

130148
// #then: returns null
131149
expect(result).toBeNull()
@@ -160,6 +178,20 @@ describe("resolveMultipleSkillsAsync", () => {
160178
expect(result.resolved.get("playwright")).toContain("Playwright Browser Automation")
161179
})
162180

181+
it("should treat disabled skills as not found async", async () => {
182+
// #given: frontend-ui-ux disabled
183+
const skillNames = ["frontend-ui-ux", "playwright"]
184+
const options = { disabledSkills: new Set(["frontend-ui-ux"]) }
185+
186+
// #when: resolving multiple skills async with disabled one
187+
const result = await resolveMultipleSkillsAsync(skillNames, options)
188+
189+
// #then: frontend-ui-ux in notFound, playwright resolved
190+
expect(result.resolved.size).toBe(1)
191+
expect(result.resolved.has("playwright")).toBe(true)
192+
expect(result.notFound).toEqual(["frontend-ui-ux"])
193+
})
194+
163195
it("should NOT inject watermark when both options are disabled", async () => {
164196
// #given: git-master skill with watermark disabled
165197
const skillNames = ["git-master"]

src/features/opencode-skill-loader/skill-content.ts

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import type { GitMasterConfig } from "../../config/schema"
77

88
export interface SkillResolutionOptions {
99
gitMasterConfig?: GitMasterConfig
10+
disabledSkills?: Set<string>
1011
}
1112

1213
let cachedSkills: LoadedSkill[] | null = null
@@ -15,12 +16,12 @@ function clearSkillCache(): void {
1516
cachedSkills = null
1617
}
1718

18-
async function getAllSkills(): Promise<LoadedSkill[]> {
19-
if (cachedSkills) return cachedSkills
19+
async function getAllSkills(options?: SkillResolutionOptions): Promise<LoadedSkill[]> {
20+
if (cachedSkills && !options?.disabledSkills) return cachedSkills
2021

2122
const [discoveredSkills, builtinSkillDefs] = await Promise.all([
2223
discoverSkills({ includeClaudeCodePaths: true }),
23-
Promise.resolve(createBuiltinSkills()),
24+
Promise.resolve(createBuiltinSkills({ disabledSkills: options?.disabledSkills })),
2425
])
2526

2627
const builtinSkillsAsLoaded: LoadedSkill[] = builtinSkillDefs.map((skill) => ({
@@ -44,8 +45,11 @@ async function getAllSkills(): Promise<LoadedSkill[]> {
4445
const discoveredNames = new Set(discoveredSkills.map((s) => s.name))
4546
const uniqueBuiltins = builtinSkillsAsLoaded.filter((s) => !discoveredNames.has(s.name))
4647

47-
cachedSkills = [...discoveredSkills, ...uniqueBuiltins]
48-
return cachedSkills
48+
const skills = [...discoveredSkills, ...uniqueBuiltins]
49+
if (!options?.disabledSkills) {
50+
cachedSkills = skills
51+
}
52+
return skills
4953
}
5054

5155
async function extractSkillTemplate(skill: LoadedSkill): Promise<string> {
@@ -118,7 +122,7 @@ export function injectGitMasterConfig(template: string, config?: GitMasterConfig
118122
}
119123

120124
export function resolveSkillContent(skillName: string, options?: SkillResolutionOptions): string | null {
121-
const skills = createBuiltinSkills()
125+
const skills = createBuiltinSkills({ disabledSkills: options?.disabledSkills })
122126
const skill = skills.find((s) => s.name === skillName)
123127
if (!skill) return null
124128

@@ -133,7 +137,7 @@ export function resolveMultipleSkills(skillNames: string[], options?: SkillResol
133137
resolved: Map<string, string>
134138
notFound: string[]
135139
} {
136-
const skills = createBuiltinSkills()
140+
const skills = createBuiltinSkills({ disabledSkills: options?.disabledSkills })
137141
const skillMap = new Map(skills.map((s) => [s.name, s.template]))
138142

139143
const resolved = new Map<string, string>()
@@ -159,7 +163,7 @@ export async function resolveSkillContentAsync(
159163
skillName: string,
160164
options?: SkillResolutionOptions
161165
): Promise<string | null> {
162-
const allSkills = await getAllSkills()
166+
const allSkills = await getAllSkills(options)
163167
const skill = allSkills.find((s) => s.name === skillName)
164168
if (!skill) return null
165169

@@ -179,7 +183,7 @@ export async function resolveMultipleSkillsAsync(
179183
resolved: Map<string, string>
180184
notFound: string[]
181185
}> {
182-
const allSkills = await getAllSkills()
186+
const allSkills = await getAllSkills(options)
183187
const skillMap = new Map<string, LoadedSkill>()
184188
for (const skill of allSkills) {
185189
skillMap.set(skill.name, skill)

src/index.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -248,8 +248,7 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => {
248248
});
249249
const disabledSkills = new Set(pluginConfig.disabled_skills ?? []);
250250
const systemMcpNames = getSystemMcpServerNames();
251-
const builtinSkills = createBuiltinSkills().filter((skill) => {
252-
if (disabledSkills.has(skill.name as never)) return false;
251+
const builtinSkills = createBuiltinSkills({ disabledSkills }).filter((skill) => {
253252
if (skill.mcpConfig) {
254253
for (const mcpName of Object.keys(skill.mcpConfig)) {
255254
if (systemMcpNames.has(mcpName)) return false;

src/plugin-handlers/config-handler.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ export function createConfigHandler(deps: ConfigHandlerDeps) {
165165
...discoveredUserSkills,
166166
];
167167

168+
const disabledSkills = new Set(pluginConfig.disabled_skills ?? []);
168169
const builtinAgents = await createBuiltinAgents(
169170
migratedDisabledAgents,
170171
pluginConfig.agents,
@@ -173,7 +174,8 @@ export function createConfigHandler(deps: ConfigHandlerDeps) {
173174
pluginConfig.categories,
174175
pluginConfig.git_master,
175176
allDiscoveredSkills,
176-
ctx.client
177+
ctx.client,
178+
disabledSkills
177179
);
178180

179181
// Claude Code agents: Do NOT apply permission migration

0 commit comments

Comments
 (0)