diff --git a/src/features/builtin-skills/skills.test.ts b/src/features/builtin-skills/skills.test.ts index afbca82de..525978e03 100644 --- a/src/features/builtin-skills/skills.test.ts +++ b/src/features/builtin-skills/skills.test.ts @@ -25,8 +25,30 @@ describe("createBuiltinSkills", () => { // then const playwrightSkill = skills.find((s) => s.name === "playwright") const agentBrowserSkill = skills.find((s) => s.name === "agent-browser") + const devBrowserSkill = skills.find((s) => s.name === "dev-browser") expect(playwrightSkill).toBeDefined() expect(agentBrowserSkill).toBeUndefined() + expect(devBrowserSkill).toBeUndefined() + }) + + test("returns dev-browser skill when browserProvider is 'dev-browser'", () => { + // given + const options = { browserProvider: "dev-browser" as const } + + // when + const skills = createBuiltinSkills(options) + + // then + const skillNames = skills.map((skill) => skill.name) + const devBrowserSkill = skills.find((skill) => skill.name === "dev-browser") + const playwrightSkill = skills.find((skill) => skill.name === "playwright") + const agentBrowserSkill = skills.find((skill) => skill.name === "agent-browser") + expect(devBrowserSkill).toBeDefined() + expect(devBrowserSkill!.description).toContain("Browser automation") + expect(playwrightSkill).toBeUndefined() + expect(agentBrowserSkill).toBeUndefined() + expect(skillNames).not.toContain("playwright-cli") + expect(skills.some((skill) => skill.allowedTools?.includes("Bash(playwright-cli:*)"))).toBe(false) }) test("returns agent-browser skill when browserProvider is 'agent-browser'", () => { @@ -67,9 +89,10 @@ describe("createBuiltinSkills", () => { // when const defaultSkills = createBuiltinSkills() const agentBrowserSkills = createBuiltinSkills({ browserProvider: "agent-browser" }) + const devBrowserSkills = createBuiltinSkills({ browserProvider: "dev-browser" }) // then - for (const skills of [defaultSkills, agentBrowserSkills]) { + for (const skills of [defaultSkills, agentBrowserSkills, devBrowserSkills]) { expect(skills.find((s) => s.name === "frontend-ui-ux")).toBeDefined() expect(skills.find((s) => s.name === "git-master")).toBeDefined() expect(skills.find((s) => s.name === "review-work")).toBeDefined() @@ -77,16 +100,18 @@ describe("createBuiltinSkills", () => { } }) - test("returns exactly 6 skills regardless of provider", () => { + test("returns exactly 5 skills regardless of provider", () => { // given // when const defaultSkills = createBuiltinSkills() const agentBrowserSkills = createBuiltinSkills({ browserProvider: "agent-browser" }) + const devBrowserSkills = createBuiltinSkills({ browserProvider: "dev-browser" }) // then - expect(defaultSkills).toHaveLength(6) - expect(agentBrowserSkills).toHaveLength(6) + expect(defaultSkills).toHaveLength(5) + expect(agentBrowserSkills).toHaveLength(5) + expect(devBrowserSkills).toHaveLength(5) }) test("should exclude playwright when it is in disabledSkills", () => { @@ -100,10 +125,10 @@ describe("createBuiltinSkills", () => { 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.map((s) => s.name)).not.toContain("dev-browser") expect(skills.map((s) => s.name)).toContain("review-work") expect(skills.map((s) => s.name)).toContain("ai-slop-remover") - expect(skills.length).toBe(5) + expect(skills.length).toBe(4) }) test("should exclude multiple skills when they are in disabledSkills", () => { @@ -117,17 +142,15 @@ describe("createBuiltinSkills", () => { 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.map((s) => s.name)).not.toContain("dev-browser") expect(skills.map((s) => s.name)).toContain("review-work") expect(skills.map((s) => s.name)).toContain("ai-slop-remover") - expect(skills.length).toBe(4) + expect(skills.length).toBe(3) }) 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", "review-work", "ai-slop-remover"]), - } + const options = { disabledSkills: new Set(["playwright", "frontend-ui-ux", "git-master", "review-work", "ai-slop-remover"]) } // #when const skills = createBuiltinSkills(options) @@ -144,7 +167,7 @@ describe("createBuiltinSkills", () => { const skills = createBuiltinSkills(options) // #then - expect(skills.length).toBe(6) + expect(skills.length).toBe(5) }) test("review-work skill has correct structure", () => { diff --git a/src/features/builtin-skills/skills.ts b/src/features/builtin-skills/skills.ts index 484d3adf4..82be5e974 100644 --- a/src/features/builtin-skills/skills.ts +++ b/src/features/builtin-skills/skills.ts @@ -21,15 +21,17 @@ export function createBuiltinSkills(options: CreateBuiltinSkillsOptions = {}): B const { browserProvider = "playwright", disabledSkills } = options let browserSkill: BuiltinSkill - if (browserProvider === "agent-browser") { - browserSkill = agentBrowserSkill - } else if (browserProvider === "playwright-cli") { - browserSkill = playwrightCliSkill - } else { - browserSkill = playwrightSkill - } + if (browserProvider === "agent-browser") { + browserSkill = agentBrowserSkill + } else if (browserProvider === "dev-browser") { + browserSkill = devBrowserSkill + } else if (browserProvider === "playwright-cli") { + browserSkill = playwrightCliSkill + } else { + browserSkill = playwrightSkill + } - const skills = [browserSkill, frontendUiUxSkill, gitMasterSkill, devBrowserSkill, reviewWorkSkill, aiSlopRemoverSkill] + const skills = [browserSkill, frontendUiUxSkill, gitMasterSkill, reviewWorkSkill, aiSlopRemoverSkill] if (!disabledSkills) { return skills diff --git a/src/plugin/skill-context.test.ts b/src/plugin/skill-context.test.ts index 4c80b2b61..75397fb0b 100644 --- a/src/plugin/skill-context.test.ts +++ b/src/plugin/skill-context.test.ts @@ -85,4 +85,68 @@ describe("createSkillContext", () => { getSystemMcpServerNamesSpy.mockRestore() } }) + + it("excludes discovered dev-browser skill when browser provider is playwright", async () => { + // given + const discoveredDevBrowserSkill = { + name: "dev-browser", + definition: { description: "Discovered dev-browser skill" }, + scope: "user" as const, + } + + const discoverConfigSourceSkillsSpy = spyOn( + skillLoader, + "discoverConfigSourceSkills", + ).mockResolvedValue([]) + const discoverUserClaudeSkillsSpy = spyOn( + skillLoader, + "discoverUserClaudeSkills", + ).mockResolvedValue([discoveredDevBrowserSkill]) + const discoverProjectClaudeSkillsSpy = spyOn( + skillLoader, + "discoverProjectClaudeSkills", + ).mockResolvedValue([]) + const discoverOpencodeGlobalSkillsSpy = spyOn( + skillLoader, + "discoverOpencodeGlobalSkills", + ).mockResolvedValue([]) + const discoverProjectAgentsSkillsSpy = spyOn( + skillLoader, + "discoverProjectAgentsSkills", + ).mockResolvedValue([]) + const discoverGlobalAgentsSkillsSpy = spyOn( + skillLoader, + "discoverGlobalAgentsSkills", + ).mockResolvedValue([]) + const getSystemMcpServerNamesSpy = spyOn( + mcpLoader, + "getSystemMcpServerNames", + ).mockReturnValue(new Set()) + + const pluginConfig = OhMyOpenCodeConfigSchema.parse({ + browser_automation_engine: { provider: "playwright" }, + }) + + try { + // when + const result = await createSkillContext({ + directory: testDirectory, + pluginConfig, + }) + + // then + expect(result.browserProvider).toBe("playwright") + expect(result.mergedSkills.some((skill) => skill.name === "playwright")).toBe(true) + expect(result.mergedSkills.some((skill) => skill.name === "dev-browser")).toBe(false) + expect(result.availableSkills.some((skill) => skill.name === "dev-browser")).toBe(false) + } finally { + discoverConfigSourceSkillsSpy.mockRestore() + discoverUserClaudeSkillsSpy.mockRestore() + discoverProjectClaudeSkillsSpy.mockRestore() + discoverOpencodeGlobalSkillsSpy.mockRestore() + discoverProjectAgentsSkillsSpy.mockRestore() + discoverGlobalAgentsSkillsSpy.mockRestore() + getSystemMcpServerNamesSpy.mockRestore() + } + }) }) diff --git a/src/plugin/skill-context.ts b/src/plugin/skill-context.ts index 05a72d688..6af00117a 100644 --- a/src/plugin/skill-context.ts +++ b/src/plugin/skill-context.ts @@ -26,7 +26,7 @@ export type SkillContext = { disabledSkills: Set } -const PROVIDER_GATED_SKILL_NAMES = new Set(["agent-browser", "playwright"]) +const PROVIDER_GATED_SKILL_NAMES = new Set(["agent-browser", "dev-browser", "playwright"]) function mapScopeToLocation(scope: SkillScope): AvailableSkill["location"] { if (scope === "user" || scope === "opencode") return "user"