diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 0d66d97d..83253924 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -13,6 +13,7 @@ ### Fixed +- Fixed `pi update` to reconcile git-pinned packages to their configured ref ([#4869](https://github.com/earendil-works/pi/issues/4869)). - Fixed OpenCode Zen/Go requests to send per-session OpenCode routing headers ([#4847](https://github.com/earendil-works/pi/issues/4847)). - Fixed Amazon Bedrock provider loading under strict package managers by inheriting the declared `@smithy/node-http-handler` dependency from `@earendil-works/pi-ai` ([#4842](https://github.com/earendil-works/pi/issues/4842)). - Fixed exported session HTML to escape quote characters in attribute values ([#4832](https://github.com/earendil-works/pi/issues/4832)). diff --git a/packages/coding-agent/docs/packages.md b/packages/coding-agent/docs/packages.md index 1f3c69a6..33527191 100644 --- a/packages/coding-agent/docs/packages.md +++ b/packages/coding-agent/docs/packages.md @@ -28,8 +28,8 @@ pi install ./relative/path/to/package pi remove npm:@foo/bar pi list # show installed packages from settings -pi update # update pi and all non-pinned packages -pi update --extensions # update all non-pinned packages only +pi update # update pi, update packages, and reconcile pinned git refs +pi update --extensions # update packages and reconcile pinned git refs only pi update --self # update pi only pi update --self --force # reinstall pi even if current pi update npm:@foo/bar # update one package @@ -85,9 +85,10 @@ 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 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. +- Refs are pinned tags or commits. `pi update` and `pi update --extensions` do not move them to newer refs, but they do reconcile an existing clone to the configured ref. +- Use `pi install git:host/user/repo@new-ref` to update settings and move an existing package to a new pinned ref. - Cloned to `~/.pi/agent/git//` (global) or `.pi/git//` (project). -- Runs `npm install` after clone, pull, or pinned ref change if `package.json` exists. +- When reconciliation changes the checkout, pi resets and cleans the clone, then runs `npm install` if `package.json` exists. **SSH examples:** ```bash diff --git a/packages/coding-agent/docs/usage.md b/packages/coding-agent/docs/usage.md index 65081090..ae69afaf 100644 --- a/packages/coding-agent/docs/usage.md +++ b/packages/coding-agent/docs/usage.md @@ -129,8 +129,8 @@ pi [options] [@files...] [messages...] pi install [-l] # Install package, -l for project-local pi remove [-l] # Remove package pi uninstall [-l] # Alias for remove -pi update [source|self|pi] # Update pi and packages; skips pinned packages -pi update --extensions # Update packages only +pi update [source|self|pi] # Update pi and packages; reconcile pinned git refs +pi update --extensions # Update packages only; reconcile pinned git refs pi update --self # Update pi only pi update --extension # Update one package pi list # List installed packages diff --git a/packages/coding-agent/src/core/package-manager.ts b/packages/coding-agent/src/core/package-manager.ts index 994adb56..06933bb2 100644 --- a/packages/coding-agent/src/core/package-manager.ts +++ b/packages/coding-agent/src/core/package-manager.ts @@ -1047,14 +1047,15 @@ export class DefaultPackageManager implements PackageManager { for (const entry of sources) { const parsed = this.parseSource(entry.source); - if (parsed.type === "local" || parsed.pinned) { - continue; - } + // Pinned npm versions are fixed. Pinned git refs are configured checkout targets, + // so include them to reconcile an existing clone when the configured ref changes. if (parsed.type === "npm") { - npmCandidates.push({ ...entry, parsed }); - continue; + if (!parsed.pinned) { + npmCandidates.push({ ...entry, parsed }); + } + } else if (parsed.type === "git") { + gitCandidates.push({ ...entry, parsed }); } - gitCandidates.push({ ...entry, parsed }); } const npmCheckTasks = npmCandidates.map((entry) => async () => ({ @@ -1766,6 +1767,11 @@ export class DefaultPackageManager implements PackageManager { return; } + 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); } @@ -1778,7 +1784,8 @@ export class DefaultPackageManager implements PackageManager { cwd: targetDir, timeoutMs: NETWORK_TIMEOUT_MS, }); - const targetHead = await this.runCommandCapture("git", ["rev-parse", ref], { + const commitRef = `${ref}^{commit}`; + const targetHead = await this.runCommandCapture("git", ["rev-parse", commitRef], { cwd: targetDir, timeoutMs: NETWORK_TIMEOUT_MS, }); @@ -1786,7 +1793,7 @@ export class DefaultPackageManager implements PackageManager { return; } - await this.runCommand("git", ["reset", "--hard", ref], { cwd: targetDir }); + await this.runCommand("git", ["reset", "--hard", commitRef], { cwd: targetDir }); // Clean untracked files (extensions should be pristine) await this.runCommand("git", ["clean", "-fdx"], { cwd: targetDir }); diff --git a/packages/coding-agent/test/git-update.test.ts b/packages/coding-agent/test/git-update.test.ts index 0f574ba6..ea8fae4d 100644 --- a/packages/coding-agent/test/git-update.test.ts +++ b/packages/coding-agent/test/git-update.test.ts @@ -282,7 +282,7 @@ describe("DefaultPackageManager git update", () => { }); describe("pinned sources", () => { - it("should not update pinned git sources (with @ref)", async () => { + it("should not move pinned git sources past their configured ref", async () => { // Create remote repo first to get the initial commit mkdirSync(remoteDir, { recursive: true }); initGitRepo(remoteDir); @@ -301,13 +301,77 @@ describe("DefaultPackageManager git update", () => { // Add new commit to remote createCommit(remoteDir, "extension.ts", "// v2", "Second commit"); - // Update should be skipped for pinned sources await packageManager.update(); // Should still be on initial commit expect(getCurrentCommit(installedDir)).toBe(initialCommit); expect(getFileContent(installedDir, "extension.ts")).toBe("// v1"); }); + + it("should checkout the configured pinned git ref during full and targeted updates", async () => { + mkdirSync(remoteDir, { recursive: true }); + initGitRepo(remoteDir); + const v1Commit = createCommit(remoteDir, "extension.ts", "// v1", "Initial commit"); + git(["tag", "v1"], remoteDir); + const v2Commit = createCommit(remoteDir, "extension.ts", "// v2", "Second commit"); + git(["tag", "v2"], remoteDir); + + mkdirSync(join(agentDir, "git", "github.com", "test"), { recursive: true }); + git(["clone", remoteDir, installedDir], tempDir); + git(["checkout", "v1"], installedDir); + expect(getCurrentCommit(installedDir)).toBe(v1Commit); + + const pinnedSource = `${gitSource}@v2`; + settingsManager.setPackages([pinnedSource]); + + await packageManager.update(); + + expect(getCurrentCommit(installedDir)).toBe(v2Commit); + expect(getFileContent(installedDir, "extension.ts")).toBe("// v2"); + + git(["checkout", "v1"], installedDir); + + await packageManager.update(pinnedSource); + + expect(getCurrentCommit(installedDir)).toBe(v2Commit); + expect(getFileContent(installedDir, "extension.ts")).toBe("// v2"); + }); + + it("should not reset an annotated tag checkout that already matches the configured ref", async () => { + mkdirSync(remoteDir, { recursive: true }); + initGitRepo(remoteDir); + const taggedCommit = createCommit(remoteDir, "extension.ts", "// v1", "Initial commit"); + git(["tag", "-a", "v1", "-m", "v1"], remoteDir); + + mkdirSync(join(agentDir, "git", "github.com", "test"), { recursive: true }); + git(["clone", remoteDir, installedDir], tempDir); + git(["checkout", "v1"], installedDir); + expect(getCurrentCommit(installedDir)).toBe(taggedCommit); + + settingsManager.setPackages([`${gitSource}@v1`]); + + const executedCommands: string[] = []; + const managerWithInternals = packageManager as unknown as { + runCommand: (command: string, args: string[], options?: { cwd?: string }) => Promise; + }; + managerWithInternals.runCommand = async (command, args, options) => { + executedCommands.push(`${command} ${args.join(" ")}`); + const result = spawnSync(command, args, { + cwd: options?.cwd, + encoding: "utf-8", + }); + if (result.status !== 0) { + throw new Error(`Command failed: ${command} ${args.join(" ")}\n${result.stderr}`); + } + }; + + await packageManager.update(); + + expect(executedCommands).toContain("git fetch origin v1"); + expect(executedCommands.some((command) => command.startsWith("git reset --hard"))).toBe(false); + expect(executedCommands).not.toContain("git clean -fdx"); + expect(getCurrentCommit(installedDir)).toBe(taggedCommit); + }); }); describe("temporary git sources", () => { diff --git a/packages/coding-agent/test/package-manager.test.ts b/packages/coding-agent/test/package-manager.test.ts index 30068da1..b464ca7d 100644 --- a/packages/coding-agent/test/package-manager.test.ts +++ b/packages/coding-agent/test/package-manager.test.ts @@ -748,7 +748,7 @@ Content`, if (args[0] === "rev-parse" && args[1] === "HEAD") { return "old-head"; } - if (args[0] === "rev-parse" && args[1] === "FETCH_HEAD") { + if (args[0] === "rev-parse" && args[1] === "FETCH_HEAD^{commit}") { return "new-head"; } throw new Error(`Unexpected runCommandCapture args: ${args.join(" ")}`); @@ -758,7 +758,9 @@ Content`, 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", ["reset", "--hard", "FETCH_HEAD^{commit}"], { + cwd: targetDir, + }); expect(runCommandSpy).toHaveBeenCalledWith("git", ["clean", "-fdx"], { cwd: targetDir }); expect(runCommandSpy).toHaveBeenCalledWith("npm", ["install", "--omit=dev"], { cwd: targetDir }); }); @@ -779,7 +781,7 @@ Content`, if (args[0] === "rev-parse" && args[1] === "HEAD") { return "old-head"; } - if (args[0] === "rev-parse" && args[1] === "origin/HEAD") { + if (args[0] === "rev-parse" && args[1] === "origin/HEAD^{commit}") { return "new-head"; } throw new Error(`Unexpected runCommandCapture args: ${args.join(" ")}`); @@ -789,7 +791,9 @@ Content`, 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", ["reset", "--hard", "origin/HEAD^{commit}"], { + cwd: targetDir, + }); expect(runCommandSpy).toHaveBeenCalledWith("git", ["clean", "-fdx"], { cwd: targetDir }); }); @@ -832,7 +836,7 @@ Content`, if (args[0] === "rev-parse" && args[1] === "--abbrev-ref" && args[2] === "@{upstream}") { return "origin/main"; } - if (args[0] === "rev-parse" && args[1] === "@{upstream}") { + if (args[0] === "rev-parse" && (args[1] === "@{upstream}" || args[1] === "@{upstream}^{commit}")) { return "remote-head"; } if (args[0] === "rev-parse" && args[1] === "HEAD") { @@ -868,7 +872,7 @@ Content`, if (args[0] === "rev-parse" && args[1] === "--abbrev-ref" && args[2] === "@{upstream}") { return "origin/main"; } - if (args[0] === "rev-parse" && args[1] === "@{upstream}") { + if (args[0] === "rev-parse" && (args[1] === "@{upstream}" || args[1] === "@{upstream}^{commit}")) { return "remote-head"; } if (args[0] === "rev-parse" && args[1] === "HEAD") { @@ -2057,7 +2061,7 @@ export default function(api) { api.registerTool({ name: "test", description: "te expect(packageManager.getInstalledPath("npm:legacy-pkg", "user")).toBe(managedPath); }); - it("should batch npm updates per scope and run git updates in parallel while skipping pinned and current packages", async () => { + it("should batch npm updates per scope and run git updates in parallel while skipping pinned npm and current packages", async () => { const userOldPath = join(agentDir, "npm", "node_modules", "user-old"); const userCurrentPath = join(agentDir, "npm", "node_modules", "user-current"); const userUnknownPath = join(agentDir, "npm", "node_modules", "user-unknown"); @@ -2159,7 +2163,7 @@ export default function(api) { api.registerTool({ name: "test", description: "te ["install", "project-old@latest", "project-missing@latest", "--prefix", join(tempDir, ".pi", "npm")], undefined, ); - expect(updateGitSpy).toHaveBeenCalledTimes(3); + expect(updateGitSpy).toHaveBeenCalledTimes(4); expect(maxConcurrentNpmUpdates).toBeGreaterThan(1); expect(maxConcurrentGitUpdates).toBeGreaterThan(1); });