Fix worktree checkout validation for existing git branch refs (#1358)
* fix(server): allow valid existing worktree branch refs (#1357) * Fix existing branch checkout validation --------- Co-authored-by: Mohamed Boudra <boudra.moha@gmail.com>
This commit is contained in:
co-authored by
Mohamed Boudra
parent
44e9287389
commit
d454fa3af4
@@ -6,6 +6,7 @@ import {
|
|||||||
createWorktree as createWorktreePrimitive,
|
createWorktree as createWorktreePrimitive,
|
||||||
deriveWorktreeProjectHash,
|
deriveWorktreeProjectHash,
|
||||||
deletePaseoWorktree,
|
deletePaseoWorktree,
|
||||||
|
InvalidGitBranchNameError,
|
||||||
getScriptConfigs,
|
getScriptConfigs,
|
||||||
getWorktreeSetupCommands,
|
getWorktreeSetupCommands,
|
||||||
getWorktreeTerminalSpecs,
|
getWorktreeTerminalSpecs,
|
||||||
@@ -270,6 +271,26 @@ describe.skipIf(isPlatform("win32"))("worktree POSIX-only", () => {
|
|||||||
expect(metadata).toMatchObject({ version: 1, baseRefName: "dev" });
|
expect(metadata).toMatchObject({ version: 1, baseRefName: "dev" });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("checks out an existing local branch whose name contains uppercase letters and dots", async () => {
|
||||||
|
execFileSync("git", ["branch", "release/1.1.15"], { cwd: repoDir });
|
||||||
|
|
||||||
|
const result = await createLegacyWorktreeForTest({
|
||||||
|
cwd: repoDir,
|
||||||
|
worktreeSlug: "release-worktree",
|
||||||
|
source: { kind: "checkout-branch", branchName: "release/1.1.15" },
|
||||||
|
runSetup: true,
|
||||||
|
paseoHome,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(existsSync(result.worktreePath)).toBe(true);
|
||||||
|
const currentBranch = execFileSync("git", ["branch", "--show-current"], {
|
||||||
|
cwd: result.worktreePath,
|
||||||
|
})
|
||||||
|
.toString()
|
||||||
|
.trim();
|
||||||
|
expect(currentBranch).toBe("release/1.1.15");
|
||||||
|
});
|
||||||
|
|
||||||
it("throws a typed error when checking out a branch already checked out in the main repo", async () => {
|
it("throws a typed error when checking out a branch already checked out in the main repo", async () => {
|
||||||
let caughtError: unknown;
|
let caughtError: unknown;
|
||||||
try {
|
try {
|
||||||
@@ -340,6 +361,51 @@ describe.skipIf(isPlatform("win32"))("worktree POSIX-only", () => {
|
|||||||
expect(metadata).toMatchObject({ baseRefName: "main" });
|
expect(metadata).toMatchObject({ baseRefName: "main" });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("fetches a GitHub PR branch when the head ref contains uppercase letters and dots", async () => {
|
||||||
|
const remoteDir = join(tempDir, "remote.git");
|
||||||
|
const remoteCloneDir = join(tempDir, "remote-clone");
|
||||||
|
execFileSync("git", ["clone", "--bare", repoDir, remoteDir]);
|
||||||
|
execFileSync("git", ["remote", "add", "origin", remoteDir], { cwd: repoDir });
|
||||||
|
|
||||||
|
execFileSync("git", ["clone", remoteDir, remoteCloneDir]);
|
||||||
|
execFileSync("git", ["config", "user.email", "test@test.com"], { cwd: remoteCloneDir });
|
||||||
|
execFileSync("git", ["config", "user.name", "Test"], { cwd: remoteCloneDir });
|
||||||
|
execFileSync("git", ["checkout", "-b", "Feature.X"], { cwd: remoteCloneDir });
|
||||||
|
writeFileSync(join(remoteCloneDir, "file.txt"), "from-uppercase-pr\n");
|
||||||
|
execFileSync("git", ["add", "file.txt"], { cwd: remoteCloneDir });
|
||||||
|
execFileSync("git", ["-c", "commit.gpgsign=false", "commit", "-m", "uppercase pr branch"], {
|
||||||
|
cwd: remoteCloneDir,
|
||||||
|
});
|
||||||
|
const prHead = execFileSync("git", ["rev-parse", "HEAD"], { cwd: remoteCloneDir })
|
||||||
|
.toString()
|
||||||
|
.trim();
|
||||||
|
execFileSync("git", ["push", "origin", "Feature.X"], { cwd: remoteCloneDir });
|
||||||
|
execFileSync("git", [`--git-dir=${remoteDir}`, "update-ref", "refs/pull/43/head", prHead]);
|
||||||
|
|
||||||
|
const result = await createLegacyWorktreeForTest({
|
||||||
|
cwd: repoDir,
|
||||||
|
worktreeSlug: "pr-43",
|
||||||
|
source: {
|
||||||
|
kind: "checkout-github-pr",
|
||||||
|
githubPrNumber: 43,
|
||||||
|
headRef: "Feature.X",
|
||||||
|
baseRefName: "main",
|
||||||
|
},
|
||||||
|
runSetup: true,
|
||||||
|
paseoHome,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(readFileSync(join(result.worktreePath, "file.txt"), "utf8")).toBe(
|
||||||
|
"from-uppercase-pr\n",
|
||||||
|
);
|
||||||
|
const currentBranch = execFileSync("git", ["branch", "--show-current"], {
|
||||||
|
cwd: result.worktreePath,
|
||||||
|
})
|
||||||
|
.toString()
|
||||||
|
.trim();
|
||||||
|
expect(currentBranch).toBe("Feature.X");
|
||||||
|
});
|
||||||
|
|
||||||
it("prefers origin/{branch} over local {branch} when both exist", async () => {
|
it("prefers origin/{branch} over local {branch} when both exist", async () => {
|
||||||
const remoteDir = join(tempDir, "remote.git");
|
const remoteDir = join(tempDir, "remote.git");
|
||||||
const remoteCloneDir = join(tempDir, "remote-clone");
|
const remoteCloneDir = join(tempDir, "remote-clone");
|
||||||
@@ -425,6 +491,42 @@ describe.skipIf(isPlatform("win32"))("worktree POSIX-only", () => {
|
|||||||
).rejects.toThrow("Invalid branch name");
|
).rejects.toThrow("Invalid branch name");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("throws a typed error when checking out an invalid existing branch name", async () => {
|
||||||
|
let caughtError: unknown;
|
||||||
|
try {
|
||||||
|
await createLegacyWorktreeForTest({
|
||||||
|
cwd: repoDir,
|
||||||
|
worktreeSlug: "invalid-existing-branch",
|
||||||
|
source: { kind: "checkout-branch", branchName: "bad..name" },
|
||||||
|
runSetup: true,
|
||||||
|
paseoHome,
|
||||||
|
});
|
||||||
|
} catch (error) {
|
||||||
|
caughtError = error;
|
||||||
|
}
|
||||||
|
|
||||||
|
expect(caughtError).toBeInstanceOf(InvalidGitBranchNameError);
|
||||||
|
expect((caughtError as InvalidGitBranchNameError).branchName).toBe("bad..name");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("throws a typed error when checking out a ref that is valid but not a branch name", async () => {
|
||||||
|
let caughtError: unknown;
|
||||||
|
try {
|
||||||
|
await createLegacyWorktreeForTest({
|
||||||
|
cwd: repoDir,
|
||||||
|
worktreeSlug: "invalid-option-like-branch",
|
||||||
|
source: { kind: "checkout-branch", branchName: "-bad" },
|
||||||
|
runSetup: true,
|
||||||
|
paseoHome,
|
||||||
|
});
|
||||||
|
} catch (error) {
|
||||||
|
caughtError = error;
|
||||||
|
}
|
||||||
|
|
||||||
|
expect(caughtError).toBeInstanceOf(InvalidGitBranchNameError);
|
||||||
|
expect((caughtError as InvalidGitBranchNameError).branchName).toBe("-bad");
|
||||||
|
});
|
||||||
|
|
||||||
it("handles branch name collision by adding suffix", async () => {
|
it("handles branch name collision by adding suffix", async () => {
|
||||||
const projectHash = await deriveWorktreeProjectHash(repoDir);
|
const projectHash = await deriveWorktreeProjectHash(repoDir);
|
||||||
// Create a branch named "hello" first
|
// Create a branch named "hello" first
|
||||||
|
|||||||
@@ -206,6 +206,16 @@ export class UnknownBranchError extends Error {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export class InvalidGitBranchNameError extends Error {
|
||||||
|
readonly branchName: string;
|
||||||
|
|
||||||
|
constructor(branchName: string) {
|
||||||
|
super(`Invalid branch name: Git rejected ref name '${branchName}'`);
|
||||||
|
this.name = "InvalidGitBranchNameError";
|
||||||
|
this.branchName = branchName;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
export type ReadPaseoConfigResult =
|
export type ReadPaseoConfigResult =
|
||||||
| { ok: true; config: PaseoConfig | null }
|
| { ok: true; config: PaseoConfig | null }
|
||||||
| { ok: false; configPath: string; error: unknown };
|
| { ok: false; configPath: string; error: unknown };
|
||||||
@@ -1261,7 +1271,7 @@ async function resolveWorktreeSourcePlan({
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
case "checkout-branch": {
|
case "checkout-branch": {
|
||||||
validateWorktreeBranchName(source.branchName);
|
await validateExistingWorktreeBranchName(cwd, source.branchName);
|
||||||
if (!(await localBranchExists(cwd, source.branchName))) {
|
if (!(await localBranchExists(cwd, source.branchName))) {
|
||||||
try {
|
try {
|
||||||
await runGitCommand(["fetch", "origin", `${source.branchName}:${source.branchName}`], {
|
await runGitCommand(["fetch", "origin", `${source.branchName}:${source.branchName}`], {
|
||||||
@@ -1284,7 +1294,7 @@ async function resolveWorktreeSourcePlan({
|
|||||||
}
|
}
|
||||||
case "checkout-github-pr": {
|
case "checkout-github-pr": {
|
||||||
const localBranchCandidate = source.localBranchName ?? source.headRef;
|
const localBranchCandidate = source.localBranchName ?? source.headRef;
|
||||||
validateWorktreeBranchName(localBranchCandidate);
|
await validateExistingWorktreeBranchName(cwd, localBranchCandidate);
|
||||||
const localBranchName = await resolveUniqueLocalBranchName(cwd, localBranchCandidate);
|
const localBranchName = await resolveUniqueLocalBranchName(cwd, localBranchCandidate);
|
||||||
const normalizedBaseRefName = normalizeRequiredBaseBranch(source.baseRefName);
|
const normalizedBaseRefName = normalizeRequiredBaseBranch(source.baseRefName);
|
||||||
await runGitCommand(
|
await runGitCommand(
|
||||||
@@ -1350,6 +1360,17 @@ function validateWorktreeBranchName(branchName: string): void {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async function validateExistingWorktreeBranchName(cwd: string, branchName: string): Promise<void> {
|
||||||
|
const result = await runGitCommand(["check-ref-format", "--branch", branchName], {
|
||||||
|
cwd,
|
||||||
|
timeout: 30_000,
|
||||||
|
acceptExitCodes: [0, 1, 128],
|
||||||
|
});
|
||||||
|
if (result.exitCode !== 0) {
|
||||||
|
throw new InvalidGitBranchNameError(branchName);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
function normalizeRequiredBaseBranch(baseBranch: string): string {
|
function normalizeRequiredBaseBranch(baseBranch: string): string {
|
||||||
const normalizedBaseBranch = normalizeBaseRefName(baseBranch);
|
const normalizedBaseBranch = normalizeBaseRefName(baseBranch);
|
||||||
if (!normalizedBaseBranch) {
|
if (!normalizedBaseBranch) {
|
||||||
|
|||||||
Reference in New Issue
Block a user