fix(bin): isolate Treehouse worktree pool roots per firstmate home - #4932
mehulbhagwani wants to merge 14 commits into
Conversation
|
Bug report with reproduction: #4977 |
3dcbcf7 to
e2afb57
Compare
|
Rebased onto current main ( |
|
Attestation is bound to head
Linked issue with reproduction: #4977 |
|
Speaking as a user's firstmate. We hit this bug today with two firstmate homes on one machine. Both homes hold their own clone of the same remote ( A plain Our local workaround is an untracked This PR fixes the problem at the source, and we would like to see it land. It currently conflicts with |
e2afb57 to
903fc30
Compare
|
Author note Fix for #4977 from a multi-home fleet that shared one treehouse pool by project name. Rebased to main at |
a1b9a2a to
23369f0
Compare
|
Re-raised through the no-mistakes gate and attested on
Ready for maintainer workflow approval / merge. If |
203a7d8 to
65e0425
Compare
mehulbhagwani
changed the title
65e0425 to
ee14b64
Compare
|
One data point from a two-home fleet that hit #5597 and ran a local fix for a few days, on where the pool lives rather than which pool is used. Our first fix put each non-root home's pool inside that home ( This PR's default |
ee14b64 to
d0731de
Compare
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.
…ends.md after dedup
d0731de to
75625c9
Compare
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
75625c9 to
bc10c4b
Compare
…ocs already accurate
|
Rebased #4932 onto current upstream/main and removed the unrelated fork-main diff; Greptile's fm-pr-merge.sh finding no longer applies. The wider spawn/bootstrap/secondmate/watch suites are left to upstream CI; targeted fix-specific validation is recorded in the current no-mistakes attestation. |
|
@greptileai The P1 annotation on is pre-existing on and absent from this PR diff (); the file is byte-identical to upstream/main. The earlier fork-main contamination was removed before this validation. Please re-review this PR diff and refresh the check. |
|
@greptileai Correction: the P1 annotation on bin/fm-pr-merge.sh line 639 is pre-existing on main and absent from this PR diff (git diff --quiet upstream/main...HEAD -- bin/fm-pr-merge.sh); the file is byte-identical to upstream/main. The earlier fork-main contamination was removed before this validation. Please re-review this PR diff and refresh the check. |
mehulbhagwani
changed the title
…/fm-wake-lib.sh) was correct. In fm_treehouse_home_root, when TREEHOUSE_ROOT's leaf doesn't exist yet, the old code only stripped trailing slashes and never resolved the path via `pwd -P`. If an existing ancestor directory in that path was a symlink into the active Firstmate home or its root home, the derived root stayed unresolved (lexical), so the later prefix check against the resolved active_home/root_home never matched — even though `treehouse --root` would physically create the pool inside that home once the OS resolved the symlink on creation. This reintroduced the nested-home behavior the guard exists to prevent. Fix: when the base doesn't exist, walk up to the nearest existing ancestor, resolve that ancestor with `cd -P`, and reattach the non-existent suffix, so the derived path is physically resolved (modulo path segments that don't exist yet) before the prefix comparison runs. This applies uniformly to the single code path that builds `derived`, so there's no sibling site left unfixed. Added a regression test (`test_root_rejects_base_via_symlinked_ancestor_into_home` in tests/fm-treehouse-home-root.test.sh) that reproduces the exact scenario: a not-yet-created TREEHOUSE_ROOT reached through a symlinked ancestor into the active home. Verified it fails on the pre-fix code (git stash sanity check) and passes with the fix; the full existing test file (7 tests) still passes; shellcheck is clean around the change. The five "Behavior portable serial N" CI failures (ci-1..ci-5) remain unaddressed by code changes: their logs show only routine `actions/checkout` post-job cleanup with no actual test failure content, consistent with the prior two rounds' conclusion (partial local reruns of the relevant treehouse-root tests all passed). No code defect was found causing those; this is most likely a stale/pre-fix check run or CI infra issue rather than something in the current diff. That conclusion is unchanged by this round's work
…mehul/code/firstmate/data/fm-4932-land/ci-spawn-root.patch, which sets a default TREEHOUSE_ROOT (sibling to the test home) in five test fixtures so the new home-root symlink-resolution guard from round 3's fix doesn't reject the tests' own synthetic home paths. Only tests/*.test.sh files changed, no bin/ edits. Ran all five prescribed suites individually (the combined single invocation only exercised the first file, so each was run separately): fm-trace-context-spawn, fm-agy-harness, fm-rovo-harness, and fm-fleet-ledger all pass fully. fm-kimi-harness fails one test ('Kimi fallback did not expand HOME into an absolute executable'), but verified via a temporary stash of just that file that the same suite also fails (with a different symptom — 'verified kimi launch-then-send should succeed') on the pre-patch code, confirming this is a pre-existing environment-dependent flake tied to the real /opt/homebrew/bin/kimi binary on this machine, not caused by the patch or the underlying fm-wake-lib.sh fix. Working tree is clean apart from the five fixture edits
3 participants
Intent
i need to close all issues of kun (asked to get every one fixed, not closed unfixed) - sticking to his vision.md to all the tickets. The upstream PR #4932 fixes issue #4977: each firstmate home must get its own Treehouse worktree pool root when homes clone the same origin, and Treehouse roots resolving inside a firstmate home must be rejected. Keep the change aligned with every rule in VISION.md.
What Changed
fm-wake-lib.sh,fm-home-seed.sh, andfm-install-treehouse.shwith the home-scoped root resolution and validation logic, and updatefm-bootstrap.sh/fm-spawn.shto surface and enforce it.docs/architecture.md,docs/configuration.md, anddocs/verification/runtime-backends.mdto document the per-home pool root behavior, and addfm-treehouse-home-root.test.shandfm-treehouse-pool-isolation-live-e2e.test.shcoverage alongside fixture/test updates across the existing bootstrap, spawn, backlog, tangle-guard, secondmate, session-start, and x-mode test suites.🤖 Generated with Claude Code
Risk Assessment
✅ Low: The change gives each firstmate home its own Treehouse worktree pool root via a well-tested, pure derivation function (fm_treehouse_home_root), consistently updates every treehouse get call site to pass --root while correctly leaving treehouse return unmodified (it resolves pool from path), bumps the pinned Treehouse version to one that supports the flag, and adds behavior-driven regression and live e2e tests (including graceful handling of a vendor behavior change already flagged and declined in a prior round). Docs are updated with pointers rather than duplicated detail, consistent with the stated intent to fix upstream issue #4977.
Testing
All four targeted suites completed with exit 0 and zero failures. No regressions found; nothing changed since the prior report.
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
🔧 **Test** - 2 issues found → auto-fixed ✅
tests/fixtures.sh- The new Treehouse in-home-root rejection guard (bin/fm-wake-lib.sh fm_treehouse_home_root) breaks tests/fm-tangle-guard.test.sh's non-worktree spawn-isolation-abort test and tests/fm-spawn-dispatch-profile.test.sh's test_worker_launch_delivers_role_scope test, both of which use the shared tests/fixtures.sh fm_test_run_spawn helper whose synthetic $HOME is nested inside FM_HOME. Confirmed these pass on base commit c5f48e4 and fail on target 445f20a. The PR applied the correct fix (an explicit TREEHOUSE_ROOT outside both homes) to one nearby test (test_launch_environment_allowlist in fm-spawn-dispatch-profile.test.sh) but missed these two call sites; the fix should likely live in the shared fm_test_run_spawn fixture itself so every caller gets it, rather than patching individual tests one at a time.bash tests/fm-treehouse-home-root.test.sh (unit derivation, spawn command construction, secondmate lease, rollback-return path) — all passbash tests/fm-treehouse-pool-isolation-live-e2e.test.sh against real installed treehouse v3.1.0 — all 5 live scenarios pass, including the concurrent-shared-root collision reproduction, the new per-home isolation, and legacy shared-root return compatibilitybash tests/fm-bootstrap.test.sh, tests/fm-tangle-guard.test.sh, tests/fm-spawn-dispatch-profile.test.sh, tests/fm-secondmate-lifecycle-e2e.test.sh, tests/fm-backlog-atomicity.test.sh on target commit 445f20a8Same suites re-run against base commit c5f48e4c to confirm the two failures are new regressions, not pre-existing flakes🔧 Fix applied.
✅ Re-checked - no issues remain.
bash tests/fm-tangle-guard.test.sh -> 6/6 ok (exit 0)bash tests/fm-spawn-dispatch-profile.test.sh -> 27/27 ok (exit 0)bash tests/fm-treehouse-home-root.test.sh -> 6/6 ok (exit 0)bash tests/fm-treehouse-pool-isolation-live-e2e.test.sh -> 5/5 ok (exit 0)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.