diff --git a/packages/coding-agent/src/core/package-manager.ts b/packages/coding-agent/src/core/package-manager.ts index a8e996ae..0c41b977 100644 --- a/packages/coding-agent/src/core/package-manager.ts +++ b/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): ResolvedResource[] => { + const mapToResolved = ( + entries: Map, + ): 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(); + return resolved.filter((entry) => { + const canonicalPath = canonicalizePath(entry.path); + if (seen.has(canonicalPath)) return false; + seen.add(canonicalPath); + return true; + }); }; - const seenCanonicalSkillPaths = new Set(); - 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), }; } diff --git a/packages/coding-agent/src/core/resource-loader.ts b/packages/coding-agent/src/core/resource-loader.ts index 9f07787e..832bb017 100644 --- a/packages/coding-agent/src/core/resource-loader.ts +++ b/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); } diff --git a/packages/coding-agent/src/core/skills.ts b/packages/coding-agent/src/core/skills.ts index ae0e07e8..c0647473 100644 --- a/packages/coding-agent/src/core/skills.ts +++ b/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)) { diff --git a/packages/coding-agent/src/modes/interactive/components/session-selector.ts b/packages/coding-agent/src/modes/interactive/components/session-selector.ts index 73f80869..229895fa 100644 --- a/packages/coding-agent/src/modes/interactive/components/session-selector.ts +++ b/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 { diff --git a/packages/coding-agent/src/utils/paths.ts b/packages/coding-agent/src/utils/paths.ts index 4884d15c..212e4f5e 100644 --- a/packages/coding-agent/src/utils/paths.ts +++ b/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 diff --git a/packages/coding-agent/test/package-manager.test.ts b/packages/coding-agent/test/package-manager.test.ts index 0df310ae..629e8b84 100644 --- a/packages/coding-agent/test/package-manager.test.ts +++ b/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 }); diff --git a/packages/coding-agent/test/paths.test.ts b/packages/coding-agent/test/paths.test.ts new file mode 100644 index 00000000..5f7c115d --- /dev/null +++ b/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); + }); +}); diff --git a/packages/coding-agent/test/resource-loader.test.ts b/packages/coding-agent/test/resource-loader.test.ts index 8d9999e1..0794af3a 100644 --- a/packages/coding-agent/test/resource-loader.test.ts +++ b/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");