From 774e6005a3dbb7f348a00ba294705ad549dce5d5 Mon Sep 17 00:00:00 2001 From: David Kong Date: Sat, 25 Jul 2026 07:23:30 +1000 Subject: [PATCH] feat: implement git_worktree_remove tool (Issue #5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add validation and error handling for removing linked worktrees: - validateRemoveWorktreeInput() — validates path is non-empty - isMainRepository() — refuses removal of main repo root - hasUncommittedChanges() — checks git status --porcelain output - buildWorktreeRemoveCommand() — builds git worktree remove args Wire up createRemoveWorktreeTool with full validation pipeline: 1. Validate input parameters (path) 2. Check if target is the main repo — refuse with prune suggestion 3. Check for uncommitted changes — refuse with stash/commit message 4. Run git worktree remove 5. Surface git errors with helpful suggestions --- .pi/extensions/pi-worktree-remove.test.ts | 117 ++++++++++++++++++++++ .pi/extensions/pi-worktree-remove.ts | 101 +++++++++++++++++++ .pi/extensions/pi-worktree.ts | 66 ++++++++++-- 3 files changed, 278 insertions(+), 6 deletions(-) create mode 100644 .pi/extensions/pi-worktree-remove.test.ts create mode 100644 .pi/extensions/pi-worktree-remove.ts diff --git a/.pi/extensions/pi-worktree-remove.test.ts b/.pi/extensions/pi-worktree-remove.test.ts new file mode 100644 index 0000000..1e5bd61 --- /dev/null +++ b/.pi/extensions/pi-worktree-remove.test.ts @@ -0,0 +1,117 @@ +/** + * Unit tests for git_worktree_remove validation and command-building functions. + * + * Tests cover input validation, main repo detection, uncommitted changes check, + * and command building for `git worktree remove`. + */ + +import { describe, it, expect } from "bun:test"; +import { + validateRemoveWorktreeInput, + isMainRepository, + hasUncommittedChanges, + buildWorktreeRemoveCommand, +} from "./pi-worktree-remove"; + +// ─── validateRemoveWorktreeInput ────────────────────────────────────── + +describe("validateRemoveWorktreeInput (Issue #5)", () => { + it("returns null for valid path", () => { + const result = validateRemoveWorktreeInput("/tmp/worktrees/feature-x"); + expect(result).toBeNull(); + }); + + it("returns error for empty path", () => { + const result = validateRemoveWorktreeInput(""); + expect(result).not.toBeNull(); + if (result) expect(result.message).toContain("cannot be empty"); + }); + + it("returns error for whitespace-only path", () => { + const result = validateRemoveWorktreeInput(" "); + expect(result).not.toBeNull(); + }); + + it("includes suggestion in error message", () => { + const result = validateRemoveWorktreeInput(""); + expect(result).not.toBeNull(); + if (result) expect(result.suggestion).toBeDefined(); + }); +}); + +// ─── isMainRepository ──────────────────────────────────────────────── + +describe("isMainRepository (Issue #5)", () => { + it("returns true when target path matches main repo root", () => { + const result = isMainRepository("/home/user/project", "/home/user/project"); + expect(result).toBe(true); + }); + + it("returns false when target path differs from main repo root", () => { + const result = isMainRepository("/tmp/feature-x", "/home/user/project"); + expect(result).toBe(false); + }); + + it("handles trailing slash differences in paths", () => { + const result = isMainRepository("/home/user/project/", "/home/user/project"); + expect(result).toBe(true); + }); + + it("returns false for subdirectory of main repo (worktree outside)", () => { + const result = isMainRepository("../worktrees/feature-x", "/home/user/project"); + expect(result).toBe(false); + }); +}); + +// ─── hasUncommittedChanges ────────────────────────────────────────── + +describe("hasUncommittedChanges (Issue #5)", () => { + it("returns true when git status output has content", () => { + const result = hasUncommittedChanges(" modified: file.txt\n"); + expect(result).toBe(true); + }); + + it("returns false when git status output is empty", () => { + const result = hasUncommittedChanges(""); + expect(result).toBe(false); + }); + + it("returns false when git status output is whitespace only", () => { + const result = hasUncommittedChanges(" \n "); + expect(result).toBe(false); + }); + + it("detects staged changes", () => { + const result = hasUncommittedChanges("new file: added.txt\n"); + expect(result).toBe(true); + }); + + it("detects untracked files in status output", () => { + const result = hasUncommittedChanges("?? untracked-file.txt\n"); + expect(result).toBe(true); + }); +}); + +// ─── buildWorktreeRemoveCommand ────────────────────────────────────── + +describe("buildWorktreeRemoveCommand (Issue #5)", () => { + it("returns basic remove command without flags", () => { + const result = buildWorktreeRemoveCommand("/tmp/worktrees/feature-x"); + expect(result).toEqual(["worktree", "remove", "/tmp/worktrees/feature-x"]); + }); + + it("includes --force flag when force is true", () => { + const result = buildWorktreeRemoveCommand("/tmp/worktrees/feature-x", { force: true }); + expect(result).toContain("--force"); + }); + + it("places path as the last argument", () => { + const result = buildWorktreeRemoveCommand("/tmp/worktrees/feature-x", { force: true }); + expect(result[result.length - 1]).toBe("/tmp/worktrees/feature-x"); + }); + + it("builds correct command with force flag", () => { + const result = buildWorktreeRemoveCommand("/tmp/worktrees/feature-x", { force: true }); + expect(result).toEqual(["worktree", "remove", "--force", "/tmp/worktrees/feature-x"]); + }); +}); diff --git a/.pi/extensions/pi-worktree-remove.ts b/.pi/extensions/pi-worktree-remove.ts new file mode 100644 index 0000000..3e94658 --- /dev/null +++ b/.pi/extensions/pi-worktree-remove.ts @@ -0,0 +1,101 @@ +/** + * Git worktree remove — pure validation and command-building functions. + * No pi SDK dependencies for testability. + */ + +import path from "path"; + +// ─── Type Definitions ────────────────────────────────────────────── + +/** Error returned when a worktree operation fails. */ +export interface WorktreeRemoveError { + /** Human-readable error message. */ + message: string; + /** Suggested fix for the user. */ + suggestion?: string; +} + +/** Options for removing a worktree. */ +export interface RemoveWorktreeOptions { + /** Force removal even if the worktree has uncommitted changes. Defaults to false. */ + force?: boolean; +} + +// ─── Input Validation ────────────────────────────────────────────── + +/** + * Validate input parameters for removing a worktree. + * + * Checks that path is non-empty and well-formed. + * + * @param worktreePath - Path of the worktree to remove + * @returns null if valid, or a WorktreeRemoveError describing the problem + */ +export function validateRemoveWorktreeInput( + worktreePath: string, +): WorktreeRemoveError | null { + if (!worktreePath || !worktreePath.trim()) { + return { + message: "Path cannot be empty.", + suggestion: "Provide a valid worktree path (e.g., '../worktrees/feature-x').", + }; + } + + return null; +} + +// ─── Main Repository Check ──────────────────────────────────────── + +/** + * Check if the target path is the main repository root. + * + * Compares resolved paths to handle trailing slashes and relative paths. + * Removing the main repo is not allowed — use `git worktree prune` instead. + * + * @param targetPath - Path of the worktree to remove + * @param mainRepoRoot - Root path of the main repository + * @returns true if target is the main repo (removal should be refused) + */ +export function isMainRepository(targetPath: string, mainRepoRoot: string): boolean { + return path.resolve(targetPath) === path.resolve(mainRepoRoot); +} + +// ─── Uncommitted Changes Check ────────────────────────────────────── + +/** + * Check if the worktree has uncommitted changes based on `git status --porcelain` output. + * + * Any non-empty porcelain output indicates changes (staged, unstaged, or untracked). + * + * @param statusOutput - Raw stdout from `git status --porcelain` in the target worktree + * @returns true if there are uncommitted changes + */ +export function hasUncommittedChanges(statusOutput: string): boolean { + return statusOutput.trim().length > 0; +} + +// ─── Command Building ────────────────────────────────────────────── + +/** + * Build the `git worktree remove` command arguments. + * + * Uses `--force` flag when force option is true to bypass uncommitted changes check. + * + * @param worktreePath - Path of the worktree to remove + * @param options - Optional removal configuration + * @returns Array of command arguments (excluding 'git' itself) + */ +export function buildWorktreeRemoveCommand( + worktreePath: string, + options?: RemoveWorktreeOptions, +): string[] { + const args = ["worktree", "remove"]; + + if (options?.force) { + args.push("--force"); + } + + args.push(worktreePath); + + return args; +} diff --git a/.pi/extensions/pi-worktree.ts b/.pi/extensions/pi-worktree.ts index 0ad2099..490fa30 100644 --- a/.pi/extensions/pi-worktree.ts +++ b/.pi/extensions/pi-worktree.ts @@ -25,6 +25,13 @@ import { isCurrentBranch, type WorktreeError, } from "./pi-worktree-create"; +import { + validateRemoveWorktreeInput, + isMainRepository, + hasUncommittedChanges, + buildWorktreeRemoveCommand, + type WorktreeRemoveError, +} from "./pi-worktree-remove"; // ─── Type Definitions ────────────────────────────────────────────── @@ -117,7 +124,7 @@ function resolveWorktreeTarget(target: string, worktrees: WorktreeEntry[]): stri * @param error - The validation error * @returns Formatted error string with suggestion if available */ -function formatValidationError(error: { message: string; suggestion?: string }): string { +function formatValidationError(error: WorktreeError | WorktreeRemoveError): string { const base = `Error: ${error.message}`; return error.suggestion ? `${base} Suggestion: ${error.suggestion}` : base; } @@ -285,19 +292,66 @@ function createSwitchWorktreeTool(_pi: ExtensionAPI) { /** * Create the git_worktree_remove tool definition. - * @param _pi - Extension API (unused in stub) + * @param pi - Extension API for executing commands * @returns The configured tool definition */ -function createRemoveWorktreeTool(_pi: ExtensionAPI) { +function createRemoveWorktreeTool(pi: ExtensionAPI) { return defineTool({ name: "git_worktree_remove", label: "Git Worktree Remove", description: "Remove a linked worktree.", parameters: removeWorktreeParams, - async execute(_toolCallId, _params, _signal, _onUpdate, _ctx) { + async execute(_toolCallId, params, _signal, _onUpdate, ctx) { + const worktreePath = params.path; + + // 1. Validate input parameters + const inputError = validateRemoveWorktreeInput(worktreePath); + if (inputError) { + return { + content: [{ type: "text", text: formatValidationError(inputError) }], + isError: true, + }; + } + + // 2. Get main repo root and check if target is the main repo + const toplevelResult = await pi.exec("git", ["rev-parse", "--show-toplevel"], { cwd: ctx.cwd }); + if (toplevelResult.exitCode !== 0) { + return { + content: [{ type: "text", text: `Error getting repository root: ${toplevelResult.stderr}. Suggestion: Ensure you are inside a git repository.` }], + isError: true, + }; + } + + const mainRepoRoot = toplevelResult.stdout.trim(); + if (isMainRepository(worktreePath, mainRepoRoot)) { + return { + content: [{ type: "text", text: `Error: Can't remove the main repository. Suggestion: Use 'git worktree prune' instead.` }], + isError: true, + }; + } + + // 3. Check for uncommitted changes in the target worktree + const statusResult = await pi.exec("git", ["status", "--porcelain"], { cwd: worktreePath }); + if (statusResult.exitCode === 0 && hasUncommittedChanges(statusResult.stdout)) { + return { + content: [{ type: "text", text: `Error: Worktree '${worktreePath}' has uncommitted changes. Suggestion: Stash or commit them first, or use --force to remove anyway.` }], + isError: true, + }; + } + + // 4. Build and run git worktree remove command + const commandArgs = buildWorktreeRemoveCommand(worktreePath); + const result = await pi.exec("git", commandArgs, { cwd: ctx.cwd }); + + if (result.exitCode !== 0) { + return { + content: [{ type: "text", text: `Error removing worktree: ${result.stderr}. Suggestion: Check that the path is a valid worktree, or use --force if the worktree has uncommitted changes.` }], + isError: true, + }; + } + return { - content: [{ type: "text", text: "Error: git_worktree_remove is not yet implemented. Suggestion: Use `git worktree remove` manually until this tool is complete." }], - isError: true, + content: [{ type: "text", text: formatSuccessResponse("Removed", { at: worktreePath }) }], }; }, });