Skip to content

fix(miner): compare-and-set the orphan reclaim so a stale probe cannot free a live re-acquired slot - #8992

Merged
JSONbored merged 2 commits into
mainfrom
fix/worktree-reclaim-cas
Jul 26, 2026
Merged

fix(miner): compare-and-set the orphan reclaim so a stale probe cannot free a live re-acquired slot#8992
JSONbored merged 2 commits into
mainfrom
fix/worktree-reclaim-cas

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Advances #8828 (found while landing its follow-ups — this flake was sinking unrelated PR CI).

reclaimOrphanedAllocations probed active slots with one SELECT, then freed each by bare slot_index with no re-check. Since #8859 made the sweep run on EVERY acquire(), the probe→apply window is hit under real concurrency: a peer legitimately frees and re-acquires the slot in that window, the stale reclaim force-frees the peer's LIVE lease, and a third process double-books the same worktree path. CI reproduced it on an unrelated PR: miner-worktree-allocator-collisions returned 4 of 5 distinct paths (failing run). Fleet mode shares this store across containers, so this is a real production race, not just test noise.

The reclaim UPDATE now compare-and-sets on the exact probed lease evidence (status='active' AND allocated_at IS ? AND owner_pid IS ? AND owner_host IS ?) — a stale apply becomes a no-op instead of a force-free. Regression test stages the exact interleave via the injectable probe seam: a stale snapshot applied after a peer re-acquire must not touch the slot; matching evidence still frees.

…t free a live re-acquired slot

reclaimOrphanedAllocations probed active slots with one SELECT and then
freed each by bare slot_index. Since #8859 the sweep runs on every
acquire(), so under real concurrency a peer can free-and-re-acquire a
slot between the probe and the apply — the stale reclaim then force-
freed the peer's LIVE lease and a third process double-booked the same
worktree path (reproduced in CI: the collisions test returned 4 of 5
distinct paths). The reclaim UPDATE now guards on the exact probed
lease evidence (status, allocated_at, owner_pid, owner_host), making a
stale apply a no-op.
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

…ke's true mechanism

The acquire-child fixture exited immediately after acquiring, violating
the lease contract: later children's on-acquire sweeps (#8859) then
CORRECTLY reclaimed the dead winners' slots via the same-host dead-pid
fast path and re-issued the same paths — CI's 4-of-5 and 2-of-5 distinct
failures, and a potential third success at a 2-slot cap. Children now
hold their lease (process alive, allocator open) until the harness has
observed every result, matching how production owners live for the whole
worktree lifetime; results resolve from stdout instead of exit, and the
release broadcast tolerates already-exited children.
@JSONbored

Copy link
Copy Markdown
Owner Author

Second commit lands the flake's TRUE mechanism, found by tracing why CI also produced a 2-of-5-distinct failure: the acquire-child fixture exited immediately after acquiring, so later children's on-acquire sweeps (#8859) correctly reclaimed the dead winners' slots via the same-host dead-pid fast path and re-issued the same paths. That's a lease-contract violation in the fixture, not an allocator bug — production owners stay alive for the whole worktree lifetime, and now the fixture does too (children hold their lease until the harness has observed every result; results resolve from stdout, not exit).

Both changes stand: the CAS guard closes the genuine cross-container probe→apply window for the age-based reclaim path, and the fixture lifecycle removes the flake mechanism structurally rather than statistically. Stress-verified 15/15 locally; the failure mode is no longer reachable (reclaim-during-observation requires a dead owner, and no owner dies before observation).

@JSONbored JSONbored self-assigned this Jul 26, 2026
@JSONbored
JSONbored merged commit 8444812 into main Jul 26, 2026
4 checks passed
@JSONbored
JSONbored deleted the fix/worktree-reclaim-cas branch July 26, 2026 15:43
@codecov

codecov Bot commented Jul 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.65%. Comparing base (d9d6833) to head (ea23f73).
⚠️ Report is 10 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8992      +/-   ##
==========================================
- Coverage   93.87%   92.65%   -1.23%     
==========================================
  Files         807      807              
  Lines       80463    80463              
  Branches    24398    24399       +1     
==========================================
- Hits        75537    74552     -985     
- Misses       3561     4841    +1280     
+ Partials     1365     1070     -295     
Flag Coverage Δ
backend 93.46% <100.00%> (-1.70%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
packages/loopover-miner/lib/worktree-allocator.ts 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

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