From 5bc15a1e6916601f52b61f750de9c8a9fe915a06 Mon Sep 17 00:00:00 2001 From: David Kong Date: Fri, 24 Jul 2026 23:18:54 +1000 Subject: [PATCH] Milestone 2.1: Implement git_worktree_create tool (#4) - Add pi-worktree-create.ts with pure validation/command-building functions - validateCreateWorktreeInput: checks path and branch name validity - validatePathDoesNotExist: refuses if target path exists (injectable for testing) - isCurrentBranch: prevents creating worktree on current branch - buildWorktreeAddCommand: builds correct git command with -b flag support - Full error messages match issue acceptance criteria exactly - 17 new tests covering all validation paths and edge cases --- .pi/extensions/pi-worktree-create.ts | 146 +++++++++++++++++++++++++++ .pi/extensions/pi-worktree.test.ts | 140 +++++++++++++++++++++++++ .pi/extensions/pi-worktree.ts | 130 ++++++++++++++++++++---- 3 files changed, 397 insertions(+), 19 deletions(-) create mode 100644 .pi/extensions/pi-worktree-create.ts diff --git a/.pi/extensions/pi-worktree-create.ts b/.pi/extensions/pi-worktree-create.ts new file mode 100644 index 0000000..439b16e --- /dev/null +++ b/.pi/extensions/pi-worktree-create.ts @@ -0,0 +1,146 @@ +/** + * Git worktree create — pure validation and command-building functions. + * No pi SDK dependencies for testability. + */ + +// ─── Type Definitions ────────────────────────────────────────────── + +/** Error returned when a worktree operation fails. */ +export interface WorktreeError { + /** Human-readable error message from git or validation. */ + message: string; + /** Suggested fix for the user. */ + suggestion?: string; +} + +/** Parameters for creating a new worktree. */ +export interface CreateWorktreeParams { + /** Path for the new worktree (relative or absolute). */ + path: string; + /** Branch name to check out. */ + branch: string; + /** Whether to create the branch if it doesn't exist. Defaults to true. */ + createBranch?: boolean; +} + +// ─── Validation ──────────────────────────────────────────────────── + +/** + * Validate input parameters for creating a worktree. + * + * Checks that path and branch name are non-empty and well-formed. + * Branch names follow git conventions: no spaces, no leading/trailing whitespace, + * no special characters like ~, ^, :, ?, *, [, " (except / for nesting). + * + * @param worktreePath - Path for the new worktree + * @param branchName - Branch name to check out + * @returns null if valid, or a WorktreeError describing the problem + */ +export function validateCreateWorktreeInput( + worktreePath: string, + branchName: string, +): WorktreeError | null { + // Validate path is non-empty + if (!worktreePath || !worktreePath.trim()) { + return { + message: "Path cannot be empty.", + suggestion: "Provide a valid path for the new worktree (e.g., '../worktrees/feature-x').", + }; + } + + // Validate branch name is non-empty + if (!branchName || !branchName.trim()) { + return { + message: "Branch name cannot be empty.", + suggestion: "Provide a valid branch name (e.g., 'feature-x' or 'bugfix/issue-42').", + }; + } + + // Validate branch name doesn't contain spaces + if (branchName.includes(" ")) { + return { + message: `Branch name '${branchName}' contains invalid characters (spaces).`, + suggestion: "Use hyphens or underscores instead of spaces (e.g., 'feature-x' not 'my feature').", + }; + } + + // Validate branch name doesn't start/end with special chars + if (/^[\/\-~]/.test(branchName) || /[\/\-\.\s]$/.test(branchName)) { + return { + message: `Branch name '${branchName}' starts or ends with an invalid character.`, + suggestion: "Branch names should start with a letter and not end with '/', '-', '.', or whitespace.", + }; + } + + // Validate branch name doesn't contain dangerous characters + if (/[?*\[\]"'\\]/.test(branchName)) { + return { + message: `Branch name '${branchName}' contains invalid characters.`, + suggestion: "Avoid characters like ?, *, [, ], \", ' in branch names.", + }; + } + + return null; +} + +// ─── Path Validation ────────────────────────────────────────────── + +/** + * Check if the target path already exists and return an error if so. + * + * @param worktreePath - Path to check + * @param checkExists - Function that checks if a path exists (injected for testability) + * @returns null if path is free, or a WorktreeError with the exact message format + */ +export function validatePathDoesNotExist( + worktreePath: string, + checkExists: (path: string) => boolean, +): WorktreeError | null { + if (checkExists(worktreePath)) { + return { + message: `Path '${worktreePath}' already exists.`, + suggestion: "Choose a different location.", + }; + } + return null; +} + +// ─── Branch Validation ────────────────────────────────────────────── + +/** + * Check if the target branch is the same as the current branch. + * + * @param targetBranch - The branch we want to create the worktree on + * @param currentBranch - The currently checked out branch + * @returns true if they match (worktree creation should be refused) + */ +export function isCurrentBranch(targetBranch: string, currentBranch: string): boolean { + return targetBranch === currentBranch; +} + +// ─── Command Building ────────────────────────────────────────────── + +/** + * Build the `git worktree add` command arguments. + * + * Uses `-b` flag when createBranch is true to create a new branch. + * Without `-b`, checks out an existing branch. + * + * @param params - Worktree creation parameters + * @returns Array of command arguments (excluding 'git' itself) + */ +export function buildWorktreeAddCommand(params: CreateWorktreeParams): string[] { + const { path: worktreePath, branch, createBranch = true } = params; + + const args = ["worktree", "add"]; + + if (createBranch) { + args.push("-b", branch); + } else { + args.push(branch); + } + + args.push(worktreePath); + + return args; +} diff --git a/.pi/extensions/pi-worktree.test.ts b/.pi/extensions/pi-worktree.test.ts index ff1a8fe..9f0677f 100644 --- a/.pi/extensions/pi-worktree.test.ts +++ b/.pi/extensions/pi-worktree.test.ts @@ -6,6 +6,12 @@ import { describe, it, expect } from "bun:test"; import { parseWorktreeList } from "./pi-worktree-parser"; +import { + validateCreateWorktreeInput, + buildWorktreeAddCommand, + validatePathDoesNotExist, + isCurrentBranch, +} from "./pi-worktree-create"; describe("parseWorktreeList", () => { describe("normal output", () => { @@ -115,3 +121,137 @@ describe("parseWorktreeList", () => { }); }); }); + +describe("validateCreateWorktreeInput (Issue #4)", () => { + it("returns null for valid path and branch", () => { + const result = validateCreateWorktreeInput("/tmp/new-worktree", "feature-x"); + expect(result).toBeNull(); + }); + + it("returns error for empty path", () => { + const result = validateCreateWorktreeInput("", "feature-x"); + expect(result).not.toBeNull(); + if (result) expect(result.message.toLowerCase()).toContain("path"); + }); + + it("returns error for empty branch name", () => { + const result = validateCreateWorktreeInput("/tmp/new-worktree", ""); + expect(result).not.toBeNull(); + if (result) expect(result.message.toLowerCase()).toContain("branch"); + }); + + it("returns error for path with only whitespace", () => { + const result = validateCreateWorktreeInput(" ", "feature-x"); + expect(result).not.toBeNull(); + }); + + it("returns error for branch name with invalid characters (spaces)", () => { + const result = validateCreateWorktreeInput("/tmp/new-worktree", "my bad branch"); + expect(result).not.toBeNull(); + if (result) expect(result.message).toContain("branch"); + }); + + it("accepts branch names with hyphens and slashes", () => { + const result = validateCreateWorktreeInput("/tmp/new-worktree", "feature/my-branch-name"); + expect(result).toBeNull(); + }); + + it("accepts branch names with underscores", () => { + const result = validateCreateWorktreeInput("/tmp/new-worktree", "issue_42_fix"); + expect(result).toBeNull(); + }); +}); + +describe("buildWorktreeAddCommand (Issue #4)", () => { + it("returns command with -b flag when create_branch is true", () => { + const result = buildWorktreeAddCommand({ + path: "/tmp/new-worktree", + branch: "feature-x", + createBranch: true, + }); + + expect(result).toEqual([ + "worktree", + "add", + "-b", + "feature-x", + "/tmp/new-worktree", + ]); + }); + + it("returns command without -b flag when create_branch is false", () => { + const result = buildWorktreeAddCommand({ + path: "/tmp/new-worktree", + branch: "existing-branch", + createBranch: false, + }); + + expect(result).toEqual([ + "worktree", + "add", + "existing-branch", + "/tmp/new-worktree", + ]); + }); + + it("defaults createBranch to true when not provided", () => { + const result = buildWorktreeAddCommand({ + path: "/tmp/new-worktree", + branch: "new-feature", + }); + + expect(result).toContain("-b"); + }); + + it("places path as the last argument", () => { + const result = buildWorktreeAddCommand({ + path: "/tmp/new-worktree", + branch: "feature-x", + createBranch: true, + }); + + expect(result[result.length - 1]).toBe("/tmp/new-worktree"); + }); +}); + +describe("validatePathDoesNotExist (Issue #4)", () => { + it("returns null when path does not exist", () => { + const result = validatePathDoesNotExist( + "/tmp/nonexistent-path-abc123", + (p) => p === "/existing/path", + ); + expect(result).toBeNull(); + }); + + it("returns error with path in message when path exists", () => { + const existingPath = "/tmp/existing-worktree"; + const result = validatePathDoesNotExist(existingPath, (p) => p === existingPath); + + expect(result).not.toBeNull(); + if (result) expect(result.message).toContain("/tmp/existing-worktree"); + if (result) expect(result.message.toLowerCase()).toContain("already exists"); + }); + + it("suggests choosing a different location", () => { + const existingPath = "/tmp/existing-worktree"; + const result = validatePathDoesNotExist(existingPath, (p) => p === existingPath); + + expect(result).not.toBeNull(); + if (result) expect(result.suggestion?.toLowerCase()).toContain("different"); + }); +}); + +describe("isCurrentBranch (Issue #4)", () => { + it("returns true when branch matches current branch", () => { + expect(isCurrentBranch("main", "main")).toBe(true); + }); + + it("returns false when branch differs from current branch", () => { + expect(isCurrentBranch("feature-x", "main")).toBe(false); + }); + + it("handles branch names with slashes", () => { + expect(isCurrentBranch("feature/my-branch", "main")).toBe(false); + expect(isCurrentBranch("feature/my-branch", "feature/my-branch")).toBe(true); + }); +}); diff --git a/.pi/extensions/pi-worktree.ts b/.pi/extensions/pi-worktree.ts index 15dd068..2aa257a 100644 --- a/.pi/extensions/pi-worktree.ts +++ b/.pi/extensions/pi-worktree.ts @@ -17,6 +17,12 @@ import type { ExtensionAPI, ExtensionContext } from "@earendil-works/pi-coding-a import { defineTool } from "@earendil-works/pi-coding-agent"; import { Type } from "@earendil-works/pi-ai"; import { parseWorktreeList, type WorktreeEntry } from "./pi-worktree-parser"; +import { + validateCreateWorktreeInput, + buildWorktreeAddCommand, + validatePathDoesNotExist, + isCurrentBranch, +} from "./pi-worktree-create"; // ─── Type Definitions ────────────────────────────────────────────── @@ -30,14 +36,6 @@ interface WorktreeListResult { currentPath: string; } -/** Error returned when a worktree operation fails. */ -interface WorktreeError { - /** Human-readable error message from git or validation. */ - message: string; - /** Suggested fix for the user. */ - suggestion?: string; -} - // ─── Tool Parameter Schemas ──────────────────────────────────────── const createWorktreeParams = Type.Object({ @@ -60,16 +58,40 @@ const removeWorktreeParams = Type.Object({ path: Type.String({ description: "Path of the worktree to remove." }), }); -// ─── Helper Functions (stubs) ────────────────────────────────────── +// ─── Helper Functions ────────────────────────────────────────────── /** - * Check if the target path exists. - * @param checkPath - Path to check - * @param ctx - Extension context for file system access + * Check if the target path exists using Bun's fs module. + * @param checkPath - Path to check (resolved against cwd) + * @param ctx - Extension context for working directory * @returns True if the path exists */ async function pathExists(checkPath: string, ctx: ExtensionContext): Promise { - throw new Error("not implemented"); + const { stat } = await import("node:fs/promises"); + const resolvedPath = checkPath.startsWith("/") ? checkPath : `${ctx.cwd}/${checkPath}`; + + try { + await stat(resolvedPath); + return true; + } catch { + return false; + } +} + +/** + * Get the current branch name from git. + * @param pi - Extension API for executing commands + * @param ctx - Extension context for working directory + * @returns Current branch name, or "unknown" if detection fails + */ +async function getCurrentBranch(pi: ExtensionAPI, ctx: ExtensionContext): Promise { + const result = await pi.exec("git", ["rev-parse", "--abbrev-ref", "HEAD"], { cwd: ctx.cwd }); + + if (result.exitCode !== 0) { + return "unknown"; + } + + return result.stdout.trim(); } /** @@ -79,7 +101,21 @@ async function pathExists(checkPath: string, ctx: ExtensionContext): Promise w.path === target); + if (exactMatch) return exactMatch.path; + + const branchMatch = worktrees.find((w) => w.branch === target); + return branchMatch?.path; +} + +/** + * Format a WorktreeError into the tool response text. + * @param error - The validation error + * @returns Formatted error string with suggestion if available + */ +function formatValidationError(error: { message: string; suggestion?: string }): string { + const base = `Error: ${error.message}`; + return error.suggestion ? `${base} Suggestion: ${error.suggestion}` : base; } // ─── Tool Factories ──────────────────────────────────────────────── @@ -137,19 +173,75 @@ function createListWorktreeTool(pi: ExtensionAPI) { /** * Create the git_worktree_create tool definition. - * @param _pi - Extension API (unused in stub) + * @param pi - Extension API for registering and executing tools * @returns The configured tool definition */ -function createCreateWorktreeTool(_pi: ExtensionAPI) { +function createCreateWorktreeTool(pi: ExtensionAPI) { return defineTool({ name: "git_worktree_create", label: "Git Worktree Create", description: "Create a linked worktree for a branch in the current repo.", parameters: createWorktreeParams, - async execute(_toolCallId, _params, _signal, _onUpdate, _ctx) { + async execute(_toolCallId, params, _signal, _onUpdate, ctx) { + const worktreePath = params.path; + const branchName = params.branch || "main"; + const createBranch = params.create_branch ?? true; + + // 1. Validate input parameters + const inputError = validateCreateWorktreeInput(worktreePath, branchName); + if (inputError) { + return { + content: [{ type: "text", text: formatValidationError(inputError) }], + isError: true, + }; + } + + // 2. Check if path already exists — refuse if so + const pathExistsResult = await pathExists(worktreePath, ctx); + const pathError = validatePathDoesNotExist(worktreePath, () => pathExistsResult); + if (pathError) { + return { + content: [{ type: "text", text: formatValidationError(pathError) }], + isError: true, + }; + } + + // 3. Check if creating on current branch — refuse if so + const currentBranch = await getCurrentBranch(pi, ctx); + if (isCurrentBranch(branchName, currentBranch)) { + return { + content: [{ + type: "text", + text: `Error: Can't create worktree on the current branch — switch first. Suggestion: Choose a different branch name or switch branches with 'git switch'.`, + }], + isError: true, + }; + } + + // 4. Build and run git worktree add command + const commandArgs = buildWorktreeAddCommand({ + path: worktreePath, + branch: branchName, + createBranch, + }); + + const result = await pi.exec("git", commandArgs, { cwd: ctx.cwd }); + + if (result.exitCode !== 0) { + return { + content: [{ + type: "text", + text: `Error creating worktree: ${result.stderr}. Suggestion: Check that the branch exists (or use create_branch=true) and that the parent directory is writable.`, + }], + isError: true, + }; + } + return { - content: [{ type: "text", text: "Error: git_worktree_create is not yet implemented. Suggestion: Use `git worktree add` manually until this tool is complete." }], - isError: true, + content: [{ + type: "text", + text: `Created worktree at '${worktreePath}' on branch '${branchName}'.`, + }], }; }, });