From 7013406f0b147d5bbde6d2202ce8b9350159c83b Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 22 Jun 2026 12:33:13 -0400 Subject: [PATCH] fix(#1586): pin withPlanningLock liveness probe in perf-407 for determinism MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1531/#1532 replaced withPlanningLock's mtime-staleness with PID-liveness (process.kill(pid,0)). perf-407 plants pid: process.pid + 1 and relied on the old mtime model to force the retry/sleep path; under the new model that pid's liveness is environment-dependent, so the retry path was taken on some runners and skipped on others (sleepCallCount: 0 precondition failure) — flaky CI red on next that blocks the merge queue. Pin the planted holder live via the _setLockProbes seam that #1532 added, and _resetLockProbes() in afterEach. The retry/sleep path is now exercised deterministically on every runner. No assertion weakened; no variable renamed. perf-316 is unaffected (its worker writes the parent's own, always-live pid). Co-Authored-By: Claude Opus 4.8 --- ...rf-407-planning-lock-buffer-alloc.test.cjs | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/tests/perf-407-planning-lock-buffer-alloc.test.cjs b/tests/perf-407-planning-lock-buffer-alloc.test.cjs index 2f99dc8de..9282b1f1c 100644 --- a/tests/perf-407-planning-lock-buffer-alloc.test.cjs +++ b/tests/perf-407-planning-lock-buffer-alloc.test.cjs @@ -93,6 +93,9 @@ function spySAB() { describe('perf #407: withPlanningLock hoists sleep buffer — exactly one SAB per call', () => { let tmpDir; let lockPath; + // Keep a reference to the module so _setLockProbes/_resetLockProbes are + // accessible across beforeEach/afterEach boundaries. + let mod; beforeEach(() => { tmpDir = makeTempDir(); @@ -100,6 +103,12 @@ describe('perf #407: withPlanningLock hoists sleep buffer — exactly one SAB pe }); afterEach(() => { + // Reset the liveness probe to the real implementation so other tests + // (or subsequent runs) are not affected by our deterministic override. + if (mod) { + mod._resetLockProbes(); + mod = null; + } try { fs.unlinkSync(lockPath); } catch { /* already gone */ } removeTempDir(tmpDir); // Purge module cache so each test gets a fresh require (and fresh SAB spy window). @@ -119,12 +128,24 @@ describe('perf #407: withPlanningLock hoists sleep buffer — exactly one SAB pe // Purge any previously cached versions so the spy catches module-level allocs. delete require.cache[PLANNING_WORKSPACE_CJS_PATH]; delete require.cache[CLOCK_CJS_PATH]; - withPlanningLock = require(PLANNING_WORKSPACE_CJS_PATH).withPlanningLock; + mod = require(PLANNING_WORKSPACE_CJS_PATH); + withPlanningLock = mod.withPlanningLock; } finally { spy.restore(); } const sabCountAtLoad = spy.getCount(); + // ── Inject deterministic liveness probe ─────────────────────────────── + // PR #1532 replaced mtime-staleness with PID-liveness (process.kill(pid,0)) + // to decide whether a contending lock holder should be waited on (live) or + // immediately stolen (dead). The test plants pid: process.pid + 1, which is + // environment-dependent: on some runners that pid is alive, on others it is + // not, making the retry/sleep path non-deterministic and causing CI flakiness + // (issue #1531). The _setLockProbes seam lets us pin the decision: treating + // the planted pid as LIVE deterministically forces the SUT into the retry path + // on every runner, which is exactly what the test intends to exercise. + mod._setLockProbes({ isPidAlive: (pid) => pid === process.pid + 1 }); + // ── Step 2: pre-create the lock file (simulates a contending process) ── // writing a valid lock JSON so withPlanningLock's stale-check doesn't // delete it immediately (mtime is NOW, well within the 30s stale window).