Evaluate treehouse stable lease identities in place of path-comparison collision guard

[atqamz]

treehouse v2.1.0 added stable lease identities (kunchenguid/treehouse#68), surfaced through treehouse get --lease --json alongside the path hand already reads.

internal/worktree/worktree.go Get() currently discards everything in that payload except path, and CheckCollision then hand-rolls a guard by comparing the returned worktree path against every other active task's recorded path. That comparison exists to catch the stale-lease-after-crash bug (firstmate #947).

Worth evaluating whether a lease identity is a sounder key than a path string. A path is reused across leases by design: treehouse keeps the slot directory when a worktree is returned, so a path cannot distinguish a returned worktree from a leased one, which Return's own doc comment already spells out. A stable identity presumably can.

Not urgent, and not obviously a win until someone reads what the identity actually guarantees across a crash. Filing so it is not lost.

Context: #41 (comment)

[atqamz]

Scouted. The answer is yes, and the reasoning changed what this issue is about, so the summary is worth reading before anyone writes the code. Full report: /home/atqa/secondhand/data/secondhand-lease-identity/report.md.

The fact it turns on

treehouse regenerates a new 128-bit lease_id on every acquisition, explicitly including same-holder same-path reuse. Path is recycled; identity never is. Verified two ways: kunchenguid/treehouse#68's own test TestReturnConditionalLeaseIdentityLifecycle asserts it, and the scout reproduced it against this host's installed binary - get --lease returned slot path .../repo-a3d4ba/1/repo twice with lease_id 876b3a05... then 7027ba55....

Version floor is treehouse v2.1.0. This host runs exactly v2.1.0, so no upgrade is needed.

The guard is broken in the opposite direction from what this issue assumed

This issue was filed suspecting path comparison misses a real collision. It does not, and the scout could not construct a sequence where it does: treehouse's Acquire already refuses a slot that is currently leased, and worktree.Get already always passes --lease, so two live leases on one path cannot coexist. That guarantee comes from treehouse's pool lock, not from CheckCollision. The firstmate #947 bug this guard was named after predates durable leases entirely - it used pid-based ownership, which a crash-and-resume invalidates.

What path comparison actually produces is false positives that abort legitimate spawns, through the teardown retry gap the code already documents as expected:

1. Teardown runs worktree.Return(P) at cmd/teardown.go:75 - treehouse genuinely frees P.
2. state.Delete at cmd/teardown.go:102 fails - sqlite busy, disk hiccup, process killed between the two. Both that call site and internal/state/task.go's Delete doc comment name this as expected and retryable. Task A's row survives with Worktree = P.
3. Task B spawns, treehouse hands P back out - correct pool behaviour.
4. CheckCollision(home, P, "B") at cmd/spawn.go:95 scans every row (state.List has no status filter) and hits A's stale row.
5. cmd/spawn.go:97-98 force-returns B's genuinely exclusive lease and fails the spawn, reporting a collision that never existed.

So this is a correctness fix for defense-in-depth bookkeeping, not a backstop for the crash bug in its name. No recorded incident of it firing in this fleet yet.

Depends on #111

The scout found the cost is not the comparison, it is the storage. internal/store/store.go's task table has no lease_id column and there is no schema-version mechanism at all - CREATE TABLE IF NOT EXISTS is a silent no-op against an existing table missing a column, and internal/store/migrate.go is a one-shot JSON importer, not a schema migrator. That gap is bigger than this issue and is now filed separately as #111.

Sequencing follows from that. Once #111 lands, this becomes one registered migration step rather than hand-written detection logic, which is why it is not worth dispatching before it.

Depends on: #111

What the ship task has to do

• ALTER TABLE task ADD COLUMN lease_id TEXT NOT NULL DEFAULT '', as a step in whatever state/hand.db has no schema-version mechanism, so the first column addition silently fails on existing homes #111 lands.
• internal/worktree/worktree.go's Get() parses the get --lease --json payload into an anonymous struct holding only Path and discards the rest. Keep lease_id too.
• CheckCollision compares lease_id when both sides have one, falls back to path when either is empty. Rows written before this change keep an empty lease_id until their task is torn down and respawned, so an existing home with live tasks has to keep working through that window. This is the same compatibility shape treehouse's own #68 used for its state file.

What the scout could not establish

Crash and reboot survival of lease_id was not empirically tested - it rests on treehouse's design (no daemon, atomic file persistence, identity not pid-derived) and its documented crash recovery, not on a reproduction. Behaviour across a future treehouse upgrade past v2.1.0 is unknowable since no later version exists.

[atqamz]

Re-verified independently, and one thing has changed since the scout report above.

That report's verdict was worth doing but not yet, blocked on #111: there was no
schema-migration mechanism, so a new lease_id column would silently fail to apply to an existing fleet
home. #111 has since closed via #115, which built PRAGMA user_version-gated
migrations in internal/store/schemaversion.go. That merged after the report and the comment above were
written, so the sole named blocker is gone and the verdict updates to worth doing.

The technical case was re-checked against treehouse's own source rather than the kunchenguid/treehouse#68
PR description, and reproduced live against this host's installed v2.1.0:

• lease_id is 128 bits from crypto/rand, minted in internal/pool/state.go's newLeaseID.
• It is regenerated on every acquisition, including same-holder reuse of the same slot directory.
  treehouse's own TestReturnConditionalLeaseIdentityLifecycle asserts same path, different lease_id
  across a return-and-reacquire. Reproduced here: same path, 5fe5412a... then 26ffa0df....
• It survives a crash or reboot, because treehouse is not a daemon and persists lease state through an
  atomic write-temp-fsync-rename.

The correction worth recording is about what the current guard actually gets wrong. CheckCollision's
doc comment names firstmate #947, the stale-lease-after-crash bug, as its origin. That bug is not reachable
in hand: worktree.Get has always passed --lease, and treehouse's pool lock will not hand out a
currently-leased slot, so two tasks can never concurrently hold one path. No sequence was found where path
comparison misses a real collision.

What it does do is produce a false positive. cmd/teardown.go returns the worktree before calling
state.Delete, and state.Delete's doc comment says that ordering is deliberate so a failure is
retryable. If Delete fails, a stale row keeps Worktree = P while treehouse considers P free; the next
spawn or promote legitimately acquires P, CheckCollision matches the stale row on path equality, and the
new lease is force-returned and the spawn failed over a collision that never existed. Both call sites are
affected (cmd/spawn.go, cmd/promote.go). Defense-in-depth that produces false positives is a net
negative rather than neutral, which is the actual argument for the change.

Shape, unchanged from the report above: key on lease_id, fall back to path comparison when either side
lacks one, so existing rows keep working and gain an identity as they are respawned.

Full report: /home/atqa/secondhand/data/secondhand-lease-identity-scout/report.md

Not established: no recorded incident of that false positive actually firing in this fleet yet, and lease
semantics on treehouse 2.1.1 (offered on this host, not installed) are unverified.

[atqamz]

PR open: #132

Keys worktree.CheckCollision on treehouse's lease_id with a path fallback when either side lacks one, and adds the column as schema version 2 through the PRAGMA user_version mechanism. Framed as the false-positive fix the second scout described, not a missed-collision fix, and CheckCollision's doc comment no longer claims firstmate #947 as its origin.

Row status filtering was left as-is (every row compared, done and failed included) - a task holds its lease until teardown returns it, so status says nothing about whether the slot is still held. Reasoning is in the PR body and in SPECS.md's new "Collision guard" section.

Waiting on the merge gate; not started.