mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
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 <triagent@localhost>
This commit is contained in:
parent
911fbab32f
commit
5f960a0e2b
3 changed files with 57 additions and 12 deletions
|
|
@ -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))
|
||||
|
|
|
|||
|
|
@ -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<void> {
|
||||
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]);
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue