fix(update): honor locked paths with mirrored skills

src/skill-relocation.test.ts

@@ -111,6 +111,30 @@ describe('resolveSkillLocations', () => {
     expect(result.resolvedPaths.size).toBe(0);
   });
 
+  it('uses an exact locked path when the installer can target it directly', () => {
+    const result = resolveSkillLocations(
+      ['swiftui-expert-skill'],
+      lock('skills/swiftui-expert-skill/SKILL.md'),
+      [
+        {
+          name: 'swiftui-expert-skill',
+          skillPath: 'skills/swiftui-expert-skill/SKILL.md',
+        },
+        {
+          name: 'swiftui-expert-skill',
+          skillPath: 'plugins/swiftui-expert-skill/SKILL.md',
+        },
+      ],
+      { exactPathDisambiguates: true }
+    );
+
+    expect(result.resolvedPaths.get('swiftui-expert-skill')).toBe(
+      'skills/swiftui-expert-skill/SKILL.md'
+    );
+    expect(result.deletedSkills).toEqual([]);
+    expect(result.ambiguousSkills).toEqual([]);
+  });
+
   it('normalizes path separators before exact matching', () => {
     const result = resolveSkillLocations(
       ['swiftui-expert-skill'],

src/skill-relocation.ts

@@ -9,6 +9,10 @@ export interface SkillLocationResolution {
   resolvedPaths: Map<string, string>;
 }
 
+export interface SkillLocationResolutionOptions {
+  exactPathDisambiguates?: boolean;
+}
+
 function normalizeSkillName(name: string): string {
   return name.toLowerCase().replace(/[\s_]+/g, '-');
 }
@@ -20,14 +24,16 @@ function normalizeSkillPath(path: string): string {
 /**
  * Resolve locked skills against their currently discovered locations.
  *
- * Exact paths always win. A missing path is treated as a relocation only when
- * exactly one discovered skill has the same normalized name. Ambiguous matches
- * fail closed: they are neither migrated nor offered for deletion.
+ * An exact path wins when the installer can target that path directly. A
+ * missing path is treated as a relocation only when exactly one discovered
+ * skill has the same normalized name. Other ambiguous matches fail closed:
+ * they are neither migrated nor offered for deletion.
  */
 export function resolveSkillLocations(
   lockedSkillNames: string[],
   lockSkills: Record<string, { skillPath?: string }>,
-  discovered: DiscoveredSkillLocation[]
+  discovered: DiscoveredSkillLocation[],
+  options: SkillLocationResolutionOptions = {}
 ): SkillLocationResolution {
   const discoveredPaths = new Set(discovered.map((skill) => normalizeSkillPath(skill.skillPath)));
   const pathsByName = new Map<string, Set<string>>();
@@ -49,16 +55,21 @@ export function resolveSkillLocations(
 
     const normalizedLockedPath = normalizeSkillPath(lockedPath);
     const candidates = [...(pathsByName.get(normalizeSkillName(name)) ?? [])];
+    const lockedPathStillExists = discoveredPaths.has(normalizedLockedPath);
+
+    if (lockedPathStillExists && options.exactPathDisambiguates) {
+      resolvedPaths.set(name, normalizedLockedPath);
+      continue;
+    }
 
-    // Update reinstallation ultimately selects by skill name. If more than one
-    // current location has that name, even an exact locked path is not enough
-    // to guarantee that every source type will reinstall the same one.
+    // Sources that cannot target a subpath reinstall by skill name. For those
+    // sources, even an exact locked path cannot guarantee which copy is used.
     if (candidates.length > 1) {
       ambiguousSkills.push(name);
       continue;
     }
 
-    if (discoveredPaths.has(normalizedLockedPath)) {
+    if (lockedPathStillExists) {
       resolvedPaths.set(name, normalizedLockedPath);
       continue;
     }

src/update.ts

@@ -21,6 +21,7 @@ import {
   resolveSkillLocations,
   type DiscoveredSkillLocation,
   type SkillLocationResolution,
+  type SkillLocationResolutionOptions,
 } from './skill-relocation.ts';
 import { wellKnownProvider, computeWellKnownSkillDigest } from './providers/index.ts';
 import { removeCommand } from './remove.ts';
@@ -299,9 +300,15 @@ export async function checkAndPromptForDeletions(
   lockSkills: Record<string, { skillPath?: string }>,
   isGlobal: boolean,
   options: UpdateCheckOptions,
-  discovered: DiscoveredSkillLocation[]
+  discovered: DiscoveredSkillLocation[],
+  locationOptions: SkillLocationResolutionOptions = {}
 ): Promise<SkillLocationResolution> {
-  const resolution = resolveSkillLocations(allLockedForSource, lockSkills, discovered);
+  const resolution = resolveSkillLocations(
+    allLockedForSource,
+    lockSkills,
+    discovered,
+    locationOptions
+  );
 
   if (resolution.ambiguousSkills.length > 0) {
     console.log();
@@ -624,7 +631,11 @@ export async function updateGlobalSkills(
         lock.skills,
         true,
         options,
-        discoveredLocations
+        discoveredLocations,
+        // Path-addressable sources reinstall only the locked directory, so a
+        // same-name mirror elsewhere in the repo is not ambiguous. Generic Git
+        // sources reinstall from the whole repo and remain fail-closed.
+        { exactPathDisambiguates: !shouldUseFullDepthForUpdate(firstEntry) }
       );
 
       const deletedSkillSet = new Set(resolution.deletedSkills);
@@ -884,7 +895,8 @@ export async function updateProjectSkills(
         localLock.skills,
         false,
         options,
-        discoveredLocations
+        discoveredLocations,
+        { exactPathDisambiguates: !shouldUseFullDepthForUpdate(firstEntry) }
       );
       deletedSkills = resolution.deletedSkills;
       resolvedPaths = resolution.resolvedPaths;

tests/private-repo-add-security.test.ts

@@ -42,23 +42,10 @@ vi.mock('../src/detect-agent.ts', () => ({
   ensureUniversalAgents: vi.fn((agents: string[]) => agents),
 }));
 
-vi.mock('../src/git.ts', () => {
-  class GitCloneError extends Error {
-    readonly url: string;
-    readonly isTimeout: boolean;
-    readonly isAuthError: boolean;
-
-    constructor(message: string, url: string, isTimeout = false, isAuthError = false) {
-      super(message);
-      this.name = 'GitCloneError';
-      this.url = url;
-      this.isTimeout = isTimeout;
-      this.isAuthError = isAuthError;
-    }
-  }
-
+vi.mock('../src/git.ts', async (importActual) => {
+  const actual = await importActual<typeof import('../src/git.ts')>();
   return {
-    GitCloneError,
+    ...actual,
     cloneRepo: vi.fn(),
     cleanupTempDir: vi.fn().mockResolvedValue(undefined),
   };

tests/update.test.ts

@@ -373,6 +373,44 @@ describe('Update Cleanup Unit Tests', () => {
       expect(spawnSync).not.toHaveBeenCalled();
     });
 
+    it('updates a GitHub skill from its locked path when a same-name mirror exists', async () => {
+      vi.mocked(localLock.readLocalLock).mockResolvedValue({
+        version: 1,
+        skills: {
+          'skill-a': {
+            source: 'owner/repo',
+            sourceType: 'github',
+            skillPath: 'skills/skill-a/SKILL.md',
+            computedHash: 'old-hash',
+          },
+        },
+      });
+      vi.mocked(git.cloneRepo).mockResolvedValue('/tmp/repo');
+      vi.mocked(skills.discoverSkills).mockResolvedValue([
+        {
+          name: 'skill-a',
+          path: '/tmp/repo/skills/skill-a',
+          description: 'Locked location',
+          rawContent: '',
+        },
+        {
+          name: 'skill-a',
+          path: '/tmp/repo/.openclaw/skills/skill-a',
+          description: 'Mirror',
+          rawContent: '',
+        },
+      ]);
+
+      await updateProjectSkills({ yes: true });
+
+      const installCall = vi
+        .mocked(spawnSync)
+        .mock.calls.find((call) => Array.isArray(call[1]) && call[1].includes('add'));
+      expect(installCall).toBeDefined();
+      expect(installCall![1]).toContain('owner/repo/skills/skill-a');
+      expect(installCall![1]).not.toContain('owner/repo/.openclaw/skills/skill-a');
+    });
+
     it('does not reinstall an ambiguous exact-path skill from a generic Git source', async () => {
       vi.mocked(localLock.readLocalLock).mockResolvedValue({
         version: 1,
@@ -609,6 +647,51 @@ describe('Update Cleanup Unit Tests', () => {
       );
     });
 
+    it('updates an exact-path global GitHub skill when fallback discovery finds a mirror', async () => {
+      const oldHash = 'a'.repeat(40);
+      const newHash = 'b'.repeat(40);
+      vi.mocked(skillLock.readSkillLock).mockResolvedValue({
+        version: 3,
+        skills: {
+          'skill-a': {
+            source: 'owner/repo',
+            sourceType: 'github',
+            sourceUrl: 'https://github.com/owner/repo',
+            skillPath: 'skills/skill-a/SKILL.md',
+            skillFolderHash: oldHash,
+            installedAt: '',
+            updatedAt: '',
+          },
+        },
+      });
+      vi.mocked(blob.fetchRepoTree).mockResolvedValue(null);
+      vi.mocked(git.cloneRepo).mockResolvedValue('/tmp/repo');
+      vi.mocked(skills.discoverSkills).mockResolvedValue([
+        {
+          name: 'skill-a',
+          path: '/tmp/repo/skills/skill-a',
+          description: 'Locked location',
+          rawContent: '',
+        },
+        {
+          name: 'skill-a',
+          path: '/tmp/repo/.openclaw/skills/skill-a',
+          description: 'Mirror',
+          rawContent: '',
+        },
+      ]);
+      vi.mocked(git.getGitTreeHash).mockResolvedValue(newHash);
+
+      await updateGlobalSkills({ yes: true });
+
+      expect(git.getGitTreeHash).toHaveBeenCalledWith('/tmp/repo', 'skills/skill-a/SKILL.md');
+      const installCall = vi
+        .mocked(spawnSync)
+        .mock.calls.find((call) => Array.isArray(call[1]) && call[1].includes('add'));
+      expect(installCall).toBeDefined();
+      expect(installCall![1]).toContain('owner/repo/skills/skill-a');
+    });
+
     it('reinstalls a relocated global skill even when its content hash is unchanged', async () => {
       const treeHash = 'a'.repeat(40);
       vi.mocked(skillLock.readSkillLock).mockResolvedValue({