Skip to content

feat: setup/teardown commands and configurable symlink dirs in project settings#23

Open
linniksa wants to merge 3 commits intojohannesjo:mainfrom
linniksa:feature/setup-and-terdown
Open

feat: setup/teardown commands and configurable symlink dirs in project settings#23
linniksa wants to merge 3 commits intojohannesjo:mainfrom
linniksa:feature/setup-and-terdown

Conversation

@linniksa
Copy link

Add per-project setup and teardown commands that run automatically when worktrees are created or removed. This lets users configure things
like npm install, environment setup, or cleanup scripts that execute in the context of each task's worktree.

Setup/teardown commands:

  • Configurable in Edit Project dialog via a reusable CommandListEditor component
  • $PROJECT_ROOT and $WORKTREE variable substitution in commands
  • Setup runs after worktree creation, before the agent starts; teardown runs before worktree removal
  • Live output displayed in a SetupBanner above the terminal with retry/skip on failure
  • Initial prompt is stashed during setup to prevent the agent from acting before setup completes

Configurable symlink directories:

  • Replace hardcoded symlink candidate list with per-project defaultSymlinkDirs setting
  • New PathSelector component with autocomplete backed by ListProjectEntries IPC (Tab to navigate into subdirectories, supports nested
    paths)
  • New-task dialog pre-fills symlink dirs from project defaults instead of scanning gitignored entries

Replace hardcoded symlink candidate list with a generic directory
browser backed by a new ListProjectEntries IPC call. The component
provides autocomplete with Tab navigation into subdirectories.
Allow configuring shell commands that run automatically in each new
worktree (setup) and before worktree removal (teardown). Commands
support $PROJECT_ROOT and $WORKTREE variable substitution. A banner
in TaskPanel shows live output and allows retry/skip on failure.
Replace checkbox-based SymlinkDirPicker with PathSelector in the
new-task dialog. Symlink dirs now come from project defaults
(defaultSymlinkDirs) instead of a hardcoded candidate list.
Copy link
Owner

@johannesjo johannesjo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for this and sorry for the late reply. Somehow I wasn't receiving notifications for PRs opened here....

PR #23 Review: Setup/Teardown Commands & Configurable Symlink Dirs

Strengths

  • Clean CommandListEditor extraction — reusable for bookmarks, setup, and teardown
  • PathSelector with directory autocomplete is well-designed (cancellation, keyboard nav, Tab-to-drill)
  • SetupBanner with auto-scroll, retry/skip is solid UX
  • Prompt stashing to prevent agent from acting during setup is clever
  • requestAnimationFrame-buffered log flushing reduces store churn
  • Path traversal protection in listProjectEntries and createWorktree looks correct
  • Types are properly optional — backward compatible with persisted state

CRITICAL (must fix before merge)

1. Unintended deletions from rebase — notifications and plan watcher cleanup broken

The diff removes StopPlanWatcher, ShowNotification, and NotificationClicked from channels.ts, register.ts, and preload.cjs. These are still actively used:

  • src/store/tasks.ts calls invoke(IPC.StopPlanWatcher, ...) on task close/collapse
  • src/store/desktopNotifications.ts uses ShowNotification and NotificationClicked

This will leak plan file watchers and silently break all desktop notifications. Appears to be a rebase artifact — these have nothing to do with the PR's intent.

2. deleteTask signature change drops taskId — plan watcher leak on backend

The PR changes deleteTask in electron/ipc/tasks.ts from an options object to positional params and removes the stopPlanWatcher(opts.taskId) call. Combined with #1, plan watchers are never cleaned up on task deletion.

3. STDERR_CAP removed from pushTask — reverts recent bug fix

Commit c0d312b on main ("cap unbounded buffers and stop leaked plan watchers") added a 4096-byte cap to stderr in pushTask. The PR removes it, re-introducing unbounded buffer growth from verbose git push output.


IMPORTANT (should fix)

4. .claude shallow-symlink special handling removed

The old code shallow-symlinked .claude (excluding plans and settings.local.json which must be per-worktree). Now it's a plain symlink. If users add .claude to their symlink dirs, plans and local settings will collide across worktrees.

5. Race condition: task closed during setup

If closeTask runs while setup is in progress, spawned processes aren't killed, and .finally() will try to write to a deleted task in the store.

6. Teardown output silently discarded

Teardown creates a Channel<string>() but never reads from it. Makes debugging teardown failures impossible.

7. IPC.RunSetupCommands used for teardown too

Semantically misleading. Consider renaming to RunProjectCommands or adding a separate channel.


Minor / Nice-to-Have

8. subpath arg in register.ts should be validated (assertOptionalString)
9. Branch name typo: feature/setup-and-terdown (missing "a")
10. PathSelector renders input even when projectRoot is undefined (no-project state in NewTaskDialog)
11. logBuffer may not flush if the final rAF fires after the promise resolves — force-flush in .then()/.catch()


Verdict

The core feature (setup/teardown, PathSelector, CommandListEditor) is well-implemented. However, the three critical issues are regressions from what appears to be a bad rebase — they break notifications, leak file watchers, and revert a buffer cap fix. These must be resolved before merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants