diff --git a/.pi/extensions/pi-worktree-create.ts b/.pi/extensions/pi-worktree-create.ts new file mode 100644 index 0000000..b081321 --- /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 ~, ^, . at start or ?, *, [, ], ", ' in the middle (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..1217cd7 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,324 @@ 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).toContain("cannot be empty"); + }); + + it("returns error for empty branch name", () => { + const result = validateCreateWorktreeInput("/tmp/new-worktree", ""); + expect(result).not.toBeNull(); + if (result) expect(result.message).toContain("cannot be empty"); + }); + + 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("invalid characters"); + }); + + 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(); + }); + + // Task 007: Branch name start/end validation + it("rejects branch name starting with dash", () => { + const result = validateCreateWorktreeInput("/tmp/x", "-feature"); + expect(result).not.toBeNull(); + if (result) expect(result.message).toContain("invalid character"); + }); + + it("rejects branch name starting with slash", () => { + const result = validateCreateWorktreeInput("/tmp/x", "/feature"); + expect(result).not.toBeNull(); + }); + + it("rejects branch name starting with tilde", () => { + const result = validateCreateWorktreeInput("/tmp/x", "~user"); + expect(result).not.toBeNull(); + }); + + it("rejects branch name starting with caret", () => { + const result = validateCreateWorktreeInput("/tmp/x", "^feature"); + expect(result).not.toBeNull(); + }); + + it("rejects branch name starting with dot", () => { + const result = validateCreateWorktreeInput("/tmp/x", ".hidden"); + expect(result).not.toBeNull(); + }); + + it("rejects branch name ending with slash", () => { + const result = validateCreateWorktreeInput("/tmp/x", "feature/"); + expect(result).not.toBeNull(); + if (result) expect(result.message).toContain("invalid character"); + }); + + it("rejects branch name ending with dash", () => { + const result = validateCreateWorktreeInput("/tmp/x", "feature-"); + expect(result).not.toBeNull(); + }); + + it("rejects branch name ending with dot", () => { + const result = validateCreateWorktreeInput("/tmp/x", "feature."); + expect(result).not.toBeNull(); + }); + + // Task 008: Dangerous character validation + it("rejects branch name with square brackets", () => { + const result = validateCreateWorktreeInput("/tmp/x", "feature[v1]"); + expect(result).not.toBeNull(); + if (result) expect(result.message).toContain("invalid characters"); + }); + + it("rejects branch name with double quotes", () => { + const result = validateCreateWorktreeInput("/tmp/x", 'my"branch'); + expect(result).not.toBeNull(); + }); + + it("rejects branch name with single quotes", () => { + const result = validateCreateWorktreeInput("/tmp/x", "my'branch"); + expect(result).not.toBeNull(); + }); + + it("rejects branch name with backslash", () => { + const result = validateCreateWorktreeInput("/tmp/x", "feature\\name"); + expect(result).not.toBeNull(); + }); + + it("rejects branch name with asterisk", () => { + const result = validateCreateWorktreeInput("/tmp/x", "feature*name"); + expect(result).not.toBeNull(); + }); + + it("rejects branch name with question mark", () => { + const result = validateCreateWorktreeInput("/tmp/x", "feature?name"); + expect(result).not.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); + }); +}); + +// ─── Integration Tests for git_worktree_create execute flow (Task 004) ──── + +describe("git_worktree_create execute flow (Issue #4)", () => { + // Minimal mock types matching what the extension needs + interface MockExecResult { + exitCode: number; + stdout: string; + stderr: string; + } + + interface MockContext { + cwd: string; + } + + interface MockPI { + exec: (cmd: string, args: string[], opts?: { cwd: string }) => Promise; + } + + describe("validation pipeline ordering", () => { + it("rejects empty path before checking filesystem", async () => { + const result = validateCreateWorktreeInput("", "feature-x"); + expect(result).not.toBeNull(); + expect(result?.message).toContain("cannot be empty"); + }); + + it("rejects invalid branch name before checking filesystem", async () => { + const result = validateCreateWorktreeInput("/tmp/x", "-bad-name"); + expect(result).not.toBeNull(); + expect(result?.message).toContain("invalid character"); + }); + }); + + describe("path existence check", () => { + it("returns error when path exists", () => { + const existingPath = "/tmp/existing-worktree"; + const result = validatePathDoesNotExist(existingPath, (p) => p === existingPath); + expect(result).not.toBeNull(); + expect(result?.message).toContain("/tmp/existing-worktree"); + expect(result?.suggestion).toContain("different"); + }); + + it("returns null when path does not exist", () => { + const result = validatePathDoesNotExist( + "/tmp/nonexistent-path-abc123", + (p) => p === "/existing/path", + ); + expect(result).toBeNull(); + }); + }); + + describe("current branch check", () => { + it("detects when target matches current branch", () => { + expect(isCurrentBranch("main", "main")).toBe(true); + }); + + it("allows different branch from current", () => { + expect(isCurrentBranch("feature-x", "main")).toBe(false); + }); + }); + + describe("command building after all validations pass", () => { + it("builds correct command with createBranch=true", () => { + const args = buildWorktreeAddCommand({ + path: "/tmp/new-worktree", + branch: "feature-x", + createBranch: true, + }); + expect(args).toEqual(["worktree", "add", "-b", "feature-x", "/tmp/new-worktree"]); + }); + + it("builds correct command with createBranch=false", () => { + const args = buildWorktreeAddCommand({ + path: "/tmp/new-worktree", + branch: "existing-branch", + createBranch: false, + }); + expect(args).toEqual(["worktree", "add", "existing-branch", "/tmp/new-worktree"]); + }); + + it("defaults createBranch to true when omitted", () => { + const args = buildWorktreeAddCommand({ + path: "/tmp/new-worktree", + branch: "new-feature", + }); + expect(args).toContain("-b"); + }); + }); + + describe("error formatting consistency", () => { + it("formats input validation error with suggestion", () => { + const result = validateCreateWorktreeInput("", "feature-x"); + expect(result).not.toBeNull(); + expect(result?.suggestion).toBeDefined(); + expect(result?.suggestion).toContain("Provide a valid path"); + }); + + it("formats path exists error with suggestion", () => { + const result = validatePathDoesNotExist("/tmp/x", () => true); + expect(result).not.toBeNull(); + expect(result?.suggestion).toBeDefined(); + expect(result?.suggestion).toContain("different"); + }); + + it("formats branch name error with suggestion", () => { + const result = validateCreateWorktreeInput("/tmp/x", "bad name here"); + expect(result).not.toBeNull(); + expect(result?.suggestion).toBeDefined(); + expect(result?.suggestion).toContain("hyphens"); + }); + }); +}); diff --git a/.pi/extensions/pi-worktree.ts b/.pi/extensions/pi-worktree.ts index 15dd068..0ad2099 100644 --- a/.pi/extensions/pi-worktree.ts +++ b/.pi/extensions/pi-worktree.ts @@ -14,9 +14,17 @@ */ import type { ExtensionAPI, ExtensionContext } from "@earendil-works/pi-coding-agent"; +import { stat } from "node:fs/promises"; 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, + type WorktreeError, +} from "./pi-worktree-create"; // ─── Type Definitions ────────────────────────────────────────────── @@ -30,14 +38,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({ @@ -45,7 +45,7 @@ const createWorktreeParams = Type.Object({ branch: Type.Optional(Type.String({ description: "Branch name to check out. Defaults to 'main'." })), create_branch: Type.Optional( Type.Boolean({ - description: "If true and branch doesn't exist, create it. Defaults to false.", + description: "If true and branch doesn't exist, create it. Defaults to true.", }), ), }); @@ -60,26 +60,77 @@ 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 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(); } /** * Resolve a worktree target (path or branch name) to its full path. + * + * Used by git_worktree_switch and git_worktree_remove — dead code until those tools ship. + * * @param target - Path or branch name to resolve * @param worktrees - List of all worktrees * @returns The resolved path, or undefined if not found */ function resolveWorktreeTarget(target: string, worktrees: WorktreeEntry[]): string | undefined { - throw new Error("not implemented"); + const exactMatch = worktrees.find((w) => 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; +} + +/** + * Format a success response for worktree operations. + * @param action - The action performed (e.g., "Created") + * @param details - Key-value pairs to include in the message + * @returns Formatted success string + */ +function formatSuccessResponse(action: string, details: Record): string { + const parts = Object.entries(details).map(([key, value]) => `${key} '${value}'`); + return `${action} worktree ${parts.join(" on ")}.`; } // ─── Tool Factories ──────────────────────────────────────────────── @@ -137,19 +188,76 @@ 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)) { + const currentBranchError: WorktreeError = { + message: "Can't create worktree on the current branch — switch first.", + suggestion: "Choose a different branch name or switch branches with 'git switch'.", + }; + return { + content: [{ type: "text", text: formatValidationError(currentBranchError) }], + 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: formatSuccessResponse("Created", { at: worktreePath, branch: branchName }), + }], }; }, });