diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 32fcca39..ce69a3aa 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -12,6 +12,7 @@ ### Fixed +- Fixed git package installs to reconcile existing checkouts to the requested ref and update package settings without losing filters ([#4870](https://github.com/earendil-works/pi/issues/4870)). - Published a 0.74.2 rescue release that tells Node 20 users to upgrade Node before updating to newer Pi versions ([#4876](https://github.com/earendil-works/pi/issues/4876)). - Fixed final bash tool cards to avoid rendering duplicate full-output truncation paths ([#4819](https://github.com/earendil-works/pi/issues/4819)). - Fixed bash tool truncation line counts to ignore the trailing newline as an extra output line ([#4818](https://github.com/earendil-works/pi/issues/4818)). diff --git a/packages/coding-agent/README.md b/packages/coding-agent/README.md index 79f284a0..00fca81e 100644 --- a/packages/coding-agent/README.md +++ b/packages/coding-agent/README.md @@ -408,7 +408,7 @@ pi update npm:@foo/pi-tools # update one package pi config # enable/disable extensions, skills, prompts, themes ``` -Packages install to `~/.pi/agent/git/` (git) or `~/.pi/agent/npm/` (npm). Use `-l` for project-local installs (`.pi/git/`, `.pi/npm/`). Git packages install dependencies with `npm install --omit=dev` by default, so runtime deps must be listed under `dependencies`; when `npmCommand` is configured, git packages use plain `install` for compatibility with wrappers. If you use a Node version manager and want package installs to reuse a stable npm context, set `npmCommand` in `settings.json`, for example `["mise", "exec", "node@20", "--", "npm"]`. +Packages install to `~/.pi/agent/git/` (git) or `~/.pi/agent/npm/` (npm). Use `-l` for project-local installs (`.pi/git/`, `.pi/npm/`). Git `@ref` values are pinned tags or commits; pinned packages are skipped by `pi update`, so use `pi install git:host/user/repo@new-ref` to move an existing package to a new ref. Git packages install dependencies with `npm install --omit=dev` by default, so runtime deps must be listed under `dependencies`; when `npmCommand` is configured, git packages use plain `install` for compatibility with wrappers. If you use a Node version manager and want package installs to reuse a stable npm context, set `npmCommand` in `settings.json`, for example `["mise", "exec", "node@20", "--", "npm"]`. Create a package by adding a `pi` key to `package.json`: diff --git a/packages/coding-agent/docs/packages.md b/packages/coding-agent/docs/packages.md index db5ab6cc..1f3c69a6 100644 --- a/packages/coding-agent/docs/packages.md +++ b/packages/coding-agent/docs/packages.md @@ -85,9 +85,9 @@ ssh://git@github.com/user/repo@v1 - HTTPS and SSH URLs are both supported. - SSH URLs use your configured SSH keys automatically (respects `~/.ssh/config`). - For non-interactive runs (for example CI), you can set `GIT_TERMINAL_PROMPT=0` to disable credential prompts and set `GIT_SSH_COMMAND` (for example `ssh -o BatchMode=yes -o ConnectTimeout=5`) to fail fast. -- Refs pin the package and skip package updates (`pi update`, `pi update --extensions`). +- Refs are pinned tags or commits and skip package updates (`pi update`, `pi update --extensions`). Use `pi install git:host/user/repo@new-ref` to move an existing package to a new pinned ref. - Cloned to `~/.pi/agent/git//` (global) or `.pi/git//` (project). -- Runs `npm install` after clone or pull if `package.json` exists. +- Runs `npm install` after clone, pull, or pinned ref change if `package.json` exists. **SSH examples:** ```bash diff --git a/packages/coding-agent/src/core/package-manager.ts b/packages/coding-agent/src/core/package-manager.ts index 90c41942..b8c333bc 100644 --- a/packages/coding-agent/src/core/package-manager.ts +++ b/packages/coding-agent/src/core/package-manager.ts @@ -778,9 +778,21 @@ export class DefaultPackageManager implements PackageManager { scope === "project" ? this.settingsManager.getProjectSettings() : this.settingsManager.getGlobalSettings(); const currentPackages = currentSettings.packages ?? []; const normalizedSource = this.normalizePackageSourceForSettings(source, scope); - const exists = currentPackages.some((existing) => this.packageSourcesMatch(existing, source, scope)); - if (exists) { - return false; + const matchIndex = currentPackages.findIndex((existing) => this.packageSourcesMatch(existing, source, scope)); + if (matchIndex !== -1) { + const existing = currentPackages[matchIndex]; + if (this.getPackageSourceString(existing) === normalizedSource) { + return false; + } + const nextPackages = [...currentPackages]; + nextPackages[matchIndex] = + typeof existing === "string" ? normalizedSource : { ...existing, source: normalizedSource }; + if (scope === "project") { + this.settingsManager.setProjectPackages(nextPackages); + } else { + this.settingsManager.setPackages(nextPackages); + } + return true; } const nextPackages = [...currentPackages, normalizedSource]; if (scope === "project") { @@ -1723,6 +1735,12 @@ export class DefaultPackageManager implements PackageManager { private async installGit(source: GitSource, scope: SourceScope): Promise { const targetDir = this.getGitInstallPath(source, scope); if (existsSync(targetDir)) { + if (source.ref) { + await this.ensureGitRef(targetDir, ["fetch", "origin", source.ref], "FETCH_HEAD"); + return; + } + const target = await this.getLocalGitUpdateTarget(targetDir); + await this.ensureGitRef(targetDir, target.fetchArgs, target.ref); return; } const gitRoot = this.getGitInstallRoot(scope); @@ -1749,23 +1767,26 @@ export class DefaultPackageManager implements PackageManager { } const target = await this.getLocalGitUpdateTarget(targetDir); + await this.ensureGitRef(targetDir, target.fetchArgs, target.ref); + } + private async ensureGitRef(targetDir: string, fetchArgs: string[], ref: string): Promise { // Fetch only the ref we will reset to, avoiding unrelated branch/tag noise. - await this.runCommand("git", target.fetchArgs, { cwd: targetDir }); + await this.runCommand("git", fetchArgs, { cwd: targetDir }); const localHead = await this.runCommandCapture("git", ["rev-parse", "HEAD"], { cwd: targetDir, timeoutMs: NETWORK_TIMEOUT_MS, }); - const refreshedTargetHead = await this.runCommandCapture("git", ["rev-parse", target.ref], { + const targetHead = await this.runCommandCapture("git", ["rev-parse", ref], { cwd: targetDir, timeoutMs: NETWORK_TIMEOUT_MS, }); - if (localHead.trim() === refreshedTargetHead.trim()) { + if (localHead.trim() === targetHead.trim()) { return; } - await this.runCommand("git", ["reset", "--hard", target.ref], { cwd: targetDir }); + await this.runCommand("git", ["reset", "--hard", ref], { cwd: targetDir }); // Clean untracked files (extensions should be pristine) await this.runCommand("git", ["clean", "-fdx"], { cwd: targetDir }); diff --git a/packages/coding-agent/test/package-manager.test.ts b/packages/coding-agent/test/package-manager.test.ts index 9466d733..9f3476a8 100644 --- a/packages/coding-agent/test/package-manager.test.ts +++ b/packages/coding-agent/test/package-manager.test.ts @@ -25,6 +25,16 @@ class MockSpawnedProcess extends EventEmitter { } } +interface PackageManagerInternals { + runCommand(command: string, args: string[], options?: { cwd?: string }): Promise; + runCommandCapture( + command: string, + args: string[], + options?: { cwd?: string; timeoutMs?: number; env?: Record }, + ): Promise; + getLocalGitUpdateTarget(installedPath: string): Promise<{ ref: string; head: string; fetchArgs: string[] }>; +} + // Helper to check if a resource is enabled const isEnabled = (r: ResolvedResource, pathMatch: string, matchFn: "endsWith" | "includes" = "endsWith") => { const normalizedPath = normalizeForMatch(r.path); @@ -693,6 +703,62 @@ Content`, expect(runCommandSpy).toHaveBeenCalledWith("npm", ["install", "--omit=dev"], { cwd: targetDir }); }); + it("should reconcile an existing git checkout to a pinned ref during install", async () => { + const source = "git:github.com/user/repo@v2"; + const targetDir = join(agentDir, "git", "github.com", "user", "repo"); + mkdirSync(targetDir, { recursive: true }); + writeFileSync(join(targetDir, "package.json"), JSON.stringify({ name: "repo", version: "1.0.0" })); + + const managerWithInternals = packageManager as unknown as PackageManagerInternals; + vi.spyOn(managerWithInternals, "runCommandCapture").mockImplementation(async (_command, args) => { + if (args[0] === "rev-parse" && args[1] === "HEAD") { + return "old-head"; + } + if (args[0] === "rev-parse" && args[1] === "FETCH_HEAD") { + return "new-head"; + } + throw new Error(`Unexpected runCommandCapture args: ${args.join(" ")}`); + }); + const runCommandSpy = vi.spyOn(managerWithInternals, "runCommand").mockResolvedValue(undefined); + + await packageManager.install(source); + + expect(runCommandSpy).toHaveBeenCalledWith("git", ["fetch", "origin", "v2"], { cwd: targetDir }); + expect(runCommandSpy).toHaveBeenCalledWith("git", ["reset", "--hard", "FETCH_HEAD"], { cwd: targetDir }); + expect(runCommandSpy).toHaveBeenCalledWith("git", ["clean", "-fdx"], { cwd: targetDir }); + expect(runCommandSpy).toHaveBeenCalledWith("npm", ["install", "--omit=dev"], { cwd: targetDir }); + }); + + it("should reconcile an existing git checkout to its update target when installing without a ref", async () => { + const source = "git:github.com/user/repo"; + const targetDir = join(agentDir, "git", "github.com", "user", "repo"); + const fetchArgs = ["fetch", "--prune", "--no-tags", "origin", "+refs/heads/main:refs/remotes/origin/main"]; + mkdirSync(targetDir, { recursive: true }); + + const managerWithInternals = packageManager as unknown as PackageManagerInternals; + vi.spyOn(managerWithInternals, "getLocalGitUpdateTarget").mockResolvedValue({ + ref: "origin/HEAD", + head: "new-head", + fetchArgs, + }); + vi.spyOn(managerWithInternals, "runCommandCapture").mockImplementation(async (_command, args) => { + if (args[0] === "rev-parse" && args[1] === "HEAD") { + return "old-head"; + } + if (args[0] === "rev-parse" && args[1] === "origin/HEAD") { + return "new-head"; + } + throw new Error(`Unexpected runCommandCapture args: ${args.join(" ")}`); + }); + const runCommandSpy = vi.spyOn(managerWithInternals, "runCommand").mockResolvedValue(undefined); + + await packageManager.install(source); + + expect(runCommandSpy).toHaveBeenCalledWith("git", fetchArgs, { cwd: targetDir }); + expect(runCommandSpy).toHaveBeenCalledWith("git", ["reset", "--hard", "origin/HEAD"], { cwd: targetDir }); + expect(runCommandSpy).toHaveBeenCalledWith("git", ["clean", "-fdx"], { cwd: targetDir }); + }); + it("should use plain install for git package dependencies when npmCommand is configured", async () => { settingsManager = SettingsManager.inMemory({ npmCommand: ["pnpm"], @@ -1061,6 +1127,47 @@ Content`, expect(removed).toBe(true); expect(settingsManager.getGlobalSettings().packages ?? []).toHaveLength(0); }); + + it("should return false when adding the same git source with the same ref", () => { + const first = packageManager.addSourceToSettings("git:github.com/user/repo@v1"); + expect(first).toBe(true); + + const second = packageManager.addSourceToSettings("git:github.com/user/repo@v1"); + expect(second).toBe(false); + expect(settingsManager.getGlobalSettings().packages).toEqual(["git:github.com/user/repo@v1"]); + }); + + it("should update the ref when adding the same git source with a different ref", () => { + packageManager.addSourceToSettings("git:github.com/user/repo@v1"); + + const updated = packageManager.addSourceToSettings("git:github.com/user/repo@v2"); + expect(updated).toBe(true); + expect(settingsManager.getGlobalSettings().packages).toEqual(["git:github.com/user/repo@v2"]); + }); + + it("should preserve package filters when replacing a package source ref", () => { + settingsManager.setPackages([ + { + source: "git:github.com/user/repo@v1", + extensions: ["extensions/main.ts"], + skills: [], + prompts: ["prompts/review.md"], + themes: ["themes/dark.json"], + }, + ]); + + const updated = packageManager.addSourceToSettings("git:github.com/user/repo@v2"); + expect(updated).toBe(true); + expect(settingsManager.getGlobalSettings().packages).toEqual([ + { + source: "git:github.com/user/repo@v2", + extensions: ["extensions/main.ts"], + skills: [], + prompts: ["prompts/review.md"], + themes: ["themes/dark.json"], + }, + ]); + }); }); describe("HTTPS git URL parsing (old behavior)", () => {