diff --git a/.pi/extensions/pi-worktree-create.ts b/.pi/extensions/pi-worktree-create.ts index 439b16e..b081321 100644 --- a/.pi/extensions/pi-worktree-create.ts +++ b/.pi/extensions/pi-worktree-create.ts @@ -30,7 +30,7 @@ export interface CreateWorktreeParams { * * 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 ~, ^, :, ?, *, [, " (except / for nesting). + * 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 @@ -65,7 +65,7 @@ export function validateCreateWorktreeInput( } // Validate branch name doesn't start/end with special chars - if (/^[\/\-~]/.test(branchName) || /[\/\-\.\s]$/.test(branchName)) { + 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.", diff --git a/.pi/extensions/pi-worktree.test.ts b/.pi/extensions/pi-worktree.test.ts index 9f0677f..1217cd7 100644 --- a/.pi/extensions/pi-worktree.test.ts +++ b/.pi/extensions/pi-worktree.test.ts @@ -131,13 +131,13 @@ describe("validateCreateWorktreeInput (Issue #4)", () => { it("returns error for empty path", () => { const result = validateCreateWorktreeInput("", "feature-x"); expect(result).not.toBeNull(); - if (result) expect(result.message.toLowerCase()).toContain("path"); + 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.toLowerCase()).toContain("branch"); + if (result) expect(result.message).toContain("cannot be empty"); }); it("returns error for path with only whitespace", () => { @@ -148,7 +148,7 @@ describe("validateCreateWorktreeInput (Issue #4)", () => { 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("branch"); + if (result) expect(result.message).toContain("invalid characters"); }); it("accepts branch names with hyphens and slashes", () => { @@ -160,6 +160,81 @@ describe("validateCreateWorktreeInput (Issue #4)", () => { 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)", () => { @@ -255,3 +330,115 @@ describe("isCurrentBranch (Issue #4)", () => { 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 2aa257a..0ad2099 100644 --- a/.pi/extensions/pi-worktree.ts +++ b/.pi/extensions/pi-worktree.ts @@ -14,6 +14,7 @@ */ 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"; @@ -22,6 +23,7 @@ import { buildWorktreeAddCommand, validatePathDoesNotExist, isCurrentBranch, + type WorktreeError, } from "./pi-worktree-create"; // ─── Type Definitions ────────────────────────────────────────────── @@ -43,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.", }), ), }); @@ -67,7 +69,6 @@ const removeWorktreeParams = Type.Object({ * @returns True if the path exists */ async function pathExists(checkPath: string, ctx: ExtensionContext): Promise { - const { stat } = await import("node:fs/promises"); const resolvedPath = checkPath.startsWith("/") ? checkPath : `${ctx.cwd}/${checkPath}`; try { @@ -96,6 +97,9 @@ 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. + * * @param target - Path or branch name to resolve * @param worktrees - List of all worktrees * @returns The resolved path, or undefined if not found @@ -118,6 +122,17 @@ function formatValidationError(error: { message: string; suggestion?: string }): 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 ──────────────────────────────────────────────── /** @@ -209,11 +224,12 @@ function createCreateWorktreeTool(pi: ExtensionAPI) { // 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: `Error: Can't create worktree on the current branch — switch first. Suggestion: Choose a different branch name or switch branches with 'git switch'.`, - }], + content: [{ type: "text", text: formatValidationError(currentBranchError) }], isError: true, }; } @@ -240,7 +256,7 @@ function createCreateWorktreeTool(pi: ExtensionAPI) { return { content: [{ type: "text", - text: `Created worktree at '${worktreePath}' on branch '${branchName}'.`, + text: formatSuccessResponse("Created", { at: worktreePath, branch: branchName }), }], }; },