diff --git a/src/agents/utils.test.ts b/src/agents/utils.test.ts index 689e4d31..4b78566e 100644 --- a/src/agents/utils.test.ts +++ b/src/agents/utils.test.ts @@ -41,7 +41,7 @@ describe("createBuiltinAgents with model overrides", () => { } // #when - const agents = await createBuiltinAgents([], overrides, undefined, TEST_DEFAULT_MODEL) + const agents = await createBuiltinAgents([], overrides, undefined, TEST_DEFAULT_MODEL, undefined, undefined, [], undefined, undefined) // #then expect(agents.sisyphus.model).toBe("github-copilot/gpt-5.2") @@ -103,7 +103,7 @@ describe("createBuiltinAgents with model overrides", () => { const cacheSpy = spyOn(connectedProvidersCache, "readConnectedProvidersCache").mockReturnValue(["openai"]) // #when - const agents = await createBuiltinAgents([], {}, undefined, TEST_DEFAULT_MODEL) + const agents = await createBuiltinAgents([], {}, undefined, TEST_DEFAULT_MODEL, undefined, undefined, [], undefined, undefined) // #then - oracle resolves via connected cache fallback to openai/gpt-5.2 (not system default) expect(agents.oracle.model).toBe("openai/gpt-5.2") @@ -132,7 +132,7 @@ describe("createBuiltinAgents with model overrides", () => { } // #when - const agents = await createBuiltinAgents([], overrides, undefined, TEST_DEFAULT_MODEL) + const agents = await createBuiltinAgents([], overrides, undefined, TEST_DEFAULT_MODEL, undefined, undefined, [], undefined, undefined) // #then expect(agents.oracle.model).toBe("openai/gpt-5.2") @@ -148,7 +148,7 @@ describe("createBuiltinAgents with model overrides", () => { } // #when - const agents = await createBuiltinAgents([], overrides, undefined, TEST_DEFAULT_MODEL) + const agents = await createBuiltinAgents([], overrides, undefined, TEST_DEFAULT_MODEL, undefined, undefined, [], undefined, undefined) // #then expect(agents.oracle.model).toBe("anthropic/claude-sonnet-4") @@ -164,12 +164,25 @@ describe("createBuiltinAgents with model overrides", () => { } // #when - const agents = await createBuiltinAgents([], overrides, undefined, TEST_DEFAULT_MODEL) + const agents = await createBuiltinAgents([], overrides, undefined, TEST_DEFAULT_MODEL, undefined, undefined, [], undefined, undefined) // #then expect(agents.sisyphus.model).toBe("github-copilot/gpt-5.2") expect(agents.sisyphus.temperature).toBe(0.5) }) + + test("createBuiltinAgents excludes disabled skills from availableSkills", async () => { + // #given + const disabledSkills = new Set(["playwright"]) + + // #when + const agents = await createBuiltinAgents([], {}, undefined, TEST_DEFAULT_MODEL, undefined, undefined, [], undefined, undefined, undefined, disabledSkills) + + // #then + expect(agents.sisyphus.prompt).not.toContain("playwright") + expect(agents.sisyphus.prompt).toContain("frontend-ui-ux") + expect(agents.sisyphus.prompt).toContain("git-master") + }) }) describe("createBuiltinAgents without systemDefaultModel", () => { diff --git a/src/agents/utils.ts b/src/agents/utils.ts index 426a8bd0..35a33b2a 100644 --- a/src/agents/utils.ts +++ b/src/agents/utils.ts @@ -57,7 +57,8 @@ export function buildAgent( model: string, categories?: CategoriesConfig, gitMasterConfig?: GitMasterConfig, - browserProvider?: BrowserAutomationProvider + browserProvider?: BrowserAutomationProvider, + disabledSkills?: Set ): AgentConfig { const base = isFactory(source) ? source(model) : source const categoryConfigs: Record = categories @@ -81,7 +82,7 @@ export function buildAgent( } if (agentWithCategory.skills?.length) { - const { resolved } = resolveMultipleSkills(agentWithCategory.skills, { gitMasterConfig, browserProvider }) + const { resolved } = resolveMultipleSkills(agentWithCategory.skills, { gitMasterConfig, browserProvider, disabledSkills }) if (resolved.size > 0) { const skillContent = Array.from(resolved.values()).join("\n\n") base.prompt = skillContent + (base.prompt ? "\n\n" + base.prompt : "") @@ -234,7 +235,8 @@ export async function createBuiltinAgents( discoveredSkills: LoadedSkill[] = [], client?: any, browserProvider?: BrowserAutomationProvider, - uiSelectedModel?: string + uiSelectedModel?: string, + disabledSkills?: Set ): Promise> { const connectedProviders = readConnectedProvidersCache() // IMPORTANT: Do NOT pass client to fetchAvailableModels during plugin initialization. @@ -258,7 +260,7 @@ export async function createBuiltinAgents( description: categories?.[name]?.description ?? CATEGORY_DESCRIPTIONS[name] ?? "General tasks", })) - const builtinSkills = createBuiltinSkills({ browserProvider }) + const builtinSkills = createBuiltinSkills({ browserProvider, disabledSkills }) const builtinSkillNames = new Set(builtinSkills.map(s => s.name)) const builtinAvailable: AvailableSkill[] = builtinSkills.map((skill) => ({ @@ -291,16 +293,16 @@ export async function createBuiltinAgents( const override = agentOverrides[agentName] ?? Object.entries(agentOverrides).find(([key]) => key.toLowerCase() === agentName.toLowerCase())?.[1] const requirement = AGENT_MODEL_REQUIREMENTS[agentName] - + // Check if agent requires a specific model if (requirement?.requiresModel && availableModels) { if (!isModelAvailable(requirement.requiresModel, availableModels)) { continue } } - + const isPrimaryAgent = isFactory(source) && source.mode === "primary" - + const resolution = applyModelResolution({ uiSelectedModel: isPrimaryAgent ? uiSelectedModel : undefined, userModel: override?.model, @@ -311,7 +313,7 @@ export async function createBuiltinAgents( if (!resolution) continue const { model, variant: resolvedVariant } = resolution - let config = buildAgent(source, model, mergedCategories, gitMasterConfig, browserProvider) + let config = buildAgent(source, model, mergedCategories, gitMasterConfig, browserProvider, disabledSkills) // Apply resolved variant from model fallback chain if (resolvedVariant) { @@ -375,7 +377,7 @@ export async function createBuiltinAgents( availableSkills, availableCategories ) - + if (sisyphusResolvedVariant) { sisyphusConfig = { ...sisyphusConfig, variant: sisyphusResolvedVariant } } @@ -420,7 +422,7 @@ export async function createBuiltinAgents( availableSkills, availableCategories ) - + hephaestusConfig = { ...hephaestusConfig, variant: hephaestusResolvedVariant ?? "medium" } const hepOverrideCategory = (hephaestusOverride as Record | undefined)?.category as string | undefined @@ -468,7 +470,7 @@ export async function createBuiltinAgents( availableSkills, userCategories: categories, }) - + if (atlasResolvedVariant) { orchestratorConfig = { ...orchestratorConfig, variant: atlasResolvedVariant } } diff --git a/src/features/builtin-skills/skills.test.ts b/src/features/builtin-skills/skills.test.ts index a5323a4a..33f0cb56 100644 --- a/src/features/builtin-skills/skills.test.ts +++ b/src/features/builtin-skills/skills.test.ts @@ -86,4 +86,58 @@ describe("createBuiltinSkills", () => { expect(defaultSkills).toHaveLength(4) expect(agentBrowserSkills).toHaveLength(4) }) + + test("should exclude playwright when it is in disabledSkills", () => { + // #given + const options = { disabledSkills: new Set(["playwright"]) } + + // #when + const skills = createBuiltinSkills(options) + + // #then + expect(skills.map((s) => s.name)).not.toContain("playwright") + expect(skills.map((s) => s.name)).toContain("frontend-ui-ux") + expect(skills.map((s) => s.name)).toContain("git-master") + expect(skills.map((s) => s.name)).toContain("dev-browser") + expect(skills.length).toBe(3) + }) + + test("should exclude multiple skills when they are in disabledSkills", () => { + // #given + const options = { disabledSkills: new Set(["playwright", "git-master"]) } + + // #when + const skills = createBuiltinSkills(options) + + // #then + expect(skills.map((s) => s.name)).not.toContain("playwright") + expect(skills.map((s) => s.name)).not.toContain("git-master") + expect(skills.map((s) => s.name)).toContain("frontend-ui-ux") + expect(skills.map((s) => s.name)).toContain("dev-browser") + expect(skills.length).toBe(2) + }) + + test("should return an empty array when all skills are disabled", () => { + // #given + const options = { + disabledSkills: new Set(["playwright", "frontend-ui-ux", "git-master", "dev-browser"]), + } + + // #when + const skills = createBuiltinSkills(options) + + // #then + expect(skills.length).toBe(0) + }) + + test("should return all skills when disabledSkills set is empty", () => { + // #given + const options = { disabledSkills: new Set() } + + // #when + const skills = createBuiltinSkills(options) + + // #then + expect(skills.length).toBe(4) + }) }) diff --git a/src/features/builtin-skills/skills.ts b/src/features/builtin-skills/skills.ts index 955184e0..2f872698 100644 --- a/src/features/builtin-skills/skills.ts +++ b/src/features/builtin-skills/skills.ts @@ -11,12 +11,19 @@ import { export interface CreateBuiltinSkillsOptions { browserProvider?: BrowserAutomationProvider + disabledSkills?: Set } export function createBuiltinSkills(options: CreateBuiltinSkillsOptions = {}): BuiltinSkill[] { - const { browserProvider = "playwright" } = options + const { browserProvider = "playwright", disabledSkills } = options const browserSkill = browserProvider === "agent-browser" ? agentBrowserSkill : playwrightSkill - return [browserSkill, frontendUiUxSkill, gitMasterSkill, devBrowserSkill] + const skills = [browserSkill, frontendUiUxSkill, gitMasterSkill, devBrowserSkill] + + if (!disabledSkills) { + return skills + } + + return skills.filter((skill) => !disabledSkills.has(skill.name)) } diff --git a/src/features/opencode-skill-loader/skill-content.test.ts b/src/features/opencode-skill-loader/skill-content.test.ts index 9118b04d..1c21e14a 100644 --- a/src/features/opencode-skill-loader/skill-content.test.ts +++ b/src/features/opencode-skill-loader/skill-content.test.ts @@ -33,10 +33,12 @@ describe("resolveSkillContent", () => { expect(result).toBeNull() }) - it("should return null for empty string", () => { - // given: builtin skills - // when: resolving content for empty string - const result = resolveSkillContent("") + it("should return null for disabled skill", () => { + // given: frontend-ui-ux skill disabled + const options = { disabledSkills: new Set(["frontend-ui-ux"]) } + + // when: resolving content for disabled skill + const result = resolveSkillContent("frontend-ui-ux", options) // then: returns null expect(result).toBeNull() @@ -96,6 +98,20 @@ describe("resolveMultipleSkills", () => { expect(result.notFound).toEqual(["skill-one", "skill-two", "skill-three"]) }) + it("should treat disabled skills as not found", () => { + // #given: frontend-ui-ux disabled, playwright not disabled + const skillNames = ["frontend-ui-ux", "playwright"] + const options = { disabledSkills: new Set(["frontend-ui-ux"]) } + + // #when: resolving multiple skills with disabled one + const result = resolveMultipleSkills(skillNames, options) + + // #then: frontend-ui-ux in notFound, playwright resolved + expect(result.resolved.size).toBe(1) + expect(result.resolved.has("playwright")).toBe(true) + expect(result.notFound).toEqual(["frontend-ui-ux"]) + }) + it("should preserve skill order in resolved map", () => { // given: list of skill names in specific order const skillNames = ["playwright", "frontend-ui-ux"] @@ -111,21 +127,24 @@ describe("resolveMultipleSkills", () => { }) describe("resolveSkillContentAsync", () => { - it("should return template for builtin skill", async () => { + it("should return template for builtin skill async", async () => { // given: builtin skill 'frontend-ui-ux' // when: resolving content async - const result = await resolveSkillContentAsync("frontend-ui-ux") + const options = { disabledSkills: new Set(["frontend-ui-ux"]) } + const result = await resolveSkillContentAsync("git-master", options) // then: returns template string expect(result).not.toBeNull() expect(typeof result).toBe("string") - expect(result).toContain("Role: Designer-Turned-Developer") + expect(result).toContain("Git Master Agent") }) - it("should return null for non-existent skill", async () => { - // given: non-existent skill name - // when: resolving content async - const result = await resolveSkillContentAsync("definitely-not-a-skill-12345") + it("should return null for disabled skill async", async () => { + // given: frontend-ui-ux disabled + const options = { disabledSkills: new Set(["frontend-ui-ux"]) } + + // when: resolving content async for disabled skill + const result = await resolveSkillContentAsync("frontend-ui-ux", options) // then: returns null expect(result).toBeNull() @@ -133,9 +152,9 @@ describe("resolveSkillContentAsync", () => { }) describe("resolveMultipleSkillsAsync", () => { - it("should resolve builtin skills", async () => { + it("should resolve builtin skills async", async () => { // given: builtin skill names - const skillNames = ["playwright", "frontend-ui-ux"] + const skillNames = ["playwright", "git-master"] // when: resolving multiple skills async const result = await resolveMultipleSkillsAsync(skillNames) @@ -144,10 +163,10 @@ describe("resolveMultipleSkillsAsync", () => { expect(result.resolved.size).toBe(2) expect(result.notFound).toEqual([]) expect(result.resolved.get("playwright")).toContain("Playwright Browser Automation") - expect(result.resolved.get("frontend-ui-ux")).toContain("Designer-Turned-Developer") + expect(result.resolved.get("git-master")).toContain("Git Master Agent") }) - it("should handle partial success with non-existent skills", async () => { + it("should handle partial success with non-existent skills async", async () => { // given: mix of existing and non-existing skills const skillNames = ["playwright", "nonexistent-skill-12345"] @@ -160,6 +179,20 @@ describe("resolveMultipleSkillsAsync", () => { expect(result.resolved.get("playwright")).toContain("Playwright Browser Automation") }) + it("should treat disabled skills as not found async", async () => { + // #given: frontend-ui-ux disabled + const skillNames = ["frontend-ui-ux", "playwright"] + const options = { disabledSkills: new Set(["frontend-ui-ux"]) } + + // #when: resolving multiple skills async with disabled one + const result = await resolveMultipleSkillsAsync(skillNames, options) + + // #then: frontend-ui-ux in notFound, playwright resolved + expect(result.resolved.size).toBe(1) + expect(result.resolved.has("playwright")).toBe(true) + expect(result.notFound).toEqual(["frontend-ui-ux"]) + }) + it("should NOT inject watermark when both options are disabled", async () => { // given: git-master skill with watermark disabled const skillNames = ["git-master"] diff --git a/src/features/opencode-skill-loader/skill-content.ts b/src/features/opencode-skill-loader/skill-content.ts index 0a4bf81b..3dec3161 100644 --- a/src/features/opencode-skill-loader/skill-content.ts +++ b/src/features/opencode-skill-loader/skill-content.ts @@ -8,6 +8,7 @@ import type { GitMasterConfig, BrowserAutomationProvider } from "../../config/sc export interface SkillResolutionOptions { gitMasterConfig?: GitMasterConfig browserProvider?: BrowserAutomationProvider + disabledSkills?: Set } const cachedSkillsByProvider = new Map() @@ -23,7 +24,12 @@ async function getAllSkills(options?: SkillResolutionOptions): Promise ({ @@ -122,7 +128,10 @@ export function injectGitMasterConfig(template: string, config?: GitMasterConfig } export function resolveSkillContent(skillName: string, options?: SkillResolutionOptions): string | null { - const skills = createBuiltinSkills({ browserProvider: options?.browserProvider }) + const skills = createBuiltinSkills({ + browserProvider: options?.browserProvider, + disabledSkills: options?.disabledSkills, + }) const skill = skills.find((s) => s.name === skillName) if (!skill) return null @@ -137,7 +146,10 @@ export function resolveMultipleSkills(skillNames: string[], options?: SkillResol resolved: Map notFound: string[] } { - const skills = createBuiltinSkills({ browserProvider: options?.browserProvider }) + const skills = createBuiltinSkills({ + browserProvider: options?.browserProvider, + disabledSkills: options?.disabledSkills, + }) const skillMap = new Map(skills.map((s) => [s.name, s.template])) const resolved = new Map() diff --git a/src/index.ts b/src/index.ts index 1e74d0e4..7a67349d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -388,6 +388,7 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => { const lookAt = isMultimodalLookerEnabled ? createLookAt(ctx) : null; const browserProvider = pluginConfig.browser_automation_engine?.provider ?? "playwright"; + const disabledSkills = new Set(pluginConfig.disabled_skills ?? []); const delegateTask = createDelegateTask({ manager: backgroundManager, client: ctx.client, @@ -396,6 +397,7 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => { gitMasterConfig: pluginConfig.git_master, sisyphusJuniorModel: pluginConfig.agents?.["sisyphus-junior"]?.model, browserProvider, + disabledSkills, onSyncSessionCreated: async (event) => { log("[index] onSyncSessionCreated callback", { sessionID: event.sessionID, @@ -414,11 +416,8 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => { }); }, }); - const disabledSkills = new Set(pluginConfig.disabled_skills ?? []); const systemMcpNames = getSystemMcpServerNames(); - const builtinSkills = createBuiltinSkills({ browserProvider }).filter( - (skill) => { - if (disabledSkills.has(skill.name as never)) return false; + const builtinSkills = createBuiltinSkills({ browserProvider, disabledSkills }).filter((skill) => { if (skill.mcpConfig) { for (const mcpName of Object.keys(skill.mcpConfig)) { if (systemMcpNames.has(mcpName)) return false; @@ -450,6 +449,7 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => { mcpManager: skillMcpManager, getSessionID: getSessionIDForMcp, gitMasterConfig: pluginConfig.git_master, + disabledSkills }); const skillMcpTool = createSkillMcpTool({ manager: skillMcpManager, diff --git a/src/plugin-handlers/config-handler.ts b/src/plugin-handlers/config-handler.ts index f809e5d5..8573b801 100644 --- a/src/plugin-handlers/config-handler.ts +++ b/src/plugin-handlers/config-handler.ts @@ -157,6 +157,7 @@ export function createConfigHandler(deps: ConfigHandlerDeps) { // config.model represents the currently active model in OpenCode (including UI selection) // Pass it as uiSelectedModel so it takes highest priority in model resolution const currentModel = config.model as string | undefined; + const disabledSkills = new Set(pluginConfig.disabled_skills ?? []); const builtinAgents = await createBuiltinAgents( migratedDisabledAgents, pluginConfig.agents, @@ -167,7 +168,8 @@ export function createConfigHandler(deps: ConfigHandlerDeps) { allDiscoveredSkills, ctx.client, browserProvider, - currentModel // uiSelectedModel - takes highest priority + currentModel, // uiSelectedModel - takes highest priority + disabledSkills ); // Claude Code agents: Do NOT apply permission migration @@ -358,7 +360,8 @@ export function createConfigHandler(deps: ConfigHandlerDeps) { : {}; const planDemoteConfig = shouldDemotePlan - ? { mode: "subagent" as const } + ? { mode: "subagent" as const + } : undefined; config.agent = { diff --git a/src/tools/delegate-task/executor.ts b/src/tools/delegate-task/executor.ts index df761c08..ee908535 100644 --- a/src/tools/delegate-task/executor.ts +++ b/src/tools/delegate-task/executor.ts @@ -44,7 +44,7 @@ interface SessionMessage { export async function resolveSkillContent( skills: string[], - options: { gitMasterConfig?: GitMasterConfig; browserProvider?: BrowserAutomationProvider } + options: { gitMasterConfig?: GitMasterConfig; browserProvider?: BrowserAutomationProvider, disabledSkills?: Set } ): Promise<{ content: string | undefined; error: string | null }> { if (skills.length === 0) { return { content: undefined, error: null } diff --git a/src/tools/delegate-task/tools.ts b/src/tools/delegate-task/tools.ts index 965a82ee..0cf1d4ff 100644 --- a/src/tools/delegate-task/tools.ts +++ b/src/tools/delegate-task/tools.ts @@ -83,6 +83,7 @@ Prompts MUST be in English.` const { content: skillContent, error: skillError } = await resolveSkillContent(args.load_skills, { gitMasterConfig: options.gitMasterConfig, browserProvider: options.browserProvider, + disabledSkills: options.disabledSkills, }) if (skillError) { return skillError diff --git a/src/tools/delegate-task/types.ts b/src/tools/delegate-task/types.ts index aa0c512d..0b82836d 100644 --- a/src/tools/delegate-task/types.ts +++ b/src/tools/delegate-task/types.ts @@ -41,6 +41,7 @@ export interface DelegateTaskToolOptions { gitMasterConfig?: GitMasterConfig sisyphusJuniorModel?: string browserProvider?: BrowserAutomationProvider + disabledSkills?: Set onSyncSessionCreated?: (event: SyncSessionCreatedEvent) => Promise } diff --git a/src/tools/skill/tools.ts b/src/tools/skill/tools.ts index 3a2eca1f..006a07b4 100644 --- a/src/tools/skill/tools.ts +++ b/src/tools/skill/tools.ts @@ -133,7 +133,7 @@ export function createSkillTool(options: SkillLoadOptions = {}): ToolDefinition const getSkills = async (): Promise => { if (options.skills) return options.skills if (cachedSkills) return cachedSkills - cachedSkills = await getAllSkills() + cachedSkills = await getAllSkills({disabledSkills: options?.disabledSkills}) return cachedSkills } diff --git a/src/tools/skill/types.ts b/src/tools/skill/types.ts index d22951e0..3babfeef 100644 --- a/src/tools/skill/types.ts +++ b/src/tools/skill/types.ts @@ -28,4 +28,5 @@ export interface SkillLoadOptions { getSessionID?: () => string /** Git master configuration for watermark/co-author settings */ gitMasterConfig?: GitMasterConfig + disabledSkills?: Set }