fix(coding-agent): reconcile pinned git update refs

closes #4869

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)).

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                   # update pi, update packages, and reconcile pinned git refs
-pi update --extensions      # update all non-pinned packages only
+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/<host>/<path>` (global) or `.pi/git/<host>/<path>` (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

packages/coding-agent/docs/usage.md

@@ -129,8 +129,8 @@ pi [options] [@files...] [messages...]
 pi install <source> [-l]     # Install package, -l for project-local
 pi remove <source> [-l]      # Remove package
 pi uninstall <source> [-l]   # Alias for remove
-pi update [source|self|pi]   # Update pi and packages; skips pinned packages
+pi update [source|self|pi]   # Update pi and packages; reconcile pinned git refs
-pi update --extensions       # Update packages only
+pi update --extensions       # Update packages only; reconcile pinned git refs
 pi update --self             # Update pi only
 pi update --extension <src>  # Update one package
 pi list                      # List installed packages

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) {
+			// Pinned npm versions are fixed. Pinned git refs are configured checkout targets,
-				continue;
+			// so include them to reconcile an existing clone when the configured ref changes.
-			}
 			if (parsed.type === "npm") {
-				npmCandidates.push({ ...entry, parsed });
+				if (!parsed.pinned) {
-				continue;
+					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 });

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<void>;
+			};
+			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", () => {

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);
 		});