Skip to content

fix(setup): use a heartbeat lease for the config lock - #863

Merged
tt-a1i merged 3 commits into
mainfrom
cursor/heartbeat-lock-liveness-5682
Oct 16, 2026
Merged

tt-a1i merged 3 commits into
mainfrom
cursor/heartbeat-lock-liveness-5682

Conversation

@tt-a1i

@tt-a1i tt-a1i commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Setup config lock recovery decides liveness from a PID plus that process's start time. The start-time query is per platform: POSIX ps, and on Windows the WMI/cscript path from #857. A UTF-16 misparse or a slow query either blocks recovery until the deadline or refuses to recover a dead holder. Refs #862, #829, #857.

CI on 3915277 also failed the required Windows UI job in tests/web/workbar-session-position.spec.ts. The README button was missing, and the review-restore case hit Vitest's 5s test budget. Node 22.19.0 and Background terminals (Windows) failed only because that UI result failed; Node 24 and Node 26 were then cancelled. The same README miss is the Windows UI flake noted on #861 (run 38764433340, attempt 3).

CI on 2f6bd28 then failed Web E2E shard 1/2 in tests/web/openpi-web.e2e.ts ("trajectory inspects bounded prompt and tool evidence on desktop and mobile") with apiResponse.json: Response has been disposed. The page snapshot and screenshots show the trajectory fixture had already rendered and the test had switched back to 对话. Web E2E (Node 22.19.0) and the Node 22/24/26 gates failed because that shard failed. This is the same disposed-response flake #861 recorded on run 38774705615.

Value

A heartbeat lease is the same on every platform and can be tested by moving the lock file's timestamp. A live holder is not stolen, a stale lock is recovered, and a crashed holder becomes recoverable once it stops refreshing. The process-start query, including extensions/shared/win32-process-started-at.js, is gone.

The workbar tests still fail when the file list or review snapshot never arrives, and they no longer treat a stalled timer batch as a missing README.

A snapshot route that throws while reading a disposed upstream body no longer fails a test whose assertions already passed. A handler that fails for any other reason still fails the test.

Approach

  • The holder refreshes the lock file's mtime every 1s from a worker thread, using futimes on the descriptor it opened. A blocked main thread still renews the lease, and a file created later at the same path is not refreshed.
  • The lease is stale 5s after the last refresh. One delayed tick does not look like a crash.
  • A waiter stops after 5s when the timestamp moved during the wait and a refresh is still inside the last 2s. If refreshes stop, the waiter keeps the lease already on disk, bounded by one extra stale period.
  • Lock documents are version 2 (pid, createdAt, token). PID is diagnostic identity only. Version 1 files, including an in-progress recovery name, stay protected until their existing timestamp is stale, then use the same companion-link recovery. Unreadable bytes and unknown versions are never taken.
  • Tests hard-code the 5s lease instead of importing the implementation constant. Fault injection confirmed the bad outcomes still fail: no refresh fails the blocked-holder test, a lease that never expires fails stale recovery, treating every lock as stale fails both the fresh-lock test and crash recovery, and dropping the version 1 recovery suffix fails legacy takeover.
  • File listings and git review publish on a timer, and React commits on a later task. findByRole / waitFor arm their deadline on that same timer phase, so after a Windows stall the timeout callback rejects before the commit. Those waits now take one check-phase turn and flush act before judging. The budget stays 1s. A listing that never resolves still fails with Unable to find ... README.md (probe: 1923ms).
  • route.fetch() stores the upstream body on the per-test request context. Playwright's page fixture closes that context with reason "Test ended" and clears the body. A refresh still between fetch() and json() then throws Response has been disposed, which is what failed the trajectory test after the UI was already correct. test.afterEach now waits for in-flight route handlers before that close, the same drain several tests in the file already did by hand. Timeouts are unchanged. Tests that already closed the fixture page skip the drain.

Validation

  • bun run check
  • node --test --experimental-strip-types --test-concurrency=1 tests/extensions/shared/setup-config.test.ts (46 passed)
  • vitest run tests/web/workbar-session-position.spec.ts (12 passed)
  • A never-resolving workspaceFiles mock still fails the Files tooltip test at the README button inside the 1s budget.
  • bun run typecheck after the route drain. Playwright browsers are not installed in this environment, so the e2e file was not re-executed locally. The failure artifact for run 38872519896 shows the trajectory UI and both viewport screenshots already written, and Playwright's context.close clears fetchResponses before response.json().

Impact

  • User-visible behavior: a crashed setup writer is recoverable about 5s after its last refresh, rather than by PID death. A live writer that keeps refreshing is not stolen; waiting on one still fails after about 5s. A writer that refreshed during the wait and then died can take until that lease expires, at most about 10s from the start of the wait.
  • Model-visible context/tools: none.
  • Runtime/lifecycle: each setup-config write starts a short-lived worker to refresh the lock and stops it before release.
  • Persisted config/data: the lock file schema is version 2. The setup document itself is unchanged.
  • Compatibility or risk: version 1 lock files are honored only until their timestamp is 5s old, because that generation cannot refresh. After that they can be recovered. Unparseable lock files are still left in place.
Open in Web Open in Cursor 

Replace PID and process-start-time liveness with a worker-thread mtime
refresh. A lock is stale five seconds after its last refresh, including
locks written by the previous version, and a live holder is not stolen
while that lease is renewed.

Refs #862

Co-authored-by: tt-a1i <tt-a1i@users.noreply.github.com>
@github-actions github-actions Bot added the area:setup OpenPI setup, configuration, or setup documentation label Oct 16, 2026
cursoragent and others added 2 commits October 16, 2026 08:15
Windows CI can stall long enough that Testing Library's wait timer and the
deferred file or review read expire in the same timer batch. The timeout
then rejects before React commits, so the README button is reported missing
and the review restore case burns the 5s test budget. Poll on a check-phase
flush inside the existing 1s budget.

Co-authored-by: tt-a1i <tt-a1i@users.noreply.github.com>
route.fetch() bodies live on the per-test request context. Closing that
context drops them, so a refresh still between fetch() and json() fails
a finished test with "Response has been disposed".

Co-authored-by: tt-a1i <tt-a1i@users.noreply.github.com>
@tt-a1i
tt-a1i marked this pull request as ready for review October 16, 2026 08:38
@tt-a1i
tt-a1i merged commit 78dc574 into main Oct 16, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:setup OpenPI setup, configuration, or setup documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants