fix(coding-agent): dedupe symlinked resources (#3818)

* fix(coding-agent): dedupe symlinked resources

Fixes #3767

* refactor(coding-agent): extract canonicalizePath util for symlink resolution

packages/coding-agent/src/core/package-manager.ts

@@ -1,15 +1,6 @@
 import { type ChildProcess, type ChildProcessByStdio, spawn, spawnSync } from "node:child_process";
 import { createHash } from "node:crypto";
-import {
-	existsSync,
-	mkdirSync,
-	readdirSync,
-	readFileSync,
-	realpathSync,
-	rmSync,
-	statSync,
-	writeFileSync,
-} from "node:fs";
+import { existsSync, mkdirSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs";
 import { homedir, tmpdir } from "node:os";
 import { basename, dirname, join, relative, resolve, sep } from "node:path";
 import type { Readable } from "node:stream";
@@ -18,7 +9,7 @@ import ignore from "ignore";
 import { minimatch } from "minimatch";
 import { CONFIG_DIR_NAME } from "../config.js";
 import { type GitSource, parseGitUrl } from "../utils/git.js";
-import { isLocalPath } from "../utils/paths.js";
+import { canonicalizePath, isLocalPath } from "../utils/paths.js";
 import { isStdoutTakenOver } from "./output-guard.js";
 import type { PackageSource, SettingsManager } from "./settings-manager.js";
 
@@ -2286,40 +2277,30 @@ export class DefaultPackageManager implements PackageManager {
 	}
 
 	private toResolvedPaths(accumulator: ResourceAccumulator): ResolvedPaths {
-		const toResolved = (entries: Map<string, { metadata: PathMetadata; enabled: boolean }>): ResolvedResource[] => {
+		const mapToResolved = (
+			entries: Map<string, { metadata: PathMetadata; enabled: boolean }>,
+		): ResolvedResource[] => {
 			const resolved = Array.from(entries.entries()).map(([path, { metadata, enabled }]) => ({
 				path,
 				enabled,
 				metadata,
 			}));
 			resolved.sort((a, b) => resourcePrecedenceRank(a.metadata) - resourcePrecedenceRank(b.metadata));
-			return resolved;
+
+			const seen = new Set<string>();
+			return resolved.filter((entry) => {
+				const canonicalPath = canonicalizePath(entry.path);
+				if (seen.has(canonicalPath)) return false;
+				seen.add(canonicalPath);
+				return true;
+			});
 		};
 
-		const seenCanonicalSkillPaths = new Set<string>();
-		const resolvedSkills = toResolved(accumulator.skills).filter((entry) => {
-			let canonicalPath: string;
-			try {
-				// Resolve symlink aliases to detect duplicate files.
-				canonicalPath = realpathSync(entry.path);
-			} catch {
-				// Fallback to raw path to match loadSkills() behavior.
-				canonicalPath = entry.path;
-			}
-
-			if (seenCanonicalSkillPaths.has(canonicalPath)) {
-				return false;
-			}
-
-			seenCanonicalSkillPaths.add(canonicalPath);
-			return true;
-		});
-
 		return {
-			extensions: toResolved(accumulator.extensions),
-			skills: resolvedSkills,
-			prompts: toResolved(accumulator.prompts),
-			themes: toResolved(accumulator.themes),
+			extensions: mapToResolved(accumulator.extensions),
+			skills: mapToResolved(accumulator.skills),
+			prompts: mapToResolved(accumulator.prompts),
+			themes: mapToResolved(accumulator.themes),
 		};
 	}
 

packages/coding-agent/src/core/resource-loader.ts

@@ -8,7 +8,7 @@ import type { ResourceDiagnostic } from "./diagnostics.js";
 
 export type { ResourceCollision, ResourceDiagnostic } from "./diagnostics.js";
 
-import { isLocalPath } from "../utils/paths.js";
+import { canonicalizePath, isLocalPath } from "../utils/paths.js";
 import { createEventBus, type EventBus } from "./event-bus.js";
 import { createExtensionRuntime, loadExtensionFromFactory, loadExtensions } from "./extensions/loader.js";
 import type { Extension, ExtensionFactory, ExtensionRuntime, LoadExtensionsResult } from "./extensions/types.js";
@@ -664,8 +664,9 @@ export class DefaultResourceLoader implements ResourceLoader {
 
 		for (const p of [...primary, ...additional]) {
 			const resolved = this.resolveResourcePath(p);
-			if (seen.has(resolved)) continue;
-			seen.add(resolved);
+			const canonicalPath = canonicalizePath(resolved);
+			if (seen.has(canonicalPath)) continue;
+			seen.add(canonicalPath);
 			merged.push(resolved);
 		}
 

packages/coding-agent/src/core/skills.ts

@@ -1,9 +1,10 @@
-import { existsSync, readdirSync, readFileSync, realpathSync, statSync } from "fs";
+import { existsSync, readdirSync, readFileSync, statSync } from "fs";
 import ignore from "ignore";
 import { homedir } from "os";
 import { basename, dirname, isAbsolute, join, relative, resolve, sep } from "path";
 import { CONFIG_DIR_NAME, getAgentDir } from "../config.js";
 import { parseFrontmatter } from "../utils/frontmatter.js";
+import { canonicalizePath } from "../utils/paths.js";
 import type { ResourceDiagnostic } from "./diagnostics.js";
 import { createSyntheticSourceInfo, type SourceInfo } from "./source-info.js";
 
@@ -416,12 +417,7 @@ export function loadSkills(options: LoadSkillsOptions): LoadSkillsResult {
 		allDiagnostics.push(...result.diagnostics);
 		for (const skill of result.skills) {
 			// Resolve symlinks to detect duplicate files
-			let realPath: string;
-			try {
-				realPath = realpathSync(skill.filePath);
-			} catch {
-				realPath = skill.filePath;
-			}
+			const realPath = canonicalizePath(skill.filePath);
 
 			// Skip silently if we've already loaded this exact file (via symlink)
 			if (realPathSet.has(realPath)) {

packages/coding-agent/src/modes/interactive/components/session-selector.ts

@@ -1,5 +1,5 @@
 import { spawnSync } from "node:child_process";
-import { existsSync, realpathSync } from "node:fs";
+import { existsSync } from "node:fs";
 import { unlink } from "node:fs/promises";
 import * as os from "node:os";
 import {
@@ -13,6 +13,7 @@ import {
 	truncateToWidth,
 	visibleWidth,
 } from "@mariozechner/pi-tui";
+import { canonicalizePath as _canonicalizePath } from "../../../../src/utils/paths.js";
 import { KeybindingsManager } from "../../../core/keybindings.js";
 import type { SessionInfo, SessionListProgress } from "../../../core/session-manager.js";
 import { theme } from "../theme/theme.js";
@@ -49,11 +50,7 @@ function formatSessionDate(date: Date): string {
 
 function canonicalizePath(path: string | undefined): string | undefined {
 	if (!path) return path;
-	try {
-		return realpathSync(path);
-	} catch {
-		return path;
-	}
+	return _canonicalizePath(path);
 }
 
 class SessionSelectorHeader implements Component {

packages/coding-agent/src/utils/paths.ts

@@ -1,3 +1,19 @@
+import { realpathSync } from "node:fs";
+
+/**
+ * Resolve a path to its canonical (real) form, following symlinks.
+ * Falls back to the raw path if resolution fails (e.g. the target does
+ * not exist yet), so that callers never crash on missing filesystem
+ * entries.
+ */
+export function canonicalizePath(path: string): string {
+	try {
+		return realpathSync(path);
+	} catch {
+		return path;
+	}
+}
+
 /**
  * Returns true if the value is NOT a package source (npm:, git:, etc.)
  * or a URL protocol. Bare names and relative paths without ./ prefix

packages/coding-agent/test/package-manager.test.ts

@@ -158,6 +158,63 @@ Content`,
 			expect(result.prompts.some((r) => r.path === promptPath && !r.enabled)).toBe(true);
 		});
 
+		it("should resolve symlinked user and project resources once", async () => {
+			const sharedDir = join(tempDir, "shared-resources");
+			const sharedExtensionsDir = join(sharedDir, "extensions");
+			const sharedSkillsDir = join(sharedDir, "skills");
+			const sharedPromptsDir = join(sharedDir, "prompts");
+			const sharedThemesDir = join(sharedDir, "themes");
+			mkdirSync(sharedExtensionsDir, { recursive: true });
+			mkdirSync(sharedSkillsDir, { recursive: true });
+			mkdirSync(sharedPromptsDir, { recursive: true });
+			mkdirSync(sharedThemesDir, { recursive: true });
+
+			writeFileSync(join(sharedExtensionsDir, "shared.ts"), "export default function() {}");
+			mkdirSync(join(sharedSkillsDir, "shared-skill"), { recursive: true });
+			writeFileSync(
+				join(sharedSkillsDir, "shared-skill", "SKILL.md"),
+				`---
+name: shared-skill
+description: Shared skill
+---
+Content`,
+			);
+			writeFileSync(join(sharedPromptsDir, "shared.md"), "Shared prompt");
+			writeFileSync(join(sharedThemesDir, "shared.json"), JSON.stringify({ name: "shared-theme" }));
+
+			mkdirSync(join(agentDir), { recursive: true });
+			mkdirSync(join(tempDir, ".pi"), { recursive: true });
+			symlinkSync(sharedExtensionsDir, join(agentDir, "extensions"), "dir");
+			symlinkSync(sharedSkillsDir, join(agentDir, "skills"), "dir");
+			symlinkSync(sharedPromptsDir, join(agentDir, "prompts"), "dir");
+			symlinkSync(sharedThemesDir, join(agentDir, "themes"), "dir");
+			symlinkSync(sharedExtensionsDir, join(tempDir, ".pi", "extensions"), "dir");
+			symlinkSync(sharedSkillsDir, join(tempDir, ".pi", "skills"), "dir");
+			symlinkSync(sharedPromptsDir, join(tempDir, ".pi", "prompts"), "dir");
+			symlinkSync(sharedThemesDir, join(tempDir, ".pi", "themes"), "dir");
+
+			const result = await packageManager.resolve();
+
+			expect({
+				extensions: result.extensions.length,
+				skills: result.skills.length,
+				prompts: result.prompts.length,
+				themes: result.themes.length,
+			}).toEqual({
+				extensions: 1,
+				skills: 1,
+				prompts: 1,
+				themes: 1,
+			});
+
+			// Project auto-discovered has higher precedence than user auto-discovered,
+			// so the surviving entry should be scoped to project.
+			expect(result.extensions[0].metadata.scope).toBe("project");
+			expect(result.skills[0].metadata.scope).toBe("project");
+			expect(result.prompts[0].metadata.scope).toBe("project");
+			expect(result.themes[0].metadata.scope).toBe("project");
+		});
+
 		it("should auto-discover project prompts with overrides", async () => {
 			const promptsDir = join(tempDir, ".pi", "prompts");
 			mkdirSync(promptsDir, { recursive: true });

packages/coding-agent/test/paths.test.ts

@@ -0,0 +1,84 @@
+import { mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { afterEach, describe, expect, it } from "vitest";
+import { canonicalizePath, isLocalPath } from "../src/utils/paths.js";
+
+let tempDir: string;
+
+afterEach(() => {
+	if (tempDir) {
+		rmSync(tempDir, { recursive: true, force: true });
+		tempDir = "";
+	}
+});
+
+function createTempDir(): string {
+	tempDir = mkdtempSync(join(tmpdir(), "pi-paths-"));
+	return tempDir;
+}
+
+describe("canonicalizePath", () => {
+	it("returns the real path for a regular file", () => {
+		const dir = createTempDir();
+		const file = join(dir, "file.txt");
+		writeFileSync(file, "hello");
+		expect(canonicalizePath(file)).toBe(realpathSync(file));
+	});
+
+	it("resolves symlinks to their targets", () => {
+		const dir = createTempDir();
+		const target = join(dir, "target.txt");
+		const link = join(dir, "link.txt");
+		writeFileSync(target, "hello");
+		symlinkSync(target, link);
+		expect(canonicalizePath(link)).toBe(realpathSync(target));
+	});
+
+	it("resolves directory symlinks", () => {
+		const dir = createTempDir();
+		const targetDir = join(dir, "target-dir");
+		const linkDir = join(dir, "link-dir");
+		mkdirSync(targetDir);
+		symlinkSync(targetDir, linkDir, "dir");
+		expect(canonicalizePath(linkDir)).toBe(realpathSync(targetDir));
+	});
+
+	it("falls back to the raw path when the target does not exist", () => {
+		const dir = createTempDir();
+		const nonexistent = join(dir, "no-such-file");
+		expect(canonicalizePath(nonexistent)).toBe(nonexistent);
+	});
+
+	it("falls back to the raw path for a dangling symlink", () => {
+		const dir = createTempDir();
+		const target = join(dir, "target.txt");
+		const link = join(dir, "link.txt");
+		// Create a symlink whose target does not exist.
+		symlinkSync(target, link);
+		// realpathSync would throw, so canonicalizePath returns the link path.
+		expect(canonicalizePath(link)).toBe(link);
+	});
+});
+
+describe("isLocalPath", () => {
+	it("returns true for bare names", () => {
+		expect(isLocalPath("my-package")).toBe(true);
+	});
+
+	it("returns true for relative paths", () => {
+		expect(isLocalPath("./foo")).toBe(true);
+	});
+
+	it("returns false for npm: protocol", () => {
+		expect(isLocalPath("npm:package")).toBe(false);
+	});
+
+	it("returns false for git: protocol", () => {
+		expect(isLocalPath("git://repo")).toBe(false);
+	});
+
+	it("returns false for https: protocol", () => {
+		expect(isLocalPath("https://example.com")).toBe(false);
+	});
+});

packages/coding-agent/test/resource-loader.test.ts

@@ -1,4 +1,4 @@
-import { mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs";
+import { mkdirSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { afterEach, beforeEach, describe, expect, it } from "vitest";
@@ -156,6 +156,36 @@ Project skill`,
 			expect(theme?.sourcePath).toBe(projectThemePath);
 		});
 
+		it("should load symlinked user and project extensions once", async () => {
+			const sharedExtDir = join(tempDir, "shared-extensions");
+			mkdirSync(sharedExtDir, { recursive: true });
+			writeFileSync(
+				join(sharedExtDir, "shared.ts"),
+				`export default function(pi) {
+	pi.registerCommand("shared", {
+		description: "shared command",
+		handler: async () => {},
+	});
+}`,
+			);
+
+			mkdirSync(agentDir, { recursive: true });
+			mkdirSync(join(cwd, ".pi"), { recursive: true });
+			symlinkSync(sharedExtDir, join(agentDir, "extensions"), "dir");
+			symlinkSync(sharedExtDir, join(cwd, ".pi", "extensions"), "dir");
+
+			const loader = new DefaultResourceLoader({ cwd, agentDir });
+			await loader.reload();
+
+			const extensionsResult = loader.getExtensions();
+			expect(extensionsResult.extensions).toHaveLength(1);
+			expect(extensionsResult.errors).toEqual([]);
+
+			// mergePaths processes project paths before user paths, so the project
+			// alias is the canonical survivor.
+			expect(extensionsResult.extensions[0].path).toBe(join(cwd, ".pi", "extensions", "shared.ts"));
+		});
+
 		it("should keep both extensions loaded when command names collide", async () => {
 			const userExtDir = join(agentDir, "extensions");
 			const projectExtDir = join(cwd, ".pi", "extensions");