From 2b45243817564cbd5e744ce24801464e19b3150f Mon Sep 17 00:00:00 2001 From: Ramiz Wachtler Date: Mon, 20 Apr 2026 13:23:00 +0200 Subject: [PATCH] fix(coding-agent): avoid duplicate symlinked skills in pi config (#3417) - dedupe resolved skill entries by canonical path (realpathSync) with raw-path fallback matching loadSkills() - preserve precedence by applying skill dedupe after resource precedence sorting (first winner retained) - add tests to verify expected behavior - closes #3405 --- packages/coding-agent/CHANGELOG.md | 4 +++ .../coding-agent/src/core/package-manager.ts | 32 +++++++++++++++++-- .../coding-agent/test/package-manager.test.ts | 31 +++++++++++++++++- 3 files changed, 64 insertions(+), 3 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index fc587a95..9a6f12bf 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed skill resolution to dedupe symlinked aliases by canonical path, so `pi config` no longer shows duplicate skill entries when `~/.pi/agent/skills` points to `~/.agents/skills` ([#3405](https://github.com/badlogic/pi-mono/issues/3405)) + ## [0.67.68] - 2026-04-17 ## [0.67.67] - 2026-04-17 diff --git a/packages/coding-agent/src/core/package-manager.ts b/packages/coding-agent/src/core/package-manager.ts index e4f9311b..51a78461 100644 --- a/packages/coding-agent/src/core/package-manager.ts +++ b/packages/coding-agent/src/core/package-manager.ts @@ -1,6 +1,15 @@ import { type ChildProcess, type ChildProcessByStdio, spawn, spawnSync } from "node:child_process"; import { createHash } from "node:crypto"; -import { existsSync, mkdirSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs"; +import { + existsSync, + mkdirSync, + readdirSync, + readFileSync, + realpathSync, + 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"; @@ -2190,9 +2199,28 @@ export class DefaultPackageManager implements PackageManager { return resolved; }; + 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: toResolved(accumulator.skills), + skills: resolvedSkills, prompts: toResolved(accumulator.prompts), themes: toResolved(accumulator.themes), }; diff --git a/packages/coding-agent/test/package-manager.test.ts b/packages/coding-agent/test/package-manager.test.ts index 82a7a6fd..907d8f4f 100644 --- a/packages/coding-agent/test/package-manager.test.ts +++ b/packages/coding-agent/test/package-manager.test.ts @@ -1,5 +1,5 @@ import { EventEmitter } from "node:events"; -import { mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { mkdirSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join, relative } from "node:path"; import { PassThrough } from "node:stream"; @@ -315,6 +315,35 @@ Content`, } } }); + + it("should dedupe user skill entries when ~/.pi/agent/skills is a symlink to ~/.agents/skills", async () => { + const previousHome = process.env.HOME; + process.env.HOME = tempDir; + + try { + const agentSkillsDir = join(agentDir, "skills"); + const agentsSkillsDir = join(tempDir, ".agents", "skills"); + mkdirSync(agentsSkillsDir, { recursive: true }); + // Use junction on Windows to avoid EPERM when symlink privileges are unavailable. + const directoryLinkType = process.platform === "win32" ? "junction" : "dir"; + symlinkSync(agentsSkillsDir, agentSkillsDir, directoryLinkType); + + const skillPath = join(agentsSkillsDir, "foo", "SKILL.md"); + mkdirSync(join(agentsSkillsDir, "foo"), { recursive: true }); + writeFileSync(skillPath, "---\nname: foo\ndescription: foo\n---\n"); + + const result = await packageManager.resolve(); + const fooSkills = result.skills.filter((r) => pathEndsWith(r.path, "foo/SKILL.md")); + + expect(fooSkills).toHaveLength(1); + } finally { + if (previousHome === undefined) { + delete process.env.HOME; + } else { + process.env.HOME = previousHome; + } + } + }); }); describe("ignore files", () => {