From 159ed7de26d45b64c3683c6f5fa3db6f3e29cc59 Mon Sep 17 00:00:00 2001
From: Szymon Chmal <szymon@chmal.it>
Date: Tue, 14 Jul 2026 09:52:01 +0200
Subject: [PATCH] fix: keep subprocess handles intact when diagnostics
 instrumentation is enabled

With diagnostics enabled, instrumented() collapsed every thenable through
.then(), turning nano-spawn Subprocess objects (Promise & AsyncIterable &
{ nodeChildProcess }) into plain promises. Teardown's stopSubprocess()
then found no nodeChildProcess to kill, so background adb logcat /
app-launch processes survived and kept the harness process alive after
tests finished. Track spans as a side effect and return the original
thenable untouched.
---
 .../fix-diagnostics-lingering-process.md      |  5 ++++
 .../tools/src/__tests__/diagnostics.test.ts   | 29 +++++++++++++++++++
 packages/tools/src/diagnostics.ts             | 19 ++++++------
 3 files changed, 44 insertions(+), 9 deletions(-)
 create mode 100644 .nx/version-plans/fix-diagnostics-lingering-process.md

diff --git a/.nx/version-plans/fix-diagnostics-lingering-process.md b/.nx/version-plans/fix-diagnostics-lingering-process.md
new file mode 100644
index 00000000..e2597485
--- /dev/null
+++ b/.nx/version-plans/fix-diagnostics-lingering-process.md
@@ -0,0 +1,5 @@
+---
+__default__: patch
+---
+
+Fixes a hang where the harness process kept running after tests finished whenever diagnostics were enabled (via the `diagnostics` config option or `RN_HARNESS_DIAGNOSTICS`). Diagnostics instrumentation replaced subprocess handles returned by adb/simctl with plain promises, so teardown could no longer kill the background `adb logcat` / app-launch processes and they kept the harness alive. Instrumented calls now return the original subprocess handles untouched, so runs with diagnostics enabled terminate cleanly.
diff --git a/packages/tools/src/__tests__/diagnostics.test.ts b/packages/tools/src/__tests__/diagnostics.test.ts
index cc6833c0..f0f6f901 100644
--- a/packages/tools/src/__tests__/diagnostics.test.ts
+++ b/packages/tools/src/__tests__/diagnostics.test.ts
@@ -231,6 +231,35 @@ describe('instrumented()', () => {
     expect(listeners.size).toBe(0);
   });
 
+  it('returns hybrid thenables (e.g. nano-spawn Subprocess) untouched, not collapsed to a plain Promise', async () => {
+    const diagnostics = createDiagnostics({ enabled: true });
+    const nodeChildProcess = Promise.resolve({ kill: vi.fn() });
+    const hybrid = Object.assign(Promise.resolve('done'), {
+      nodeChildProcess,
+      [Symbol.asyncIterator]: async function* () {
+        yield 'line-1';
+      },
+    });
+    const target = { startLogcat: () => hybrid };
+
+    const wrapped = instrumented(target, 'platform.android.adb', diagnostics);
+    const result = wrapped.startLogcat();
+
+    expect(result).toBe(hybrid);
+    expect((result as typeof hybrid).nodeChildProcess).toBe(nodeChildProcess);
+
+    const lines: string[] = [];
+    for await (const line of result as AsyncIterable<string>) {
+      lines.push(line);
+    }
+    expect(lines).toEqual(['line-1']);
+
+    await wait(5);
+    const entries = diagnostics.flush();
+    expect(entries).toHaveLength(1);
+    expect(entries[0]?.status).toBe('ok');
+  });
+
   it('spy-based check that fn is invoked once per call', () => {
     const diagnostics = createDiagnostics({ enabled: true });
     const spy = vi.fn(() => 'ok');
diff --git a/packages/tools/src/diagnostics.ts b/packages/tools/src/diagnostics.ts
index 503e54f1..382a287e 100644
--- a/packages/tools/src/diagnostics.ts
+++ b/packages/tools/src/diagnostics.ts
@@ -258,16 +258,17 @@ export const instrumented = <T extends object>(
           'then' in result &&
           typeof (result as { then: unknown }).then === 'function'
         ) {
-          return (result as Promise<unknown>).then(
-            (resolved) => {
-              span.end();
-              return resolved;
-            },
-            (error: unknown) => {
-              span.fail(error);
-              throw error;
-            }
+          // Some thenables (e.g. nano-spawn's Subprocess) carry extra
+          // properties/behavior beyond `.then()` (async iteration, a handle
+          // to the underlying child process, etc). Calling `.then()` and
+          // returning its result would collapse them into a plain Promise
+          // and silently drop that shape, so track the span as a side
+          // effect and always return the original object untouched.
+          (result as Promise<unknown>).then(
+            () => span.end(),
+            (error: unknown) => span.fail(error)
           );
+          return result;
         }
 
         span.end();