From b170345abac6cec903cbfd86c4d1aecc3f3e78b8 Mon Sep 17 00:00:00 2001
From: tx tests <tx-tests@example.com>
Date: Fri, 14 Aug 2026 10:31:48 +0100
Subject: [PATCH] fix(test): stop child-process cleanup from masking real test
 failures

terminateChild runs from finally blocks, but its SIGKILL path awaited
waitForExit without a try/catch. When the exit event was slow to arrive
under CI load, cleanup threw and replaced whatever the test had actually
asserted.

That is what the SIGINT watchdog failure on main was: the reported error
was "Timed out waiting for process 16248 to exit" from the cleanup helper,
and the 4065ms duration matches its two 2s waits rather than the 15s the
test itself budgets. The real outcome of the assertion was never visible.
SIGTERM passed in 86ms in the same run, so the process teardown itself was
not the problem.

Cleanup is now best effort and cannot throw. The SIGKILL wait is also
given more room, since SIGKILL cannot be caught and a missing exit event
inside the window is a reporting artifact rather than a live process. The
harness still reaps by pid afterwards.
---
 test/integration/ralph-script.test.ts         | 20 +++++++++++++++++--
 .../integration/ralph-watchdog-script.test.ts | 20 +++++++++++++++++--
 2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/test/integration/ralph-script.test.ts b/test/integration/ralph-script.test.ts
index eca7032..f02cf69 100644
--- a/test/integration/ralph-script.test.ts
+++ b/test/integration/ralph-script.test.ts
@@ -359,18 +359,34 @@ function isPidLive(pid: number): boolean {
   }
 }
 
+/**
+ * Best-effort teardown for a spawned child.
+ *
+ * This runs from `finally` blocks, so it must never throw: an exception here
+ * replaces the real test outcome with a cleanup error. That is exactly what
+ * happened in CI, where a slow `exit` event under load surfaced as
+ * "Timed out waiting for process N to exit" and masked what the test actually
+ * asserted. SIGKILL cannot be caught, so a missing exit event within the
+ * window is a reporting artifact, not a process still running.
+ */
 async function terminateChild(proc: ReturnType<typeof spawn>): Promise<void> {
   if (proc.exitCode !== null) {
     return
   }
 
-  proc.kill("SIGTERM")
   try {
+    proc.kill("SIGTERM")
     await waitForExit(proc, 2000)
     return
   } catch {
+    // Fall through to SIGKILL.
+  }
+
+  try {
     proc.kill("SIGKILL")
-    await waitForExit(proc, 2000)
+    await waitForExit(proc, 5000)
+  } catch {
+    // Best effort: the harness cleanup also reaps by pid.
   }
 }
 
diff --git a/test/integration/ralph-watchdog-script.test.ts b/test/integration/ralph-watchdog-script.test.ts
index cbe8a51..b133b4a 100644
--- a/test/integration/ralph-watchdog-script.test.ts
+++ b/test/integration/ralph-watchdog-script.test.ts
@@ -262,18 +262,34 @@ function terminatePid(pid: number): void {
   }
 }
 
+/**
+ * Best-effort teardown for a spawned child.
+ *
+ * This runs from `finally` blocks, so it must never throw: an exception here
+ * replaces the real test outcome with a cleanup error. That is exactly what
+ * happened in CI, where a slow `exit` event under load surfaced as
+ * "Timed out waiting for process N to exit" and masked what the test actually
+ * asserted. SIGKILL cannot be caught, so a missing exit event within the
+ * window is a reporting artifact, not a process still running.
+ */
 async function terminateChild(proc: ReturnType<typeof spawn>): Promise<void> {
   if (proc.exitCode !== null) {
     return
   }
 
-  proc.kill("SIGTERM")
   try {
+    proc.kill("SIGTERM")
     await waitForExit(proc, 2000)
     return
   } catch {
+    // Fall through to SIGKILL.
+  }
+
+  try {
     proc.kill("SIGKILL")
-    await waitForExit(proc, 2000)
+    await waitForExit(proc, 5000)
+  } catch {
+    // Best effort: the harness cleanup also reaps by pid.
   }
 }