Skip to content

fix(bin): isolate treehouse worktree pools per firstmate home - #19

Closed
mehulbhagwani wants to merge 9 commits into
upstream-mainfrom
fm/pool-roots
Closed

mehulbhagwani wants to merge 9 commits into
upstream-mainfrom
fm/pool-roots

Conversation

@mehulbhagwani

Copy link
Copy Markdown
Owner

Intent

Upstream PR kunchenguid#4932 (fix: isolate treehouse pools per home, so each firstmate home gets its own Treehouse worktree pool root and roots inside a firstmate home are rejected) is focused on that fix and mergeable, but it is 7 commits behind Kun's main, and Kun's reviewer needs a clean current tip with a matching no-mistakes validation record before approving workflows and considering a merge.

Lift the build phase for this (bring the upstream Firstmate PRs up to Kun's bar so they can merge).

What Changed

  • Give each firstmate home its own Treehouse worktree pool root, seeded during home creation/install, instead of sharing a single global pool.
  • Reject Treehouse pool roots that resolve inside a firstmate home, with the new guard wired into bootstrap and spawn paths, and update bootstrap-diagnostics and architecture/configuration docs to describe the new per-home root and rejection behavior.
  • Update existing tests across bootstrap, spawn dispatch-profile, secondmate lifecycle/liveness/sync/safety/harness, backlog atomicity, tangle guard, session start, startup memory budget, x-mode, and Herdr presentation e2e to account for per-home pool roots, plus add new coverage in tests/fm-treehouse-home-root.test.sh and tests/fm-treehouse-pool-isolation-live-e2e.test.sh.

🤖 Generated with Claude Code

Risk Assessment

✅ Low: The PR's actual scope (fm_treehouse_home_root derivation, its two call sites in fm-home-seed.sh and fm-spawn.sh, the bootstrap capability probe, and the treehouse CI pin bump) is well-bounded, matches the stated intent exactly, is guarded against nested-home path collisions, and is backed by tests that exercise real commands/derivations rather than source-text matching; the remaining bulk of the diff (fm-path-lib.sh, recovery-marker rework, claude --add-dir grant, etc.) is pre-existing work pulled in by rebasing onto Kun's main, not new work introduced by this change.

Testing

Ran the targeted live/unit tests that exercise this branch's actual behavior changes (per-home Treehouse pool root derivation, concurrent acquisition, interactive acquire, legacy-root return compatibility, and the fork-free path/epoch/stat helpers) directly against the real treehouse v3.1.0 binary and real git; every fix-specific assertion passed. The one failure encountered is a vendor-baseline sanity check in fm-treehouse-pool-isolation-live-e2e.test.sh whose own comments anticipate exactly this ("re-derive this guard against the new behavior") — it is asserting that the installed treehouse still exhibits the pre-fix collision bug, which it no longer does on this machine's v3.1.0. This is the same untested/declined vendor-baseline item recorded from the prior test round, not a new regression. Did not run the full bootstrap/spawn/secondmate/watch regression suites (broad, long-running, previously flagged as out of scope for targeted validation and at risk of the same budget timeout already declined in round 1).

  • Live validation: ⚠️ inconclusive - 5 of 7 scenarios driven live against the product
Scenario Result Live Evidence
Per-home Treehouse worktree root: two firstmate homes cloning the same origin derive different, isolated pool roots ✅ pass live tests/fm-treehouse-home-root.test.sh: 'the worktree root is per home and independent of how the home or base is spelled', 'the derived worktree root stays outside the active and root Firstmate homes'
Concurrent acquisition: two homes acquire worktrees from their own roots at the same time with no contention or cross-ownership ✅ pass live fm-treehouse-pool-isolation-live-e2e.test.sh::test_per_home_roots_acquire_concurrently_without_contention -> 'ok - two homes cloning one origin acquire concurrently from their own roots with no conten…
Interactive spawn path: fm-spawn's interactive treehouse get enters a worktree under the home's own derived root ✅ pass live fm-treehouse-pool-isolation-live-e2e.test.sh::test_interactive_get_honors_the_root_it_is_given -> 'ok - the interactive acquire the worker's shell runs enters a worktree below the root it is given'
Migration compatibility: a worktree leased under the old shared root still returns correctly once the home moves to its own per-home root ✅ pass live fm-treehouse-pool-isolation-live-e2e.test.sh::test_a_worktree_from_another_root_still_returns -> 'ok - a worktree leased under a different root still returns to its own pool, so nothing in flight has…
Fork-free path/epoch/stat helper library behaves identically to the shelled-out dirname/basename/date/tr pipelines it replaces ✅ pass live tests/fm-fork-free-helpers.test.sh: all 6 assertions ok, e.g. 'fm_dirname_to and fm_basename_to match dirname and basename on every edge case'
Vendor-baseline case: a slot freed under a shared root is still handed to another clone's checkout on the currently installed treehouse ⏸️ untested no Already recorded as declined/untested in a prior test round on this branch (the installed treehouse v3.1.0 no longer exhibits the pre-fix vendor collision this guard assumes, so the pretest itself can…
Wider spawn/bootstrap/secondmate/watch regression suites remain green after this change ⏸️ untested no Out of scope for targeted validation per the no-mistakes test-phase rule against running the full suite, and previously flagged/declined in round 1 as at risk of the same 30-minute agent budget timeou…
  • Outcome: ⚠️ 1 warning across 1 run (4m13s)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

⚠️ **Test** - 1 warning
  • ⚠️ live validation verdict: inconclusive (5 of 7 scenarios were driven live against the product); untested: Vendor-baseline case: a slot freed under a shared root is still handed to another clone's checkout on the currently installed treehouse, Wider spawn/bootstrap/secondmate/watch regression suites remain green after this change
  • Live validation: ⚠️ inconclusive - 5 of 7 scenarios driven live against the product
Scenario Result Live Evidence
Per-home Treehouse worktree root: two firstmate homes cloning the same origin derive different, isolated pool roots ✅ pass live tests/fm-treehouse-home-root.test.sh: 'the worktree root is per home and independent of how the home or base is spelled', 'the derived worktree root stays outside the active and root Firstmate homes'
Concurrent acquisition: two homes acquire worktrees from their own roots at the same time with no contention or cross-ownership ✅ pass live fm-treehouse-pool-isolation-live-e2e.test.sh::test_per_home_roots_acquire_concurrently_without_contention -> 'ok - two homes cloning one origin acquire concurrently from their own roots with no conten…
Interactive spawn path: fm-spawn's interactive treehouse get enters a worktree under the home's own derived root ✅ pass live fm-treehouse-pool-isolation-live-e2e.test.sh::test_interactive_get_honors_the_root_it_is_given -> 'ok - the interactive acquire the worker's shell runs enters a worktree below the root it is given'
Migration compatibility: a worktree leased under the old shared root still returns correctly once the home moves to its own per-home root ✅ pass live fm-treehouse-pool-isolation-live-e2e.test.sh::test_a_worktree_from_another_root_still_returns -> 'ok - a worktree leased under a different root still returns to its own pool, so nothing in flight has…
Fork-free path/epoch/stat helper library behaves identically to the shelled-out dirname/basename/date/tr pipelines it replaces ✅ pass live tests/fm-fork-free-helpers.test.sh: all 6 assertions ok, e.g. 'fm_dirname_to and fm_basename_to match dirname and basename on every edge case'
Vendor-baseline case: a slot freed under a shared root is still handed to another clone's checkout on the currently installed treehouse ⏸️ untested no Already recorded as declined/untested in a prior test round on this branch (the installed treehouse v3.1.0 no longer exhibits the pre-fix vendor collision this guard assumes, so the pretest itself can…
Wider spawn/bootstrap/secondmate/watch regression suites remain green after this change ⏸️ untested no Out of scope for targeted validation per the no-mistakes test-phase rule against running the full suite, and previously flagged/declined in round 1 as at risk of the same 30-minute agent budget timeou…
  • bash tests/fm-treehouse-home-root.test.sh
  • bash tests/fm-treehouse-pool-isolation-live-e2e.test.sh (partial run, see scenario notes)
  • bash tests/fm-fork-free-helpers.test.sh
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

mehulbhagwani and others added 9 commits September 28, 2026 03:58
Treehouse keys a worktree pool by the repository's resolved origin rather
than by the clone, so every home holding its own clone of one repository
allocated from a single shared pool. Homes competed for the same numbered
slots, and a returned slot read free while its checkout was still a linked
worktree of another home's clone; fm-spawn.sh's isolation assertion then
refused it and the task stopped until a human released the slot from the
home that owned it.

Derive a per-home worktree root from the home's own resolved path and pass
it to both acquisition sites, so two homes never see each other's slots.
Returns stay flagless on purpose: treehouse resolves a return's pool from
the worktree path it is handed, so every worktree already leased under the
previously shared root stays returnable and nothing is migrated.

Bootstrap now probes the global --root flag alongside the durable lease,
and the CI treehouse pin moves to v2.3.0 because v2.0.1 has no such flag.
… treehouse

Adding the global --root flag and the second capability probe changed two
things the suites pin: the acquire command sent to a worker's shell, and
what a fake treehouse must advertise to be accepted.

Match the acquire around its root rather than pinning the bare command, so
the assertions stay meaningful instead of passing because the old spelling
disappeared, and teach the stubs that --root precedes the subcommand.
The bootstrap suite gains the has-the-lease-but-no-root case beside its
existing lease case, and a both-present case so neither probe can pass for
the other's reason.
Treehouse keys a worktree pool by the repository's resolved origin rather
than by the clone, so every home holding its own clone of one repository
allocated from a single shared pool. Homes competed for the same numbered
slots, and a returned slot read free while its checkout was still a linked
worktree of another home's clone; fm-spawn.sh's isolation assertion then
refused it and the task stopped until a human released the slot from the
home that owned it.

Derive a per-home worktree root from the home's own resolved path and pass
it to both acquisition sites, so two homes never see each other's slots.
Returns stay flagless on purpose: treehouse resolves a return's pool from
the worktree path it is handed, so every worktree already leased under the
previously shared root stays returnable and nothing is migrated.

Bootstrap now probes the global --root flag alongside the durable lease,
and the CI treehouse pin moves to v2.3.0 because v2.0.1 has no such flag.
@mehulbhagwani

Copy link
Copy Markdown
Owner Author

Closing this fork-side PR; the validated change belongs on kunchenguid#4932.

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