Skip to content

feat(pool): add durable worktree leases#35

Merged
kunchenguid merged 5 commits into
mainfrom
fm/treehouse-lease-t4
Jun 22, 2026
Merged

feat(pool): add durable worktree leases#35
kunchenguid merged 5 commits into
mainfrom
fm/treehouse-lease-t4

Conversation

@kunchenguid

Copy link
Copy Markdown
Owner

Intent

Add a durable worktree lease to treehouse so a caller (firstmate's persistent secondmate homes) can permanently reserve a pooled worktree without keeping a live process inside it. Today treehouse decides in-use vs available from live processes plus short-lived owner reservations, and 'get' opens an interactive subshell whose lifetime IS the hold - so a process-less home is unreclaimable-safe.

Design decisions:

  • Introduced NEW persistent state on WorktreeEntry: Leased/LeaseHolder/LeasedAt, all json omitempty so existing on-disk state files keep today's behavior (no lease recorded == old behavior). A lease is deliberately process-independent: it carries NO owner PID, and healState (which only clears dead owner reservations) never clears it, so it survives with zero processes inside.
  • New 'treehouse get --lease' flag: non-interactive acquire that opens no subshell, marks the worktree durably leased, and prints ONLY the worktree's absolute path to stdout while routing all banners AND post-create hook stdout to stderr, so path=$(treehouse get --lease) works in scripts. Per the task's design note the no-subshell mode intentionally IMPLIES the lease (a non-leasing detach would be instantly reclaimable). Optional --lease-holder / $TREEHOUSE_LEASE_HOLDER records the holder for diagnostics.
  • A leased worktree is skipped by Acquire and by prune regardless of running processes, is treated as in-use by worktreeInUse so non-force destroy rejects it (--force still overrides), shows a distinct 'leased' state (magenta, with holder) in status, and is cleared by Release so 'treehouse return ' releases it while keeping its existing lingering-process termination.
  • Concurrency reuses the existing WithStateLock (flock) around all pool mutation; both Acquire and AcquireLease delegate to a shared acquire(...) core, and markAcquired stamps either a lease or an owner reservation.

Back-compat: default interactive 'get' subshell behavior is unchanged. Tests added at pool level (lease marking, skipped-by-get, skipped-by-prune with no process, return clears lease, status shows leased, healState preserves lease, non-force destroy rejected, concurrent acquires never double-lease - the last verified under -race) and e2e CLI level (stdout-only path, holder recorded, leased skipped by get and prune, return releases). README and AGENTS.md document the flag and the lease concept. Windows/Linux cross-builds, go vet, and gofmt all clean.

What Changed

  • Added durable worktree lease support via treehouse get --lease, including holder metadata, path-only stdout for scripting, and persisted lease state that survives without live processes.
  • Updated pool, return, status, prune, and destroy behavior so leased worktrees are skipped by normal acquisition/pruning, shown distinctly in status, rejected by non-force destroy, and released with treehouse return <path>.
  • Documented the lease workflow and added focused pool/CLI coverage for lease acquisition, release, status, prune/destroy behavior, return-by-path handling, and concurrent lease acquisition.

Risk Assessment

⚠️ Medium: The lease feature touches persistent pool state and core acquire/return/prune/destroy behavior, but the implementation is cohesive and I found no material correctness issues in the reviewed diff.

Testing

No separate baseline output was supplied; I ran the lease-focused Go tests, the concurrent acquire race test, the full Go suite, and an isolated CLI workflow that demonstrates clean stdout capture, persisted holder state without an owner PID, skip-by-get, survival through prune, non-force destroy rejection, and release plus reuse. All checks passed, and the reviewer-visible transcript is saved in the requested evidence directory.

Evidence: durable lease CLI E2E transcript

Source: durable lease CLI E2E transcript

Key assertions: stdout_line_count=1, stdout_contains_banner=no, owner_pid_present_after_lease=no, normal_get_reused_leased_path=no, leased_path_exists_after_prune=yes, destroy_exit_code=1, released_worktree_reused=yes.

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 1 issue found → auto-fixed (3) ✅
  • ⚠️ internal/pool/state.go:26 - omitempty does not omit a zero time.Time, so cleared or never-leased entries will still write &#34;leased_at&#34;: &#34;0001-01-01T00:00:00Z&#34; after WriteState. That contradicts the intended sparse lease metadata and leaves stale-looking lease fields in the state file. Use omitzero or make this a *time.Time so non-leased entries omit it.

🔧 Fix: Omit cleared lease timestamps from state
1 warning still open:

  • ⚠️ cmd/get.go:122 - The lease banner advertises treehouse return &lt;path&gt; as the release command, but return still derives the repo and pool from the caller's cwd rather than from the provided path. A script that captures a leased path and later runs this command outside the original repo, or from another repo, cannot release the lease. Either make explicit-path return resolve config from that worktree path, or document the cwd requirement here.

🔧 Fix: Fix return-by-path lease release
1 warning still open:

  • ⚠️ cmd/return_cmd.go:29 - return &lt;path&gt; now resolves the pool from the managed worktree's main repo, but get --lease still creates the pool from the caller's git.FindRepoRoot() result. If the lease was acquired while running inside a linked worktree whose directory name or relative config root differs from the main checkout, return-by-path from another directory will look in a different pool and report the path as unmanaged. Use the same canonical repo root on acquire and return, or add a deliberate fallback for existing linked-worktree pool identities.

🔧 Fix: Fix return-by-path pool lookup
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • TREEHOUSE_NO_UPDATE_CHECK=1 go test ./internal/pool ./cmd -run 'Test(AcquireLease|Release_ClearsLease|GetLease|LeasedWorktree|Return.*Lease)' -count=1
  • TREEHOUSE_NO_UPDATE_CHECK=1 go test -race ./internal/pool -run TestAcquireLease_ConcurrentAcquiresNeverDoubleLease -count=1
  • TREEHOUSE_NO_UPDATE_CHECK=1 go test ./...
  • Manual CLI E2E: built treehouse, then in an isolated fixture repo ran treehouse get --lease --lease-holder secondmate-home, treehouse status, SHELL=&lt;true&gt; treehouse get, treehouse prune --yes, printf &#39;y\n&#39; | treehouse destroy &lt;leased-path&gt;, treehouse return &lt;leased-path&gt;, and SHELL=&lt;true&gt; treehouse get; transcript saved to .no-mistakes/evidence/fm/treehouse-lease-t4/lease-cli-e2e.txt.
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

Add a persistent, process-independent worktree reservation so a caller can
permanently hold a pooled worktree as a home without keeping a live process
inside it. This replaces the fragile process-based hold that consumers like
firstmate's persistent secondmate homes relied on.

- New WorktreeEntry.Leased/LeaseHolder/LeasedAt persistent state, all
  omitempty so pre-lease state files keep today's behavior.
- `treehouse get --lease` acquires without a subshell, marks the worktree
  leased, and prints only its absolute path to stdout (all banners and hook
  output go to stderr), so `path=$(treehouse get --lease)` is clean in
  scripts. `--lease-holder` / $TREEHOUSE_LEASE_HOLDER records the holder.
- Leased worktrees are skipped by get and prune regardless of running
  processes, require --force to destroy, and healState never clears them.
- `treehouse return <path>` clears the lease and returns the worktree,
  keeping its existing lingering-process termination behavior.
- `treehouse status` shows a distinct `leased` state with the holder.
- Concurrency is guarded by the existing WithStateLock (flock); both Acquire
  and AcquireLease delegate to a shared acquire() core.

Tests cover: lease prints only the path to stdout with info on stderr; a
leased worktree is skipped by get and prune with no process inside; return
releases it; status shows the leased state; concurrent acquires never
double-lease. README and AGENTS.md document the flag and the lease concept.
@kunchenguid kunchenguid merged commit 97d6708 into main Jun 22, 2026
4 checks passed
@kunchenguid kunchenguid deleted the fm/treehouse-lease-t4 branch June 22, 2026 23:23
@abhinav abhinav mentioned this pull request Jun 26, 2026
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.

1 participant