fix(cli): resolve bundled skills by proximity, not candidate order

.changeset/staged-skill-resolution-proximity.md

@@ -0,0 +1,5 @@
+---
+"hunkdiff": patch
+---
+
+Stop an unrelated `skills/` directory above the install location from shadowing the bundled skill a source install staged beside the Hunk executable.

packages/hunk/src/core/run/paths.test.ts

@@ -96,6 +96,64 @@ describe("paths", () => {
     }
   });
 
+  test("prefers a staged skill beside the binary over a generic one further up", () => {
+    const tempRoot = createTempRoot("hunk-skill-proximity-");
+
+    try {
+      // A source install stages its skills under `hunkdiff/` beside the executable, so a
+      // reviewer with their own `skills/` directory anywhere above the bin directory must
+      // not shadow it. Exhausting the generic shape to the filesystem root first did.
+      const installDir = join(tempRoot, ".local", "bin");
+      const installedSkill = join(installDir, "hunkdiff", "skills", "hunk-review", "SKILL.md");
+      const unrelatedSkill = join(tempRoot, "skills", "hunk-review", "SKILL.md");
+      const fakeBinary = join(installDir, "hunk");
+
+      mkdirSync(dirname(installedSkill), { recursive: true });
+      mkdirSync(dirname(unrelatedSkill), { recursive: true });
+      writeFileSync(installedSkill, "# installed\n");
+      writeFileSync(unrelatedSkill, "# unrelated\n");
+      writeFileSync(fakeBinary, "binary\n");
+
+      expect(resolveBundledSkillPath("hunk-review", [fakeBinary])).toBe(installedSkill);
+    } finally {
+      rmSync(tempRoot, { recursive: true, force: true });
+    }
+  });
+
+  test("prefers a standalone build's own skills over a stale nested package", () => {
+    const tempRoot = createTempRoot("hunk-skill-specificity-");
+
+    try {
+      // All three shapes at one ancestor. A standalone build ships its skills beside the
+      // executable, and a `node_modules/hunkdiff` in the same directory belongs to whatever
+      // project happens to live there — possibly pinned to another version — so the shipped
+      // copy has to win rather than whichever shape is checked first.
+      const shippedSkill = join(tempRoot, "hunkdiff", "skills", "hunk-review", "SKILL.md");
+      const siblingSkill = join(tempRoot, "skills", "hunk-review", "SKILL.md");
+      const staleNestedSkill = join(
+        tempRoot,
+        "node_modules",
+        "hunkdiff",
+        "skills",
+        "hunk-review",
+        "SKILL.md",
+      );
+      const fakeBinary = join(tempRoot, "hunk");
+
+      for (const skill of [shippedSkill, siblingSkill, staleNestedSkill]) {
+        mkdirSync(dirname(skill), { recursive: true });
+      }
+      writeFileSync(shippedSkill, "# shipped\n");
+      writeFileSync(siblingSkill, "# sibling\n");
+      writeFileSync(staleNestedSkill, "# stale\n");
+      writeFileSync(fakeBinary, "binary\n");
+
+      expect(resolveBundledSkillPath("hunk-review", [fakeBinary])).toBe(shippedSkill);
+    } finally {
+      rmSync(tempRoot, { recursive: true, force: true });
+    }
+  });
+
   test("canonicalizes two spellings of one directory to the same path", () => {
     // Canonicalize with the same resolver the code under test uses: plain
     // realpathSync leaves Windows 8.3 short names (RUNNER~1) in place, which

packages/hunk/src/core/run/paths.ts

@@ -118,8 +118,14 @@ export function resolveInstalledExtensionsRoot(env: NodeJS.ProcessEnv = process.
   return extensionsDir ? join(extensionsDir, INSTALLED_EXTENSIONS_DIR_NAME) : undefined;
 }
 
-/** Search one path and its parents for one relative child path. */
-function findRelativePathFromAncestors(startPath: string, relativePath: string) {
+/**
+ * Search one path and its parents for the first of several relative child paths.
+ *
+ * Proximity wins over candidate order: every shape is tested at one ancestor before the
+ * walk moves up. Exhausting one shape to the filesystem root first would let a generic
+ * match far above the start path beat the specific match sitting right at it.
+ */
+function findRelativePathFromAncestors(startPath: string, relativePaths: readonly string[]) {
   let current = resolve(startPath);
 
   try {
@@ -131,9 +137,11 @@ function findRelativePathFromAncestors(startPath: string, relativePath: string)
   }
 
   for (;;) {
-    const candidate = join(current, relativePath);
-    if (fs.existsSync(candidate)) {
-      return candidate;
+    for (const relativePath of relativePaths) {
+      const candidate = join(current, relativePath);
+      if (fs.existsSync(candidate)) {
+        return candidate;
+      }
     }
 
     const parent = dirname(current);
@@ -157,18 +165,19 @@ export function resolveBundledSkillPath(
 ) {
   const roots = searchRoots ?? [import.meta.dir, process.execPath];
   const skillRelativePath = join("skills", name, "SKILL.md");
+  // Order within one directory, closest-shipped first. A standalone build carries its skills
+  // beside the executable, so that copy wins over a `node_modules/hunkdiff` that a nearby
+  // project may have installed at a different version and left stale.
   const relativeCandidates = [
-    skillRelativePath,
     join("hunkdiff", skillRelativePath),
+    skillRelativePath,
     join("node_modules", "hunkdiff", skillRelativePath),
   ];
 
   for (const root of roots) {
-    for (const relativePath of relativeCandidates) {
-      const resolvedPath = findRelativePathFromAncestors(root, relativePath);
-      if (resolvedPath) {
-        return resolvedPath;
-      }
+    const resolvedPath = findRelativePathFromAncestors(root, relativeCandidates);
+    if (resolvedPath) {
+      return resolvedPath;
     }
   }