Skip to content

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

Open
Lifferado wants to merge 2 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 2 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, with lease acquisition, abort cleanup, task-record ownership, backend path settlement, and teardown release forming a consistent lifecycle.

No actionable defect remains: affected backends reflect the top-level worktree change, failed pre-publication spawns return their lease, and successful tasks persist the leased path and project needed by teardown’s forced return.

Reviews (1): Last reviewed commit: "no-mistakes(review): fix: add lease stub..." | 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.

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