From 5e2a72d148baf87ded1ab564ad7cf9909e17a340 Mon Sep 17 00:00:00 2001
From: arzafran <franco@darkroom.engineering>
Date: Wed, 29 Jul 2026 11:42:57 -0300
Subject: [PATCH] fix: stop subprocess tests timing out when the machine is
 busy

Two test helpers waited for a subprocess by handing the wait off to a
shared background worker. That worker is taken from a pool everyone else
is also using, and it stays occupied for as long as the subprocess runs.
When the whole suite runs at once the pool fills up, the wait never gets
a turn, and the test gives up on its own timer long after the subprocess
has actually finished.

The result was tests failing after a minute and a half on scripts that
finish in fifteen milliseconds. Two of them took turns breaking the build
on main today.

Now the wait is driven by the notification the system already sends when
a process ends, which occupies nothing.
---
 .../WorkspaceRemoteConnectionTests.swift      | 29 +++++++++++--------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/programaTests/WorkspaceRemoteConnectionTests.swift b/programaTests/WorkspaceRemoteConnectionTests.swift
index 7c863205..164018db 100644
--- a/programaTests/WorkspaceRemoteConnectionTests.swift
+++ b/programaTests/WorkspaceRemoteConnectionTests.swift
@@ -28,6 +28,18 @@ final class WorkspaceRemoteConnectionTests: XCTestCase {
         process.standardOutput = stdoutPipe
         process.standardError = stderrPipe
 
+        // Signalled from Foundation's own termination callback rather than by parking a
+        // `DispatchQueue.global()` worker in `waitUntilExit()`. The old shape blocked one
+        // pooled thread for the entire lifetime of every child process; run enough of these
+        // concurrently (the suite runs in parallel) and the global pool saturates, so the
+        // block that signals this semaphore can sit unscheduled long past the child's actual
+        // exit. The test then reports a timeout at exactly its configured limit for a
+        // subprocess that finished in milliseconds -- observed repeatedly on CI as 90s
+        // failures of scripts that run in ~15ms locally. `terminationHandler` is delivered on
+        // Foundation's private queue and blocks nothing. Must be set before `run()`.
+        let exitSignal = DispatchSemaphore(value: 0)
+        process.terminationHandler = { _ in exitSignal.signal() }
+
         do {
             try process.run()
         } catch {
@@ -39,12 +51,6 @@ final class WorkspaceRemoteConnectionTests: XCTestCase {
             )
         }
 
-        let exitSignal = DispatchSemaphore(value: 0)
-        DispatchQueue.global(qos: .userInitiated).async {
-            process.waitUntilExit()
-            exitSignal.signal()
-        }
-
         let timedOut = exitSignal.wait(timeout: .now() + timeout) == .timedOut
         if timedOut {
             process.terminate()
@@ -1486,6 +1492,11 @@ final class CLINotifyProcessIntegrationTests: XCTestCase {
         process.standardOutput = stdoutPipe
         process.standardError = stderrPipe
 
+        // See the sibling `runProcess` above for why this uses `terminationHandler` instead of
+        // parking a pooled thread in `waitUntilExit()`.
+        let exitSignal = DispatchSemaphore(value: 0)
+        process.terminationHandler = { _ in exitSignal.signal() }
+
         do {
             try process.run()
         } catch {
@@ -1497,12 +1508,6 @@ final class CLINotifyProcessIntegrationTests: XCTestCase {
             )
         }
 
-        let exitSignal = DispatchSemaphore(value: 0)
-        DispatchQueue.global(qos: .userInitiated).async {
-            process.waitUntilExit()
-            exitSignal.signal()
-        }
-
         let timedOut = exitSignal.wait(timeout: .now() + timeout) == .timedOut
         if timedOut {
             process.terminate()