From a51dc2d9c8ef2b69f4018d9c15d764b8dafeef98 Mon Sep 17 00:00:00 2001
From: antianqi <75944423+antianqi@users.noreply.github.com>
Date: Tue, 10 Nov 2026 17:59:10 +0800
Subject: [PATCH] fix(webui): the native acceptance host goes when it is done
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The native PTY case failed at random on Windows with an "afterAll" hook
timeout. The cause was the host staying alive after it had finished.

Node-pty's Windows console-list helper is forked on every terminal kill to
enumerate the processes attached to the shell's console. A shell hosted by
ConPTY has a pseudo-console rather than a console, so the helper's
AttachConsole always fails there; the helper dies without answering, and
node-pty falls back to a five-second timer to resolve the promise. That
timer lives in the process that killed the terminal, so every kill left the
process five seconds of work it had not asked for. Measured on this host:
killing five terminals finished in 720ms, and the process still lived for
5.74s.

The acceptance host has nothing left to do once it has reported, so it now
exits there — in the send callback, which runs after the message has been
flushed, so the report is not lost. The case went from 13-15 seconds to
about 8.5.

The case now says so: the host has to be gone within two seconds of
reporting that it closed, and a shutdown that does not arrive carries the
host's own trace of where it was. Before this, a timeout said only that
the hook ran out of time; it now says which step stopped answering.

The alternative — node-pty's useConptyDll option, which skips the helper
entirely — was measured and is worse: the process did not exit at all.
---
 .../test/browser/native-runtime.spec.mjs      | 39 ++++++++++++++++++-
 .../browser/support/native-browser-host.ts    | 25 +++++++++++-
 2 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/packages/webui/test/browser/native-runtime.spec.mjs b/packages/webui/test/browser/native-runtime.spec.mjs
index 5be9f151..06e8d2cd 100644
--- a/packages/webui/test/browser/native-runtime.spec.mjs
+++ b/packages/webui/test/browser/native-runtime.spec.mjs
@@ -22,14 +22,49 @@ test.beforeAll(async () => {
     once(child, "exit").then(([code]) => { throw new Error(`Native host exited ${code}: ${logs}`); }),
   ]);
 });
+/**
+ * Waits for a promise, and says what the host was doing when it does not
+ * arrive.
+ */
+async function within(promise, ms, makeError) {
+  let timer;
+  try {
+    return await Promise.race([
+      promise,
+      new Promise((_, reject) => {
+        timer = setTimeout(() => reject(makeError()), ms);
+      }),
+    ]);
+  } finally {
+    clearTimeout(timer);
+  }
+}
+
 test.afterAll(async () => {
   if (!child || child.exitCode !== null || !child.connected) return;
   const stopped = once(child, "message");
   child.send("close");
-  const [message] = await stopped;
+  // The host's own trace of where it is in the shutdown is the only thing
+  // that says which step stopped answering, so a shutdown that does not
+  // finish has to carry it.
+  const [message] = await within(
+    stopped,
+    30_000,
+    () => new Error(`the native host did not finish closing\n${logs}`),
+  );
   expect(message).toMatchObject({ closed: true, terminals: 0 });
   expect(message.exited).toBeGreaterThanOrEqual(6);
-  await once(child, "exit");
+  // Once the host has said it closed, it has nothing left to do and it must
+  // be gone. Node-pty's Windows console-list helper cannot attach to a
+  // ConPTY shell, so every terminal it kills leaves a five-second fallback
+  // timer behind, and that timer used to keep the host alive long after the
+  // acceptance was over — long enough for this hook to miss its deadline on a
+  // loaded machine.
+  await within(
+    once(child, "exit"),
+    2_000,
+    () => new Error(`the native host did not exit\n${logs}`),
+  );
 });
 
 async function rpc(page, operation, body = operation === "listTerminals" ? {} : undefined) {
diff --git a/packages/webui/test/browser/support/native-browser-host.ts b/packages/webui/test/browser/support/native-browser-host.ts
index a609f8f7..bb62972e 100644
--- a/packages/webui/test/browser/support/native-browser-host.ts
+++ b/packages/webui/test/browser/support/native-browser-host.ts
@@ -65,13 +65,34 @@ let closing: Promise<void> | undefined;
 async function close() {
   if (closing) return closing;
   closing = (async () => {
+    // Each step says where it is before it runs: a shutdown that does not
+    // finish is otherwise a silence, and the suite cannot tell which step
+    // stopped answering.
+    const step = (name: string) => { process.stderr.write(`[native-host] ${name}\n`); };
+    step("service.close");
     await service.close();
+    step("terminals exited");
     await Promise.all(exits);
+    step("apiHost.close");
     await assembled.host.apiHost.close();
+    step("terminals checked");
     if (terminal.list().length !== 0) throw new Error("Native terminal service leaked processes");
+    step("data dir removed");
     await rm(directory, { recursive: true, force: true });
-    process.send?.({ closed: true, terminals: 0, exited: exits.length });
-    process.disconnect?.();
+    step("reported");
+    // The acceptance is over and the host has said so, so it goes: the exit is
+    // taken here rather than left to the event loop. Node-pty's Windows
+    // console-list helper cannot attach to a ConPTY shell — the shell has a
+    // pseudo-console, not a console — so every terminal it kills leaves a
+    // five-second fallback timer behind in this process. Waiting for that
+    // timer kept the host alive long after there was nothing left to do, and
+    // under load that was enough for the suite's shutdown hook to miss its
+    // deadline. The callback runs once the message has been flushed, so the
+    // report is not lost by exiting here.
+    process.send?.({ closed: true, terminals: 0, exited: exits.length }, () => {
+      process.disconnect?.();
+      process.exit(0);
+    });
   })();
   return closing;
 }