From 618288aa795e8840851004cc86d35038c2b06feb Mon Sep 17 00:00:00 2001 From: David Kong Date: Fri, 24 Jul 2026 18:16:00 +1000 Subject: [PATCH 1/3] Create extension file structure with tool scaffolding - Add .pi/extensions/pi-worktree.ts with correct imports and default export - Define WorktreeEntry, WorktreeListResult, WorktreeError types - Register 4 tools: create, list, switch, remove (stubs) - Register /worktree command (stub) - Add helper function stubs: parseWorktreeList, pathExists, resolveWorktreeTarget - Uses defineTool() and Type from pi SDK for type-safe registration --- .pi/extensions/pi-worktree.ts | 162 ++++++++++++++++++++++++++++++++++ findings.md | 40 +++++++++ progress.md | 26 ++++++ task_plan.md | 18 ++++ 4 files changed, 246 insertions(+) create mode 100644 .pi/extensions/pi-worktree.ts create mode 100644 findings.md create mode 100644 progress.md create mode 100644 task_plan.md diff --git a/.pi/extensions/pi-worktree.ts b/.pi/extensions/pi-worktree.ts new file mode 100644 index 0000000..0edd2dd --- /dev/null +++ b/.pi/extensions/pi-worktree.ts @@ -0,0 +1,162 @@ +/** + * pi-worktree Extension + * + * Manages git worktrees through tool commands and an interactive `/worktree` command. + * + * Tools: + * - git_worktree_create — Create a linked worktree for a branch + * - git_worktree_list — List all linked worktrees + * - git_worktree_switch — Switch working directory to a worktree + * - git_worktree_remove — Remove a linked worktree + * + * Command: + * - /worktree — Interactive picker to switch between worktrees + */ + +import type { ExtensionAPI, ExtensionContext } from "@earendil-works/pi-coding-agent"; +import { defineTool } from "@earendil-works/pi-coding-agent"; +import { Type } from "@earendil-works/pi-ai"; + +// ─── Type Definitions ────────────────────────────────────────────── + +/** A single worktree entry parsed from `git worktree list`. */ +interface WorktreeEntry { + /** Full path to the worktree directory. */ + path: string; + /** Short commit hash checked out in this worktree. */ + commit: string; + /** Branch name, or "(detached)" for detached HEAD. */ + branch: string; + /** Whether this worktree is the current working directory. */ + isCurrent: boolean; +} + +/** Result of listing all worktrees. */ +interface WorktreeListResult { + /** All worktrees in the repository. */ + worktrees: WorktreeEntry[]; + /** Total number of worktrees. */ + count: number; +} + +/** Error returned when a worktree operation fails. */ +interface WorktreeError { + /** Human-readable error message from git or validation. */ + message: string; + /** Suggested fix for the user. */ + suggestion?: string; +} + +// ─── Tool Parameter Schemas ──────────────────────────────────────── + +const createWorktreeParams = Type.Object({ + path: Type.String({ description: "Path for the new worktree (relative or absolute)" }), + 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.", + }), + ), +}); + +const listWorktreeParams = Type.Object({}); + +const switchWorktreeParams = Type.Object({ + target: Type.String({ description: "Path or branch name of the target worktree to switch to." }), +}); + +const removeWorktreeParams = Type.Object({ + path: Type.String({ description: "Path of the worktree to remove." }), +}); + +// ─── Helper Functions (stubs) ────────────────────────────────────── + +/** + * Parse raw `git worktree list` output into structured entries. + * @param rawOutput - Raw stdout from `git worktree list` + * @param currentDir - Current working directory for comparison + * @returns Array of parsed worktree entries + */ +function parseWorktreeList(rawOutput: string, currentDir: string): WorktreeEntry[] { + throw new Error("not implemented"); +} + +/** + * Check if the target path exists. + * @param path - Path to check + * @param ctx - Extension context for file system access + * @returns True if the path exists + */ +async function pathExists(path: string, ctx: ExtensionContext): Promise { + throw new Error("not implemented"); +} + +/** + * Resolve a worktree target (path or branch name) to its full path. + * @param target - Path or branch name to resolve + * @param worktrees - List of all worktrees + * @returns The resolved path, or undefined if not found + */ +function resolveWorktreeTarget(target: string, worktrees: WorktreeEntry[]): string | undefined { + throw new Error("not implemented"); +} + +// ─── Tool Definitions ────────────────────────────────────────────── + +const gitWorktreeCreateTool = defineTool({ + name: "git_worktree_create", + label: "Git Worktree Create", + description: "Create a linked worktree for a branch in the current repo.", + parameters: createWorktreeParams, + async execute(_toolCallId, _params, _signal, _onUpdate, _ctx) { + throw new Error("not implemented"); + }, +}); + +const gitWorktreeListTool = defineTool({ + name: "git_worktree_list", + label: "Git Worktree List", + description: "List all linked worktrees in the current repo.", + parameters: listWorktreeParams, + async execute(_toolCallId, _params, _signal, _onUpdate, _ctx) { + throw new Error("not implemented"); + }, +}); + +const gitWorktreeSwitchTool = defineTool({ + name: "git_worktree_switch", + label: "Git Worktree Switch", + description: "Switch pi's working directory to a linked worktree.", + parameters: switchWorktreeParams, + async execute(_toolCallId, _params, _signal, _onUpdate, _ctx) { + throw new Error("not implemented"); + }, +}); + +const gitWorktreeRemoveTool = defineTool({ + name: "git_worktree_remove", + label: "Git Worktree Remove", + description: "Remove a linked worktree.", + parameters: removeWorktreeParams, + async execute(_toolCallId, _params, _signal, _onUpdate, _ctx) { + throw new Error("not implemented"); + }, +}); + +// ─── Extension Entry Point ──────────────────────────────────────── + +export default function (pi: ExtensionAPI) { + // Register tools + pi.registerTool(gitWorktreeCreateTool); + pi.registerTool(gitWorktreeListTool); + pi.registerTool(gitWorktreeSwitchTool); + pi.registerTool(gitWorktreeRemoveTool); + + // Register /worktree command + pi.registerCommand("worktree", { + description: "Interactive picker to switch between linked worktrees.", + handler: async (_args, _ctx) => { + throw new Error("not implemented"); + }, + }); +} diff --git a/findings.md b/findings.md new file mode 100644 index 0000000..338c464 --- /dev/null +++ b/findings.md @@ -0,0 +1,40 @@ +# Findings — Issue #1: Create Extension File Structure + +## Git Workflow +- Branch: `issue-1` (based on origin/main) +- Remote: `https://git.excelera.net/david/pi-worktrees.git` +- Tool: `fj` CLI for Forgejo issues (auto-detected from git remote) +- PR: Will use `fj pr create` when complete + +## Project Context +- **Extension location**: `.pi/extensions/pi-worktree.ts` +- **Pattern**: Single-file extension with default export function +- **API imports**: `@earendil-works/pi-coding-agent` (ExtensionAPI, ExtensionContext) +- **Schema library**: TypeBox (`typebox`) for tool parameter schemas +- **No npm dependencies** beyond what pi provides + +## Validation Approach +This project has no traditional test suite. Validation is: +1. File exists at `.pi/extensions/pi-worktree.ts` +2. Correct imports from `@earendil-works/pi-coding-agent` and `typebox` +3. Follows single-file extension pattern (default export function) +4. pi loads without errors via `/reload` or restart + +## Issue #1 Acceptance Criteria +- [ ] `.pi/extensions/pi-worktree.ts` exists +- [ ] Correct imports from `@earendil-works/pi-coding-agent` and `typebox` +- [ ] Extension loads without errors when pi starts +- [ ] Follows the single-file extension pattern with default export function + +## Related Files (from main branch) +| File | Purpose | +|------|---------| +| `README.md` | User-facing docs — describes 4 tools + `/worktree` command | +| `DESIGN.md` | Architecture decisions for the extension | +| `DOMAIN.md` | Git worktree domain concepts and terminology | +| `AGENTS.md` | Agent conventions, repo details, fj CLI usage | + +## Session Discoveries +- **TypeBox import**: Use `{ Type } from "@earendil-works/pi-ai"` (re-exported) rather than direct `typebox` import — matches pi's extension examples. +- **Tool definition helper**: Use `defineTool()` from `@earendil-works/pi-coding-agent` for type-safe tool registration. +- **Validation command**: `bun build --no-bundle --emit=metadata` compiles and validates TypeScript without bundling. diff --git a/progress.md b/progress.md new file mode 100644 index 0000000..67722ac --- /dev/null +++ b/progress.md @@ -0,0 +1,26 @@ +# Progress Log — Issue #1: Create Extension File Structure + +## Session 1 — [2025-07-24] + +### Task 001: Scaffold `.pi/extensions/pi-worktree.ts` + +| Phase | Status | Notes | +|-------|--------|-------| +| Phase 0 (Context) | ✅ Done | Branch issue-1 from origin/main, docs found, TypeScript + TypeBox confirmed | +| Step 1 (Read Task) | ✅ Done | Issue #1: Create Extension File Structure. Acceptance: file exists, correct imports, loads without errors | +| Step 2 (Scaffold) | ✅ Done | Created full scaffold with types, tool schemas, helper stubs, and entry point | +| Step 3 (RED) | ✅ Done | Scaffold compiles — stub functions throw "not implemented" as expected | +| Step 4 (GREEN) | ✅ Done | `bun build` succeeds with no errors. All imports resolve from pi's global bun install. + +### Scaffold Contents +- **Types**: `WorktreeEntry`, `WorktreeListResult`, `WorktreeError` +- **Tool schemas**: `createWorktreeParams`, `listWorktreeParams`, `switchWorktreeParams`, `removeWorktreeParams` +- **Helper stubs**: `parseWorktreeList()`, `pathExists()`, `resolveWorktreeTarget()` +- **Tools registered**: `git_worktree_create`, `git_worktree_list`, `git_worktree_switch`, `git_worktree_remove` +- **Command registered**: `/worktree` (stub) + +--- + +### Test Results + +_N/A — validation is pi loading + TypeScript syntax. `bun build` passes with zero errors._ diff --git a/task_plan.md b/task_plan.md new file mode 100644 index 0000000..2c59211 --- /dev/null +++ b/task_plan.md @@ -0,0 +1,18 @@ +# Task Plan — Issue #1: Create Extension File Structure + +| Task | Description | TDD Status | Done | +|------|-------------|------------|------| +| 001 | Scaffold `.pi/extensions/pi-worktree.ts` with correct imports, tools, command, and helper stubs | ☑ RED ✅ GREEN ✅ REFACTOR | ☑ | + +## Milestone Context (from implementation plan) + +- **Milestone 1: Foundation** — Set up extension structure and register basic tools +- **Task 1.1**: Create Extension File — empty file with correct imports, loads without errors +- **Task 1.2**: Register Tool Scaffolding — four tool registrations with placeholder execute functions +- **Task 1.3**: Implement `git_worktree_list` + +## Notes + +- No traditional test framework — validation is: file exists at `.pi/extensions/pi-worktree.ts`, correct imports, pi loads without errors via `/reload` or restart. +- Single-file extension pattern: default export function receiving `ExtensionAPI`. +- Uses fj CLI for issue operations (auto-detected from git remote). -- 2.45.3 From c71a81183ec1ffef6638745aea065c2012844f17 Mon Sep 17 00:00:00 2001 From: David Kong Date: Fri, 24 Jul 2026 18:31:12 +1000 Subject: [PATCH 2/3] Fix code review issues: structured error returns, remove unused count, add JSDoc - Replace throw Error() with { content, isError: true } in all execute functions and command handler - Remove unused count field from WorktreeListResult interface - Add JSDoc comments above all defineTool() calls - Clarify task_plan.md subtask structure (001.1-001.3) --- .pi/extensions/pi-worktree.ts | 33 +++-- issue-1-review.md | 229 ++++++++++++++++++++++++++++++++++ task_plan.md | 10 +- 3 files changed, 260 insertions(+), 12 deletions(-) create mode 100644 issue-1-review.md diff --git a/.pi/extensions/pi-worktree.ts b/.pi/extensions/pi-worktree.ts index 0edd2dd..ea6bbbe 100644 --- a/.pi/extensions/pi-worktree.ts +++ b/.pi/extensions/pi-worktree.ts @@ -31,12 +31,10 @@ interface WorktreeEntry { isCurrent: boolean; } -/** Result of listing all worktrees. */ +/** Result of listing all worktrees. Count available via `worktrees.length`. */ interface WorktreeListResult { /** All worktrees in the repository. */ worktrees: WorktreeEntry[]; - /** Total number of worktrees. */ - count: number; } /** Error returned when a worktree operation fails. */ @@ -103,43 +101,59 @@ function resolveWorktreeTarget(target: string, worktrees: WorktreeEntry[]): stri // ─── Tool Definitions ────────────────────────────────────────────── +/** Create a linked worktree for a branch in the current repo. */ const gitWorktreeCreateTool = defineTool({ name: "git_worktree_create", label: "Git Worktree Create", description: "Create a linked worktree for a branch in the current repo.", parameters: createWorktreeParams, async execute(_toolCallId, _params, _signal, _onUpdate, _ctx) { - throw new Error("not implemented"); + return { + content: [{ type: "text", text: "Error: git_worktree_create is not yet implemented. Suggestion: Use `git worktree add` manually until this tool is complete." }], + isError: true, + }; }, }); +/** List all linked worktrees in the current repo. */ const gitWorktreeListTool = defineTool({ name: "git_worktree_list", label: "Git Worktree List", description: "List all linked worktrees in the current repo.", parameters: listWorktreeParams, async execute(_toolCallId, _params, _signal, _onUpdate, _ctx) { - throw new Error("not implemented"); + return { + content: [{ type: "text", text: "Error: git_worktree_list is not yet implemented. Suggestion: Use `git worktree list` manually until this tool is complete." }], + isError: true, + }; }, }); +/** Switch pi's working directory to a linked worktree. */ const gitWorktreeSwitchTool = defineTool({ name: "git_worktree_switch", label: "Git Worktree Switch", description: "Switch pi's working directory to a linked worktree.", parameters: switchWorktreeParams, async execute(_toolCallId, _params, _signal, _onUpdate, _ctx) { - throw new Error("not implemented"); + return { + content: [{ type: "text", text: "Error: git_worktree_switch is not yet implemented. Suggestion: Use the `/worktree` command or `git worktree` manually until this tool is complete." }], + isError: true, + }; }, }); +/** Remove a linked worktree. */ const gitWorktreeRemoveTool = defineTool({ name: "git_worktree_remove", label: "Git Worktree Remove", description: "Remove a linked worktree.", parameters: removeWorktreeParams, async execute(_toolCallId, _params, _signal, _onUpdate, _ctx) { - throw new Error("not implemented"); + 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, + }; }, }); @@ -156,7 +170,10 @@ export default function (pi: ExtensionAPI) { pi.registerCommand("worktree", { description: "Interactive picker to switch between linked worktrees.", handler: async (_args, _ctx) => { - throw new Error("not implemented"); + return { + content: [{ type: "text", text: "Error: /worktree command is not yet implemented. Suggestion: Use `git worktree list` and navigate manually until this command is complete." }], + isError: true, + }; }, }); } diff --git a/issue-1-review.md b/issue-1-review.md new file mode 100644 index 0000000..9f451a3 --- /dev/null +++ b/issue-1-review.md @@ -0,0 +1,229 @@ +# Code Review: issue-1 vs main + +**Date:** 2025-07-24 +**Language:** TypeScript (pi extension) +**Files changed:** 4 +**Lines added:** +246 / Lines removed: -0 + +## Files Under Review + +| File | Status | Purpose | +|------|--------|---------| +| `.pi/extensions/pi-worktree.ts` | New | Extension scaffold — types, schemas, tool/command stubs | +| `findings.md` | New | Research findings and session discoveries | +| `progress.md` | New | Progress log with TDD phase tracking | +| `task_plan.md` | New | Task plan for issue #1 implementation | + +--- + +## Critical (0) + +_None._ + +## Major (2) + +### [MAJOR] Execute functions throw errors instead of returning structured error objects + +**File:** `.pi/extensions/pi-worktree.ts` +**Lines:** 97, 105, 113, 121, 138 + +**Detail:** All `execute()` functions and the command handler use `throw new Error("not implemented")`. Per AGENTS.md error handling pattern, tools should return structured objects: + +```typescript +return { + content: [{ type: "text", text: `Error: ${message}. Suggestion: ` }], + isError: true +}; +``` + +Even stub implementations should follow this pattern so the extension loads without unhandled exceptions. Throwing from an execute function will crash the tool invocation rather than surfacing a clean error to the user. + +**Fix:** Replace `throw new Error("not implemented")` with structured return objects that include a suggestion, e.g.: + +```typescript +async execute(_toolCallId, _params, _signal, _onUpdate, _ctx) { + return { + content: [{ type: "text", text: "Error: git_worktree_create not yet implemented. Suggestion: Use `git worktree add` manually until this tool is complete." }], + isError: true + }; +} +``` + +### [MAJOR] `WorktreeListResult.count` field has no consumer and duplicates available info + +**File:** `.pi/extensions/pi-worktree.ts` +**Line:** 32 + +**Detail:** The interface defines `count: number` but nothing in the scaffold uses it. Since callers can derive count from `worktrees.length`, this field adds maintenance burden (must stay in sync) without clear value. If included for API convenience, document why it's separate from `.length`. + +**Fix:** Either remove `count` from the interface or add a comment justifying its presence (e.g., "cached to avoid repeated .length calls" or "included for tool response schema consistency"). + +--- + +## Minor (3) + +### [MINOR] `WorktreeError` interface is defined but unused + +**File:** `.pi/extensions/pi-worktree.ts` +**Lines:** 38–43 + +**Detail:** The `WorktreeError` interface (`message`, `suggestion?`) is well-designed and aligns with AGENTS.md conventions, but isn't referenced anywhere in the current code. It will be needed when implementing error returns. + +**Fix:** No action needed for scaffold phase — this is forward-looking type design. Confirm it's used when implementing actual tool logic. + +### [MINOR] `ExtensionContext` imported but only used indirectly via type definitions + +**File:** `.pi/extensions/pi-worktree.ts` +**Line:** 16 + +**Detail:** `ExtensionContext` is imported as a type and referenced in the `pathExists()` helper signature. This is correct forward-looking design — it will be needed when implementing file system checks. No issue, just noting for completeness. + +### [MINOR] Tool definitions lack JSDoc comments unlike helper functions + +**File:** `.pi/extensions/pi-worktree.ts` +**Lines:** 95–123 + +**Detail:** Helper functions (`parseWorktreeList`, `pathExists`, `resolveWorktreeTarget`) have thorough JSDoc with parameter descriptions and return types. The tool definitions (`gitWorktreeCreateTool`, etc.) have no comments. For a single-file extension, inline documentation helps future maintainers understand each tool's contract at a glance. + +**Fix:** Add brief JSDoc above each `defineTool()` call summarizing its purpose and key behaviors: + +```typescript +/** + * Create a linked worktree for a branch. + * Runs `git worktree add [branch]` with optional `-b` flag. + */ +const gitWorktreeCreateTool = defineTool({ ... }); +``` + +--- + +## Suggestions (2) + +### [SUGGESTION] Consider adding a `pi.exec()` usage example in comments for future implementers + +**File:** `.pi/extensions/pi-worktree.ts` + +**Detail:** AGENTS.md specifies "Git: native via `pi.exec("git", [...])`" but the scaffold has no indication of how git commands will be invoked. A comment near the helper functions showing the expected pattern would reduce friction for the next implementation phase: + +```typescript +// Example: const result = await ctx.exec("git", ["worktree", "add", path, branch]); +``` + +### [SUGGESTION] `task_plan.md` lists only Task 001 but milestone context references Tasks 1.1–1.3 + +**File:** `task_plan.md` +**Lines:** 6, 12–14 + +**Detail:** The task table has one entry (Task 001) marked done, but the milestone context below references three subtasks (1.1: Create Extension File, 1.2: Register Tool Scaffolding, 1.3: Implement `git_worktree_list`). This creates ambiguity about whether Task 001 encompasses all three or if 1.2 and 1.3 are still pending. + +**Fix:** Either expand the task table to list subtasks 1.1–1.3 separately with individual status, or clarify that Task 001 covers all three milestones. + +--- + +## Summary + +| Severity | Count | +|----------|-------| +| Critical | 0 | +| Major | 2 | +| Minor | 3 | +| Suggestions | 2 | +| **Total** | **7** | + +⚡ No critical issues. Major issues should be addressed before the extension is loadable in pi — specifically, execute functions must return structured error objects instead of throwing to avoid unhandled exceptions at runtime. + +--- + +## Review Fix Plan + +| Task | Issue | Severity | File | Description | +|------|-------|----------|------|-------------| +| 001 | Execute stubs throw instead of returning errors | major | `.pi/extensions/pi-worktree.ts` | Replace `throw new Error()` with structured `{ content, isError: true }` returns in all execute functions and command handler | +| 002 | Unused `count` field in `WorktreeListResult` | major | `.pi/extensions/pi-worktree.ts` | Remove or document the `count` field | +| 003 | Tool definitions lack JSDoc | minor | `.pi/extensions/pi-worktree.ts` | Add JSDoc comments above each `defineTool()` call | +| 004 | Task plan subtask ambiguity | suggestion | `task_plan.md` | Clarify relationship between Task 001 and milestone subtasks 1.1–1.3 | + +### Task 001: Execute stubs throw instead of returning errors + +**Severity:** major +**File:** `.pi/extensions/pi-worktree.ts` +**Lines:** 97, 105, 113, 121, 138 + +**Issue:** All five execute/handler functions use `throw new Error("not implemented")`. When pi invokes a tool, an uncaught exception will crash the tool invocation rather than returning a clean error message to the user. AGENTS.md specifies the pattern: return `{ content: [{ type: "text", text }], isError: true }`. + +**Fix:** For each of the 4 tools and 1 command handler, replace the throw with: + +```typescript +async execute(_toolCallId, _params, _signal, _onUpdate, _ctx) { + return { + content: [{ type: "text", text: `Error: is not yet implemented. Suggestion: Use git worktree commands directly until this tool is complete.` }], + isError: true + }; +} +``` + +**Verification:** Extension loads in pi without errors; calling any tool returns an error message instead of crashing. + +### Task 002: Unused `count` field in `WorktreeListResult` + +**Severity:** major +**File:** `.pi/extensions/pi-worktree.ts` +**Line:** 32 + +**Issue:** `count: number` is defined but never populated or consumed. Callers can use `worktrees.length`. Dead fields create maintenance burden and confusion about whether they're intentional API design or leftover scaffolding. + +**Fix:** Remove the `count` field from `WorktreeListResult`: + +```typescript +interface WorktreeListResult { + worktrees: WorktreeEntry[]; +} +``` + +Or add a comment if intentionally kept for API schema reasons. + +**Verification:** No references to `.count` in code; interface matches actual usage. + +### Task 003: Tool definitions lack JSDoc + +**Severity:** minor +**File:** `.pi/extensions/pi-worktree.ts` +**Lines:** 95–123 + +**Issue:** Helper functions have thorough JSDoc but tool definitions don't. In a single-file extension, inline docs help readers scan the file and understand each tool's purpose without reading implementation details. + +**Fix:** Add JSDoc above each `defineTool()`: + +```typescript +/** Create a linked worktree for a branch in the current repo. */ +const gitWorktreeCreateTool = defineTool({ ... }); + +/** List all linked worktrees in the current repo. */ +const gitWorktreeListTool = defineTool({ ... }); + +/** Switch pi's working directory to a linked worktree. */ +const gitWorktreeSwitchTool = defineTool({ ... }); + +/** Remove a linked worktree. */ +const gitWorktreeRemoveTool = defineTool({ ... }); +``` + +**Verification:** Each tool definition has a JSDoc comment matching its description field. + +### Task 004: Task plan subtask ambiguity + +**Severity:** suggestion +**File:** `task_plan.md` +**Lines:** 6, 12–14 + +**Issue:** The task table shows one completed Task 001, but the milestone context references three distinct subtasks (1.1, 1.2, 1.3). It's unclear whether all are done or only partially complete. + +**Fix:** Expand the task table: + +| Task | Description | Done | +|------|-------------|------| +| 001.1 | Create extension file with correct imports | ☑ | +| 001.2 | Register tool scaffolding (4 tools + command) | ☑ | +| 001.3 | Implement `git_worktree_list` | ☐ | + +**Verification:** Task plan clearly shows what's done vs pending within the issue scope. diff --git a/task_plan.md b/task_plan.md index 2c59211..8055bd5 100644 --- a/task_plan.md +++ b/task_plan.md @@ -2,14 +2,16 @@ | Task | Description | TDD Status | Done | |------|-------------|------------|------| -| 001 | Scaffold `.pi/extensions/pi-worktree.ts` with correct imports, tools, command, and helper stubs | ☑ RED ✅ GREEN ✅ REFACTOR | ☑ | +| 001.1 | Create extension file with correct imports and entry point | ✅ GREEN ✅ REFACTOR | ✅ | +| 001.2 | Register tool scaffolding (4 tools + command) with structured error returns | ✅ GREEN ✅ REFACTOR | ✅ | +| 001.3 | Implement `git_worktree_list` | ☐ RED ☐ GREEN ☐ REFACTOR | ☐ | ## Milestone Context (from implementation plan) - **Milestone 1: Foundation** — Set up extension structure and register basic tools -- **Task 1.1**: Create Extension File — empty file with correct imports, loads without errors -- **Task 1.2**: Register Tool Scaffolding — four tool registrations with placeholder execute functions -- **Task 1.3**: Implement `git_worktree_list` +- **Task 1.1**: Create Extension File — empty file with correct imports, loads without errors ✅ +- **Task 1.2**: Register Tool Scaffolding — four tool registrations with placeholder execute functions (structured error returns) ✅ +- **Task 1.3**: Implement `git_worktree_list` ☐ ## Notes -- 2.45.3 From ce0a350181d89485092f266c1a1f2330a666b2c8 Mon Sep 17 00:00:00 2001 From: David Kong Date: Fri, 24 Jul 2026 18:39:32 +1000 Subject: [PATCH 3/3] Add .gitignore for planning files and remove them from tracking --- .gitignore | 5 + findings.md | 40 -------- issue-1-review.md | 229 ---------------------------------------------- progress.md | 26 ------ task_plan.md | 20 ---- 5 files changed, 5 insertions(+), 315 deletions(-) create mode 100644 .gitignore delete mode 100644 findings.md delete mode 100644 issue-1-review.md delete mode 100644 progress.md delete mode 100644 task_plan.md diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..8fa6188 --- /dev/null +++ b/.gitignore @@ -0,0 +1,5 @@ +# Planning files (local working documents) +findings.md +progress.md +task_plan.md +issue-*-review.md diff --git a/findings.md b/findings.md deleted file mode 100644 index 338c464..0000000 --- a/findings.md +++ /dev/null @@ -1,40 +0,0 @@ -# Findings — Issue #1: Create Extension File Structure - -## Git Workflow -- Branch: `issue-1` (based on origin/main) -- Remote: `https://git.excelera.net/david/pi-worktrees.git` -- Tool: `fj` CLI for Forgejo issues (auto-detected from git remote) -- PR: Will use `fj pr create` when complete - -## Project Context -- **Extension location**: `.pi/extensions/pi-worktree.ts` -- **Pattern**: Single-file extension with default export function -- **API imports**: `@earendil-works/pi-coding-agent` (ExtensionAPI, ExtensionContext) -- **Schema library**: TypeBox (`typebox`) for tool parameter schemas -- **No npm dependencies** beyond what pi provides - -## Validation Approach -This project has no traditional test suite. Validation is: -1. File exists at `.pi/extensions/pi-worktree.ts` -2. Correct imports from `@earendil-works/pi-coding-agent` and `typebox` -3. Follows single-file extension pattern (default export function) -4. pi loads without errors via `/reload` or restart - -## Issue #1 Acceptance Criteria -- [ ] `.pi/extensions/pi-worktree.ts` exists -- [ ] Correct imports from `@earendil-works/pi-coding-agent` and `typebox` -- [ ] Extension loads without errors when pi starts -- [ ] Follows the single-file extension pattern with default export function - -## Related Files (from main branch) -| File | Purpose | -|------|---------| -| `README.md` | User-facing docs — describes 4 tools + `/worktree` command | -| `DESIGN.md` | Architecture decisions for the extension | -| `DOMAIN.md` | Git worktree domain concepts and terminology | -| `AGENTS.md` | Agent conventions, repo details, fj CLI usage | - -## Session Discoveries -- **TypeBox import**: Use `{ Type } from "@earendil-works/pi-ai"` (re-exported) rather than direct `typebox` import — matches pi's extension examples. -- **Tool definition helper**: Use `defineTool()` from `@earendil-works/pi-coding-agent` for type-safe tool registration. -- **Validation command**: `bun build --no-bundle --emit=metadata` compiles and validates TypeScript without bundling. diff --git a/issue-1-review.md b/issue-1-review.md deleted file mode 100644 index 9f451a3..0000000 --- a/issue-1-review.md +++ /dev/null @@ -1,229 +0,0 @@ -# Code Review: issue-1 vs main - -**Date:** 2025-07-24 -**Language:** TypeScript (pi extension) -**Files changed:** 4 -**Lines added:** +246 / Lines removed: -0 - -## Files Under Review - -| File | Status | Purpose | -|------|--------|---------| -| `.pi/extensions/pi-worktree.ts` | New | Extension scaffold — types, schemas, tool/command stubs | -| `findings.md` | New | Research findings and session discoveries | -| `progress.md` | New | Progress log with TDD phase tracking | -| `task_plan.md` | New | Task plan for issue #1 implementation | - ---- - -## Critical (0) - -_None._ - -## Major (2) - -### [MAJOR] Execute functions throw errors instead of returning structured error objects - -**File:** `.pi/extensions/pi-worktree.ts` -**Lines:** 97, 105, 113, 121, 138 - -**Detail:** All `execute()` functions and the command handler use `throw new Error("not implemented")`. Per AGENTS.md error handling pattern, tools should return structured objects: - -```typescript -return { - content: [{ type: "text", text: `Error: ${message}. Suggestion: ` }], - isError: true -}; -``` - -Even stub implementations should follow this pattern so the extension loads without unhandled exceptions. Throwing from an execute function will crash the tool invocation rather than surfacing a clean error to the user. - -**Fix:** Replace `throw new Error("not implemented")` with structured return objects that include a suggestion, e.g.: - -```typescript -async execute(_toolCallId, _params, _signal, _onUpdate, _ctx) { - return { - content: [{ type: "text", text: "Error: git_worktree_create not yet implemented. Suggestion: Use `git worktree add` manually until this tool is complete." }], - isError: true - }; -} -``` - -### [MAJOR] `WorktreeListResult.count` field has no consumer and duplicates available info - -**File:** `.pi/extensions/pi-worktree.ts` -**Line:** 32 - -**Detail:** The interface defines `count: number` but nothing in the scaffold uses it. Since callers can derive count from `worktrees.length`, this field adds maintenance burden (must stay in sync) without clear value. If included for API convenience, document why it's separate from `.length`. - -**Fix:** Either remove `count` from the interface or add a comment justifying its presence (e.g., "cached to avoid repeated .length calls" or "included for tool response schema consistency"). - ---- - -## Minor (3) - -### [MINOR] `WorktreeError` interface is defined but unused - -**File:** `.pi/extensions/pi-worktree.ts` -**Lines:** 38–43 - -**Detail:** The `WorktreeError` interface (`message`, `suggestion?`) is well-designed and aligns with AGENTS.md conventions, but isn't referenced anywhere in the current code. It will be needed when implementing error returns. - -**Fix:** No action needed for scaffold phase — this is forward-looking type design. Confirm it's used when implementing actual tool logic. - -### [MINOR] `ExtensionContext` imported but only used indirectly via type definitions - -**File:** `.pi/extensions/pi-worktree.ts` -**Line:** 16 - -**Detail:** `ExtensionContext` is imported as a type and referenced in the `pathExists()` helper signature. This is correct forward-looking design — it will be needed when implementing file system checks. No issue, just noting for completeness. - -### [MINOR] Tool definitions lack JSDoc comments unlike helper functions - -**File:** `.pi/extensions/pi-worktree.ts` -**Lines:** 95–123 - -**Detail:** Helper functions (`parseWorktreeList`, `pathExists`, `resolveWorktreeTarget`) have thorough JSDoc with parameter descriptions and return types. The tool definitions (`gitWorktreeCreateTool`, etc.) have no comments. For a single-file extension, inline documentation helps future maintainers understand each tool's contract at a glance. - -**Fix:** Add brief JSDoc above each `defineTool()` call summarizing its purpose and key behaviors: - -```typescript -/** - * Create a linked worktree for a branch. - * Runs `git worktree add [branch]` with optional `-b` flag. - */ -const gitWorktreeCreateTool = defineTool({ ... }); -``` - ---- - -## Suggestions (2) - -### [SUGGESTION] Consider adding a `pi.exec()` usage example in comments for future implementers - -**File:** `.pi/extensions/pi-worktree.ts` - -**Detail:** AGENTS.md specifies "Git: native via `pi.exec("git", [...])`" but the scaffold has no indication of how git commands will be invoked. A comment near the helper functions showing the expected pattern would reduce friction for the next implementation phase: - -```typescript -// Example: const result = await ctx.exec("git", ["worktree", "add", path, branch]); -``` - -### [SUGGESTION] `task_plan.md` lists only Task 001 but milestone context references Tasks 1.1–1.3 - -**File:** `task_plan.md` -**Lines:** 6, 12–14 - -**Detail:** The task table has one entry (Task 001) marked done, but the milestone context below references three subtasks (1.1: Create Extension File, 1.2: Register Tool Scaffolding, 1.3: Implement `git_worktree_list`). This creates ambiguity about whether Task 001 encompasses all three or if 1.2 and 1.3 are still pending. - -**Fix:** Either expand the task table to list subtasks 1.1–1.3 separately with individual status, or clarify that Task 001 covers all three milestones. - ---- - -## Summary - -| Severity | Count | -|----------|-------| -| Critical | 0 | -| Major | 2 | -| Minor | 3 | -| Suggestions | 2 | -| **Total** | **7** | - -⚡ No critical issues. Major issues should be addressed before the extension is loadable in pi — specifically, execute functions must return structured error objects instead of throwing to avoid unhandled exceptions at runtime. - ---- - -## Review Fix Plan - -| Task | Issue | Severity | File | Description | -|------|-------|----------|------|-------------| -| 001 | Execute stubs throw instead of returning errors | major | `.pi/extensions/pi-worktree.ts` | Replace `throw new Error()` with structured `{ content, isError: true }` returns in all execute functions and command handler | -| 002 | Unused `count` field in `WorktreeListResult` | major | `.pi/extensions/pi-worktree.ts` | Remove or document the `count` field | -| 003 | Tool definitions lack JSDoc | minor | `.pi/extensions/pi-worktree.ts` | Add JSDoc comments above each `defineTool()` call | -| 004 | Task plan subtask ambiguity | suggestion | `task_plan.md` | Clarify relationship between Task 001 and milestone subtasks 1.1–1.3 | - -### Task 001: Execute stubs throw instead of returning errors - -**Severity:** major -**File:** `.pi/extensions/pi-worktree.ts` -**Lines:** 97, 105, 113, 121, 138 - -**Issue:** All five execute/handler functions use `throw new Error("not implemented")`. When pi invokes a tool, an uncaught exception will crash the tool invocation rather than returning a clean error message to the user. AGENTS.md specifies the pattern: return `{ content: [{ type: "text", text }], isError: true }`. - -**Fix:** For each of the 4 tools and 1 command handler, replace the throw with: - -```typescript -async execute(_toolCallId, _params, _signal, _onUpdate, _ctx) { - return { - content: [{ type: "text", text: `Error: is not yet implemented. Suggestion: Use git worktree commands directly until this tool is complete.` }], - isError: true - }; -} -``` - -**Verification:** Extension loads in pi without errors; calling any tool returns an error message instead of crashing. - -### Task 002: Unused `count` field in `WorktreeListResult` - -**Severity:** major -**File:** `.pi/extensions/pi-worktree.ts` -**Line:** 32 - -**Issue:** `count: number` is defined but never populated or consumed. Callers can use `worktrees.length`. Dead fields create maintenance burden and confusion about whether they're intentional API design or leftover scaffolding. - -**Fix:** Remove the `count` field from `WorktreeListResult`: - -```typescript -interface WorktreeListResult { - worktrees: WorktreeEntry[]; -} -``` - -Or add a comment if intentionally kept for API schema reasons. - -**Verification:** No references to `.count` in code; interface matches actual usage. - -### Task 003: Tool definitions lack JSDoc - -**Severity:** minor -**File:** `.pi/extensions/pi-worktree.ts` -**Lines:** 95–123 - -**Issue:** Helper functions have thorough JSDoc but tool definitions don't. In a single-file extension, inline docs help readers scan the file and understand each tool's purpose without reading implementation details. - -**Fix:** Add JSDoc above each `defineTool()`: - -```typescript -/** Create a linked worktree for a branch in the current repo. */ -const gitWorktreeCreateTool = defineTool({ ... }); - -/** List all linked worktrees in the current repo. */ -const gitWorktreeListTool = defineTool({ ... }); - -/** Switch pi's working directory to a linked worktree. */ -const gitWorktreeSwitchTool = defineTool({ ... }); - -/** Remove a linked worktree. */ -const gitWorktreeRemoveTool = defineTool({ ... }); -``` - -**Verification:** Each tool definition has a JSDoc comment matching its description field. - -### Task 004: Task plan subtask ambiguity - -**Severity:** suggestion -**File:** `task_plan.md` -**Lines:** 6, 12–14 - -**Issue:** The task table shows one completed Task 001, but the milestone context references three distinct subtasks (1.1, 1.2, 1.3). It's unclear whether all are done or only partially complete. - -**Fix:** Expand the task table: - -| Task | Description | Done | -|------|-------------|------| -| 001.1 | Create extension file with correct imports | ☑ | -| 001.2 | Register tool scaffolding (4 tools + command) | ☑ | -| 001.3 | Implement `git_worktree_list` | ☐ | - -**Verification:** Task plan clearly shows what's done vs pending within the issue scope. diff --git a/progress.md b/progress.md deleted file mode 100644 index 67722ac..0000000 --- a/progress.md +++ /dev/null @@ -1,26 +0,0 @@ -# Progress Log — Issue #1: Create Extension File Structure - -## Session 1 — [2025-07-24] - -### Task 001: Scaffold `.pi/extensions/pi-worktree.ts` - -| Phase | Status | Notes | -|-------|--------|-------| -| Phase 0 (Context) | ✅ Done | Branch issue-1 from origin/main, docs found, TypeScript + TypeBox confirmed | -| Step 1 (Read Task) | ✅ Done | Issue #1: Create Extension File Structure. Acceptance: file exists, correct imports, loads without errors | -| Step 2 (Scaffold) | ✅ Done | Created full scaffold with types, tool schemas, helper stubs, and entry point | -| Step 3 (RED) | ✅ Done | Scaffold compiles — stub functions throw "not implemented" as expected | -| Step 4 (GREEN) | ✅ Done | `bun build` succeeds with no errors. All imports resolve from pi's global bun install. - -### Scaffold Contents -- **Types**: `WorktreeEntry`, `WorktreeListResult`, `WorktreeError` -- **Tool schemas**: `createWorktreeParams`, `listWorktreeParams`, `switchWorktreeParams`, `removeWorktreeParams` -- **Helper stubs**: `parseWorktreeList()`, `pathExists()`, `resolveWorktreeTarget()` -- **Tools registered**: `git_worktree_create`, `git_worktree_list`, `git_worktree_switch`, `git_worktree_remove` -- **Command registered**: `/worktree` (stub) - ---- - -### Test Results - -_N/A — validation is pi loading + TypeScript syntax. `bun build` passes with zero errors._ diff --git a/task_plan.md b/task_plan.md deleted file mode 100644 index 8055bd5..0000000 --- a/task_plan.md +++ /dev/null @@ -1,20 +0,0 @@ -# Task Plan — Issue #1: Create Extension File Structure - -| Task | Description | TDD Status | Done | -|------|-------------|------------|------| -| 001.1 | Create extension file with correct imports and entry point | ✅ GREEN ✅ REFACTOR | ✅ | -| 001.2 | Register tool scaffolding (4 tools + command) with structured error returns | ✅ GREEN ✅ REFACTOR | ✅ | -| 001.3 | Implement `git_worktree_list` | ☐ RED ☐ GREEN ☐ REFACTOR | ☐ | - -## Milestone Context (from implementation plan) - -- **Milestone 1: Foundation** — Set up extension structure and register basic tools -- **Task 1.1**: Create Extension File — empty file with correct imports, loads without errors ✅ -- **Task 1.2**: Register Tool Scaffolding — four tool registrations with placeholder execute functions (structured error returns) ✅ -- **Task 1.3**: Implement `git_worktree_list` ☐ - -## Notes - -- No traditional test framework — validation is: file exists at `.pi/extensions/pi-worktree.ts`, correct imports, pi loads without errors via `/reload` or restart. -- Single-file extension pattern: default export function receiving `ExtensionAPI`. -- Uses fj CLI for issue operations (auto-detected from git remote). -- 2.45.3