From 5f960a0e2b9decdbe9183f28be947798756abecd Mon Sep 17 00:00:00 2001 From: "robopipi[bot]" <336283572+robopipi[bot]@users.noreply.github.com> Date: Sun, 25 Oct 2026 08:47:34 +0100 Subject: [PATCH] fix(coding-agent): pass real path as npm prefix when agent dir is a symlink (#11396) When the agent directory is reached through a symlink (for example `~/.pi` linked into a dotfiles repo), pi passed the symlinked path as `--prefix` to `npm install` and `npm uninstall`. npm then writes lock entries such as `../../../dotfiles/pi/agent/npm/node_modules/foo` instead of `node_modules/foo`. Each later install adds one more nesting level, so `package-lock.json` keeps growing. This change resolves the install root with `realpathSync` before passing it to the package manager: - in `getNpmInstallArgs()`, which covers install and update for npm, pnpm and bun; - in `uninstallNpm()`. The paths pi loads extensions from are unchanged. **Testing** - Added a regression test in `test/package-manager.test.ts`. It uses a symlinked agent dir and checks that install and uninstall get the real path as `--prefix`. The test fails without the fix and passes with it. - Wrapped the existing `--prefix`/`--cwd` expectations in `realpathSync(...)`. They now match on systems where the temp dir is itself a symlink, such as macOS. - Ran `npm run check` (clean) and `test/package-manager.test.ts` plus `test/package-manager-ssh.test.ts` (all pass). - Manual reproduction with `~/.pi` as a symlink, running `pi install npm:ms` several times: - before: lock keys grew to `../../../dotfiles/pi/agent/npm/node_modules/ms`, `../../../dotfiles/dotfiles/pi/...`, and so on; - after: `[ '', 'node_modules/ms' ]`, and `pi remove npm:ms` leaves `[ '' ]`. Fixes #11358 Requested by @badlogic Co-authored-by: triagent --- packages/coding-agent/CHANGELOG.md | 1 + .../coding-agent/src/core/package-manager.ts | 11 +++- .../coding-agent/test/package-manager.test.ts | 57 ++++++++++++++++--- 3 files changed, 57 insertions(+), 12 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 246c1225e..080bb8913 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -13,6 +13,7 @@ ### Fixed +- Fixed `pi install` and `pi remove` adding ever deeper nested entries to the npm `package-lock.json` when the agent directory is reached through a symlink, such as `~/.pi` linked into a dotfiles repo. pi now passes the resolved real path to npm ([#11358](https://github.com/earendil-works/pi/issues/11358)) - MCP sign-in now rejects authorization endpoints and other OAuth metadata URLs that are not `http:` or `https:`. Before, a malicious MCP server could supply a `file:` or custom protocol URL, which pi passed to the platform URL handler. - Automatic `fd` and `rg` downloads no longer fetch x86-64 binaries on ARM32 and other architectures without a prebuilt release. pi now warns and recommends installing the tool with the system package manager ([#11209](https://github.com/earendil-works/pi/issues/11209)) - Fixed `/model` search ranking letters scattered over an earlier part of an entry above a real match, for example `opus` listing `gpt-oss-120b` and `claude-sonnet-4.5` before `claude-opus-4.1` ([#11220](https://github.com/earendil-works/pi/issues/11220), [#11296](https://github.com/earendil-works/pi/pull/11296)) diff --git a/packages/coding-agent/src/core/package-manager.ts b/packages/coding-agent/src/core/package-manager.ts index b5015b5ba..2535b1f93 100644 --- a/packages/coding-agent/src/core/package-manager.ts +++ b/packages/coding-agent/src/core/package-manager.ts @@ -7,6 +7,7 @@ import { mkdirSync, readdirSync, readFileSync, + realpathSync, rmSync, statSync, writeFileSync, @@ -1868,7 +1869,9 @@ export class DefaultPackageManager implements PackageManager { return this.runCommandSync(npmCommand.command, [...npmCommand.args, ...args]); } - private getNpmInstallArgs(specs: string[], installRoot: string): string[] { + private getNpmInstallArgs(specs: string[], root: string): string[] { + // npm writes broken nested package-lock.json paths when --prefix goes through a symlink. + const installRoot = existsSync(root) ? realpathSync(root) : root; const packageManagerName = this.getPackageManagerName(); // Extension packages run inside pi and resolve pi APIs through loader aliases/virtual modules. // Disable peer dependency resolution for managed installs (npm's --legacy-peer-deps, and @@ -1898,10 +1901,12 @@ export class DefaultPackageManager implements PackageManager { } private async uninstallNpm(source: NpmSource, scope: SourceScope): Promise { - const installRoot = this.getNpmInstallRoot(scope, false); - if (!existsSync(installRoot)) { + const root = this.getNpmInstallRoot(scope, false); + if (!existsSync(root)) { return; } + // npm writes broken nested package-lock.json paths when --prefix goes through a symlink. + const installRoot = realpathSync(root); const packageManagerName = this.getPackageManagerName(); if (packageManagerName === "bun") { await this.runNpmCommand(["uninstall", source.name, "--cwd", installRoot]); diff --git a/packages/coding-agent/test/package-manager.test.ts b/packages/coding-agent/test/package-manager.test.ts index 4aaa60ab3..f8446b657 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 { existsSync, mkdirSync, rmSync, statSync, symlinkSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, realpathSync, rmSync, statSync, symlinkSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join, relative } from "node:path"; import { PassThrough } from "node:stream"; @@ -754,7 +754,7 @@ Content`, "install", "@scope/pkg", "--prefix", - join(agentDir, "npm"), + realpathSync(join(agentDir, "npm")), "--legacy-peer-deps", ], undefined, @@ -769,7 +769,36 @@ Content`, expect(runCommandSpy).toHaveBeenCalledWith( "npm", - ["uninstall", "@scope/pkg", "--prefix", join(agentDir, "npm"), "--legacy-peer-deps"], + ["uninstall", "@scope/pkg", "--prefix", realpathSync(join(agentDir, "npm")), "--legacy-peer-deps"], + undefined, + ); + }); + + // Regression test for #11358: npm writes nested package-lock.json entries for a symlinked --prefix + it("should pass the real path as --prefix when the agent dir is a symlink", async () => { + const realAgentDir = join(tempDir, "dotfiles", "agent"); + mkdirSync(realAgentDir, { recursive: true }); + const linkedAgentDir = join(tempDir, "linked-agent"); + symlinkSync(realAgentDir, linkedAgentDir, "dir"); + packageManager = new DefaultPackageManager({ + cwd: tempDir, + agentDir: linkedAgentDir, + settingsManager, + }); + const runCommandSpy = vi.spyOn(packageManager as any, "runCommand").mockResolvedValue(undefined); + const realNpmRoot = join(realpathSync(realAgentDir), "npm"); + + await packageManager.install("npm:@scope/pkg"); + await packageManager.remove("npm:@scope/pkg"); + + expect(runCommandSpy).toHaveBeenCalledWith( + "npm", + ["install", "@scope/pkg", "--prefix", realNpmRoot, "--legacy-peer-deps"], + undefined, + ); + expect(runCommandSpy).toHaveBeenCalledWith( + "npm", + ["uninstall", "@scope/pkg", "--prefix", realNpmRoot, "--legacy-peer-deps"], undefined, ); }); @@ -790,7 +819,17 @@ Content`, expect(runCommandSpy).toHaveBeenCalledWith( "mise", - ["exec", "bun@1", "--", "bun", "install", "@scope/pkg", "--cwd", join(agentDir, "npm"), "--omit=peer"], + [ + "exec", + "bun@1", + "--", + "bun", + "install", + "@scope/pkg", + "--cwd", + realpathSync(join(agentDir, "npm")), + "--omit=peer", + ], undefined, ); }); @@ -1206,7 +1245,7 @@ Content`, "install", "pnpm-pkg", "--prefix", - join(agentDir, "npm"), + realpathSync(join(agentDir, "npm")), "--config.auto-install-peers=false", "--config.strict-peer-dependencies=false", "--config.strict-dep-builds=false", @@ -2467,7 +2506,7 @@ export default function(api) { api.registerTool({ name: "test", description: "te ); expect(runCommandSpy).toHaveBeenCalledWith( "npm", - ["install", "example@^1.0.0", "--prefix", join(tempDir, ".pi", "npm"), "--legacy-peer-deps"], + ["install", "example@^1.0.0", "--prefix", realpathSync(join(tempDir, ".pi", "npm")), "--legacy-peer-deps"], undefined, ); }); @@ -2531,7 +2570,7 @@ export default function(api) { api.registerTool({ name: "test", description: "te "install", "legacy-pkg@latest", "--prefix", - join(agentDir, "npm"), + realpathSync(join(agentDir, "npm")), "--legacy-peer-deps", ]); mkdirSync(managedPath, { recursive: true }); @@ -2648,7 +2687,7 @@ export default function(api) { api.registerTool({ name: "test", description: "te "user-old@latest", "user-unknown@latest", "--prefix", - join(agentDir, "npm"), + realpathSync(join(agentDir, "npm")), "--legacy-peer-deps", ], undefined, @@ -2661,7 +2700,7 @@ export default function(api) { api.registerTool({ name: "test", description: "te "project-old@latest", "project-missing@latest", "--prefix", - join(tempDir, ".pi", "npm"), + realpathSync(join(tempDir, ".pi", "npm")), "--legacy-peer-deps", ], undefined,