fix(opencode): resolve duplicate skill names deterministically, preferring .claude

packages/opencode/src/skill/index.ts

@@ -24,6 +24,27 @@ const EXTERNAL_SKILL_PATTERN = "skills/**/SKILL.md"
 const CODEV_SKILL_PATTERN = "{skill,skills}/**/SKILL.md"
 const SKILL_PATTERN = "**/SKILL.md"
 
+// When two SKILL.md files declare the same `name`, the winner is chosen by
+// this rank (higher wins), NOT by which parse finishes first — skills are
+// parsed with unbounded concurrency, so a completion-order tie-break would be
+// non-deterministic run to run. `.claude` outranks `.agents` so a Claude Code
+// skill deterministically shadows an `.agents` copy of the same name (the two
+// are frequently the very same skill reached through a symlink); the config
+// dir, `skills.paths` and `skills.urls` fall below the external dirs. Equal
+// ranks tie-break on the lexicographically smaller path, which is also
+// order-independent — so the resolved set is identical on every run.
+const SkillPriority = {
+  claudeExternal: 50,
+  agentsExternal: 40,
+  configDir: 30,
+  skillsPath: 20,
+  skillsUrl: 10,
+} as const
+
+function externalDirPriority(dir: string): number {
+  return dir === CLAUDE_EXTERNAL_DIR ? SkillPriority.claudeExternal : SkillPriority.agentsExternal
+}
+
 // Built-in skill that ships with opencode. The model's intuition for what an
 // codev.json should look like is often wrong, and opencode hard-fails on
 // invalid config, so users hit cryptic startup errors. Loading this skill
@@ -81,16 +102,27 @@ export class NotFoundError extends Schema.TaggedErrorClass<NotFoundError>()("Ski
 
 type State = {
   skills: Record<string, Info>
+  // name -> priority of the skill currently occupying skills[name]; lets a
+  // later, higher-ranked match replace an earlier one regardless of the order
+  // the concurrent parses complete in.
+  priorities: Record<string, number>
   dirs: Set<string>
 }
 
+type Match = {
+  path: string
+  priority: number
+}
+
 type DiscoveryState = {
-  matches: string[]
+  matches: Match[]
   dirs: string[]
 }
 
 type ScanState = {
-  matches: Set<string>
+  // path -> highest priority it was scanned at (a path reached from two roots
+  // keeps the stronger claim).
+  matches: Map<string, number>
   dirs: Set<string>
 }
 
@@ -102,7 +134,12 @@ export interface Interface {
   readonly available: (agent?: Agent.Info) => Effect.Effect<Info[]>
 }
 
-const add = Effect.fnUntraced(function* (state: State, match: string, events: EventV2Bridge.Service["Service"]) {
+const add = Effect.fnUntraced(function* (
+  state: State,
+  match: string,
+  priority: number,
+  events: EventV2Bridge.Service["Service"],
+) {
   const md = yield* Effect.tryPromise({
     try: () => ConfigMarkdown.parse(match),
     catch: (err) => err,
@@ -122,15 +159,26 @@ const add = Effect.fnUntraced(function* (state: State, match: string, events: Ev
 
   if (!isSkillFrontmatter(md.data)) return
 
-  if (state.skills[md.data.name]) {
+  state.dirs.add(path.dirname(match))
+
+  const existing = state.skills[md.data.name]
+  if (existing) {
+    // A built-in skill (registered before disk discovery) has no priority
+    // entry; treat it as the weakest so any disk skill overrides it.
+    const existingPriority = state.priorities[md.data.name] ?? Number.NEGATIVE_INFINITY
+    // Deterministic winner, independent of parse-completion order: higher
+    // rank wins; equal rank keeps the lexicographically smaller path.
+    const keepExisting =
+      priority < existingPriority || (priority === existingPriority && existing.location <= match)
     yield* Effect.logWarning("duplicate skill name", {
       name: md.data.name,
-      existing: state.skills[md.data.name].location,
-      duplicate: match,
+      kept: keepExisting ? existing.location : match,
+      dropped: keepExisting ? match : existing.location,
     })
+    if (keepExisting) return
   }
 
-  state.dirs.add(path.dirname(match))
+  state.priorities[md.data.name] = priority
   state.skills[md.data.name] = {
     name: md.data.name,
     description: md.data.description,
@@ -143,6 +191,7 @@ const scan = Effect.fnUntraced(function* (
   state: ScanState,
   root: string,
   pattern: string,
+  priority: number,
   opts?: { dot?: boolean; scope?: string },
 ) {
   const matches = yield* Effect.tryPromise({
@@ -165,7 +214,8 @@ const scan = Effect.fnUntraced(function* (
   )
 
   for (const match of matches) {
-    state.matches.add(match)
+    const prev = state.matches.get(match)
+    state.matches.set(match, prev === undefined ? priority : Math.max(prev, priority))
     state.dirs.add(path.dirname(match))
   }
 })
@@ -180,7 +230,7 @@ const discoverSkills = Effect.fnUntraced(function* (
   directory: string,
   worktree: string,
 ) {
-  const state: ScanState = { matches: new Set(), dirs: new Set() }
+  const state: ScanState = { matches: new Map(), dirs: new Set() }
 
   const externalDirs: string[] = []
   if (!disableExternalSkills) {
@@ -190,21 +240,26 @@ const discoverSkills = Effect.fnUntraced(function* (
     for (const dir of externalDirs) {
       const root = path.join(global.home, dir)
       if (!(yield* fsys.isDir(root))) continue
-      yield* scan(state, root, EXTERNAL_SKILL_PATTERN, { dot: true, scope: "global" })
+      yield* scan(state, root, EXTERNAL_SKILL_PATTERN, externalDirPriority(dir), { dot: true, scope: "global" })
     }
 
     const upDirs = yield* fsys
       .up({ targets: externalDirs, start: directory, stop: worktree })
       .pipe(Effect.catch(() => Effect.succeed([] as string[])))
 
     for (const root of upDirs) {
-      yield* scan(state, root, EXTERNAL_SKILL_PATTERN, { dot: true, scope: "project" })
+      // up() returns each matched dir as `<ancestor>/<target>`, so its
+      // basename is the external dir name that fixes its priority.
+      yield* scan(state, root, EXTERNAL_SKILL_PATTERN, externalDirPriority(path.basename(root)), {
+        dot: true,
+        scope: "project",
+      })
     }
   }
 
   const configDirs = yield* config.directories()
   for (const dir of configDirs) {
-    yield* scan(state, dir, CODEV_SKILL_PATTERN)
+    yield* scan(state, dir, CODEV_SKILL_PATTERN, SkillPriority.configDir)
   }
 
   const cfg = yield* config.get()
@@ -216,18 +271,18 @@ const discoverSkills = Effect.fnUntraced(function* (
       continue
     }
 
-    yield* scan(state, dir, SKILL_PATTERN)
+    yield* scan(state, dir, SKILL_PATTERN, SkillPriority.skillsPath)
   }
 
   for (const url of cfg.skills?.urls ?? []) {
     const pulledDirs = yield* discovery.pull(url)
     for (const dir of pulledDirs) {
-      yield* scan(state, dir, SKILL_PATTERN)
+      yield* scan(state, dir, SKILL_PATTERN, SkillPriority.skillsUrl)
     }
   }
 
   return {
-    matches: Array.from(state.matches),
+    matches: Array.from(state.matches, ([path, priority]) => ({ path, priority })),
     dirs: Array.from(state.dirs),
   }
 })
@@ -237,7 +292,7 @@ const loadSkills = Effect.fnUntraced(function* (
   discovered: DiscoveryState,
   events: EventV2Bridge.Service["Service"],
 ) {
-  yield* Effect.forEach(discovered.matches, (match) => add(state, match, events), {
+  yield* Effect.forEach(discovered.matches, (match) => add(state, match.path, match.priority, events), {
     concurrency: "unbounded",
     discard: true,
   })
@@ -272,7 +327,7 @@ const layer = Layer.effect(
     )
     const state = yield* InstanceState.make(
       Effect.fn("Skill.state")(function* () {
-        const s: State = { skills: {}, dirs: new Set() }
+        const s: State = { skills: {}, priorities: {}, dirs: new Set() }
         // Register the built-in skill BEFORE disk discovery so a user-disk
         // skill with the same name can override it.
         s.skills[CUSTOMIZE_CODEV_SKILL_NAME] = {

packages/opencode/test/skill/skill.test.ts

@@ -443,6 +443,49 @@ description: A skill in the .agents/skills directory.
     ),
   )
 
+  it.live("prefers the .claude copy when the same skill name is in .claude and .agents", () =>
+    provideTmpdirInstance(
+      (dir) =>
+        Effect.gen(function* () {
+          yield* Effect.promise(() =>
+            Promise.all([
+              Bun.write(
+                path.join(dir, ".claude", "skills", "dup-skill", "SKILL.md"),
+                `---
+name: dup-skill
+description: The .claude copy.
+---
+
+# Claude copy
+`,
+              ),
+              Bun.write(
+                path.join(dir, ".agents", "skills", "dup-skill", "SKILL.md"),
+                `---
+name: dup-skill
+description: The .agents copy.
+---
+
+# Agents copy
+`,
+              ),
+            ]),
+          )
+
+          const skill = yield* Skill.Service
+          const list = (yield* skill.all()).filter((s) => s.location !== "<built-in>")
+          // A single winner (the duplicate name collapses to one entry), and
+          // it is deterministically the .claude copy — never the .agents one,
+          // regardless of parse-completion order.
+          expect(list.filter((s) => s.name === "dup-skill")).toHaveLength(1)
+          const resolved = list.find((s) => s.name === "dup-skill")!
+          expect(resolved.location).toContain(path.join(".claude", "skills", "dup-skill", "SKILL.md"))
+          expect(resolved.description).toBe("The .claude copy.")
+        }),
+      { git: true },
+    ),
+  )
+
   itWithoutClaudeCodeSkills.live("skips Claude Code skills when disabled", () =>
     provideTmpdirInstance(
       (dir) =>