fix(spawn): capture worktree lease by repo-identity, not treehouse root#218
Open
maximebrmd wants to merge 4 commits into
Open
fix(spawn): capture worktree lease by repo-identity, not treehouse root#218maximebrmd wants to merge 4 commits into
maximebrmd wants to merge 4 commits into
Conversation
The worktree wait loop accepted the first pane cwd that differed from the project dir, so shell-init noise (e.g. a transient ~/.oh-my-zsh cwd during zsh startup - itself a git repo, so the post-loop isolation guard would fail the spawn rather than wait) could be captured. Require the captured path to be an existing git worktree under the treehouse root, so the loop keeps polling for the genuine lease; the isolation guard remains the backstop.
…spawn worktree test
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Changed
bin/fm-spawn.sh: instead of accepting the first pane cwd that merely differs from the project dir (which could latch onto shell-init noise like a transient~/.oh-my-zshcwd), it now accepts a path only when it is a genuine worktree of the same repository, comparing canonical git common dirs and resolving every path withpwd -Pso a symlinked$HOMEor a non-default treehouse pool root no longer breaks the match.FM_SPAWN_WORKTREE_WAIT(default 60s), threaded through the loop and the timeout error message, and documented it indocs/configuration.md.bin/fm-pr-merge.shand updatedtests/fm-tangle-guard.test.shfor the new repo-identity worktree check.Risk Assessment
✅ Low: A well-bounded 46-line change to a single spawn helper that faithfully replaces a brittle treehouse-root containment check with a symlink-safe repository-identity check, keeps the existing isolation guard as a backstop, and leaves no orphaned code.
Testing
Completed 1 recorded test check.
Pipeline
Updates from git push no-mistakes
⏭️ **intent** - skipped
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 2 issues found → auto-fixed ✅
bin/fm-spawn.sh:655- is_treehouse_worktree gates lease capture on containment under TH_ROOT=${FM_TREEHOUSE_ROOT:-$HOME/.treehouse}, but treehouse's pool root is configurable via treehouse.toml (rootkey: worktrees are placed under {root}/.treehouse/, e.g.root = "./"puts them under <repo>/.treehouse/). For any repo with a non-default treehouse root, the captured pane cwd will never be under $HOME/.treehouse, so the loop never selects, and spawn fails with the confusing 'treehouse get did not enter a worktree within 60s' timeout unless the operator knows to set the undocumented FM_TREEHOUSE_ROOT env var. Since this repo is a shared template for every user, consider deriving the root from treehouse.toml (or relaxing to 'is a git worktree distinct from PROJ_ABS' and relying on the existing isolation guard) rather than assuming $HOME/.treehouse.bin/fm-spawn.sh:429- The containment check uses raw string-prefix matching (path_is_ancestor_of) between the tmux pane_current_path (a resolved physical path on macOS/Linux) and the unresolved TH_ROOT ($HOME/.treehouse). If $HOME contains a symlinked component (common on some Linux hosts where /home is symlinked), the physical pane path won't be prefixed by the logical $HOME and the match silently fails, causing the same 60s timeout. The later isolation guard resolves both sides withpwd -P; this new pre-check does not.🔧 Fix: replace treehouse-root check with repo-identity worktree check
✅ Re-checked - no issues remain.
🔧 **Test** - 1 issue found → auto-fixed ✅
command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"🔧 Fix: guard bash 3.2 empty array in pr-merge; fix stale spawn worktree test
✅ Re-checked - no issues remain.
command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.