feat: implement git_worktree_remove tool (Issue #5)
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 <path> 5. Surface git errors with helpful suggestions
This commit is contained in:
parent
a18e6fe64a
commit
774e6005a3
3 changed files with 278 additions and 6 deletions
117
.pi/extensions/pi-worktree-remove.test.ts
Normal file
117
.pi/extensions/pi-worktree-remove.test.ts
Normal file
|
|
@ -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"]);
|
||||
});
|
||||
});
|
||||
101
.pi/extensions/pi-worktree-remove.ts
Normal file
101
.pi/extensions/pi-worktree-remove.ts
Normal file
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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 }) }],
|
||||
};
|
||||
},
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue