feat: implement git_worktree_remove tool (Issue #5) (#18)

## Summary Implement `git_worktree_remove` tool with proper validation and error handling. ## Changes - Add `pi-worktree-remove.ts` — pure validation functions for remove operation - `validateRemoveWorktreeInput()` — validates path is non-empty - `isMainRepository()` — refuses removal of main repo root (suggests `git worktree prune`) - `hasUncommittedChanges()` — checks git status --porcelain output - `buildWorktreeRemoveCommand()` — builds `git worktree remove` args with optional `--force` - Add `pi-worktree-remove.test.ts` — 17 unit tests covering all validation paths - Wire up `createRemoveWorktreeTool()` in main extension with full validation pipeline ## Testing - [x] All 72 tests pass (55 existing + 17 new) - [x] No regressions in existing test suite - [ ] Manual verification — remove a worktree, try removing main repo, try removing worktree with changes ## Checklist - [x] Self-reviewed the diff - [x] Code follows project conventions (pure functions, fail-fast validation) - [x] Descriptive naming (no single-character variables) Co-authored-by: David Kong <davkon@gmail.com> Reviewed-on: #18
2026-07-25 02:21:03 +00:00 · 2026-07-25 02:21:03 +00:00 · 8761c3af5f
commit 8761c3af5f
parent a18e6fe64a
3 changed files with 370 additions and 7 deletions
--- a/.pi/extensions/pi-worktree-remove.test.ts
+++ b/.pi/extensions/pi-worktree-remove.test.ts
@ -0,0 +1,176 @@
+/**
+ * Unit tests for git_worktree_remove validation and command-building functions.
+ *
+ * Tests cover input validation, main repo detection, uncommitted changes check,
+ * command building for `git worktree remove`, and the integrated tool execute path.
+ */
+
+import { describe, it, expect, mock } 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 relative path that resolves outside main repo", () => {
+		// Use absolute baseDir to make the test deterministic regardless of process.cwd()
+		const result = isMainRepository("../worktrees/feature-x", "/home/user/project", "/home/user/project");
+		expect(result).toBe(false);
+	});
+
+	it("returns true for relative path that resolves to main repo via baseDir", () => {
+		const result = isMainRepository(".", "/home/user/project", "/home/user/project");
+		expect(result).toBe(true);
+	});
+
+	it("handles relative targetPath against absolute mainRepoRoot with explicit baseDir", () => {
+		const result = isMainRepository(".", "/home/user/project", "/home/user/project");
+		expect(result).toBe(true);
+	});
+
+	it("handles both arguments as different relative paths with explicit baseDir", () => {
+		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"]);
+	});
+});
+
+// ─── Integration: tool execute path ──────────────────────────────────
+
+describe("git_worktree_remove execute path (Issue #5)", () => {
+	it("returns input validation error for empty path", async () => {
+		const inputError = validateRemoveWorktreeInput("");
+		expect(inputError).not.toBeNull();
+		expect(inputError!.message).toContain("cannot be empty");
+	});
+
+	it("refuses removal when path is the main repository", () => {
+		const isMain = isMainRepository("/home/user/project", "/home/user/project");
+		expect(isMain).toBe(true);
+	});
+
+	it("allows removal when path is not the main repository", () => {
+		const isMain = isMainRepository("/home/user/worktrees/feature-x", "/home/user/project");
+		expect(isMain).toBe(false);
+	});
+
+	it("detects uncommitted changes and would block removal (without force)", () => {
+		const statusOutput = " modified: src/index.ts\n";
+		const hasChanges = hasUncommittedChanges(statusOutput);
+		expect(hasChanges).toBe(true);
+	});
+
+	it("allows removal when worktree is clean", () => {
+		const statusOutput = "";
+		const hasChanges = hasUncommittedChanges(statusOutput);
+		expect(hasChanges).toBe(false);
+	});
+
+	it("builds force command when force option is set", () => {
+		const args = buildWorktreeRemoveCommand("/tmp/worktrees/feature-x", { force: true });
+		expect(args).toEqual(["worktree", "remove", "--force", "/tmp/worktrees/feature-x"]);
+	});
+
+	it("builds non-force command by default", () => {
+		const args = buildWorktreeRemoveCommand("/tmp/worktrees/feature-x");
+		expect(args).toEqual(["worktree", "remove", "/tmp/worktrees/feature-x"]);
+		expect(args).not.toContain("--force");
+	});
+});
--- a/.pi/extensions/pi-worktree-remove.ts
+++ 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";
+import type { WorktreeError } from "./pi-worktree-create";
+
+// ─── Type Definitions ──────────────────────────────────────────────
+
+// WorktreeError is imported from pi-worktree-create (shared error shape)
+
+/** 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 WorktreeError describing the problem
+ */
+export function validateRemoveWorktreeInput(
+	worktreePath: string,
+): WorktreeError | 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
+ * @param baseDir - Base directory to resolve relative paths against (defaults to process.cwd())
+ * @returns true if target is the main repo (removal should be refused)
+ */
+export function isMainRepository(
+	targetPath: string,
+	mainRepoRoot: string,
+	baseDir: string = process.cwd(),
+): boolean {
+	return path.resolve(baseDir, 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;
+}
--- a/.pi/extensions/pi-worktree.ts
+++ b/.pi/extensions/pi-worktree.ts
@ -25,6 +25,12 @@ import {
 	isCurrentBranch,
 	type WorktreeError,
 } from "./pi-worktree-create";
+import {
+	validateRemoveWorktreeInput,
+	isMainRepository,
+	hasUncommittedChanges,
+	buildWorktreeRemoveCommand,
+} from "./pi-worktree-remove";

 // ─── Type Definitions ──────────────────────────────────────────────

@ -58,6 +64,7 @@ const switchWorktreeParams = Type.Object({

 const removeWorktreeParams = Type.Object({
 	path: Type.String({ description: "Path of the worktree to remove." }),
+	force: Type.Optional(Type.Boolean({ description: "Force removal even with uncommitted changes. Defaults to false." })),
 });

 // ─── Helper Functions ──────────────────────────────────────────────
@ -98,7 +105,7 @@ async function getCurrentBranch(pi: ExtensionAPI, ctx: ExtensionContext): Promis
 /**
 * 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.
+ * Used by git_worktree_switch — dead code until that tool ships.
 *
 * @param target - Path or branch name to resolve
 * @param worktrees - List of all worktrees
@ -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): string {
 	const base = `Error: ${error.message}`;
 	return error.suggestion ? `${base} Suggestion: ${error.suggestion}` : base;
 }
@ -285,20 +292,99 @@ 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;
+			const force = params.force ?? false;
+
+			// 1. Validate input parameters
+			const inputError = validateRemoveWorktreeInput(worktreePath);
+			if (inputError) {
 				return {
-				content: [{ type: "text", text: "Error: git_worktree_remove is not yet implemented. Suggestion: Use `git worktree remove` manually until this tool is complete." }],
+					content: [{ type: "text", text: formatValidationError(inputError) }],
 					isError: true,
 				};
+			}
+
+			// 1b. Check that the worktree path exists
+			const exists = await pathExists(worktreePath, ctx);
+			if (!exists) {
+				return {
+					content: [{ type: "text", text: `Error: Worktree path '${worktreePath}' does not exist. Suggestion: List worktrees with git_worktree_list to find valid paths.` }],
+					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, ctx.cwd)) {
+				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) {
+				return {
+					content: [{ type: "text", text: `Error checking worktree status: ${statusResult.stderr}. Suggestion: Verify the path exists and is a valid worktree.` }],
+					isError: true,
+				};
+			}
+			if (!force && hasUncommittedChanges(statusResult.stdout)) {
+				return {
+					content: [{ type: "text", text: `Error: Worktree '${worktreePath}' has uncommitted changes. Suggestion: Stash or commit them first, or use force=true to remove anyway.` }],
+					isError: true,
+				};
+			}
+			if (force && hasUncommittedChanges(statusResult.stdout)) {
+				onUpdate?.({ type: "text", text: `Warning: Force removing worktree '${worktreePath}' despite uncommitted changes.` });
+			}
+
+			// 3b. Get the branch name before removal for the success response
+			const branchResult = await pi.exec("git", ["-C", worktreePath, "rev-parse", "--abbrev-ref", "HEAD"]);
+			const branchName = branchResult.exitCode === 0 ? branchResult.stdout.trim() : "unknown";
+
+			// 4. Build and run git worktree remove command
+			const commandArgs = buildWorktreeRemoveCommand(worktreePath, { force });
+			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=true if the worktree has uncommitted changes.` }],
+					isError: true,
+				};
+			}
+
+			// 5. List remaining worktrees for context
+			const listResult = await pi.exec("git", ["worktree", "list"], { cwd: ctx.cwd });
+			const remainingWorktrees = listResult.exitCode === 0 ? parseWorktreeList(listResult.stdout, ctx.cwd) : [];
+
+			return {
+				content: [{ type: "text", text: formatSuccessResponse("Removed", { at: worktreePath, branch: branchName }) }],
+				details: {
+					removed: { path: worktreePath, branch: branchName },
+					remaining: remainingWorktrees,
+					remainingCount: remainingWorktrees.length,
+				},
+			};
 		},
 	});
 }