fix(cli): stop the cancellable-process test reading a pid file mid-write (#4080)

Same defect shape as the earlier preflight cancel test fix: the wait
condition only checked that the ready file existed, which goes true
as soon as the write opens (and truncates) it, before its content
lands. A read landing in that window parsed an empty string to 0.
The wait condition now also validates the parsed value is a real
positive pid before accepting it, retrying otherwise. Reproduced
with a busy-wait harness that hit the empty-file race in roughly
two of five attempts and confirmed clean after the fix.
This commit is contained in:
Miguel Ángel
2026-09-18 10:37:31 -04:00
committed by GitHub
parent 8fe480c0a0
commit 2a9912ec99
@@ -52,8 +52,15 @@ describe.skipIf(!IS_POSIX)("cancellable process tree teardown", () => {
},
},
);
await waitForTestCondition(() => existsSync(readyPath), 2_000);
grandchildPid = Number(readFileSync(readyPath, "utf8"));
// existsSync goes true as soon as the write opens (and truncates) the
// file, before its content lands — read must retry, not just exist-check.
await waitForTestCondition(() => {
if (!existsSync(readyPath)) return false;
const pid = Number(readFileSync(readyPath, "utf8"));
if (!Number.isInteger(pid) || pid <= 0) return false;
grandchildPid = pid;
return true;
}, 2_000);
controller.abort(abortReason);
await expect(setup).rejects.toBe(abortReason);
@@ -61,7 +68,7 @@ describe.skipIf(!IS_POSIX)("cancellable process tree teardown", () => {
expect(rootPid).toBeDefined();
expect(grandchildPid).toBeGreaterThan(0);
expect(testProcessIsAlive(rootPid!)).toBe(false);
expect(testProcessIsAlive(grandchildPid)).toBe(false);
expect(testProcessIsAlive(grandchildPid!)).toBe(false);
await new Promise((resolve) => setTimeout(resolve, completionDelayMs + 100));
expect(existsSync(completionPath)).toBe(false);
} finally {