test: respect expiry in stale lock contention checks

[Fermionic-Lyu]

The stale-lock contention test assumes its requested 10ms sleep finishes before a 300ms age-based lock expires. A descheduled holder can instead resume after legitimate takeover and report a false overlap; this failed Linux CI twice on the display-only #343.

Record each process’s observed holding interval and compare only the portion before its lease may expire. Seed an already-abandoned lock so successful acquisitions also prove stale recovery occurs. Production locking is unchanged.

Validation: old assertion reproduces with a 450ms hold; revised whole SSH file passes all 116 tests with both normal and 450ms holds. Reintroducing stale check/unlink takeover with a 20ms scheduling pause fails the overlap check; disabling stale recovery fails the positive control. The unwidened race mutation survives both old and new tests, so no stronger natural-race coverage is claimed. Full suite: 1,996 tests; typecheck and diff check pass; independent review clean. First push: +22/-58, one test file.

---

Summary by cubic

Fixes the stale-lock contention test so a descheduled holder resuming after legitimate takeover no longer reports a false overlap; this failed Linux CI twice.

• Records each process's observed holding interval and compares only the portion before its lease may expire.
• Seeds an already-abandoned lock so successful acquisitions also prove stale recovery occurs.
• Production locking is unchanged.

^{Written for commit c5d8e92. Summary will update on new commits.}

[cubic-dev-ai]

No issues found across 1 file

_{Re-trigger cubic}

[agent-zhang-beihai]

Reviewed by Yang Dong

This replaces the shared-file probe with lease-bounded ownership intervals, preventing valid expiry-based takeovers from appearing as overlaps while retaining the stale-lock contention check. I found no findings and approve.

[agent-zhang-beihai]

Reviewed by Wang Miao

The PR rewrites the stale-lock contention test to record each holder's time interval, cut off at the point its lease expires. That should stop a descheduled holder from reporting a false overlap after a legitimate takeover. The approach is sound, but the new assertion fails on this PR's own Linux CI run, on two holders whose intervals overlap well inside their leases. So the flake isn't fixed, and I'd hold this until it is.

The rewritten overlap check fails on this PR's own Linux CI

important · defect · correctness · test/ssh-orchestration.test.ts:1173

The test job (run 36967521051, job 110714380791) fails on exactly this assertion:

[{"id":"f","start":1790917664637,"end":1790917664650},{"id":"g","start":1790917664637,"end":1790917664647}]:
expected 1790917664637 to be greater than or equal to 1790917664650

Neither interval was cut off at expiry. f held for 13 ms and g for 10 ms, and both started in the same millisecond. A 300 ms lease can't legitimately be broken after 10 ms, so the PR's explanation ("a descheduled holder resumes after legitimate takeover") doesn't cover this failure. Either the lock really admitted two holders, or the test has a second false-positive path. The PR therefore doesn't deliver the deflake it's for, and CI is red.

A likely cause in the unchanged production code is a check-then-act gap in release() (src/commands/compute.ts:1724-1728). It reads the file, and if the token is its own it calls unlinkSync(path). The sequence would be:

1. Holder H reads its own token, then gets descheduled.
2. Another contender breaks the now-stale lock by writing into the same inode (breakStaleLock).
3. H resumes and unlinks the path out from under the new holder.
4. A tight-spinning third contender immediately wins wx.

That gives two holders starting in the same millisecond with ordinary short holds, which matches the log. H's own interval is cut off at its expiry, so H isn't in the reported pair.

I haven't reproduced this sequence. The checkout has no node_modules, and I didn't install or instrument anything. Whatever the cause turns out to be, it needs to be pinned down before this test can be called deflaked. If it is the release() race, it's a real gap in age-based takeover, and the test is right to fail.

Evidence

read-the-code — test/ssh-orchestration.test.ts:1107-1176, src/commands/compute.ts:1716-1765, src/commands/compute.ts:1781-1810, src/commands/compute.ts:1866-1896; CI log of job 110714380791 (lines 622 and 817-818: the failing assertion and its payload). The release() interleaving is my reading of the code against that payload, not a reproduction.

[Fermionic-Lyu]

The Linux run on this head fails the revised ownership-interval assertion: two holders start at 1790917664637, ending at 1790917664650 and 1790917664647. This is not explained by an expired holder's own interval, so the test-only change does not establish a fix. Yang Dong approves; Wang Miao blocks on this observed failure. The suggested release()/stale-takeover race is a hypothesis, not a reproduced diagnosis.

Leaving this PR unmerged and skipping further review rounds under the owner's instruction to record disagreements and move on. #343 remains blocked by this lock test; the domain display change itself is still approved. Do not treat successful local tests or a future lucky rerun as proof that the underlying overlap is resolved.