From e3da248ea9100a3b5d53f9c701b6a47ef7f58d6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Hou=C5=A1ka?= Date: Sat, 31 Oct 2026 18:19:44 +0100 Subject: [PATCH] fix(coding-agent): keep updating git packages when git clean fails (#11648) Closes #11597 ## Problem On Windows, `pi update` stopped with `git clean -fdx failed with code 1` for git extensions such as pi-lens. 1. Another pi session has the extension loaded, so a native module (`@ast-grep/napi` `.node`) or `ast-grep.exe` is open in that process. 2. npm renames the in-use package dir to `node_modules/@ast-grep/.napi-win32-x64-msvc-`. Windows allows the rename, but npm cannot delete the dir afterwards. 3. The next `pi update` runs `git reset --hard`, then `git clean -fdx`. git prints `warning: failed to remove ...: Invalid argument` and exits 1. 4. pi threw, leaving the checkout at the new commit with the old dependencies. The suggested `-ffdx` does not help. Reproduced locally (git 2.52, Windows): with a running `.exe`, or a `.node` loaded by another node process and its dir renamed, both `-fdx` and `-ffdx` fail with `Invalid argument`. Once the process exits, `-fdx` succeeds. ## Fix In `cleanAndInstallGitDependencies`, a failed `git clean` no longer aborts the update. pi: - prints a warning that files may be in use by another pi session - still runs `npm install`, so dependencies match the new commit - keeps the `.pi-update-incomplete` marker, so the next update retries the clean ## Tests - `test/git-update.test.ts`: new regression test. A failing clean still updates and installs dependencies, warns, and keeps the marker. The next update cleans and removes the marker. - `test/package-manager.test.ts`: updated the test that expected the throw. `npm run check` reports `install-lock is out of date`, and `package-manager.test.ts > should expand ~ in exclusion and exact patterns` fails on Windows. Both also happen on `main` without this change. --- packages/coding-agent/CHANGELOG.md | 1 + .../coding-agent/src/core/package-manager.ts | 21 ++++++-- packages/coding-agent/test/git-update.test.ts | 53 +++++++++++++++++++ .../coding-agent/test/package-manager.test.ts | 5 +- 4 files changed, 73 insertions(+), 7 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 8a916e873..359250c80 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -48,6 +48,7 @@ - Fixed the external editor failing silently, for example when the editor command is wrong or a Windows editor cannot start from a deep WSL directory. pi now shows a status message such as `External editor exited with code 1; prompt unchanged` ([#11592](https://github.com/earendil-works/pi/issues/11592)) - Fixed `tool_search` and codemode `searchTools()` ranking a tool below tools that only mention its name in their descriptions, for example `peekaboo app` ranking `mcp__peekaboo__app` 14th. A tool matching more of the query's words now ranks above one matching fewer, a query word in a tool's name scores more than any number of mentions in its description, a tool whose whole name is in the query gets a large bonus, and long server instructions no longer lower the scores of that server's tools ([#11331](https://github.com/earendil-works/pi/issues/11331)) - Fixed `tool_search` and codemode `searchTools()` finding nothing for Chinese, Japanese, Thai, and other non-ASCII queries, because the tokenizer kept only ASCII letters and digits. Words in any script now match, text without spaces between words such as `搜索记录` is split into words, and canonically equivalent text such as `café` and `cafe\u0301` matches ([#10816](https://github.com/earendil-works/pi/issues/10816), [#11037](https://github.com/earendil-works/pi/pull/11037) by [@haoqixu](https://github.com/haoqixu)) +- Fixed `pi update` stopping with `git clean -fdx failed with code 1` on Windows when another pi session had a git extension's native module or executable open. Windows cannot delete such files, so the update left the new code with the old dependencies. pi now warns, installs the dependencies anyway, and retries the clean on the next update ([#11597](https://github.com/earendil-works/pi/issues/11597)) ### Removed diff --git a/packages/coding-agent/src/core/package-manager.ts b/packages/coding-agent/src/core/package-manager.ts index 1a0c96932..5f4229b24 100644 --- a/packages/coding-agent/src/core/package-manager.ts +++ b/packages/coding-agent/src/core/package-manager.ts @@ -2090,20 +2090,31 @@ export class DefaultPackageManager implements PackageManager { } private async cleanAndInstallGitDependencies(targetDir: string, markerPath: string): Promise { - // Clean untracked files (extensions should be pristine). If this fails after - // deleting dependencies, repair them so the existing extension still loads. + // Clean untracked files (extensions should be pristine). On Windows, files loaded by + // another process (e.g. a native module of this extension in another pi session) cannot + // be deleted, so git clean fails. Still install dependencies for the new commit, but keep + // the marker so the next update retries the clean. + let cleaned = true; try { await this.runCommand("git", ["clean", "-fdx"], { cwd: targetDir }); } catch (error) { - await this.repairMissingGitDependencies(targetDir).catch(() => {}); - throw error; + cleaned = false; + const reason = error instanceof Error ? error.message : String(error); + this.emitProgress({ + type: "progress", + action: "update", + source: targetDir, + message: `Could not remove all untracked files in ${targetDir} (${reason}). Files may be in use by another pi session; close it and run the update again.`, + }); } const packageJsonPath = join(targetDir, "package.json"); if (existsSync(packageJsonPath)) { await this.runNpmCommand(this.getGitDependencyInstallArgs(), { cwd: targetDir }); } - rmSync(markerPath, { force: true }); + if (cleaned) { + rmSync(markerPath, { force: true }); + } } private async ensureGitRef(targetDir: string, fetchArgs: string[], ref: string): Promise { diff --git a/packages/coding-agent/test/git-update.test.ts b/packages/coding-agent/test/git-update.test.ts index 80308277c..8b771f688 100644 --- a/packages/coding-agent/test/git-update.test.ts +++ b/packages/coding-agent/test/git-update.test.ts @@ -178,6 +178,59 @@ describe("DefaultPackageManager git update", () => { }); }); + describe("git clean failures", () => { + // https://github.com/earendil-works/pi/issues/11597 + it("should install dependencies and retry the clean later when git clean fails", async () => { + setupRemoteAndInstall(); + const newCommit = createCommit( + remoteDir, + "package.json", + JSON.stringify({ name: "test-extension", version: "1.0.0" }), + "Add package.json", + ); + + const executedCommands: string[] = []; + let failClean = true; + const managerWithInternals = packageManager as unknown as { + runCommand: (command: string, args: string[], options?: { cwd?: string }) => Promise; + }; + managerWithInternals.runCommand = async (command, args, options) => { + const commandLine = `${command} ${args.join(" ")}`; + executedCommands.push(commandLine); + if (command === "npm") return; + if (commandLine === "git clean -fdx" && failClean) { + throw new Error("git clean -fdx failed with code 1"); + } + const result = spawnSync(command, args, { cwd: options?.cwd, encoding: "utf-8" }); + if (result.status !== 0) { + throw new Error(`Command failed: ${commandLine}\n${result.stderr}`); + } + }; + const progressMessages: string[] = []; + packageManager.setProgressCallback((event) => { + if (event.type === "progress" && event.message) progressMessages.push(event.message); + }); + const markerPath = join(agentDir, "git", "github.com", "test", ".extension.pi-update-incomplete"); + + await packageManager.update(); + + expect(getCurrentCommit(installedDir)).toBe(newCommit); + expect(executedCommands).toContain("git clean -fdx"); + expect(executedCommands.some((command) => command.startsWith("npm install"))).toBe(true); + expect(progressMessages.some((message) => message.includes("Could not remove all untracked files"))).toBe( + true, + ); + expect(existsSync(markerPath)).toBe(true); + + failClean = false; + executedCommands.length = 0; + await packageManager.update(); + + expect(executedCommands).toContain("git clean -fdx"); + expect(existsSync(markerPath)).toBe(false); + }); + }); + describe("force-push scenarios", () => { it("should handle complete history rewrite", async () => { setupRemoteAndInstall(); diff --git a/packages/coding-agent/test/package-manager.test.ts b/packages/coding-agent/test/package-manager.test.ts index bdeffba40..b5895b331 100644 --- a/packages/coding-agent/test/package-manager.test.ts +++ b/packages/coding-agent/test/package-manager.test.ts @@ -1211,7 +1211,8 @@ Content`, expect(runCommandSpy).not.toHaveBeenCalledWith("git", ["clean", "-fdx"], { cwd: targetDir }); }); - it("should repair deleted git package dependencies when cleaning fails", async () => { + // https://github.com/earendil-works/pi/issues/11597 + it("should install git package dependencies when cleaning fails", 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"]; @@ -1237,7 +1238,7 @@ Content`, if (args[0] === "clean") throw new Error("simulated clean failure"); }); - await expect(packageManager.update(source)).rejects.toThrow("simulated clean failure"); + await packageManager.update(source); expect(runCommandSpy).toHaveBeenCalledWith( "npm",