From b0dfac9c0da776a12573d6ba1bb4cfa327057876 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Mon, 30 Mar 2026 00:23:02 -0400 Subject: [PATCH 1/4] feat(penpal): live-detect worktree additions and removals Watch each project's .git/worktrees/ directory so that `git worktree add` and `git worktree remove` are detected without restarting the server. On change, full re-discovery runs and an SSE event pushes the updated worktree list to the frontend. Co-Authored-By: Claude Opus 4.6 --- apps/penpal/ERD.md | 3 + apps/penpal/PRODUCT.md | 2 +- apps/penpal/TESTING.md | 1 + apps/penpal/internal/discovery/worktree.go | 27 ++++ .../internal/discovery/worktree_test.go | 122 +++++++++++++++ apps/penpal/internal/watcher/watcher.go | 45 +++++- apps/penpal/internal/watcher/watcher_test.go | 140 ++++++++++++++++++ 7 files changed, 335 insertions(+), 5 deletions(-) diff --git a/apps/penpal/ERD.md b/apps/penpal/ERD.md index 1089932b2..e207e0f48 100644 --- a/apps/penpal/ERD.md +++ b/apps/penpal/ERD.md @@ -91,6 +91,9 @@ see-also: - **E-PENPAL-WORKTREE-DISCOVERY**: Worktrees are discovered by parsing `git worktree list --porcelain` output. Each worktree gets a name, path, branch, and `IsMain` flag. The `refs/heads/` prefix is stripped from branch names. ← [P-PENPAL-WORKTREE](PRODUCT.md#P-PENPAL-WORKTREE) +- **E-PENPAL-WORKTREE-WATCH**: The watcher monitors each project's `.git/worktrees/` directory (for the main worktree) or the equivalent resolved via `git rev-parse --git-common-dir`. When entries are created or removed in that directory, the watcher re-runs `DiscoverWorktrees` for the affected project, updates the cached worktree list, and broadcasts a `projects` SSE event so the frontend reflects the change. + ← [P-PENPAL-WORKTREE](PRODUCT.md#P-PENPAL-WORKTREE) + - **E-PENPAL-CLAUDE-PLANS-DETECT**: `DiscoverClaudePlans()` checks `~/.claude/plans/` for existence and at least one `.md` file. If found, a synthetic standalone project is injected. If the user already manually added the same path, a tree source is injected into the existing entry instead of duplicating. ← [P-PENPAL-CLAUDE-PLANS](PRODUCT.md#P-PENPAL-CLAUDE-PLANS) diff --git a/apps/penpal/PRODUCT.md b/apps/penpal/PRODUCT.md index 5f132a7ad..cec77bf54 100644 --- a/apps/penpal/PRODUCT.md +++ b/apps/penpal/PRODUCT.md @@ -22,7 +22,7 @@ Penpal is a desktop application and local web server for collaborative review of - **P-PENPAL-STANDALONE**: Users can add standalone projects (directories or individual files) outside of any workspace, via the home view "+" button or the `penpal open` CLI command. -- **P-PENPAL-WORKTREE**: Git worktrees for a project are discovered automatically. In the home view, multi-worktree projects expand to show each worktree as a child item with its branch name. In the project view, a worktree dropdown in the breadcrumb bar lets the user switch between worktrees. Each worktree has its own branch name and independent comment storage. +- **P-PENPAL-WORKTREE**: Git worktrees for a project are discovered automatically. In the home view, multi-worktree projects expand to show each worktree as a child item with its branch name. In the project view, a worktree dropdown in the breadcrumb bar lets the user switch between worktrees. Each worktree has its own branch name and independent comment storage. When worktrees are added or removed (via `git worktree add`/`remove`), the worktree list updates without restarting the server. - **P-PENPAL-DEDUP**: When multiple directories in a workspace share the same git repository (one is a worktree of the other), only the main worktree is shown as a project to avoid duplicates. diff --git a/apps/penpal/TESTING.md b/apps/penpal/TESTING.md index a184ebafb..2d8a4fb86 100644 --- a/apps/penpal/TESTING.md +++ b/apps/penpal/TESTING.md @@ -66,6 +66,7 @@ see-also: | Source Types — manual (P-PENPAL-SRC-MANUAL) | — | — | grouping_test.go (TestBuildFileGroups_ManualSourceDirHeadings) | — | | Cache & File Scanning (E-PENPAL-CACHE, SCAN) | cache_test.go (TestCheckAllProjectsHasFiles, TestProjectHasAnyMarkdown_SkipsGitignored, TestProjectHasAnyMarkdown_SkipsVCSDirs, TestAllFiles_DeduplicatesAllMarkdown, TestEnsureProjectScanned_NoDuplicateScans) | — | — | — | | Worktree Support (P-PENPAL-WORKTREE) | discovery/worktree_test.go, cache/worktree_test.go | Layout.test.tsx | worktree_test.go (API + MCP) | — | +| Worktree Watch (E-PENPAL-WORKTREE-WATCH) | watcher_test.go | — | — | — | | Worktree Dropdown (P-PENPAL-PROJECT-WORKTREE-DROPDOWN) | — | Layout.test.tsx | — | — | | Git Integration (P-PENPAL-GIT-INFO) | — | — | — | — | | File List & Grouping (P-PENPAL-FILE-LIST) | — | ProjectPage.test.tsx | grouping_test.go, integration_test.go | — | diff --git a/apps/penpal/internal/discovery/worktree.go b/apps/penpal/internal/discovery/worktree.go index ce0af4e15..24b7a15a5 100644 --- a/apps/penpal/internal/discovery/worktree.go +++ b/apps/penpal/internal/discovery/worktree.go @@ -1,6 +1,7 @@ package discovery import ( + "os" "os/exec" "path/filepath" "strings" @@ -142,3 +143,29 @@ func FindMainWorktree(path string) string { return "" } + +// GitWorktreesDir returns the path to the .git/worktrees/ directory for the +// repository that projectPath belongs to, or "" if it doesn't exist. +// Works for both main worktrees (.git is a directory) and linked worktrees +// (.git is a file pointing to the main repo's gitdir). +// E-PENPAL-WORKTREE-WATCH: resolves the worktrees metadata directory for fs watching. +func GitWorktreesDir(projectPath string) string { + cmd := exec.Command("git", "-C", projectPath, "rev-parse", "--git-common-dir") + out, err := cmd.Output() + if err != nil { + return "" + } + commonDir := strings.TrimSpace(string(out)) + if commonDir == "" || commonDir == "." { + return "" + } + // --git-common-dir output is relative to the -C directory + if !filepath.IsAbs(commonDir) { + commonDir = filepath.Join(projectPath, commonDir) + } + wtDir := filepath.Join(filepath.Clean(commonDir), "worktrees") + if info, err := os.Stat(wtDir); err == nil && info.IsDir() { + return wtDir + } + return "" +} diff --git a/apps/penpal/internal/discovery/worktree_test.go b/apps/penpal/internal/discovery/worktree_test.go index 384859191..3e12b5a93 100644 --- a/apps/penpal/internal/discovery/worktree_test.go +++ b/apps/penpal/internal/discovery/worktree_test.go @@ -1,6 +1,9 @@ package discovery import ( + "os" + "os/exec" + "path/filepath" "testing" ) @@ -100,3 +103,122 @@ func TestParseWorktreeList_BranchStripping(t *testing.T) { t.Errorf("wt branch = %q, want %q", got[1].Branch, "feature/nested") } } + +// initGitRepo creates a git repo in dir with an initial commit. +func initGitRepo(t *testing.T, dir string) { + t.Helper() + for _, args := range [][]string{ + {"init"}, + {"config", "user.email", "test@test.com"}, + {"config", "user.name", "Test"}, + {"commit", "--allow-empty", "-m", "init"}, + } { + cmd := exec.Command("git", append([]string{"-C", dir}, args...)...) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + } +} + +// resolveSymlinks resolves symlinks in a path for reliable comparison on macOS +// where /var → /private/var. +func resolveSymlinks(t *testing.T, path string) string { + t.Helper() + resolved, err := filepath.EvalSymlinks(path) + if err != nil { + t.Fatalf("EvalSymlinks(%q): %v", path, err) + } + return resolved +} + +// E-PENPAL-WORKTREE-WATCH: verifies GitWorktreesDir returns the .git/worktrees/ dir for a repo with worktrees. +func TestGitWorktreesDir_MainWorktree(t *testing.T) { + mainDir := resolveSymlinks(t, t.TempDir()) + initGitRepo(t, mainDir) + + // Before adding a worktree, the dir doesn't exist + if got := GitWorktreesDir(mainDir); got != "" { + t.Fatalf("expected empty before worktree add, got %q", got) + } + + // Add a worktree + wtDir := filepath.Join(resolveSymlinks(t, t.TempDir()), "my-worktree") + cmd := exec.Command("git", "-C", mainDir, "worktree", "add", "-b", "test-branch", wtDir) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git worktree add: %v\n%s", err, out) + } + + // Now GitWorktreesDir should return the .git/worktrees/ path + got := GitWorktreesDir(mainDir) + want := filepath.Join(mainDir, ".git", "worktrees") + if got != want { + t.Errorf("GitWorktreesDir(main) = %q, want %q", got, want) + } + + // It should also work when called from the linked worktree + got2 := GitWorktreesDir(wtDir) + if got2 != want { + t.Errorf("GitWorktreesDir(linked) = %q, want %q", got2, want) + } +} + +// E-PENPAL-WORKTREE-WATCH: verifies GitWorktreesDir returns "" for a non-git directory. +func TestGitWorktreesDir_NotGitRepo(t *testing.T) { + dir := t.TempDir() + if got := GitWorktreesDir(dir); got != "" { + t.Errorf("GitWorktreesDir(non-git) = %q, want empty", got) + } +} + +// E-PENPAL-WORKTREE-WATCH: verifies GitWorktreesDir returns "" for a repo with no worktrees. +func TestGitWorktreesDir_NoWorktrees(t *testing.T) { + dir := t.TempDir() + initGitRepo(t, dir) + if got := GitWorktreesDir(dir); got != "" { + t.Errorf("GitWorktreesDir(no worktrees) = %q, want empty", got) + } +} + +// E-PENPAL-WORKTREE-WATCH: verifies worktree directory appears after git worktree add +// and disappears after git worktree remove. +func TestGitWorktreesDir_AddRemoveCycle(t *testing.T) { + mainDir := resolveSymlinks(t, t.TempDir()) + initGitRepo(t, mainDir) + + wtPath := filepath.Join(resolveSymlinks(t, t.TempDir()), "wt") + cmd := exec.Command("git", "-C", mainDir, "worktree", "add", "-b", "wt-branch", wtPath) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git worktree add: %v\n%s", err, out) + } + + wtDir := GitWorktreesDir(mainDir) + if wtDir == "" { + t.Fatal("expected non-empty after add") + } + + // Verify the specific worktree entry exists + entries, err := os.ReadDir(wtDir) + if err != nil { + t.Fatal(err) + } + found := false + for _, e := range entries { + if e.Name() == filepath.Base(wtPath) { + found = true + } + } + if !found { + t.Errorf("expected entry %q in %s", filepath.Base(wtPath), wtDir) + } + + // Remove the worktree + cmd = exec.Command("git", "-C", mainDir, "worktree", "remove", wtPath) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git worktree remove: %v\n%s", err, out) + } + + // After removing the last worktree, the worktrees/ dir should be gone + if got := GitWorktreesDir(mainDir); got != "" { + t.Errorf("expected empty after removing last worktree, got %q", got) + } +} diff --git a/apps/penpal/internal/watcher/watcher.go b/apps/penpal/internal/watcher/watcher.go index c5e96f32f..9686023fe 100644 --- a/apps/penpal/internal/watcher/watcher.go +++ b/apps/penpal/internal/watcher/watcher.go @@ -70,6 +70,9 @@ type Watcher struct { windowFocuses map[string]focusTarget baseWatched map[string]struct{} dynamicWatched map[string]struct{} + + // E-PENPAL-WORKTREE-WATCH: tracks .git/worktrees/ dirs to detect worktree add/remove. + worktreeWatchDirs map[string]struct{} } // New creates a new watcher @@ -87,9 +90,10 @@ func New(c *cache.Cache, act *activity.Tracker) (*Watcher, error) { done: make(chan struct{}), subs: make(map[chan Event]struct{}), debounce: make(map[string]*time.Timer), - windowFocuses: make(map[string]focusTarget), - baseWatched: make(map[string]struct{}), - dynamicWatched: make(map[string]struct{}), + windowFocuses: make(map[string]focusTarget), + baseWatched: make(map[string]struct{}), + dynamicWatched: make(map[string]struct{}), + worktreeWatchDirs: make(map[string]struct{}), } return w, nil @@ -330,6 +334,19 @@ func (w *Watcher) syncBaseWatchesLocked(workspacePaths []string, projects []disc desired[filepath.Clean(p.Path)] = struct{}{} } + // E-PENPAL-WORKTREE-WATCH: watch .git/worktrees/ dirs to detect worktree add/remove. + desiredWtDirs := make(map[string]struct{}) + for _, p := range projects { + if len(p.Worktrees) == 0 { + continue + } + if wtDir := discovery.GitWorktreesDir(p.Path); wtDir != "" { + clean := filepath.Clean(wtDir) + desiredWtDirs[clean] = struct{}{} + desired[clean] = struct{}{} + } + } + for path := range w.baseWatched { if _, ok := desired[path]; ok { continue @@ -355,6 +372,8 @@ func (w *Watcher) syncBaseWatchesLocked(workspacePaths []string, projects []disc } w.baseWatched[path] = struct{}{} } + + w.worktreeWatchDirs = desiredWtDirs } func (w *Watcher) syncDynamicWatchesLocked() { @@ -479,8 +498,26 @@ func (w *Watcher) loop() { func (w *Watcher) handleEvent(event fsnotify.Event) { path := filepath.Clean(event.Name) - // Check if this is a change in a workspace directory (new/removed project) + // E-PENPAL-WORKTREE-WATCH: detect worktree add/remove via .git/worktrees/ changes. parentDir := filepath.Clean(filepath.Dir(path)) + if _, ok := w.worktreeWatchDirs[parentDir]; ok { + w.debounceRefresh("worktrees:"+parentDir, func() { + if w.discoverFn != nil { + projects, err := w.discoverFn() + if err == nil { + w.cache.RescanWith(projects) + w.focusMu.Lock() + w.syncBaseWatchesLocked(w.workspacePaths, projects) + w.syncDynamicWatchesLocked() + w.focusMu.Unlock() + w.Broadcast(Event{Type: EventProjectsChanged}) + } + } + }) + return + } + + // Check if this is a change in a workspace directory (new/removed project) for _, ws := range w.workspacePaths { if parentDir == filepath.Clean(ws) { w.debounceRefresh("workspace:"+ws, func() { diff --git a/apps/penpal/internal/watcher/watcher_test.go b/apps/penpal/internal/watcher/watcher_test.go index 4e30fc75c..f3da87c46 100644 --- a/apps/penpal/internal/watcher/watcher_test.go +++ b/apps/penpal/internal/watcher/watcher_test.go @@ -2,10 +2,13 @@ package watcher import ( "os" + "os/exec" "path/filepath" "sort" "testing" + "time" + "github.com/fsnotify/fsnotify" "github.com/loganj/penpal/internal/cache" "github.com/loganj/penpal/internal/discovery" ) @@ -282,6 +285,143 @@ func TestWindowFocusUnionAcrossWindows(t *testing.T) { assertWatched(t, w, thoughtsDir2, true, "window B still keeps proj2 watched") } +// E-PENPAL-WORKTREE-WATCH: verifies syncBaseWatchesLocked watches .git/worktrees/ for +// projects that have worktrees, and that handleEvent triggers re-discovery on changes. +func TestWorktreeWatchDir(t *testing.T) { + // Create a real git repo with a worktree so GitWorktreesDir resolves + mainDir := t.TempDir() + for _, args := range [][]string{ + {"init"}, + {"config", "user.email", "test@test.com"}, + {"config", "user.name", "Test"}, + {"commit", "--allow-empty", "-m", "init"}, + } { + cmd := exec.Command("git", append([]string{"-C", mainDir}, args...)...) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + } + wtPath := filepath.Join(t.TempDir(), "wt1") + cmd := exec.Command("git", "-C", mainDir, "worktree", "add", "-b", "b1", wtPath) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git worktree add: %v\n%s", err, out) + } + + worktrees := discovery.DiscoverWorktrees(mainDir) + project := discovery.Project{ + Name: "myrepo", + Path: mainDir, + Worktrees: worktrees, + } + + c := cache.New() + c.SetProjects([]discovery.Project{project}) + + w, err := New(c, nil) + if err != nil { + t.Fatal(err) + } + defer w.Stop() + + // Sync base watches with the project + w.focusMu.Lock() + w.syncBaseWatchesLocked(nil, []discovery.Project{project}) + w.focusMu.Unlock() + + // The .git/worktrees/ dir should be watched + gitWtDir := filepath.Join(mainDir, ".git", "worktrees") + assertWatched(t, w, gitWtDir, true, ".git/worktrees/ should be base-watched") + + // Verify it's tracked in worktreeWatchDirs + if _, ok := w.worktreeWatchDirs[filepath.Clean(gitWtDir)]; !ok { + t.Errorf("expected %s in worktreeWatchDirs", gitWtDir) + } +} + +// E-PENPAL-WORKTREE-WATCH: verifies that projects without worktrees don't get +// a .git/worktrees/ watch. +func TestWorktreeWatchDir_NoWorktrees(t *testing.T) { + mainDir := t.TempDir() + for _, args := range [][]string{ + {"init"}, + {"config", "user.email", "test@test.com"}, + {"config", "user.name", "Test"}, + {"commit", "--allow-empty", "-m", "init"}, + } { + cmd := exec.Command("git", append([]string{"-C", mainDir}, args...)...) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + } + + project := discovery.Project{ + Name: "solo", + Path: mainDir, + // No worktrees + } + + c := cache.New() + c.SetProjects([]discovery.Project{project}) + + w, err := New(c, nil) + if err != nil { + t.Fatal(err) + } + defer w.Stop() + + w.focusMu.Lock() + w.syncBaseWatchesLocked(nil, []discovery.Project{project}) + w.focusMu.Unlock() + + if len(w.worktreeWatchDirs) != 0 { + t.Errorf("expected no worktreeWatchDirs, got %v", w.worktreeWatchDirs) + } +} + +// E-PENPAL-WORKTREE-WATCH: verifies that an event in .git/worktrees/ triggers re-discovery. +func TestWorktreeWatchDir_EventTriggersRediscovery(t *testing.T) { + // Set up a watcher with a worktreeWatchDir and a discoverFn + c := cache.New() + c.SetProjects([]discovery.Project{{Name: "proj", Path: "/tmp/proj"}}) + + w, err := New(c, nil) + if err != nil { + t.Fatal(err) + } + defer w.Stop() + + discovered := make(chan struct{}, 1) + w.discoverFn = func() ([]discovery.Project, error) { + select { + case discovered <- struct{}{}: + default: + } + return []discovery.Project{{Name: "proj", Path: "/tmp/proj"}}, nil + } + w.workspacePaths = nil + + // Manually set a worktree watch dir + fakeWtDir := t.TempDir() + w.worktreeWatchDirs = map[string]struct{}{ + filepath.Clean(fakeWtDir): {}, + } + + // Simulate an event in the worktree watch dir + fakeEvent := fsnotify.Event{ + Name: filepath.Join(fakeWtDir, "new-worktree"), + Op: fsnotify.Create, + } + w.handleEvent(fakeEvent) + + // The debounce timer fires after 100ms; wait for the discovery callback + select { + case <-discovered: + // success + case <-time.After(500 * time.Millisecond): + t.Fatal("expected discoverFn to be called after worktree dir event") + } +} + func assertWatched(t *testing.T, w *Watcher, dir string, expected bool, context string) { t.Helper() watched := w.watcher.WatchList() From 89f287c1635cd04e61b14f706dfc76a82e02711b Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Mon, 6 Apr 2026 13:40:37 -0400 Subject: [PATCH 2/4] fix(penpal): address review findings for worktree watch - Extract rediscoverProjects() helper to eliminate duplicated boilerplate across worktree, workspace, and source-detection event handlers - Protect worktreeWatchDirs and gitDirWatches reads in handleEvent with focusMu to prevent concurrent map read/write panic - Watch .git/ directory for projects without existing worktrees so the first `git worktree add` (which creates .git/worktrees/) is detected - Add GitCommonDir() helper extracted from GitWorktreesDir() - Add test for first-worktree creation detection via .git/ watch - Update no-worktrees test to verify .git/ is watched Co-Authored-By: Claude Opus 4.6 --- apps/penpal/internal/discovery/worktree.go | 24 +++-- apps/penpal/internal/watcher/watcher.go | 106 ++++++++++--------- apps/penpal/internal/watcher/watcher_test.go | 67 +++++++++++- 3 files changed, 139 insertions(+), 58 deletions(-) diff --git a/apps/penpal/internal/discovery/worktree.go b/apps/penpal/internal/discovery/worktree.go index 24b7a15a5..adaa956b3 100644 --- a/apps/penpal/internal/discovery/worktree.go +++ b/apps/penpal/internal/discovery/worktree.go @@ -144,12 +144,11 @@ func FindMainWorktree(path string) string { return "" } -// GitWorktreesDir returns the path to the .git/worktrees/ directory for the -// repository that projectPath belongs to, or "" if it doesn't exist. -// Works for both main worktrees (.git is a directory) and linked worktrees -// (.git is a file pointing to the main repo's gitdir). -// E-PENPAL-WORKTREE-WATCH: resolves the worktrees metadata directory for fs watching. -func GitWorktreesDir(projectPath string) string { +// GitCommonDir returns the shared .git directory for the repository at +// projectPath (e.g. "/repo/.git"). Returns "" for non-git directories. +// Works for both main worktrees and linked worktrees. +// E-PENPAL-WORKTREE-WATCH: resolves the git common dir for fs watching. +func GitCommonDir(projectPath string) string { cmd := exec.Command("git", "-C", projectPath, "rev-parse", "--git-common-dir") out, err := cmd.Output() if err != nil { @@ -163,7 +162,18 @@ func GitWorktreesDir(projectPath string) string { if !filepath.IsAbs(commonDir) { commonDir = filepath.Join(projectPath, commonDir) } - wtDir := filepath.Join(filepath.Clean(commonDir), "worktrees") + return filepath.Clean(commonDir) +} + +// GitWorktreesDir returns the path to the .git/worktrees/ directory for the +// repository that projectPath belongs to, or "" if it doesn't exist. +// E-PENPAL-WORKTREE-WATCH: resolves the worktrees metadata directory for fs watching. +func GitWorktreesDir(projectPath string) string { + commonDir := GitCommonDir(projectPath) + if commonDir == "" { + return "" + } + wtDir := filepath.Join(commonDir, "worktrees") if info, err := os.Stat(wtDir); err == nil && info.IsDir() { return wtDir } diff --git a/apps/penpal/internal/watcher/watcher.go b/apps/penpal/internal/watcher/watcher.go index 9686023fe..2e7d0f1b7 100644 --- a/apps/penpal/internal/watcher/watcher.go +++ b/apps/penpal/internal/watcher/watcher.go @@ -71,8 +71,12 @@ type Watcher struct { baseWatched map[string]struct{} dynamicWatched map[string]struct{} - // E-PENPAL-WORKTREE-WATCH: tracks .git/worktrees/ dirs to detect worktree add/remove. + // E-PENPAL-WORKTREE-WATCH: tracks watched dirs for worktree detection. + // worktreeWatchDirs: .git/worktrees/ dirs (projects with existing worktrees) + // gitDirWatches: .git/ dirs (projects without worktrees, to detect first add) + // Both are read in handleEvent and written in syncBaseWatchesLocked — protected by focusMu. worktreeWatchDirs map[string]struct{} + gitDirWatches map[string]struct{} } // New creates a new watcher @@ -94,6 +98,7 @@ func New(c *cache.Cache, act *activity.Tracker) (*Watcher, error) { baseWatched: make(map[string]struct{}), dynamicWatched: make(map[string]struct{}), worktreeWatchDirs: make(map[string]struct{}), + gitDirWatches: make(map[string]struct{}), } return w, nil @@ -334,16 +339,26 @@ func (w *Watcher) syncBaseWatchesLocked(workspacePaths []string, projects []disc desired[filepath.Clean(p.Path)] = struct{}{} } - // E-PENPAL-WORKTREE-WATCH: watch .git/worktrees/ dirs to detect worktree add/remove. + // E-PENPAL-WORKTREE-WATCH: watch .git/worktrees/ for projects with worktrees, + // and .git/ for projects without worktrees (to detect the first worktree add). desiredWtDirs := make(map[string]struct{}) + desiredGitDirs := make(map[string]struct{}) for _, p := range projects { - if len(p.Worktrees) == 0 { + if p.Name == "(root)" { continue } - if wtDir := discovery.GitWorktreesDir(p.Path); wtDir != "" { - clean := filepath.Clean(wtDir) - desiredWtDirs[clean] = struct{}{} - desired[clean] = struct{}{} + if len(p.Worktrees) > 0 { + if wtDir := discovery.GitWorktreesDir(p.Path); wtDir != "" { + clean := filepath.Clean(wtDir) + desiredWtDirs[clean] = struct{}{} + desired[clean] = struct{}{} + } + } else { + if gitDir := discovery.GitCommonDir(p.Path); gitDir != "" { + clean := filepath.Clean(gitDir) + desiredGitDirs[clean] = struct{}{} + desired[clean] = struct{}{} + } } } @@ -374,6 +389,7 @@ func (w *Watcher) syncBaseWatchesLocked(workspacePaths []string, projects []disc } w.worktreeWatchDirs = desiredWtDirs + w.gitDirWatches = desiredGitDirs } func (w *Watcher) syncDynamicWatchesLocked() { @@ -494,45 +510,49 @@ func (w *Watcher) loop() { } } +// rediscoverProjects runs full project discovery, updates the cache and watches, +// and broadcasts a projects-changed SSE event. +func (w *Watcher) rediscoverProjects() { + if w.discoverFn == nil { + return + } + projects, err := w.discoverFn() + if err != nil { + return + } + w.cache.RescanWith(projects) + w.focusMu.Lock() + w.syncBaseWatchesLocked(w.workspacePaths, projects) + w.syncDynamicWatchesLocked() + w.focusMu.Unlock() + w.Broadcast(Event{Type: EventProjectsChanged}) +} + // E-PENPAL-WATCHER: routes fsnotify events to project/workspace refresh or SSE broadcast. func (w *Watcher) handleEvent(event fsnotify.Event) { path := filepath.Clean(event.Name) - - // E-PENPAL-WORKTREE-WATCH: detect worktree add/remove via .git/worktrees/ changes. parentDir := filepath.Clean(filepath.Dir(path)) - if _, ok := w.worktreeWatchDirs[parentDir]; ok { - w.debounceRefresh("worktrees:"+parentDir, func() { - if w.discoverFn != nil { - projects, err := w.discoverFn() - if err == nil { - w.cache.RescanWith(projects) - w.focusMu.Lock() - w.syncBaseWatchesLocked(w.workspacePaths, projects) - w.syncDynamicWatchesLocked() - w.focusMu.Unlock() - w.Broadcast(Event{Type: EventProjectsChanged}) - } - } - }) + + // E-PENPAL-WORKTREE-WATCH: detect worktree add/remove via .git/worktrees/ changes, + // or first worktree creation via .git/ directory watch. + w.focusMu.Lock() + _, isWorktreeDir := w.worktreeWatchDirs[parentDir] + _, isGitDir := w.gitDirWatches[parentDir] + w.focusMu.Unlock() + + if isWorktreeDir { + w.debounceRefresh("worktrees:"+parentDir, w.rediscoverProjects) + return + } + if isGitDir && filepath.Base(path) == "worktrees" && event.Op&fsnotify.Create != 0 { + w.debounceRefresh("worktrees:"+parentDir, w.rediscoverProjects) return } // Check if this is a change in a workspace directory (new/removed project) for _, ws := range w.workspacePaths { if parentDir == filepath.Clean(ws) { - w.debounceRefresh("workspace:"+ws, func() { - if w.discoverFn != nil { - projects, err := w.discoverFn() - if err == nil { - w.cache.RescanWith(projects) - w.focusMu.Lock() - w.syncBaseWatchesLocked(w.workspacePaths, projects) - w.syncDynamicWatchesLocked() - w.focusMu.Unlock() - w.Broadcast(Event{Type: EventProjectsChanged}) - } - } - }) + w.debounceRefresh("workspace:"+ws, w.rediscoverProjects) return } } @@ -563,19 +583,7 @@ func (w *Watcher) handleEvent(event fsnotify.Event) { if matched { for _, p := range w.cache.Projects() { if parentDir == filepath.Clean(p.Path) { - w.debounceRefresh("sources:"+p.QualifiedName(), func() { - if w.discoverFn != nil { - projects, err := w.discoverFn() - if err == nil { - w.cache.RescanWith(projects) - w.focusMu.Lock() - w.syncBaseWatchesLocked(w.workspacePaths, projects) - w.syncDynamicWatchesLocked() - w.focusMu.Unlock() - w.Broadcast(Event{Type: EventProjectsChanged}) - } - } - }) + w.debounceRefresh("sources:"+p.QualifiedName(), w.rediscoverProjects) return } } diff --git a/apps/penpal/internal/watcher/watcher_test.go b/apps/penpal/internal/watcher/watcher_test.go index f3da87c46..8210589d2 100644 --- a/apps/penpal/internal/watcher/watcher_test.go +++ b/apps/penpal/internal/watcher/watcher_test.go @@ -338,8 +338,8 @@ func TestWorktreeWatchDir(t *testing.T) { } } -// E-PENPAL-WORKTREE-WATCH: verifies that projects without worktrees don't get -// a .git/worktrees/ watch. +// E-PENPAL-WORKTREE-WATCH: verifies that projects without worktrees get a .git/ +// watch (to detect the first worktree add) but not a .git/worktrees/ watch. func TestWorktreeWatchDir_NoWorktrees(t *testing.T) { mainDir := t.TempDir() for _, args := range [][]string{ @@ -376,6 +376,13 @@ func TestWorktreeWatchDir_NoWorktrees(t *testing.T) { if len(w.worktreeWatchDirs) != 0 { t.Errorf("expected no worktreeWatchDirs, got %v", w.worktreeWatchDirs) } + + // The .git/ dir should be watched to detect first worktree creation + gitDir := filepath.Join(mainDir, ".git") + assertWatched(t, w, gitDir, true, ".git/ should be watched for first-worktree detection") + if _, ok := w.gitDirWatches[filepath.Clean(gitDir)]; !ok { + t.Errorf("expected %s in gitDirWatches", gitDir) + } } // E-PENPAL-WORKTREE-WATCH: verifies that an event in .git/worktrees/ triggers re-discovery. @@ -422,6 +429,62 @@ func TestWorktreeWatchDir_EventTriggersRediscovery(t *testing.T) { } } +// E-PENPAL-WORKTREE-WATCH: verifies that creating a "worktrees" entry in a watched +// .git/ dir triggers re-discovery (first worktree add scenario). +func TestWorktreeWatchDir_FirstWorktreeCreation(t *testing.T) { + c := cache.New() + c.SetProjects([]discovery.Project{{Name: "proj", Path: "/tmp/proj"}}) + + w, err := New(c, nil) + if err != nil { + t.Fatal(err) + } + defer w.Stop() + + discovered := make(chan struct{}, 1) + w.discoverFn = func() ([]discovery.Project, error) { + select { + case discovered <- struct{}{}: + default: + } + return []discovery.Project{{Name: "proj", Path: "/tmp/proj"}}, nil + } + w.workspacePaths = nil + + fakeGitDir := t.TempDir() + w.focusMu.Lock() + w.gitDirWatches = map[string]struct{}{ + filepath.Clean(fakeGitDir): {}, + } + w.focusMu.Unlock() + + // A non-"worktrees" event in .git/ should NOT trigger rediscovery + w.handleEvent(fsnotify.Event{ + Name: filepath.Join(fakeGitDir, "FETCH_HEAD"), + Op: fsnotify.Write, + }) + + select { + case <-discovered: + t.Fatal("should not trigger rediscovery for non-worktrees events in .git/") + case <-time.After(200 * time.Millisecond): + // good + } + + // A Create event for "worktrees" in .git/ SHOULD trigger rediscovery + w.handleEvent(fsnotify.Event{ + Name: filepath.Join(fakeGitDir, "worktrees"), + Op: fsnotify.Create, + }) + + select { + case <-discovered: + // success + case <-time.After(500 * time.Millisecond): + t.Fatal("expected discoverFn to be called when worktrees dir is created in .git/") + } +} + func assertWatched(t *testing.T, w *Watcher, dir string, expected bool, context string) { t.Helper() watched := w.watcher.WatchList() From e2b52f72bcaa8116b7ce6fe93f872fd2451e74b4 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Wed, 8 Apr 2026 15:19:49 -0400 Subject: [PATCH 3/4] fix(penpal): address review findings for worktree watch - Remove dead code (FindMainWorktree, ResolveWorktree) - Tighten .md-only event filter (drop Create bypass for non-.md files) - Filter .git/ events to only react to worktrees/ creation - Add tests for rediscoverProjects cache update and event filtering Co-Authored-By: Claude Opus 4.6 --- apps/penpal/internal/discovery/worktree.go | 62 ----------- apps/penpal/internal/watcher/watcher.go | 12 ++- apps/penpal/internal/watcher/watcher_test.go | 108 +++++++++++++++++++ 3 files changed, 116 insertions(+), 66 deletions(-) diff --git a/apps/penpal/internal/discovery/worktree.go b/apps/penpal/internal/discovery/worktree.go index adaa956b3..9d2d2e676 100644 --- a/apps/penpal/internal/discovery/worktree.go +++ b/apps/penpal/internal/discovery/worktree.go @@ -82,68 +82,6 @@ func parseWorktreeList(projectPath string, output string) []Worktree { return worktrees } -// ResolveWorktree finds the worktree that contains the given absolute path. -// Returns the worktree name and the main project path, or empty strings if -// the path doesn't belong to any worktree. -func ResolveWorktree(projectPath string, absPath string) (worktreeName string, mainProjectPath string) { - absPath = filepath.Clean(absPath) - - // First check if this path is inside the main project - mainPath := filepath.Clean(projectPath) - if strings.HasPrefix(absPath, mainPath+"/") || absPath == mainPath { - // Check if it's inside a worktree subdirectory - worktrees := DiscoverWorktrees(projectPath) - for _, wt := range worktrees { - if !wt.IsMain && (strings.HasPrefix(absPath, wt.Path+"/") || absPath == wt.Path) { - return wt.Name, mainPath - } - } - return "", mainPath - } - - return "", "" -} - -// FindMainWorktree returns the path to the main worktree for a given path -// that might be inside a worktree. It reads the .git file to find the -// gitdir and traces back to the main worktree. -func FindMainWorktree(path string) string { - cmd := exec.Command("git", "-C", path, "rev-parse", "--git-common-dir") - out, err := cmd.Output() - if err != nil { - return "" - } - commonDir := strings.TrimSpace(string(out)) - if commonDir == "" || commonDir == "." { - return "" - } - - // commonDir is the .git directory of the main worktree - // If it's relative, resolve it relative to the path - if !filepath.IsAbs(commonDir) { - // Get the actual git dir for this worktree first - cmd2 := exec.Command("git", "-C", path, "rev-parse", "--git-dir") - out2, err := cmd2.Output() - if err != nil { - return "" - } - gitDir := strings.TrimSpace(string(out2)) - if !filepath.IsAbs(gitDir) { - gitDir = filepath.Join(path, gitDir) - } - commonDir = filepath.Join(gitDir, commonDir) - } - - commonDir = filepath.Clean(commonDir) - - // The main worktree is the parent of the .git directory - if filepath.Base(commonDir) == ".git" { - return filepath.Dir(commonDir) - } - - return "" -} - // GitCommonDir returns the shared .git directory for the repository at // projectPath (e.g. "/repo/.git"). Returns "" for non-git directories. // Works for both main worktrees and linked worktrees. diff --git a/apps/penpal/internal/watcher/watcher.go b/apps/penpal/internal/watcher/watcher.go index 2e7d0f1b7..4bf5fefdf 100644 --- a/apps/penpal/internal/watcher/watcher.go +++ b/apps/penpal/internal/watcher/watcher.go @@ -544,8 +544,11 @@ func (w *Watcher) handleEvent(event fsnotify.Event) { w.debounceRefresh("worktrees:"+parentDir, w.rediscoverProjects) return } - if isGitDir && filepath.Base(path) == "worktrees" && event.Op&fsnotify.Create != 0 { - w.debounceRefresh("worktrees:"+parentDir, w.rediscoverProjects) + if isGitDir { + if filepath.Base(path) == "worktrees" && event.Op&fsnotify.Create != 0 { + w.debounceRefresh("worktrees:"+parentDir, w.rediscoverProjects) + } + // Ignore all other .git/ events (index, FETCH_HEAD, etc.) return } @@ -623,8 +626,9 @@ func (w *Watcher) handleEvent(event fsnotify.Event) { return } - // Only care about .md files for file list updates - if !strings.HasSuffix(path, ".md") && event.Op&fsnotify.Create == 0 { + // Only care about .md files for file list updates. Directory creation + // events are handled above by the source-detection and dynamic-watch paths. + if !strings.HasSuffix(path, ".md") { return } diff --git a/apps/penpal/internal/watcher/watcher_test.go b/apps/penpal/internal/watcher/watcher_test.go index 8210589d2..6b97bfadb 100644 --- a/apps/penpal/internal/watcher/watcher_test.go +++ b/apps/penpal/internal/watcher/watcher_test.go @@ -485,6 +485,114 @@ func TestWorktreeWatchDir_FirstWorktreeCreation(t *testing.T) { } } +// E-PENPAL-WORKTREE-WATCH: verifies that rediscoverProjects updates the cache with +// new project data from discoverFn and broadcasts a projects-changed event. +func TestRediscoverProjects_UpdatesCache(t *testing.T) { + c := cache.New() + original := discovery.Project{Name: "repo", Path: "/tmp/repo"} + c.SetProjects([]discovery.Project{original}) + + w, err := New(c, nil) + if err != nil { + t.Fatal(err) + } + defer w.Stop() + + updated := discovery.Project{ + Name: "repo", Path: "/tmp/repo", + Worktrees: []discovery.Worktree{ + {Name: "repo", Path: "/tmp/repo", Branch: "main", IsMain: true}, + {Name: "feature", Path: "/tmp/wt/feature", Branch: "feature", IsMain: false}, + }, + } + w.discoverFn = func() ([]discovery.Project, error) { + return []discovery.Project{updated}, nil + } + + events := w.Subscribe() + defer w.Unsubscribe(events) + + w.rediscoverProjects() + + // Verify cache was updated with new worktree data + projects := c.Projects() + if len(projects) != 1 { + t.Fatalf("expected 1 project, got %d", len(projects)) + } + if len(projects[0].Worktrees) != 2 { + t.Errorf("expected 2 worktrees in cache, got %d", len(projects[0].Worktrees)) + } + + // Verify a projects-changed event was broadcast + select { + case evt := <-events: + if evt.Type != EventProjectsChanged { + t.Errorf("expected EventProjectsChanged, got %v", evt.Type) + } + case <-time.After(500 * time.Millisecond): + t.Fatal("expected projects-changed event to be broadcast") + } +} + +// E-PENPAL-WATCHER: verifies that non-.md file events are filtered out +// and do not trigger project rescans. +func TestEventFilter_NonMdFilesIgnored(t *testing.T) { + projDir := t.TempDir() + thoughtsDir := filepath.Join(projDir, "thoughts") + os.MkdirAll(thoughtsDir, 0o755) + + project := discovery.Project{ + Name: "proj", Path: projDir, + Sources: []discovery.FileSource{{ + Name: "thoughts", Type: "tree", SourceTypeName: "thoughts", + RootPath: thoughtsDir, Auto: true, + }}, + } + + c := cache.New() + c.SetProjects([]discovery.Project{project}) + + w, err := New(c, nil) + if err != nil { + t.Fatal(err) + } + defer w.Stop() + + events := w.Subscribe() + defer w.Unsubscribe(events) + + // Focus the project so the thoughts dir is watched and events reach the filter + w.FocusProject("proj") + + // Create event for a .js file should NOT trigger a files-changed broadcast + w.handleEvent(fsnotify.Event{ + Name: filepath.Join(thoughtsDir, "script.js"), + Op: fsnotify.Create, + }) + + select { + case evt := <-events: + t.Fatalf("expected no event for .js file, got %v", evt) + case <-time.After(200 * time.Millisecond): + // good — no event + } + + // Create event for a .md file SHOULD trigger a files-changed broadcast + w.handleEvent(fsnotify.Event{ + Name: filepath.Join(thoughtsDir, "notes.md"), + Op: fsnotify.Create, + }) + + select { + case evt := <-events: + if evt.Type != EventFilesChanged { + t.Errorf("expected EventFilesChanged, got %v", evt.Type) + } + case <-time.After(500 * time.Millisecond): + t.Fatal("expected files-changed event for .md file") + } +} + func assertWatched(t *testing.T, w *Watcher, dir string, expected bool, context string) { t.Helper() watched := w.watcher.WatchList() From 08295b3072e98b5f82451dfd35b5f937205b5646 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Thu, 9 Apr 2026 09:53:57 -0400 Subject: [PATCH 4/4] fix(penpal): cheaper worktree detection via workspace watch Replace .git/worktrees/ inotify watches and per-project subprocess calls with zero-cost workspace directory detection. Unexport unused GitCommonDir/GitWorktreesDir, serialize rediscoverProjects, remove redundant .md check, add tests for error paths and Write events. Co-Authored-By: Claude Opus 4.6 --- apps/penpal/ERD.md | 2 +- apps/penpal/internal/discovery/worktree.go | 71 +++-- .../internal/discovery/worktree_test.go | 60 +++-- apps/penpal/internal/watcher/watcher.go | 65 +---- apps/penpal/internal/watcher/watcher_test.go | 254 ++++-------------- 5 files changed, 152 insertions(+), 300 deletions(-) diff --git a/apps/penpal/ERD.md b/apps/penpal/ERD.md index e207e0f48..2ff58c763 100644 --- a/apps/penpal/ERD.md +++ b/apps/penpal/ERD.md @@ -91,7 +91,7 @@ see-also: - **E-PENPAL-WORKTREE-DISCOVERY**: Worktrees are discovered by parsing `git worktree list --porcelain` output. Each worktree gets a name, path, branch, and `IsMain` flag. The `refs/heads/` prefix is stripped from branch names. ← [P-PENPAL-WORKTREE](PRODUCT.md#P-PENPAL-WORKTREE) -- **E-PENPAL-WORKTREE-WATCH**: The watcher monitors each project's `.git/worktrees/` directory (for the main worktree) or the equivalent resolved via `git rev-parse --git-common-dir`. When entries are created or removed in that directory, the watcher re-runs `DiscoverWorktrees` for the affected project, updates the cached worktree list, and broadcasts a `projects` SSE event so the frontend reflects the change. +- **E-PENPAL-WORKTREE-WATCH**: Worktree additions and removals are detected by the existing workspace directory watch — `git worktree add`/`remove` creates or deletes a sibling directory in the workspace, triggering the workspace rescan path. The rescan re-runs full project discovery (including `DiscoverWorktrees`), updates the cache, and broadcasts a `projects` SSE event so the frontend reflects the change. No additional inotify watches or subprocess calls are needed. `GitCommonDir` resolves the shared `.git` directory using pure filesystem reads (no `git rev-parse`). ← [P-PENPAL-WORKTREE](PRODUCT.md#P-PENPAL-WORKTREE) - **E-PENPAL-CLAUDE-PLANS-DETECT**: `DiscoverClaudePlans()` checks `~/.claude/plans/` for existence and at least one `.md` file. If found, a synthetic standalone project is injected. If the user already manually added the same path, a tree source is injected into the existing entry instead of duplicating. diff --git a/apps/penpal/internal/discovery/worktree.go b/apps/penpal/internal/discovery/worktree.go index 9d2d2e676..2212485bb 100644 --- a/apps/penpal/internal/discovery/worktree.go +++ b/apps/penpal/internal/discovery/worktree.go @@ -7,6 +7,48 @@ import ( "strings" ) +// gitCommonDirFS resolves the shared .git directory using only filesystem +// reads — no subprocess. For a main worktree .git is a directory; for a +// linked worktree .git is a file containing "gitdir: " and the +// referenced gitdir contains a "commondir" file pointing back to the +// shared .git. +func gitCommonDirFS(projectPath string) string { + gitPath := filepath.Join(projectPath, ".git") + info, err := os.Lstat(gitPath) + if err != nil { + return "" + } + // Main worktree: .git is a directory — it IS the common dir. + if info.IsDir() { + return gitPath + } + // Linked worktree: .git is a file with "gitdir: ". + data, err := os.ReadFile(gitPath) + if err != nil { + return "" + } + line := strings.TrimSpace(string(data)) + if !strings.HasPrefix(line, "gitdir: ") { + return "" + } + gitDir := strings.TrimPrefix(line, "gitdir: ") + if !filepath.IsAbs(gitDir) { + gitDir = filepath.Join(projectPath, gitDir) + } + gitDir = filepath.Clean(gitDir) + // Read commondir file to find the shared .git directory. + cdPath := filepath.Join(gitDir, "commondir") + cdData, err := os.ReadFile(cdPath) + if err != nil { + return "" + } + commonDir := strings.TrimSpace(string(cdData)) + if !filepath.IsAbs(commonDir) { + commonDir = filepath.Join(gitDir, commonDir) + } + return filepath.Clean(commonDir) +} + // Worktree represents a git worktree associated with a project. type Worktree struct { Name string `json:"name"` // directory name (e.g., "fancy-name") @@ -82,32 +124,11 @@ func parseWorktreeList(projectPath string, output string) []Worktree { return worktrees } -// GitCommonDir returns the shared .git directory for the repository at -// projectPath (e.g. "/repo/.git"). Returns "" for non-git directories. -// Works for both main worktrees and linked worktrees. -// E-PENPAL-WORKTREE-WATCH: resolves the git common dir for fs watching. -func GitCommonDir(projectPath string) string { - cmd := exec.Command("git", "-C", projectPath, "rev-parse", "--git-common-dir") - out, err := cmd.Output() - if err != nil { - return "" - } - commonDir := strings.TrimSpace(string(out)) - if commonDir == "" || commonDir == "." { - return "" - } - // --git-common-dir output is relative to the -C directory - if !filepath.IsAbs(commonDir) { - commonDir = filepath.Join(projectPath, commonDir) - } - return filepath.Clean(commonDir) -} - -// GitWorktreesDir returns the path to the .git/worktrees/ directory for the +// gitWorktreesDir returns the path to the .git/worktrees/ directory for the // repository that projectPath belongs to, or "" if it doesn't exist. -// E-PENPAL-WORKTREE-WATCH: resolves the worktrees metadata directory for fs watching. -func GitWorktreesDir(projectPath string) string { - commonDir := GitCommonDir(projectPath) +// Uses pure filesystem reads via gitCommonDirFS — no subprocess calls. +func gitWorktreesDir(projectPath string) string { + commonDir := gitCommonDirFS(projectPath) if commonDir == "" { return "" } diff --git a/apps/penpal/internal/discovery/worktree_test.go b/apps/penpal/internal/discovery/worktree_test.go index 3e12b5a93..716f6834a 100644 --- a/apps/penpal/internal/discovery/worktree_test.go +++ b/apps/penpal/internal/discovery/worktree_test.go @@ -131,13 +131,13 @@ func resolveSymlinks(t *testing.T, path string) string { return resolved } -// E-PENPAL-WORKTREE-WATCH: verifies GitWorktreesDir returns the .git/worktrees/ dir for a repo with worktrees. -func TestGitWorktreesDir_MainWorktree(t *testing.T) { +// E-PENPAL-WORKTREE-WATCH: verifies gitWorktreesDir returns the .git/worktrees/ dir for a repo with worktrees. +func TestWorktreesDir_MainWorktree(t *testing.T) { mainDir := resolveSymlinks(t, t.TempDir()) initGitRepo(t, mainDir) // Before adding a worktree, the dir doesn't exist - if got := GitWorktreesDir(mainDir); got != "" { + if got := gitWorktreesDir(mainDir); got != "" { t.Fatalf("expected empty before worktree add, got %q", got) } @@ -148,40 +148,40 @@ func TestGitWorktreesDir_MainWorktree(t *testing.T) { t.Fatalf("git worktree add: %v\n%s", err, out) } - // Now GitWorktreesDir should return the .git/worktrees/ path - got := GitWorktreesDir(mainDir) + // Now gitWorktreesDir should return the .git/worktrees/ path + got := gitWorktreesDir(mainDir) want := filepath.Join(mainDir, ".git", "worktrees") if got != want { - t.Errorf("GitWorktreesDir(main) = %q, want %q", got, want) + t.Errorf("gitWorktreesDir(main) = %q, want %q", got, want) } // It should also work when called from the linked worktree - got2 := GitWorktreesDir(wtDir) + got2 := gitWorktreesDir(wtDir) if got2 != want { - t.Errorf("GitWorktreesDir(linked) = %q, want %q", got2, want) + t.Errorf("gitWorktreesDir(linked) = %q, want %q", got2, want) } } -// E-PENPAL-WORKTREE-WATCH: verifies GitWorktreesDir returns "" for a non-git directory. -func TestGitWorktreesDir_NotGitRepo(t *testing.T) { +// E-PENPAL-WORKTREE-WATCH: verifies gitWorktreesDir returns "" for a non-git directory. +func TestWorktreesDir_NotGitRepo(t *testing.T) { dir := t.TempDir() - if got := GitWorktreesDir(dir); got != "" { - t.Errorf("GitWorktreesDir(non-git) = %q, want empty", got) + if got := gitWorktreesDir(dir); got != "" { + t.Errorf("gitWorktreesDir(non-git) = %q, want empty", got) } } -// E-PENPAL-WORKTREE-WATCH: verifies GitWorktreesDir returns "" for a repo with no worktrees. -func TestGitWorktreesDir_NoWorktrees(t *testing.T) { +// E-PENPAL-WORKTREE-WATCH: verifies gitWorktreesDir returns "" for a repo with no worktrees. +func TestWorktreesDir_NoWorktrees(t *testing.T) { dir := t.TempDir() initGitRepo(t, dir) - if got := GitWorktreesDir(dir); got != "" { - t.Errorf("GitWorktreesDir(no worktrees) = %q, want empty", got) + if got := gitWorktreesDir(dir); got != "" { + t.Errorf("gitWorktreesDir(no worktrees) = %q, want empty", got) } } // E-PENPAL-WORKTREE-WATCH: verifies worktree directory appears after git worktree add // and disappears after git worktree remove. -func TestGitWorktreesDir_AddRemoveCycle(t *testing.T) { +func TestWorktreesDir_AddRemoveCycle(t *testing.T) { mainDir := resolveSymlinks(t, t.TempDir()) initGitRepo(t, mainDir) @@ -191,7 +191,7 @@ func TestGitWorktreesDir_AddRemoveCycle(t *testing.T) { t.Fatalf("git worktree add: %v\n%s", err, out) } - wtDir := GitWorktreesDir(mainDir) + wtDir := gitWorktreesDir(mainDir) if wtDir == "" { t.Fatal("expected non-empty after add") } @@ -218,7 +218,29 @@ func TestGitWorktreesDir_AddRemoveCycle(t *testing.T) { } // After removing the last worktree, the worktrees/ dir should be gone - if got := GitWorktreesDir(mainDir); got != "" { + if got := gitWorktreesDir(mainDir); got != "" { t.Errorf("expected empty after removing last worktree, got %q", got) } } + +// E-PENPAL-WORKTREE-WATCH: verifies gitCommonDirFS returns "" for malformed .git file. +func TestGitCommonDirFS_MalformedGitFile(t *testing.T) { + dir := t.TempDir() + // .git file with no "gitdir:" prefix + os.WriteFile(filepath.Join(dir, ".git"), []byte("not a gitdir line\n"), 0o644) + if got := gitCommonDirFS(dir); got != "" { + t.Errorf("expected empty for malformed .git file, got %q", got) + } +} + +// E-PENPAL-WORKTREE-WATCH: verifies gitCommonDirFS returns "" when commondir file is missing. +func TestGitCommonDirFS_MissingCommondir(t *testing.T) { + dir := t.TempDir() + gitDir := filepath.Join(dir, "fake-gitdir") + os.MkdirAll(gitDir, 0o755) + // .git file points to a valid directory but commondir file doesn't exist + os.WriteFile(filepath.Join(dir, ".git"), []byte("gitdir: "+gitDir+"\n"), 0o644) + if got := gitCommonDirFS(dir); got != "" { + t.Errorf("expected empty for missing commondir, got %q", got) + } +} diff --git a/apps/penpal/internal/watcher/watcher.go b/apps/penpal/internal/watcher/watcher.go index 4bf5fefdf..f07809622 100644 --- a/apps/penpal/internal/watcher/watcher.go +++ b/apps/penpal/internal/watcher/watcher.go @@ -71,12 +71,8 @@ type Watcher struct { baseWatched map[string]struct{} dynamicWatched map[string]struct{} - // E-PENPAL-WORKTREE-WATCH: tracks watched dirs for worktree detection. - // worktreeWatchDirs: .git/worktrees/ dirs (projects with existing worktrees) - // gitDirWatches: .git/ dirs (projects without worktrees, to detect first add) - // Both are read in handleEvent and written in syncBaseWatchesLocked — protected by focusMu. - worktreeWatchDirs map[string]struct{} - gitDirWatches map[string]struct{} + // Serializes rediscoverProjects calls from concurrent debounce timers. + rediscoverMu sync.Mutex } // New creates a new watcher @@ -97,8 +93,6 @@ func New(c *cache.Cache, act *activity.Tracker) (*Watcher, error) { windowFocuses: make(map[string]focusTarget), baseWatched: make(map[string]struct{}), dynamicWatched: make(map[string]struct{}), - worktreeWatchDirs: make(map[string]struct{}), - gitDirWatches: make(map[string]struct{}), } return w, nil @@ -339,29 +333,6 @@ func (w *Watcher) syncBaseWatchesLocked(workspacePaths []string, projects []disc desired[filepath.Clean(p.Path)] = struct{}{} } - // E-PENPAL-WORKTREE-WATCH: watch .git/worktrees/ for projects with worktrees, - // and .git/ for projects without worktrees (to detect the first worktree add). - desiredWtDirs := make(map[string]struct{}) - desiredGitDirs := make(map[string]struct{}) - for _, p := range projects { - if p.Name == "(root)" { - continue - } - if len(p.Worktrees) > 0 { - if wtDir := discovery.GitWorktreesDir(p.Path); wtDir != "" { - clean := filepath.Clean(wtDir) - desiredWtDirs[clean] = struct{}{} - desired[clean] = struct{}{} - } - } else { - if gitDir := discovery.GitCommonDir(p.Path); gitDir != "" { - clean := filepath.Clean(gitDir) - desiredGitDirs[clean] = struct{}{} - desired[clean] = struct{}{} - } - } - } - for path := range w.baseWatched { if _, ok := desired[path]; ok { continue @@ -388,8 +359,6 @@ func (w *Watcher) syncBaseWatchesLocked(workspacePaths []string, projects []disc w.baseWatched[path] = struct{}{} } - w.worktreeWatchDirs = desiredWtDirs - w.gitDirWatches = desiredGitDirs } func (w *Watcher) syncDynamicWatchesLocked() { @@ -511,13 +480,18 @@ func (w *Watcher) loop() { } // rediscoverProjects runs full project discovery, updates the cache and watches, -// and broadcasts a projects-changed SSE event. +// and broadcasts a projects-changed SSE event. Serialized by rediscoverMu so +// concurrent debounce timers don't interleave cache updates. func (w *Watcher) rediscoverProjects() { if w.discoverFn == nil { return } + w.rediscoverMu.Lock() + defer w.rediscoverMu.Unlock() + projects, err := w.discoverFn() if err != nil { + log.Printf("Warning: project discovery failed: %v", err) return } w.cache.RescanWith(projects) @@ -533,24 +507,9 @@ func (w *Watcher) handleEvent(event fsnotify.Event) { path := filepath.Clean(event.Name) parentDir := filepath.Clean(filepath.Dir(path)) - // E-PENPAL-WORKTREE-WATCH: detect worktree add/remove via .git/worktrees/ changes, - // or first worktree creation via .git/ directory watch. - w.focusMu.Lock() - _, isWorktreeDir := w.worktreeWatchDirs[parentDir] - _, isGitDir := w.gitDirWatches[parentDir] - w.focusMu.Unlock() - - if isWorktreeDir { - w.debounceRefresh("worktrees:"+parentDir, w.rediscoverProjects) - return - } - if isGitDir { - if filepath.Base(path) == "worktrees" && event.Op&fsnotify.Create != 0 { - w.debounceRefresh("worktrees:"+parentDir, w.rediscoverProjects) - } - // Ignore all other .git/ events (index, FETCH_HEAD, etc.) - return - } + // E-PENPAL-WORKTREE-WATCH: worktree add/remove is detected by the workspace + // directory watch — the new worktree directory appears as a sibling project, + // triggering the workspace rescan path below. // Check if this is a change in a workspace directory (new/removed project) for _, ws := range w.workspacePaths { @@ -633,7 +592,7 @@ func (w *Watcher) handleEvent(event fsnotify.Event) { } // Record activity for .md file changes before debouncing - if strings.HasSuffix(path, ".md") && w.activity != nil { + if w.activity != nil { if project := w.cache.FindProject(projectName); project != nil { if relPath, err := filepath.Rel(project.Path, path); err == nil { evtType := activity.FileModified diff --git a/apps/penpal/internal/watcher/watcher_test.go b/apps/penpal/internal/watcher/watcher_test.go index 6b97bfadb..8a0229720 100644 --- a/apps/penpal/internal/watcher/watcher_test.go +++ b/apps/penpal/internal/watcher/watcher_test.go @@ -1,8 +1,8 @@ package watcher import ( + "fmt" "os" - "os/exec" "path/filepath" "sort" "testing" @@ -285,206 +285,6 @@ func TestWindowFocusUnionAcrossWindows(t *testing.T) { assertWatched(t, w, thoughtsDir2, true, "window B still keeps proj2 watched") } -// E-PENPAL-WORKTREE-WATCH: verifies syncBaseWatchesLocked watches .git/worktrees/ for -// projects that have worktrees, and that handleEvent triggers re-discovery on changes. -func TestWorktreeWatchDir(t *testing.T) { - // Create a real git repo with a worktree so GitWorktreesDir resolves - mainDir := t.TempDir() - for _, args := range [][]string{ - {"init"}, - {"config", "user.email", "test@test.com"}, - {"config", "user.name", "Test"}, - {"commit", "--allow-empty", "-m", "init"}, - } { - cmd := exec.Command("git", append([]string{"-C", mainDir}, args...)...) - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatalf("git %v: %v\n%s", args, err, out) - } - } - wtPath := filepath.Join(t.TempDir(), "wt1") - cmd := exec.Command("git", "-C", mainDir, "worktree", "add", "-b", "b1", wtPath) - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatalf("git worktree add: %v\n%s", err, out) - } - - worktrees := discovery.DiscoverWorktrees(mainDir) - project := discovery.Project{ - Name: "myrepo", - Path: mainDir, - Worktrees: worktrees, - } - - c := cache.New() - c.SetProjects([]discovery.Project{project}) - - w, err := New(c, nil) - if err != nil { - t.Fatal(err) - } - defer w.Stop() - - // Sync base watches with the project - w.focusMu.Lock() - w.syncBaseWatchesLocked(nil, []discovery.Project{project}) - w.focusMu.Unlock() - - // The .git/worktrees/ dir should be watched - gitWtDir := filepath.Join(mainDir, ".git", "worktrees") - assertWatched(t, w, gitWtDir, true, ".git/worktrees/ should be base-watched") - - // Verify it's tracked in worktreeWatchDirs - if _, ok := w.worktreeWatchDirs[filepath.Clean(gitWtDir)]; !ok { - t.Errorf("expected %s in worktreeWatchDirs", gitWtDir) - } -} - -// E-PENPAL-WORKTREE-WATCH: verifies that projects without worktrees get a .git/ -// watch (to detect the first worktree add) but not a .git/worktrees/ watch. -func TestWorktreeWatchDir_NoWorktrees(t *testing.T) { - mainDir := t.TempDir() - for _, args := range [][]string{ - {"init"}, - {"config", "user.email", "test@test.com"}, - {"config", "user.name", "Test"}, - {"commit", "--allow-empty", "-m", "init"}, - } { - cmd := exec.Command("git", append([]string{"-C", mainDir}, args...)...) - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatalf("git %v: %v\n%s", args, err, out) - } - } - - project := discovery.Project{ - Name: "solo", - Path: mainDir, - // No worktrees - } - - c := cache.New() - c.SetProjects([]discovery.Project{project}) - - w, err := New(c, nil) - if err != nil { - t.Fatal(err) - } - defer w.Stop() - - w.focusMu.Lock() - w.syncBaseWatchesLocked(nil, []discovery.Project{project}) - w.focusMu.Unlock() - - if len(w.worktreeWatchDirs) != 0 { - t.Errorf("expected no worktreeWatchDirs, got %v", w.worktreeWatchDirs) - } - - // The .git/ dir should be watched to detect first worktree creation - gitDir := filepath.Join(mainDir, ".git") - assertWatched(t, w, gitDir, true, ".git/ should be watched for first-worktree detection") - if _, ok := w.gitDirWatches[filepath.Clean(gitDir)]; !ok { - t.Errorf("expected %s in gitDirWatches", gitDir) - } -} - -// E-PENPAL-WORKTREE-WATCH: verifies that an event in .git/worktrees/ triggers re-discovery. -func TestWorktreeWatchDir_EventTriggersRediscovery(t *testing.T) { - // Set up a watcher with a worktreeWatchDir and a discoverFn - c := cache.New() - c.SetProjects([]discovery.Project{{Name: "proj", Path: "/tmp/proj"}}) - - w, err := New(c, nil) - if err != nil { - t.Fatal(err) - } - defer w.Stop() - - discovered := make(chan struct{}, 1) - w.discoverFn = func() ([]discovery.Project, error) { - select { - case discovered <- struct{}{}: - default: - } - return []discovery.Project{{Name: "proj", Path: "/tmp/proj"}}, nil - } - w.workspacePaths = nil - - // Manually set a worktree watch dir - fakeWtDir := t.TempDir() - w.worktreeWatchDirs = map[string]struct{}{ - filepath.Clean(fakeWtDir): {}, - } - - // Simulate an event in the worktree watch dir - fakeEvent := fsnotify.Event{ - Name: filepath.Join(fakeWtDir, "new-worktree"), - Op: fsnotify.Create, - } - w.handleEvent(fakeEvent) - - // The debounce timer fires after 100ms; wait for the discovery callback - select { - case <-discovered: - // success - case <-time.After(500 * time.Millisecond): - t.Fatal("expected discoverFn to be called after worktree dir event") - } -} - -// E-PENPAL-WORKTREE-WATCH: verifies that creating a "worktrees" entry in a watched -// .git/ dir triggers re-discovery (first worktree add scenario). -func TestWorktreeWatchDir_FirstWorktreeCreation(t *testing.T) { - c := cache.New() - c.SetProjects([]discovery.Project{{Name: "proj", Path: "/tmp/proj"}}) - - w, err := New(c, nil) - if err != nil { - t.Fatal(err) - } - defer w.Stop() - - discovered := make(chan struct{}, 1) - w.discoverFn = func() ([]discovery.Project, error) { - select { - case discovered <- struct{}{}: - default: - } - return []discovery.Project{{Name: "proj", Path: "/tmp/proj"}}, nil - } - w.workspacePaths = nil - - fakeGitDir := t.TempDir() - w.focusMu.Lock() - w.gitDirWatches = map[string]struct{}{ - filepath.Clean(fakeGitDir): {}, - } - w.focusMu.Unlock() - - // A non-"worktrees" event in .git/ should NOT trigger rediscovery - w.handleEvent(fsnotify.Event{ - Name: filepath.Join(fakeGitDir, "FETCH_HEAD"), - Op: fsnotify.Write, - }) - - select { - case <-discovered: - t.Fatal("should not trigger rediscovery for non-worktrees events in .git/") - case <-time.After(200 * time.Millisecond): - // good - } - - // A Create event for "worktrees" in .git/ SHOULD trigger rediscovery - w.handleEvent(fsnotify.Event{ - Name: filepath.Join(fakeGitDir, "worktrees"), - Op: fsnotify.Create, - }) - - select { - case <-discovered: - // success - case <-time.After(500 * time.Millisecond): - t.Fatal("expected discoverFn to be called when worktrees dir is created in .git/") - } -} - // E-PENPAL-WORKTREE-WATCH: verifies that rediscoverProjects updates the cache with // new project data from discoverFn and broadcasts a projects-changed event. func TestRediscoverProjects_UpdatesCache(t *testing.T) { @@ -572,7 +372,20 @@ func TestEventFilter_NonMdFilesIgnored(t *testing.T) { select { case evt := <-events: - t.Fatalf("expected no event for .js file, got %v", evt) + t.Fatalf("expected no event for .js Create, got %v", evt) + case <-time.After(200 * time.Millisecond): + // good — no event + } + + // Write event for a .js file should also be filtered out + w.handleEvent(fsnotify.Event{ + Name: filepath.Join(thoughtsDir, "script.js"), + Op: fsnotify.Write, + }) + + select { + case evt := <-events: + t.Fatalf("expected no event for .js Write, got %v", evt) case <-time.After(200 * time.Millisecond): // good — no event } @@ -593,6 +406,43 @@ func TestEventFilter_NonMdFilesIgnored(t *testing.T) { } } +// E-PENPAL-WORKTREE-WATCH: verifies that rediscoverProjects does not update the +// cache or broadcast events when discoverFn returns an error. +func TestRediscoverProjects_ErrorNoUpdate(t *testing.T) { + c := cache.New() + original := discovery.Project{Name: "repo", Path: "/tmp/repo"} + c.SetProjects([]discovery.Project{original}) + + w, err := New(c, nil) + if err != nil { + t.Fatal(err) + } + defer w.Stop() + + w.discoverFn = func() ([]discovery.Project, error) { + return nil, fmt.Errorf("network error") + } + + events := w.Subscribe() + defer w.Unsubscribe(events) + + w.rediscoverProjects() + + // Cache should still have the original project, not be wiped + projects := c.Projects() + if len(projects) != 1 || projects[0].Name != "repo" { + t.Errorf("expected original project preserved, got %+v", projects) + } + + // No event should be broadcast + select { + case evt := <-events: + t.Fatalf("expected no event on error, got %v", evt) + case <-time.After(200 * time.Millisecond): + // good — no event + } +} + func assertWatched(t *testing.T, w *Watcher, dir string, expected bool, context string) { t.Helper() watched := w.watcher.WatchList()