fix(launch): include Pi native skill roots in 'all' mode; dedup 'discover' force-loads (#556)

packages/mosaic/src/commands/launch.ts

@@ -6,7 +6,15 @@
  */
 
 import { execFileSync, execSync, spawnSync } from 'node:child_process';
-import { existsSync, mkdirSync, readFileSync, writeFileSync, readdirSync, rmSync } from 'node:fs';
+import {
+  existsSync,
+  mkdirSync,
+  readFileSync,
+  writeFileSync,
+  readdirSync,
+  realpathSync,
+  rmSync,
+} from 'node:fs';
 import { createRequire } from 'node:module';
 import { homedir } from 'node:os';
 import { join, dirname } from 'node:path';
@@ -428,25 +436,74 @@ function ensureRuntimeConfig(runtime: RuntimeName, destPath: string): void {
 
 // ─── Pi skill/extension discovery ────────────────────────────────────────────
 
-function discoverPiSkills(): string[] {
+/** Resolve a skill dir to its canonical real path so symlinked duplicates
+ * (e.g. ~/.pi/agent/skills/X -> ~/.config/mosaic/skills/X) collapse to one key.
+ * Falls back to the literal path if it can't be resolved (e.g. broken link). */
+function skillRealPath(dir: string): string {
+  try {
+    return realpathSync(dir);
+  } catch {
+    return dir;
+  }
+}
+
+/** Skill roots Pi auto-discovers natively (no `--skill` needed): its global
+ * skills dir and the project-local one relative to the launch cwd. */
+function piNativeSkillRoots(cwd: string = process.cwd()): string[] {
+  return [join(homedir(), '.pi', 'agent', 'skills'), join(cwd, '.pi', 'skills')];
+}
+
+/** Enumerate skill dirs under a set of roots, deduped by real path. A directory
+ * counts as a skill when it (or its symlink target) contains a SKILL.md.
+ * Exported for tests (real-FS coverage of symlink acceptance + realpath dedup). */
+export function enumerateSkillDirs(roots: string[]): string[] {
+  const seen = new Set<string>();
   const args: string[] = [];
-  for (const skillsRoot of [join(MOSAIC_HOME, 'skills'), join(MOSAIC_HOME, 'skills-local')]) {
+  for (const skillsRoot of roots) {
     if (!existsSync(skillsRoot)) continue;
     try {
       for (const entry of readdirSync(skillsRoot, { withFileTypes: true })) {
-        if (!entry.isDirectory()) continue;
+        // Synced fleet skills land as symlinks, so accept both dirs and links.
+        if (!entry.isDirectory() && !entry.isSymbolicLink()) continue;
         const skillDir = join(skillsRoot, entry.name);
-        if (existsSync(join(skillDir, 'SKILL.md'))) {
-          args.push('--skill', skillDir);
-        }
+        if (!existsSync(join(skillDir, 'SKILL.md'))) continue;
+        const key = skillRealPath(skillDir);
+        if (seen.has(key)) continue;
+        seen.add(key);
+        args.push('--skill', skillDir);
       }
     } catch {
-      // skip
+      // skip unreadable roots
     }
   }
   return args;
 }
 
+/** Every skill dir Pi would link under `MOSAIC_PI_SKILL_MODE=all`: the Mosaic
+ * global/local catalog plus Pi's own native roots. `--no-skills` suppresses
+ * native auto-discovery, so 'all' must re-add the native roots explicitly or
+ * they would be silently dropped. Deduped by real path. */
+function discoverPiSkills(cwd: string = process.cwd()): string[] {
+  return enumerateSkillDirs([
+    join(MOSAIC_HOME, 'skills'),
+    join(MOSAIC_HOME, 'skills-local'),
+    ...piNativeSkillRoots(cwd),
+  ]);
+}
+
+/** Real paths of skills Pi will auto-discover from its native roots. Used to
+ * drop redundant force-loads in 'discover' mode (which keeps native discovery
+ * on) so the same skill is not registered twice. */
+function piNativeSkillRealPaths(cwd: string = process.cwd()): Set<string> {
+  const args = enumerateSkillDirs(piNativeSkillRoots(cwd));
+  const set = new Set<string>();
+  for (let i = 1; i < args.length; i += 2) {
+    const dir = args[i];
+    if (dir !== undefined) set.add(skillRealPath(dir));
+  }
+  return set;
+}
+
 type PiSkillMode = 'none' | 'all' | 'discover';
 
 function normalizePiSkillMode(env: NodeJS.ProcessEnv): PiSkillMode {
@@ -492,15 +549,19 @@ function forcedPiSkillArgs(env: NodeJS.ProcessEnv = process.env): string[] {
   return args;
 }
 
-/** Concatenate `--skill <dir>` arg groups, dropping any directory already seen. */
+/** Concatenate `--skill <dir>` arg groups, dropping any skill already seen.
+ * Dedup is by real path, so a forced skill and the same skill reached via a
+ * different (e.g. symlinked) directory collapse to a single `--skill`. */
 function mergeSkillArgs(...groups: string[][]): string[] {
   const seen = new Set<string>();
   const out: string[] = [];
   for (const group of groups) {
     for (let i = 0; i < group.length; i += 2) {
       const dir = group[i + 1];
-      if (group[i] !== '--skill' || dir === undefined || seen.has(dir)) continue;
-      seen.add(dir);
+      if (group[i] !== '--skill' || dir === undefined) continue;
+      const key = skillRealPath(dir);
+      if (seen.has(key)) continue;
+      seen.add(key);
       out.push('--skill', dir);
     }
   }
@@ -512,17 +573,31 @@ export function buildPiSkillArgs(
   env: NodeJS.ProcessEnv = process.env,
   discoveredSkillArgs: string[] = discoverPiSkills(),
   forcedSkillArgs: string[] = forcedPiSkillArgs(env),
+  nativeSkillRealPaths: Set<string> = piNativeSkillRealPaths(),
 ): string[] {
   const mode = normalizePiSkillMode(env);
 
   if (mode === 'discover') {
-    // Native Pi discovery handles the rest; still force-load the fleet skills.
-    return [...forcedSkillArgs];
+    // Native Pi discovery stays on, so only force-load fleet skills it will NOT
+    // already find under its native roots — otherwise the same skill is
+    // registered twice (once natively, once via --skill). mergeSkillArgs first
+    // collapses any intra-forced-set realpath duplicates, mirroring 'all' mode.
+    const deduped = mergeSkillArgs(forcedSkillArgs);
+    const out: string[] = [];
+    for (let i = 0; i < deduped.length; i += 2) {
+      const dir = deduped[i + 1];
+      if (deduped[i] !== '--skill' || dir === undefined) continue;
+      if (nativeSkillRealPaths.has(skillRealPath(dir))) continue;
+      out.push('--skill', dir);
+    }
+    return out;
   }
 
   if (mode === 'all') {
     // 'all' links the full catalog; merge in the forced set so fleet-critical
     // skills are guaranteed present even if they live only under skills-local/.
+    // discoverPiSkills already covers Pi's native roots, which `--no-skills`
+    // would otherwise suppress.
     return ['--no-skills', ...mergeSkillArgs(discoveredSkillArgs, forcedSkillArgs)];
   }