fix(coding-agent): handle git @ref parsing edge cases and pinned update tests refs #1299

This commit is contained in:
Mario Zechner 2026-02-05 21:56:57 +01:00
parent 8abcb35c62
commit f33844fe37
3 changed files with 73 additions and 29 deletions

View file

@ -66,6 +66,7 @@ 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.
- Raw `https://` URLs work without the `git:` prefix. - Raw `https://` URLs work without the `git:` prefix.
- Refs pin the package and skip `pi update`. - Refs pin the package and skip `pi update`.
- 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).

View file

@ -19,32 +19,57 @@ export type GitSource = {
}; };
function splitRef(url: string): { repo: string; ref?: string } { function splitRef(url: string): { repo: string; ref?: string } {
const lastAt = url.lastIndexOf("@"); const scpLikeMatch = url.match(/^git@([^:]+):(.+)$/);
if (lastAt <= 0) {
return { repo: url };
}
const lastSlash = url.lastIndexOf("/");
const hasScheme = url.includes("://");
const scpLikeMatch = url.match(/^[^@]+@[^:]+:/);
if (scpLikeMatch) { if (scpLikeMatch) {
const separatorIndex = scpLikeMatch[0].length - 1; const pathWithMaybeRef = scpLikeMatch[2] ?? "";
if (lastAt <= separatorIndex || lastAt <= lastSlash) { const refSeparator = pathWithMaybeRef.indexOf("@");
return { repo: url }; if (refSeparator < 0) return { repo: url };
} const repoPath = pathWithMaybeRef.slice(0, refSeparator);
} else if (hasScheme) { const ref = pathWithMaybeRef.slice(refSeparator + 1);
const schemeIndex = url.indexOf("://"); if (!repoPath || !ref) return { repo: url };
const pathStart = url.indexOf("/", schemeIndex + 3); return {
if (pathStart < 0 || lastAt <= pathStart || lastAt <= lastSlash) { repo: `git@${scpLikeMatch[1] ?? ""}:${repoPath}`,
return { repo: url }; ref,
} };
} else if (lastAt <= lastSlash) {
return { repo: url };
} }
if (url.includes("://")) {
try {
const parsed = new URL(url);
const pathWithMaybeRef = parsed.pathname.replace(/^\/+/, "");
const refSeparator = pathWithMaybeRef.indexOf("@");
if (refSeparator < 0) return { repo: url };
const repoPath = pathWithMaybeRef.slice(0, refSeparator);
const ref = pathWithMaybeRef.slice(refSeparator + 1);
if (!repoPath || !ref) return { repo: url };
parsed.pathname = `/${repoPath}`;
return {
repo: parsed.toString().replace(/\/$/, ""),
ref,
};
} catch {
return { repo: url };
}
}
const slashIndex = url.indexOf("/");
if (slashIndex < 0) {
return { repo: url };
}
const host = url.slice(0, slashIndex);
const pathWithMaybeRef = url.slice(slashIndex + 1);
const refSeparator = pathWithMaybeRef.indexOf("@");
if (refSeparator < 0) {
return { repo: url };
}
const repoPath = pathWithMaybeRef.slice(0, refSeparator);
const ref = pathWithMaybeRef.slice(refSeparator + 1);
if (!repoPath || !ref) {
return { repo: url };
}
return { return {
repo: url.slice(0, lastAt), repo: `${host}/${repoPath}`,
ref: url.slice(lastAt + 1), ref,
}; };
} }
@ -111,6 +136,9 @@ export function parseGitUrl(source: string): GitSource | null {
for (const candidate of hostedCandidates) { for (const candidate of hostedCandidates) {
const info = hostedGitInfo.fromUrl(candidate); const info = hostedGitInfo.fromUrl(candidate);
if (info) { if (info) {
if (split.ref && info.project?.includes("@")) {
continue;
}
const useHttpsPrefix = const useHttpsPrefix =
!split.repo.startsWith("http://") && !split.repo.startsWith("http://") &&
!split.repo.startsWith("https://") && !split.repo.startsWith("https://") &&
@ -120,7 +148,7 @@ export function parseGitUrl(source: string): GitSource | null {
type: "git", type: "git",
repo: useHttpsPrefix ? `https://${split.repo}` : split.repo, repo: useHttpsPrefix ? `https://${split.repo}` : split.repo,
host: info.domain || "", host: info.domain || "",
path: `${info.user}/${info.project}`, path: `${info.user}/${info.project}`.replace(/\.git$/, ""),
ref: info.committish || split.ref || undefined, ref: info.committish || split.ref || undefined,
pinned: Boolean(info.committish || split.ref), pinned: Boolean(info.committish || split.ref),
}; };
@ -133,11 +161,14 @@ export function parseGitUrl(source: string): GitSource | null {
for (const candidate of httpsCandidates) { for (const candidate of httpsCandidates) {
const info = hostedGitInfo.fromUrl(candidate); const info = hostedGitInfo.fromUrl(candidate);
if (info) { if (info) {
if (split.ref && info.project?.includes("@")) {
continue;
}
return { return {
type: "git", type: "git",
repo: `https://${split.repo}`, repo: `https://${split.repo}`,
host: info.domain || "", host: info.domain || "",
path: `${info.user}/${info.project}`, path: `${info.user}/${info.project}`.replace(/\.git$/, ""),
ref: info.committish || split.ref || undefined, ref: info.committish || split.ref || undefined,
pinned: Boolean(info.committish || split.ref), pinned: Boolean(info.committish || split.ref),
}; };

View file

@ -83,8 +83,9 @@ describe("DefaultPackageManager git update", () => {
/** /**
* Sets up a "remote" repository and clones it to the installed directory. * Sets up a "remote" repository and clones it to the installed directory.
* This simulates what packageManager.install() would do. * This simulates what packageManager.install() would do.
* @param sourceOverride Optional source string to use instead of gitSource (e.g., with @ref for pinned tests)
*/ */
function setupRemoteAndInstall(): void { function setupRemoteAndInstall(sourceOverride?: string): void {
// Create "remote" repository // Create "remote" repository
mkdirSync(remoteDir, { recursive: true }); mkdirSync(remoteDir, { recursive: true });
git(["init"], remoteDir); git(["init"], remoteDir);
@ -99,7 +100,7 @@ describe("DefaultPackageManager git update", () => {
git(["config", "--local", "user.name", "Test"], installedDir); git(["config", "--local", "user.name", "Test"], installedDir);
// Add to global packages so update() processes this source // Add to global packages so update() processes this source
settingsManager.setPackages([gitSource]); settingsManager.setPackages([sourceOverride ?? gitSource]);
} }
describe("normal updates (no force-push)", () => { describe("normal updates (no force-push)", () => {
@ -203,10 +204,21 @@ describe("DefaultPackageManager git update", () => {
describe("pinned sources", () => { describe("pinned sources", () => {
it("should not update pinned git sources (with @ref)", async () => { it("should not update pinned git sources (with @ref)", async () => {
setupRemoteAndInstall(); // Create remote repo first to get the initial commit
const initialCommit = getCurrentCommit(installedDir); mkdirSync(remoteDir, { recursive: true });
git(["init"], remoteDir);
git(["config", "--local", "user.email", "test@test.com"], remoteDir);
git(["config", "--local", "user.name", "Test"], remoteDir);
const initialCommit = createCommit(remoteDir, "extension.ts", "// v1", "Initial commit");
// Reconfigure with pinned ref // Install with pinned ref from the start - full clone to ensure commit is available
mkdirSync(join(agentDir, "git", "github.com", "test"), { recursive: true });
git(["clone", remoteDir, installedDir], tempDir);
git(["checkout", initialCommit], installedDir);
git(["config", "--local", "user.email", "test@test.com"], installedDir);
git(["config", "--local", "user.name", "Test"], installedDir);
// Add to global packages with pinned ref
settingsManager.setPackages([`${gitSource}@${initialCommit}`]); settingsManager.setPackages([`${gitSource}@${initialCommit}`]);
// Add new commit to remote // Add new commit to remote