Merge pull request #3667 from code-yeongyu/fix/dev-browser-provider-gating

fix(builtin-skills): treat dev-browser as a selectable browser provider (fixes #3411)

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(defaultSkills).toHaveLength(5)
-		expect(agentBrowserSkills).toHaveLength(6)
+		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 = {
+		const options = { disabledSkills: new Set(["playwright", "frontend-ui-ux", "git-master", "review-work", "ai-slop-remover"]) }
-			disabledSkills: new Set(["playwright", "frontend-ui-ux", "git-master", "dev-browser", "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", () => {

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") {
+	if (browserProvider === "agent-browser") {
-    browserSkill = agentBrowserSkill
+		browserSkill = agentBrowserSkill
-  } else if (browserProvider === "playwright-cli") {
+	} else if (browserProvider === "dev-browser") {
-    browserSkill = playwrightCliSkill
+		browserSkill = devBrowserSkill
-  } else {
+	} else if (browserProvider === "playwright-cli") {
-    browserSkill = playwrightSkill
+		browserSkill = playwrightCliSkill
-  }
+	} else {
+		browserSkill = playwrightSkill
+	}
 
-  const skills = [browserSkill, frontendUiUxSkill, gitMasterSkill, devBrowserSkill, reviewWorkSkill, aiSlopRemoverSkill]
+	const skills = [browserSkill, frontendUiUxSkill, gitMasterSkill, reviewWorkSkill, aiSlopRemoverSkill]
 
   if (!disabledSkills) {
     return skills

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<string>())
+
+    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()
+    }
+  })
 })

src/plugin/skill-context.ts

@@ -26,7 +26,7 @@ export type SkillContext = {
   disabledSkills: Set<string>
 }
 
-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"