Skip to content

fix(bin): lease each crew spawn's treehouse slot until teardown - #2760

Open
Lifferado wants to merge 3 commits into
kunchenguid:mainfrom
Lifferado:fm/wt-slot-t5
Open

fix(bin): lease each crew spawn's treehouse slot until teardown#2760
Lifferado wants to merge 3 commits into
kunchenguid:mainfrom
Lifferado:fm/wt-slot-t5

Conversation

@Lifferado

Copy link
Copy Markdown

Fixes #2754.

Problem

fm-spawn asked the pane's shell to run an interactive treehouse get, which holds its pool slot only while processes still run inside it. A parked worker whose occupant processes went quiet left its recorded worktree= pointing at a slot the pool considered free, so the next spawn on the same project received the same path - two concurrently-live tasks sharing one worktree, with task B doing its whole job inside task A's copy.

Fix

  • fm-spawn acquires the slot itself with durable treehouse get --lease --lease-holder <task-id> (the idiom bin/fm-home-seed.sh already uses; bootstrap already gates fleet-wide on lease support), so the lease survives process death until teardown's treehouse return --force releases it.
  • The pane is sent into that exact leased path with a top-level cd, and spawn settles only after two consecutive pane reads agree on the leased physical path.
  • An abort between lease acquisition and task-record publication releases the lease through the existing spawn_abort_cleanup EXIT-trap idiom, so failed spawns cannot strand slots; after publication the recorded task owns the slot through teardown.
  • New regression suite tests/fm-spawn-worktree-lease.test.sh proves two live tasks cannot share a slot (verified failing on pre-fix code), that an allocator refusal stops the spawn fail-closed, and that an aborted spawn releases its lease. Existing spawn suites' inert exit-0 treehouse stubs were updated via a shared fm_fake_treehouse_lease helper; the new suite is mapped into the backend-dispatch family.
  • docs/cmux-backend.md and docs/zellij-backend.md pane-cwd lines updated to describe the top-level cd flow.

Validation

Pipeline run 01M0K7B0BRTY30D1E3HG93432J passed intent, rebase, review, test, document, and lint; its two review fixes are included. The push step alone failed because the ambient credential (Lifferado) lacks push access to the upstream repo, so this branch is pushed from the fork and this PR opened per supervisor instruction. Review auto-fix findings addressed: missing lease stub in tests/fm-secondmate-harness.test.sh's real-spawn fixture; lease-disarm window closed by moving the disarm immediately after record publication.

An interactive in-pane `treehouse get` holds its slot only while
processes run inside it, so a parked worker whose occupant processes
went quiet could see its recorded worktree handed to the next spawn on
the same project - two concurrently-live tasks sharing one slot
(kunchenguid#2754). Spawn now acquires the pool slot itself with
`treehouse get --lease --lease-holder <task-id>`, whose durable lease
survives process death until teardown's `treehouse return --force`
releases it, sends the pane into that exact leased path, and settles
only when the pane reads there twice consecutively. An abort between
lease and task-record publication releases the lease through the
existing abort-cleanup trap so failed spawns cannot strand slots.
@greptile-apps

greptile-apps Bot commented Aug 21, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Reviews (2): Last reviewed commit: "fix(test): answer lease-shaped treehouse..." | Re-trigger Greptile

@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate:

VISION: aligned. This is a scripted isolation fix for a real parked-worker failure mode, not a new captain-facing surface and not inferred consent. It deepens the "restart is a non-event" rule (the lease survives process death) and fail-closes when the allocator refuses. A ready-for-pr issue is not a merge vote; VISION, CI, and default-behavior still apply. Class: corrective (default spawn path, bugfix).

Security: none.

Overlap / do not land together:

CI: first-time fork workflows approved after diff review (CI run 32536015279, Require no-mistakes 32536015277). Greptile is green. no-mistakes is still pending and is blocking. HEAD is 2 behind main, mergeable.

Waiting on CI — not on the author, not on the captain. Coordinator must sequence around the holds above even if CI goes green.

Merge-eligible: NO. Captain-flag NOW: NO.

@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate:

Corrective lease for #2754: treehouse get --lease --lease-holder so a parked worker cannot free a slot another live task then receives. Aligns (isolated copies; scripts own the allocator; fail-closed on refusal).

Not merge-eligible. Require no-mistakes FAILED on this HEAD. CI still in progress. File-level overlap with #2770 on fm-spawn.sh and with spawn-freshen #2622/#2693/#2154 on spawn/freshen tests (this PR is lease, not freshen). Distinct from held teardown pair #2637/#2692.

Waiting on the author for a green no-mistakes raise — not waiting on the captain.

@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate:

Correction on CI: after approving the first-time fork workflows, Require no-mistakes failed. The check did not find Updates from git push no-mistakes / a no-mistakes-pipeline-attestation:v1 block in the PR body. The description names pipeline run 01M0K7B0BRTY30D1E3HG93432J and says the push step lacked upstream access, but that is not a substitute for the attestation this repo's check requires (no-mistakes >= 1.46.0).

Portable CI is still in flight. no-mistakes is blocking regardless.

Waiting on the author to re-deliver through git push no-mistakes so the body carries structured attestation — not waiting on the captain. Overlap holds from the previous comment still apply. Merge-eligible remains NO.

…tation abort fixture

The post-create abort fixture armed worktree-validation refusals by making
its treehouse stub answer a bare interactive `get` with silence while the
wrapped pane reported a non-worktree cwd. fm-spawn now executes
`get --lease --lease-holder <task-id>` itself (kunchenguid#2754), so the silent answer
aborted each spawn at an empty acquisition instead of at the armed
"did not yield an isolated worktree" validation. Answer `--lease` with the
same bogus cwd the fake pane reports and intercept best-effort lease
returns, keeping the fixture's post-create abort semantics intact.
@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate:

Re-review of NEW HEAD b5bea4983fed. Corrective lease for ready-for-pr #2754: treehouse get --lease --lease-holder so a parked worker cannot free a slot another live task then receives. A ready-for-pr issue is not a merge vote.

VISION (per rule):

  • One captain, one interface — aligns. No new captain-facing surface; allocator refusal is a named fail-closed error.
  • Authority is explicit — aligns. Isolation bugfix on the existing spawn path, not a new grant.
  • Scripts own the mechanics — aligns. The allocator is a scripted treehouse lease; spawn no longer asks the pane's shell to hold the slot by occupancy.
  • A restart is a non-event — aligns. The lease survives process death until teardown's treehouse return --force.
  • Delegation with a spine — aligns. Isolated copies; an unleased slot refuses launch.
  • The fleet outlives any vendor — aligns. Treehouse remains the worktree provider; Orca still skips the lease.
  • Scope — aligns. Command-layer isolation, not workshop validation.

Security: no.

Overlap / do not land together:

CI: first-time fork workflows re-approved on this HEAD after diff review (CI 32564007475, Require no-mistakes 32564007596). Greptile is green. Portable CI has not finished. The PR body still has no Updates from git push no-mistakes / no-mistakes-pipeline-attestation:v1 block (pipeline run 01M0K7B0BRTY30D1E3HG93432J is not a substitute). no-mistakes is blocking.

Ahead 3, behind 5, mergeable, mergeStateStatus UNSTABLE.

Waiting on the author for a green no-mistakes raise whose body attestation matches this HEAD — not waiting on the captain. Coordinator must sequence around the holds even if CI later goes green.

Merge-eligible: NO. Captain-flag NOW: NO.

@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate:

Newer HEAD b5bea4983fed for #2754. Still corrective (lease until teardown). Require no-mistakes is still FAILED. Do not land with held pair #2637/#2692 or with #2770 (same fm-spawn.sh).

Waiting on the author for a green no-mistakes raise — not waiting on the captain.

@Lifferado

Copy link
Copy Markdown
Author

Crew verdict on the #2754 fix (fm/wt-slot-t5 @ b5bea49)

Primary defect and fix. fm-spawn asked the pane shell to run an interactive treehouse get, whose slot protection lasts only while processes run inside it. A parked worker whose occupant processes went quiet left its recorded worktree= pointing at a slot the pool considered free, so a later spawn on the same project received the identical path — two concurrently-live tasks sharing one worktree (#2754). This branch fixes that: fm-spawn acquires the slot itself via durable treehouse get --lease --lease-holder <task-id> (the idiom bin/fm-home-seed.sh already uses; bootstrap gates fleet-wide on lease support), sends the pane into the exact leased path with a top-level cd, settles only after two consecutive pane reads agree on the leased physical path, and releases the lease through the existing spawn_abort_cleanup EXIT trap when a spawn aborts between lease acquisition and task-record publication.

Validation. Pipeline run 01M0K7B0BRTY30D1E3HG93432J passed intent, rebase, review, test, document, and lint; only its push step failed, on credential scope (hence this fork-raised PR). New regression suite tests/fm-spawn-worktree-lease.test.sh proves two concurrently-live tasks cannot share a slot and is verified failing on pre-fix code. Latest CI: 12 of 13 behavior checks green.

Residual known failure — confirmed not flake. Behavior tests (Herdr) fails in the concurrent post-create abort fixtures of tests/fm-backend-herdr-presentation-e2e.test.sh: after the armed worktree-validation refusal, each task's projected pane is already gone (pane_not_found) when its own cleanup verifies it, so cleanup refuses ("could not verify the exact pane") and the serialized create→close audit loses its close events ("interleaved outside the presentation lock"). Reproduced identically locally and on a quiet CI runner (run 32564007475). Ruled out so far: the new cd dispatch itself (raw-Herdr probes keep panes alive through cd, seeded-prune, and concurrent gets). Not yet root-caused: which actor removes the projected workspaces before their own serialized close once spawn acquires slots by durable lease. Commit b5bea49 already restores the fixture's armed-stub contract for lease-shaped acquires, which fixed the original assertion progression; the deeper residual layer remains open.

Repo-policy check. "PR must be raised via no-mistakes" failing is expected here: the shared no-mistakes daemon could not push from this environment (upstream credential scope), so the branch was pushed from a fork per operator instruction; every pre-push pipeline step passed.

Recommendation: fix-forward. Hold merge until the residual edge is root-caused and fixed on this branch. Leading suspects are Herdr's emptying-close/husk machinery interacting with pre-launch validation aborts, or a fixture contract that needs deliberate re-specification for the lease era; both are bounded follow-up work, and the underlying #2754 race is a real data-integrity hazard worth landing promptly once green. Alternatives, in order of fallback: park this PR while the residual is investigated separately, or document-and-land with the required Herdr job knowingly red — which we do not recommend. Happy to drive the fix-forward round on this same branch on your word.

@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate:

Re-review of THIS HEAD b5bea4983fed after Lifferado's 11:41Z note. Corrective lease for ready-for-pr #2754: treehouse get --lease --lease-holder so a parked worker cannot free a slot another live task then receives. A ready-for-pr issue is not a merge vote.

VISION (per rule):

  • One captain, one interface — aligns. No new captain-facing surface; allocator refusal is a named fail-closed error.
  • Authority is explicit — aligns. Isolation bugfix on the existing spawn path, not a new grant.
  • Scripts own the mechanics — aligns. The allocator is a scripted treehouse lease; spawn no longer asks the pane's shell to hold the slot by occupancy.
  • A restart is a non-event — aligns. The lease survives process death until teardown's treehouse return --force.
  • Delegation with a spine — aligns. Isolated copies; an unleased slot refuses launch.
  • The fleet outlives any vendor — aligns. Treehouse remains the worktree provider; Orca still skips the lease.
  • Scope — aligns. Command-layer isolation, not workshop validation.

Security: none.

Overlap / do not land together:

CI: Require no-mistakes FAILED (run 32564007596). No structured no-mistakes-pipeline-attestation:v1 in the body; pipeline run 01M0K7B0BRTY30D1E3HG93432J is not a substitute. Behavior tests (Herdr) FAILED (CI 32564007475) on the concurrent post-create abort fixtures in tests/fm-backend-herdr-presentation-e2e.test.sh (author reproduced; not flake). First-time fork workflows were already approved on this HEAD. Greptile is not a merge gate.

Ahead 3, behind 6, mergeable, mergeStateStatus UNSTABLE.

Not merge-eligible. Waiting on the author for a green no-mistakes raise whose body attestation matches this HEAD and for Herdr CI green — not waiting on the captain. Coordinator must still sequence around the holds even if CI later goes green.

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.

fm-spawn/treehouse: pool assigned the same worktree slot to two concurrently-live tasks

2 participants