fix(branch): #154 branch ref format validation

package.json

@@ -55,7 +55,7 @@
     "knip": "^6.15.0",
     "semantic-release": "^25.0.3",
     "tsdown": "^0.21.10",
-    "tsx": "^3.12.3",
+    "tsx": "^4.23.12",
     "typescript": "^5.9.3",
     "valibot": "1.3.1",
     "vitest": "^3.2.4"

src/branch-args.test.ts

@@ -82,5 +82,23 @@ describe("parse_branch_runtime_flags", () => {
     expect(parsed.git_args).toBe(
       "--git-dir=/tmp/repo/.git --work-tree=/tmp/repo",
     );
+    expect(parsed.git_options).toEqual([
+      "--git-dir=/tmp/repo/.git",
+      "--work-tree=/tmp/repo",
+    ]);
+  });
+
+  it("preserves spaces in git-dir and work-tree options", () => {
+    const parsed = parse_branch_runtime_flags([
+      "--git-dir",
+      "/tmp/my repo/.git",
+      "--work-tree",
+      "/tmp/my repo",
+    ]);
+
+    expect(parsed.git_options).toEqual([
+      "--git-dir=/tmp/my repo/.git",
+      "--work-tree=/tmp/my repo",
+    ]);
   });
 });

src/branch-args.ts

@@ -8,6 +8,7 @@ type ParsedRuntimeFlags = {
   help: boolean;
   version: boolean;
   git_args: string;
+  git_options: string[];
   no_interactive: boolean;
   dry_run: boolean;
   branch_state: Partial<BranchStateRuntime>;
@@ -59,6 +60,10 @@ class BranchFlags {
     return this.#runtime.git_args;
   }
 
+  get git_options(): string[] {
+    return this.#runtime.git_options;
+  }
+
   get branch_state(): Partial<BranchStateRuntime> {
     return this.#runtime.branch_state;
   }
@@ -91,19 +96,28 @@ export function parse_branch_runtime_flags(argv: string[]): ParsedRuntimeFlags {
     }
   });
 
+  const git_options = get_git_options(
+    parsed["git-dir"],
+    parsed["work-tree"],
+  );
+
   return {
     help: parsed["help"] === true,
     version: parsed["version"] === true,
-    git_args: get_git_args(parsed["git-dir"], parsed["work-tree"]),
+    git_args: git_options.join(" "),
+    git_options,
     no_interactive: parsed["interactive"] === false,
     dry_run: parsed["dry-run"] === true,
     branch_state: branch_state,
   };
 }
 
-function get_git_args(
+function get_git_options(
   git_dir: string | undefined,
   work_tree: string | undefined,
-): string {
-  return `${git_dir ? `--git-dir=${git_dir}` : ""} ${work_tree ? `--work-tree=${work_tree}` : ""}`.trim();
+): string[] {
+  return [
+    ...(git_dir ? [`--git-dir=${git_dir}`] : []),
+    ...(work_tree ? [`--work-tree=${work_tree}`] : []),
+  ];
 }

src/branch-help.ts

@@ -1,4 +1,4 @@
-import { execSync } from "child_process";
+import { execFileSync } from "child_process";
 import { InferOutput } from "valibot";
 import { branch_flags } from "./branch-args";
 import { get_package_version } from "./utils";
@@ -56,31 +56,31 @@ export function print_help_text(
   let branch = "(none)";
   try {
     branch =
-      execSync(`git ${branch_flags.git_args} branch --show-current`, {
-        stdio: "pipe",
-      })
+      execFileSync(
+        "git",
+        [...branch_flags.git_options, "branch", "--show-current"],
+        { stdio: "pipe" },
+      )
         .toString()
         .trim() || "(none)";
-  } catch {
-    // noop
-  }
+  } catch {}
 
   const inferred_type =
-    infer_type_from_git(config.commit_type.options, branch_flags.git_args) ||
+    infer_type_from_git(config.commit_type.options, branch_flags.git_options) ||
     "Unknown";
   const inferred_ticket = config.check_ticket.infer_ticket
     ? infer_ticket_from_git(
         {
           append_hashtag: config.check_ticket.append_hashtag,
           prepend_hashtag: config.check_ticket.prepend_hashtag,
         },
-        branch_flags.git_args,
+        branch_flags.git_options,
       ) || "Unknown"
     : "Infer Disabled";
   const inferred_scope = config.commit_scope.infer_scope_from_branch
     ? infer_scope_from_git(
         config.commit_scope.options,
-        branch_flags.git_args,
+        branch_flags.git_options,
       ) || "Unknown"
     : "Infer Disabled";
 

src/branch.ts

@@ -44,7 +44,7 @@ const promptCtors: PromptCtor[] = [
 
 const { config, config_source } = load_setup(
   " better-branch ",
-  branch_flags.git_args,
+  branch_flags.git_options,
 );
 
 main(config, config_source);
@@ -53,7 +53,7 @@ async function main(
   config: InferOutput<typeof Config>,
   config_source: ConfigSource,
 ) {
-  chdir(get_git_root(branch_flags.git_args));
+  chdir(get_git_root(branch_flags.git_options));
 
   if (branch_flags.version) {
     const version = get_package_version();
@@ -77,7 +77,7 @@ async function main(
       } else {
         p.log.error(`Failed to validate branch input: ${err}`);
       }
-      process.exit(0);
+      process.exit(1);
     }
   }
 

src/prompts/branch-confirm.prompt.test.ts

@@ -1,4 +1,4 @@
-import { beforeEach, describe, expect, it, vi } from "vitest";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import { parse } from "valibot";
 import { BranchState, Config } from "../valibot-state";
 
@@ -12,13 +12,18 @@ const sq = (value: string): string => `'${value.replaceAll("'", `'\\''`)}'`;
 
 const mocked = vi.hoisted(() => ({
   dry_run: true,
+  git_options: [] as string[],
+  execFileSync: vi.fn(),
   execSync: vi.fn(),
+  spawnSync: vi.fn(),
   info: vi.fn(),
   error: vi.fn(),
 }));
 
 vi.mock("child_process", () => ({
+  execFileSync: mocked.execFileSync,
   execSync: mocked.execSync,
+  spawnSync: mocked.spawnSync,
 }));
 
 vi.mock("@clack/prompts", () => ({
@@ -41,6 +46,9 @@ vi.mock("picocolors", () => ({
 vi.mock("../branch-args", () => ({
   branch_flags: {
     git_args: "",
+    get git_options() {
+      return mocked.git_options;
+    },
     get dry_run() {
       return mocked.dry_run;
     },
@@ -56,9 +64,25 @@ vi.mock("../utils", async () => {
 });
 
 describe("BranchConfirmPrompt", () => {
+  afterEach(() => {
+    vi.restoreAllMocks();
+  });
+
   beforeEach(() => {
     mocked.dry_run = true;
+    mocked.git_options = [];
+    mocked.execFileSync.mockReset();
+    mocked.execFileSync.mockImplementation((_file, args: string[]) => {
+      if (args.includes("show-ref")) throw new Error("branch missing");
+      return Buffer.from("");
+    });
     mocked.execSync.mockReset();
+    mocked.spawnSync.mockReset();
+    mocked.spawnSync.mockImplementation((_file, args: string[]) => ({
+      status: 0,
+      stdout: args.at(-1),
+      stderr: "",
+    }));
     mocked.info.mockReset();
     mocked.error.mockReset();
   });
@@ -85,15 +109,19 @@ describe("BranchConfirmPrompt", () => {
 
     await new BranchConfirmPrompt(config, branch_state, {} as never).run();
 
-    expect(mocked.execSync).toHaveBeenCalledTimes(1);
-    expect(mocked.execSync).toHaveBeenCalledWith(
-      "git  show-ref feat/TAC-123-cli-add-parser",
-      {
-        encoding: "utf-8",
-      },
+    expect(mocked.execFileSync).toHaveBeenCalledTimes(1);
+    expect(mocked.execFileSync).toHaveBeenCalledWith(
+      "git",
+      [
+        "show-ref",
+        "--verify",
+        "--quiet",
+        "refs/heads/feat/TAC-123-cli-add-parser",
+      ],
+      { encoding: "utf-8" },
     );
     expect(mocked.info).toHaveBeenCalledWith(
-      "Dry run: git  worktree add ../worktrees/repo-TAC-123-add-parser -b feat/TAC-123-cli-add-parser",
+      "Dry run: git worktree add -b feat/TAC-123-cli-add-parser ../worktrees/repo-TAC-123-add-parser",
     );
   });
 
@@ -124,12 +152,12 @@ describe("BranchConfirmPrompt", () => {
     await new BranchConfirmPrompt(config, branch_state, {} as never).run();
 
     // Only the show-ref check runs a real command; everything else is dry-run logged.
-    expect(mocked.execSync).toHaveBeenCalledTimes(1);
+    expect(mocked.execFileSync).toHaveBeenCalledTimes(1);
     expect(mocked.info).toHaveBeenCalledWith(
       `Dry run: echo pre ${sq("TAC-123")}`,
     );
     expect(mocked.info).toHaveBeenCalledWith(
-      "Dry run: git  checkout -b feat/TAC-123-cli-add-parser",
+      "Dry run: git checkout -b feat/TAC-123-cli-add-parser",
     );
     expect(mocked.info).toHaveBeenCalledWith(
       `Dry run: echo post ${sq("feat")}/${sq("TAC-123")}`,
@@ -327,4 +355,122 @@ describe("BranchConfirmPrompt", () => {
       { stdio: "inherit" },
     );
   });
+
+  it("passes git-dir and work-tree as literal arguments", async () => {
+    mocked.dry_run = false;
+    mocked.git_options = [
+      "--git-dir=/tmp/my repo/.git",
+      "--work-tree=/tmp/my repo",
+    ];
+    const { BranchConfirmPrompt } = await import("./branch-confirm.prompt");
+    const config = parse(Config, {});
+    const branch_state = parse(BranchState, {
+      type: "feat",
+      description: "safe-branch",
+      checkout: "branch",
+    });
+
+    await new BranchConfirmPrompt(config, branch_state, {} as never).run();
+
+    expect(mocked.execFileSync).toHaveBeenNthCalledWith(
+      2,
+      "git",
+      [
+        "--git-dir=/tmp/my repo/.git",
+        "--work-tree=/tmp/my repo",
+        "checkout",
+        "-b",
+        "feat/safe-branch",
+      ],
+      { stdio: "inherit" },
+    );
+  });
+
+  it("exits non-zero when branch validation fails", async () => {
+    mocked.spawnSync.mockReturnValue({
+      status: 128,
+      stdout: "",
+      stderr: "fatal: 'feat/a..b' is not a valid branch name",
+    });
+    const exit = vi
+      .spyOn(process, "exit")
+      .mockImplementation((code): never => {
+        throw new Error(`exit ${code}`);
+      });
+    const { BranchConfirmPrompt } = await import("./branch-confirm.prompt");
+    const config = parse(Config, {});
+    const branch_state = parse(BranchState, {
+      type: "feat",
+      description: "a..b",
+      checkout: "branch",
+    });
+
+    await expect(
+      new BranchConfirmPrompt(config, branch_state, {} as never).run(),
+    ).rejects.toThrow("exit 1");
+
+    expect(exit).toHaveBeenCalledWith(1);
+    expect(mocked.execFileSync).not.toHaveBeenCalled();
+    exit.mockRestore();
+  });
+
+  it("exits non-zero when checkout fails", async () => {
+    mocked.dry_run = false;
+    mocked.execFileSync.mockImplementation(() => {
+      throw new Error("checkout failed");
+    });
+    const exit = vi
+      .spyOn(process, "exit")
+      .mockImplementation((code): never => {
+        throw new Error(`exit ${code}`);
+      });
+    const { BranchConfirmPrompt } = await import("./branch-confirm.prompt");
+    const config = parse(Config, {});
+    const branch_state = parse(BranchState, {
+      type: "feat",
+      description: "safe-branch",
+      checkout: "branch",
+    });
+
+    await expect(
+      new BranchConfirmPrompt(config, branch_state, {} as never).run(),
+    ).rejects.toThrow("exit 1");
+
+    expect(exit).toHaveBeenCalledWith(1);
+    expect(mocked.error).toHaveBeenCalledWith(
+      "Failed to checkout branch 'feat/safe-branch'",
+    );
+    exit.mockRestore();
+  });
+
+  it("exits non-zero when worktree creation fails", async () => {
+    mocked.dry_run = false;
+    mocked.execFileSync.mockImplementation(() => {
+      throw new Error("worktree failed");
+    });
+    const exit = vi
+      .spyOn(process, "exit")
+      .mockImplementation((code): never => {
+        throw new Error(`exit ${code}`);
+      });
+    const { BranchConfirmPrompt } = await import("./branch-confirm.prompt");
+    const config = parse(Config, {
+      worktrees: { base_path: "../worktrees" },
+    });
+    const branch_state = parse(BranchState, {
+      type: "feat",
+      description: "safe-worktree",
+      checkout: "worktree",
+    });
+
+    await expect(
+      new BranchConfirmPrompt(config, branch_state, {} as never).run(),
+    ).rejects.toThrow("exit 1");
+
+    expect(exit).toHaveBeenCalledWith(1);
+    expect(mocked.error).toHaveBeenCalledWith(
+      "Failed to create worktree '../worktrees/repo-safe-worktree'",
+    );
+    exit.mockRestore();
+  });
 });