fix(coding-agent): reconcile git package refs

closes #4870
This commit is contained in:
Mario Zechner
2026-05-22 11:13:26 +02:00
parent 1a2a536dba
commit bf56a86e1e
5 changed files with 139 additions and 10 deletions

View File

@@ -12,6 +12,7 @@
### Fixed ### 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)). - 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 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)). - 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)).

View File

@@ -408,7 +408,7 @@ pi update npm:@foo/pi-tools # update one package
pi config # enable/disable extensions, skills, prompts, themes 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`: Create a package by adding a `pi` key to `package.json`:

View File

@@ -85,9 +85,9 @@ ssh://git@github.com/user/repo@v1
- HTTPS and SSH URLs are both supported. - HTTPS and SSH URLs are both supported.
- SSH URLs use your configured SSH keys automatically (respects `~/.ssh/config`). - 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. - 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/<host>/<path>` (global) or `.pi/git/<host>/<path>` (project). - Cloned to `~/.pi/agent/git/<host>/<path>` (global) or `.pi/git/<host>/<path>` (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:** **SSH examples:**
```bash ```bash

View File

@@ -778,9 +778,21 @@ export class DefaultPackageManager implements PackageManager {
scope === "project" ? this.settingsManager.getProjectSettings() : this.settingsManager.getGlobalSettings(); scope === "project" ? this.settingsManager.getProjectSettings() : this.settingsManager.getGlobalSettings();
const currentPackages = currentSettings.packages ?? []; const currentPackages = currentSettings.packages ?? [];
const normalizedSource = this.normalizePackageSourceForSettings(source, scope); const normalizedSource = this.normalizePackageSourceForSettings(source, scope);
const exists = currentPackages.some((existing) => this.packageSourcesMatch(existing, source, scope)); const matchIndex = currentPackages.findIndex((existing) => this.packageSourcesMatch(existing, source, scope));
if (exists) { if (matchIndex !== -1) {
return false; 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]; const nextPackages = [...currentPackages, normalizedSource];
if (scope === "project") { if (scope === "project") {
@@ -1723,6 +1735,12 @@ export class DefaultPackageManager implements PackageManager {
private async installGit(source: GitSource, scope: SourceScope): Promise<void> { private async installGit(source: GitSource, scope: SourceScope): Promise<void> {
const targetDir = this.getGitInstallPath(source, scope); const targetDir = this.getGitInstallPath(source, scope);
if (existsSync(targetDir)) { 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; return;
} }
const gitRoot = this.getGitInstallRoot(scope); const gitRoot = this.getGitInstallRoot(scope);
@@ -1749,23 +1767,26 @@ export class DefaultPackageManager implements PackageManager {
} }
const target = await this.getLocalGitUpdateTarget(targetDir); const target = await this.getLocalGitUpdateTarget(targetDir);
await this.ensureGitRef(targetDir, target.fetchArgs, target.ref);
}
private async ensureGitRef(targetDir: string, fetchArgs: string[], ref: string): Promise<void> {
// Fetch only the ref we will reset to, avoiding unrelated branch/tag noise. // 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"], { const localHead = await this.runCommandCapture("git", ["rev-parse", "HEAD"], {
cwd: targetDir, cwd: targetDir,
timeoutMs: NETWORK_TIMEOUT_MS, 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, cwd: targetDir,
timeoutMs: NETWORK_TIMEOUT_MS, timeoutMs: NETWORK_TIMEOUT_MS,
}); });
if (localHead.trim() === refreshedTargetHead.trim()) { if (localHead.trim() === targetHead.trim()) {
return; 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) // Clean untracked files (extensions should be pristine)
await this.runCommand("git", ["clean", "-fdx"], { cwd: targetDir }); await this.runCommand("git", ["clean", "-fdx"], { cwd: targetDir });

View File

@@ -25,6 +25,16 @@ class MockSpawnedProcess extends EventEmitter {
} }
} }
interface PackageManagerInternals {
runCommand(command: string, args: string[], options?: { cwd?: string }): Promise<void>;
runCommandCapture(
command: string,
args: string[],
options?: { cwd?: string; timeoutMs?: number; env?: Record<string, string> },
): Promise<string>;
getLocalGitUpdateTarget(installedPath: string): Promise<{ ref: string; head: string; fetchArgs: string[] }>;
}
// Helper to check if a resource is enabled // Helper to check if a resource is enabled
const isEnabled = (r: ResolvedResource, pathMatch: string, matchFn: "endsWith" | "includes" = "endsWith") => { const isEnabled = (r: ResolvedResource, pathMatch: string, matchFn: "endsWith" | "includes" = "endsWith") => {
const normalizedPath = normalizeForMatch(r.path); const normalizedPath = normalizeForMatch(r.path);
@@ -693,6 +703,62 @@ Content`,
expect(runCommandSpy).toHaveBeenCalledWith("npm", ["install", "--omit=dev"], { cwd: targetDir }); 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 () => { it("should use plain install for git package dependencies when npmCommand is configured", async () => {
settingsManager = SettingsManager.inMemory({ settingsManager = SettingsManager.inMemory({
npmCommand: ["pnpm"], npmCommand: ["pnpm"],
@@ -1061,6 +1127,47 @@ Content`,
expect(removed).toBe(true); expect(removed).toBe(true);
expect(settingsManager.getGlobalSettings().packages ?? []).toHaveLength(0); 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)", () => { describe("HTTPS git URL parsing (old behavior)", () => {