fix(server): accept Git-valid worktree branch names (#3591)
Branch-off creation reused the filesystem slug validator even when callers supplied a separate branch name and worktree slug. Validate branch refs with Git at the worktree boundary so exact valid refs are preserved while path slugs remain restricted.
This commit is contained in:
@@ -96,6 +96,40 @@ test("workspace.create keeps a branch-off name separate from its worktree slug",
|
|||||||
}
|
}
|
||||||
}, 180000);
|
}, 180000);
|
||||||
|
|
||||||
|
test("workspace.create accepts a Git-valid branch-off name outside Paseo slug syntax", async () => {
|
||||||
|
const daemon = await createTestPaseoDaemon();
|
||||||
|
const { repoDir, tempRoot } = createGitRepoWithBranch();
|
||||||
|
const client = new DaemonClient({
|
||||||
|
url: `ws://127.0.0.1:${daemon.port}/ws`,
|
||||||
|
appVersion: "0.1.82",
|
||||||
|
});
|
||||||
|
|
||||||
|
try {
|
||||||
|
await client.connect();
|
||||||
|
|
||||||
|
const result = await client.createWorkspace({
|
||||||
|
source: {
|
||||||
|
kind: "worktree",
|
||||||
|
cwd: repoDir,
|
||||||
|
action: "branch-off",
|
||||||
|
branchName: "ABC-123_Fix-Broken-Model-Relationships_Author_Name",
|
||||||
|
worktreeSlug: "git-valid-branch-name",
|
||||||
|
baseBranch: "main",
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(result.error).toBeNull();
|
||||||
|
expect(result.workspace?.gitRuntime?.currentBranch).toBe(
|
||||||
|
"ABC-123_Fix-Broken-Model-Relationships_Author_Name",
|
||||||
|
);
|
||||||
|
expect(path.basename(result.workspace?.workspaceDirectory ?? "")).toBe("git-valid-branch-name");
|
||||||
|
} finally {
|
||||||
|
await client.close().catch(() => undefined);
|
||||||
|
await daemon.close();
|
||||||
|
rmSync(tempRoot, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
}, 180000);
|
||||||
|
|
||||||
test("workspace.create always creates a new worktree when the slug is already occupied", async () => {
|
test("workspace.create always creates a new worktree when the slug is already occupied", async () => {
|
||||||
const daemon = await createTestPaseoDaemon();
|
const daemon = await createTestPaseoDaemon();
|
||||||
const { repoDir, tempRoot } = createGitRepoWithBranch();
|
const { repoDir, tempRoot } = createGitRepoWithBranch();
|
||||||
|
|||||||
@@ -62,9 +62,7 @@ async function createWorktreeCoreWithPriority(
|
|||||||
const requestedWorktreeSlug = input.worktreeSlug
|
const requestedWorktreeSlug = input.worktreeSlug
|
||||||
? normalizeWorktreeSlug(input.worktreeSlug)
|
? normalizeWorktreeSlug(input.worktreeSlug)
|
||||||
: undefined;
|
: undefined;
|
||||||
const requestedBranchName = input.branchName
|
const requestedBranchName = input.branchName?.trim();
|
||||||
? validateWorktreeSlug(input.branchName.trim())
|
|
||||||
: undefined;
|
|
||||||
|
|
||||||
let intentInput: ResolveWorktreeCreationIntentInput;
|
let intentInput: ResolveWorktreeCreationIntentInput;
|
||||||
if (input.action === "checkout") {
|
if (input.action === "checkout") {
|
||||||
|
|||||||
@@ -655,15 +655,15 @@ describe.skipIf(isPlatform("win32"))("worktree POSIX-only", () => {
|
|||||||
).rejects.toThrow("Base branch not found: does-not-exist");
|
).rejects.toThrow("Base branch not found: does-not-exist");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("fails with invalid branch name", async () => {
|
it("fails with a Git-invalid branch name", async () => {
|
||||||
await expect(
|
await expect(
|
||||||
createLegacyWorktreeForTest({
|
createLegacyWorktreeForTest({
|
||||||
branchName: "INVALID_UPPERCASE",
|
branchName: "bad..name",
|
||||||
cwd: repoDir,
|
cwd: repoDir,
|
||||||
baseBranch: "main",
|
baseBranch: "main",
|
||||||
worktreeSlug: "test",
|
worktreeSlug: "test",
|
||||||
}),
|
}),
|
||||||
).rejects.toThrow("Invalid branch name");
|
).rejects.toThrow("Git rejected ref name 'bad..name'");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("throws a typed error when checking out an invalid existing branch name", async () => {
|
it("throws a typed error when checking out an invalid existing branch name", async () => {
|
||||||
|
|||||||
@@ -42,7 +42,6 @@ import { spawnProcess } from "./spawn.js";
|
|||||||
import { resolvePaseoHome } from "../server/paseo-home.js";
|
import { resolvePaseoHome } from "../server/paseo-home.js";
|
||||||
import { createExternalProcessEnv } from "../server/paseo-env.js";
|
import { createExternalProcessEnv } from "../server/paseo-env.js";
|
||||||
import { parseGitRevParsePath, resolveGitRevParsePath } from "./git-rev-parse-path.js";
|
import { parseGitRevParsePath, resolveGitRevParsePath } from "./git-rev-parse-path.js";
|
||||||
import { validateBranchSlug } from "@getpaseo/protocol/branch-slug";
|
|
||||||
import { expandTilde, getRealpathAwareRelativePath, isPathInsideRoot } from "./path.js";
|
import { expandTilde, getRealpathAwareRelativePath, isPathInsideRoot } from "./path.js";
|
||||||
import { terminateWithTreeKill } from "./tree-kill.js";
|
import { terminateWithTreeKill } from "./tree-kill.js";
|
||||||
|
|
||||||
@@ -1310,7 +1309,7 @@ async function resolveWorktreeSourcePlan({
|
|||||||
switch (source.kind) {
|
switch (source.kind) {
|
||||||
case "branch-off": {
|
case "branch-off": {
|
||||||
const branchName = source.branchName;
|
const branchName = source.branchName;
|
||||||
validateWorktreeBranchName(branchName);
|
await validateGitBranchName(cwd, branchName);
|
||||||
const normalizedBaseBranch = normalizeRequiredBaseBranch(source.baseBranch);
|
const normalizedBaseBranch = normalizeRequiredBaseBranch(source.baseBranch);
|
||||||
const resolvedBaseBranch = await resolveBaseBranchForWorktree(cwd, source.baseBranch);
|
const resolvedBaseBranch = await resolveBaseBranchForWorktree(cwd, source.baseBranch);
|
||||||
const branchExists = await localBranchExists(cwd, branchName);
|
const branchExists = await localBranchExists(cwd, branchName);
|
||||||
@@ -1330,7 +1329,7 @@ async function resolveWorktreeSourcePlan({
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
case "checkout-branch": {
|
case "checkout-branch": {
|
||||||
await validateExistingWorktreeBranchName(cwd, source.branchName);
|
await validateGitBranchName(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}`], {
|
||||||
@@ -1367,7 +1366,7 @@ async function resolveWorktreeSourcePlan({
|
|||||||
case "checkout-change-request":
|
case "checkout-change-request":
|
||||||
case "checkout-github-pr": {
|
case "checkout-github-pr": {
|
||||||
const localBranchCandidate = source.localBranchName ?? source.headRef;
|
const localBranchCandidate = source.localBranchName ?? source.headRef;
|
||||||
await validateExistingWorktreeBranchName(cwd, localBranchCandidate);
|
await validateGitBranchName(cwd, localBranchCandidate);
|
||||||
const localBranchName = await resolveUniqueLocalBranchName(cwd, localBranchCandidate);
|
const localBranchName = await resolveUniqueLocalBranchName(cwd, localBranchCandidate);
|
||||||
const normalizedBaseRefName = normalizeRequiredBaseBranch(source.baseRefName);
|
const normalizedBaseRefName = normalizeRequiredBaseBranch(source.baseRefName);
|
||||||
const changeRequestNumber =
|
const changeRequestNumber =
|
||||||
@@ -1589,14 +1588,7 @@ async function configureWorktreeTrackingRemote(options: {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
function validateWorktreeBranchName(branchName: string): void {
|
async function validateGitBranchName(cwd: string, branchName: string): Promise<void> {
|
||||||
const validation = validateBranchSlug(branchName);
|
|
||||||
if (!validation.valid) {
|
|
||||||
throw new Error(`Invalid branch name: ${validation.error}`);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
async function validateExistingWorktreeBranchName(cwd: string, branchName: string): Promise<void> {
|
|
||||||
const result = await runGitCommand(["check-ref-format", "--branch", branchName], {
|
const result = await runGitCommand(["check-ref-format", "--branch", branchName], {
|
||||||
cwd,
|
cwd,
|
||||||
timeout: 30_000,
|
timeout: 30_000,
|
||||||
|
|||||||
Reference in New Issue
Block a user