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
This commit is contained in:
@@ -2,6 +2,10 @@
|
|||||||
|
|
||||||
## [Unreleased]
|
## [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.68] - 2026-04-17
|
||||||
|
|
||||||
## [0.67.67] - 2026-04-17
|
## [0.67.67] - 2026-04-17
|
||||||
|
|||||||
@@ -1,6 +1,15 @@
|
|||||||
import { type ChildProcess, type ChildProcessByStdio, spawn, spawnSync } from "node:child_process";
|
import { type ChildProcess, type ChildProcessByStdio, spawn, spawnSync } from "node:child_process";
|
||||||
import { createHash } from "node:crypto";
|
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 { homedir, tmpdir } from "node:os";
|
||||||
import { basename, dirname, join, relative, resolve, sep } from "node:path";
|
import { basename, dirname, join, relative, resolve, sep } from "node:path";
|
||||||
import type { Readable } from "node:stream";
|
import type { Readable } from "node:stream";
|
||||||
@@ -2190,9 +2199,28 @@ export class DefaultPackageManager implements PackageManager {
|
|||||||
return resolved;
|
return resolved;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
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 {
|
return {
|
||||||
extensions: toResolved(accumulator.extensions),
|
extensions: toResolved(accumulator.extensions),
|
||||||
skills: toResolved(accumulator.skills),
|
skills: resolvedSkills,
|
||||||
prompts: toResolved(accumulator.prompts),
|
prompts: toResolved(accumulator.prompts),
|
||||||
themes: toResolved(accumulator.themes),
|
themes: toResolved(accumulator.themes),
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
import { EventEmitter } from "node:events";
|
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 { tmpdir } from "node:os";
|
||||||
import { join, relative } from "node:path";
|
import { join, relative } from "node:path";
|
||||||
import { PassThrough } from "node:stream";
|
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", () => {
|
describe("ignore files", () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user