Compare commits

..

2 commits

Author SHA1 Message Date
8761c3af5f 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
a18e6fe64a Milestone 2.1: Implement git_worktree_create tool (#4) (#17)
## Summary
Implements the `git_worktree_create` tool with full validation and error handling per issue #4 acceptance criteria.

## Changes
- New `pi-worktree-create.ts` module with pure, testable functions (no pi SDK deps)
- `validateCreateWorktreeInput()` — validates path and branch name parameters
- `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
- Integrated all helpers into `createCreateWorktreeTool()` in main extension
- 17 new tests covering all validation paths and edge cases (29 total, all passing)

## Testing
- [x] All 29 tests pass (`bun test .pi/extensions/*.test.ts`)
- [x] Full test suite run with no regressions
- [ ] Manual verification (requires pi runtime)

## Checklist
- [x] Self-reviewed the diff
- [x] Code follows project conventions
- [x] Descriptive naming (no single-character variables)
- [x] Documentation updated (JSDoc on all new functions)

Co-authored-by: David Kong <davkon@gmail.com>
Reviewed-on: #17
2026-07-24 20:50:45 +00:00
3 changed files with 370 additions and 7 deletions

View file

@ -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");
});
});

View file

@ -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;
}

View file

@ -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,19 +292,98 @@ 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: 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: "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, branch: branchName }) }],
details: {
removed: { path: worktreePath, branch: branchName },
remaining: remainingWorktrees,
remainingCount: remainingWorktrees.length,
},
};
},
});