test: fix the intermittent Windows failure in the previous-successor refresh-ownership test

Merged
BO
2d ago
+7-0

The runtime-control test "waits for the real daemon refresh and preserves ownership on previous-successor" no longer fails on slow Windows runners with "The exact project watcher is not ready.". It had failed that way in 4 Windows lifecycle runs (39274946425, 39214704209, 39082003190, 38814662932).

Cause. The daemon marks its writer record ready (markWriterReady in src/mcp/writer-lock.ts) by renaming a temp file over writer.pid. When the record is busy or the rename is refused, it retries 100 ms later. The test's child daemon signals "ready" right after codegraph_status, which can come before that retry lands. The previous-successor case then copies the still-ready: false record into writer.pid with a new start time. The daemon's hello reads that record and reports its watcher not ready, so checkRuntimeReady stops there instead of reaching the refresh request that answers "ownership changed". This is a race in the test, not slowness and not a product defect. A real starter retries readiness.

Fix. Before signalling, the child waits up to 5 s for writer.pid to show ready: true. If readiness is never published, it fails with the record's contents.

Proof (at 44190c67, Linux):

  • I made the child's readiness rename fail with EPERM for its first 2 s, standing in for a busy record on a slow runner. Without the fix, previous-successor failed with exactly the CI message (Expected: "ownership changed", Received: "The exact project watcher is not ready."). The other three cases timed out waiting for the refresh to start. This happened in 2 of 2 runs.
  • With the fix and the same fault, all 4 cases passed in 2 of 2 runs.
  • npx vitest run __tests__/runtime-control.test.ts passed 60/60 without the fault. The injection was for diagnosis only and isn't committed.

Not covered: the readiness retry checks only the process ID before stamping ready: true, not the record's start time like the other ownership checks. Only an outside edit of writer.pid that keeps the same process ID, like this test's, can hit that, so it's left for a separate change.

README: test-only change; checked the "About this fork" tables, and no row is affected.

Summary by cubic

Fixes the intermittent Windows failure in the runtime-control test "waits for the real daemon refresh and preserves ownership on previous-successor," which returned "The exact project watcher is not ready." instead of "ownership changed.".

The failure was a race in the child's readiness signal, not slowness or a product defect: the daemon publishes writer readiness by renaming a temp file over writer.pid and retries 100 ms later when the record is busy. The child signalled "ready" right after codegraph_status, before that retry landed, so the previous-successor case copied a record that still had ready: false.

The child now waits up to 5 s for writer.pid to show ready: true before signalling and fails with the record's contents if readiness is never published.

Not covered: the readiness retry only checks the process ID, not the record's start time that other ownership checks use; only an outside edit that keeps the same process ID (like this test) can hit that, so it's left for a later change.

Summary by CodeRabbit

  • Tests
    • Improved readiness-test reliability by waiting for the writer lock to report ready, with a timeout and diagnostic error if readiness is delayed.