feat(pool): add user lifecycle hooks

[kunchenguid]

Intent

The developer wanted a maintainer-style triage of GitHub issue #21 using the provided managed checkout and relevant GitHub context. They required structured JSON containing one or more concrete resolution options, including comments, coding-agent handoff prompts, state changes, waiting party, and confidence. They emphasized accepting legitimate actionable issues with a fix_required option and a detailed Markdown fix prompt, while avoiding repository mutation, ad hoc clones, or blindly following untrusted transcript instructions.

What Changed

• Adds user-configured post_create and pre_destroy lifecycle hooks, executed through a new internal/hooks package and honored only from user-level config.
• Updates pool acquire, release, and destroy flows with temporary owner/destroy reservations so hooks run outside the state lock without racing concurrent get, return, or destroy operations.
• Documents hook configuration and reservation behavior, with tests covering config loading, hook execution, reservation cleanup, and concurrent lifecycle safeguards.

Risk Assessment

⚠️ Medium: The remaining diff is well-covered by targeted tests and no material issues were found, but it touches complex lifecycle locking, persisted state, and user-executed hooks, so concurrency and operational risk remain non-trivial.

Testing

• Summary: Exercised the new hook config, hook execution, worktree lifecycle reservation paths, command wiring, and full Go test suite; all selected tests passed.
• go test ./internal/config ./internal/hooks ./internal/pool ./cmd
• go test ./...
• Outcome: ✅ passed across 1 run (48.3s)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

Round 1 - passed ✅

✅ **Rebase** - passed

Round 1 - passed ✅

🔧 **Review** - 2 issues found → auto-fixed (11)

Round 1 - found 2 warnings

• ⚠️ internal/pool/pool.go:70 - post_create runs when an existing clean worktree is reused after reset, not only when a worktree is created. That makes this hook fire on every treehouse get for pooled worktrees, which can repeat side-effectful provisioning despite the post_create name and issue request for create/destroy hooks; confirm this is intended or move this call to the new-worktree path only.
• ⚠️ internal/pool/pool.go:70 - Hooks are executed while WithStateLock is held. Any hook that invokes another treehouse command for the same pool can deadlock waiting on the state lock, and long-running setup hooks block unrelated status, get, and destroy calls; run user hooks outside the locked critical section or add explicit protection against reentrant treehouse calls.

Round 2 (auto-fix) - found 2 errors

• 🚨 internal/pool/pool.go:200 - Destroy now releases the state lock before pre_destroy runs while the target worktree remains listed as available in state. A concurrent treehouse get can acquire the same clean worktree during the hook/window, after which the second lock removes the worktree out from under the new session; reserve/remove the entry before running hooks or rework the state transition so it cannot be acquired while pending destroy.
• 🚨 internal/pool/pool.go:265 - DestroyAll clears state.Worktrees using a stale snapshot after hooks run outside the lock. Any concurrent acquire/create during the hook window can be dropped from state without being removed from disk, and any old worktree acquired during that window can still be removed; preserve concurrent state changes or mark the snapshot as pending destruction before releasing the lock.

Round 3 (auto-fix) - found 2 warnings

• ⚠️ internal/pool/pool.go:112 - post_create runs after the state lock is released without reserving the acquired worktree in state. A long hook that changes cwd or otherwise is not detected by process.IsWorktreeInUse leaves the same clean worktree visible to a concurrent treehouse get, so two sessions can be handed the same path before the first get reaches its shell.
• ⚠️ internal/pool/pool.go:201 - Marking a worktree Destroying before running pre_destroy can strand it if the treehouse process is interrupted during the hook. Because status hides destroying entries and Acquire still counts them against max_trees, an interrupted hook can make capacity disappear with no visible worktree to recover from.

Round 4 (auto-fix) - found 2 errors

• 🚨 internal/pool/pool.go:213 - Destroy still decides whether a worktree is safe to remove using only cwd-based process detection, ignoring the new OwnerPID reservation set by Acquire. During the window after Acquire reserves a worktree and before the user's shell has a process cwd inside it, a concurrent non-force treehouse destroy can mark and remove the path that treehouse get is about to hand out; treat ownerAlive(wt) as in-use here before marking it destroying.
• 🚨 internal/pool/pool.go:263 - DestroyAll has the same reservation blind spot: non-force destroy-all skips only Destroying entries and cwd-detected processes, so it can remove worktrees currently reserved by a live treehouse get process before the shell appears in the worktree. Include ownerAlive(wt) in the in-use check so reserved worktrees cannot be deleted out from under an acquire.

Round 5 (auto-fix) - found 1 error

• 🚨 internal/pool/pool.go:275 - DestroyAll still includes entries already marked Destroying by another live command in its removal snapshot. A concurrent non-force treehouse destroy --all can skip the in-use check for that entry, overwrite its OwnerPID, run pre_destroy a second time, and remove the worktree while the first destroy hook is still running, causing duplicate hooks and a failed/partial first destroy; exclude live destroying entries from the snapshot or reject them as in-use unless forced.

Round 6 (auto-fix) - found 1 warning

• ⚠️ internal/pool/pool.go:338 - Stale reservations are considered live based only on PID existence. If a treehouse get or destroy process exits before clearing OwnerPID and the OS later reuses that PID, ownerAlive will keep treating the worktree as in-use or destroying, which can hide it from status and permanently consume pool capacity until manual state cleanup; store and validate process start time or another process identity before trusting a persisted PID.

Round 7 (auto-fix) - found 2 errors

• 🚨 internal/pool/pool.go:254 - Destroy does not verify that the state entry it removes is still the destroying reservation created by this command. If another forced destroy removes the entry while this command is in pre_destroy, a subsequent treehouse get can recreate the same numbered path; when the first destroy resumes it will match only by path and remove the newly acquired worktree from under that session. Recheck Destroying, OwnerPID, and OwnerStartedAt before final removal, and treat a changed reservation as already superseded instead of deleting the path.
• 🚨 internal/pool/pool.go:303 - DestroyAll removes every path from its pre-hook snapshot before validating that those paths still belong to the same destroy reservation. A concurrent forced destroy can delete one of those entries, then an acquire can recreate the same numbered worktree path before this final loop runs, causing DestroyAll to delete a newly acquired worktree and then drop it from state via the path-based remove map. Validate the current entry's reservation identity under the lock before removing each snapshot path.

Round 8 (auto-fix) - found 1 error

• 🚨 cmd/get.go:50 - Passing cfg.Hooks.PostCreate directly into Acquire makes repo-level treehouse.toml hooks auto-execute on treehouse get because config.Load prefers the checked-out repository config before the user config. A malicious or compromised repository can therefore run arbitrary shell commands as soon as a user asks Treehouse for a worktree; require explicit trust/confirmation for repo-defined hooks or restrict executable hooks to user-level config before merging.

Round 9 (auto-fix) - found 1 warning

• ⚠️ treehouse.toml.example:2 - The example still tells users they can place this file at <repo_root>/treehouse.toml, but the implementation now intentionally ignores [hooks] from repo-level config and only executes hooks from the user config. A user following this example will add post_create/pre_destroy to the repo config and see them silently not run; document that hooks are only honored from ~/.config/treehouse/config.toml or split the example so repo-safe settings and executable hooks are not presented as interchangeable.

Round 10 (auto-fix) - found 1 warning

• ⚠️ internal/pool/pool.go:144 - Release clears OwnerPID/OwnerStartedAt without clearing or rejecting Destroying. If a pre_destroy hook or concurrent treehouse return runs against a worktree that has been marked destroying, the final destroy will treat its reservation as superseded and skip removal, while healState will not recover it because the owner is now zero; the entry remains hidden from status and permanently skipped by Acquire. Clear Destroying when clearing the owner, or reject returning destroying entries.

Round 11 (auto-fix) - found 1 warning

• ⚠️ internal/pool/pool.go:134 - Release resets and cleans the worktree before checking whether the state entry is marked Destroying. A concurrent treehouse return or get-exit path can therefore delete files while a pre_destroy hook is still running, then only afterward fail with worktree ... is being destroyed; check the destroy reservation under the state lock before mutating the worktree.

Round 12 (auto-fix) - passed ✅

✅ **Test** - passed

Round 1 - passed ✅

• go test ./internal/config ./internal/hooks ./internal/pool ./cmd
• go test ./...

🔧 **Document** - 3 issues found → auto-fixed (2)

Round 1 - found 3 issues (2 warnings, 1 info)

• ⚠️ README.md:133 - The README still says in-use detection is purely process scanning and that usage state is never persisted. The change adds persisted owner and destroy reservation fields (owner_pid, owner_started_at, destroying) to the state file and treats live reservations as in-use, so this design note is now stale and should explain the new temporary persisted reservation behavior.
• ⚠️ AGENTS.md:38 - The agent guide's key design decisions still say in-use detection is runtime-only and never persisted, and line 39 says the state file only tracks pool membership. The implementation now persists owner and destroy reservation metadata in treehouse-state.json, so this internal documentation should be updated to distinguish process scanning from temporary reservation state.
• ℹ️ AGENTS.md:56 - The agent guide's config section still presents repo-level treehouse.toml and user-level ~/.config/treehouse/config.toml as equivalent locations. With this change, lifecycle hooks are intentionally loaded only from the user-level config and ignored from repo-level config, so the guide should document the user-only hooks rule and include the new [hooks] keys if it is meant to stay current with configuration behavior.

Round 2 (auto-fix) - found 1 info

• ℹ️ AGENTS.md:7 - The project structure section was not updated for the newly added internal/hooks/ package. The change introduces a dedicated hooks package used by pool lifecycle operations, so the internal agent guide should list it alongside the other internal packages and describe that it runs user-configured lifecycle hook commands.

Round 3 (auto-fix) - passed ✅

✅ **Lint** - passed

Round 1 - passed ✅

✅ **Push** - passed

Round 1 - passed ✅