fix(webui): the native acceptance names the step it stopped at, and keeps our preload out of node-pty's helper

packages/webui/src/runtime/terminal.ts

@@ -70,6 +70,30 @@ export class WebuiTerminalManager implements WebuiTerminalProcessPort {
       signal?.removeEventListener("abort", onAbort);
     }
   }
-  disposeBySession() { for (const terminal of this.terminals.values()) terminal.pty.kill(); this.terminals.clear(); }
+  /**
+   * Kill every terminal in this session.
+   *
+   * Node-pty's Windows kill path forks a helper that lists the processes
+   * attached to the shell's console, and `child_process.fork` hands that
+   * child this process's `execArgv`. Whatever this process was started with
+   * is therefore loaded inside the helper, and the helper has no use for our
+   * arguments. Under the acceptance host that meant tsx's loader ran inside
+   * node-pty's helper, which is the shape microsoft/node-pty#974 describes;
+   * the helper then failed on `AttachConsole` and the kill fell back to
+   * node-pty's five-second timer.
+   *
+   * The kill is synchronous and the helper needs none of our flags, so the
+   * flags are put aside for the length of the kill and put back afterwards.
+   */
+  disposeBySession() {
+    const execArgv = process.execArgv;
+    process.execArgv = [];
+    try {
+      for (const terminal of this.terminals.values()) terminal.pty.kill();
+    } finally {
+      process.execArgv = execArgv;
+      this.terminals.clear();
+    }
+  }
   private get(id: string) { const terminal = this.terminals.get(id); if (!terminal) throw new Error("终端不存在"); return terminal; }
 }

packages/webui/src/server/service.ts

@@ -342,18 +342,23 @@ export class WebuiService {
    *
    * Attempt every owner in order and report all failures after release.
    */
-  close(): Promise<void> {
-    this.closing ??= this.#closeOwners();
+  close(onOwner?: (owner: string) => void): Promise<void> {
+    this.closing ??= this.#closeOwners(onOwner);
     return this.closing;
   }
 
-  async #closeOwners(): Promise<void> {
+  async #closeOwners(onOwner?: (owner: string) => void): Promise<void> {
     /** One owner that failed, kept with its name for the aggregate. */
     const failures: { readonly owner: string; readonly error: unknown }[] = [];
     const attempt = async (
       owner: string,
       step: () => void | Promise<void>,
     ): Promise<void> => {
+      // The owner is announced before it is awaited, so a caller that is
+      // waiting on the close knows which owner it is waiting for. Every
+      // await here is unbounded, and a shutdown that never finishes is
+      // otherwise a silence.
+      onOwner?.(owner);
       try {
         await step();
       } catch (error) {

packages/webui/test/browser/native-runtime.spec.mjs

@@ -47,13 +47,28 @@ test.afterAll(async () => {
   // 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.
+  // The budget is below Playwright's own hook timeout on purpose: this
+  // hook's message carries the host's trace, and a tie would leave
+  // Playwright's bare "afterAll hook timeout" as the only thing said.
   const [message] = await within(
     stopped,
-    30_000,
+    25_000,
     () => new Error(`the native host did not finish closing\n${logs}`),
   );
-  expect(message).toMatchObject({ closed: true, terminals: 0 });
-  expect(message.exited).toBeGreaterThanOrEqual(6);
+  // The host reports the step it stopped at when its own shutdown does not
+  // finish, so a stall says which step it was instead of leaving the hook
+  // with a bare timeout and no way to tell what is stuck.
+  expect(
+    message,
+    `the native host did not close${message?.stalledAt ? ` (stalled at ${message.stalledAt})` : ""}\n${logs}`,
+  ).toMatchObject({ closed: true, terminals: 0 });
+  // The case creates six terminals before the shutdown, so a smaller count
+  // means the body failed before it got that far. Saying so keeps the
+  // teardown's second complaint from reading like a second defect.
+  expect(
+    message.exited,
+    `the host closed ${message.exited} terminals; the case creates six before the shutdown, so fewer means the body failed earlier\n${logs}`,
+  ).toBeGreaterThanOrEqual(6);
   // 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

packages/webui/test/browser/support/native-browser-host.ts

@@ -62,15 +62,57 @@ const service = new WebuiService({ port, operations, terminal,
 const info = await service.start();
 process.send?.({ url: `http://127.0.0.1:${info.tcpPort}/?token=${info.credential.token}#session=${sessionId}`, workspaceDir, sessionId });
 let closing: Promise<void> | undefined;
+/**
+ * How long the service close is given before the host says it stopped there.
+ *
+ * The suite's own hook budget is thirty seconds, and the host used to spend
+ * all of it saying nothing: the service close never answered, so the hook
+ * timed out on a step it could not name. The host now answers first, and the
+ * answer names the step.
+ */
+const SERVICE_CLOSE_DEADLINE_MS = 20_000;
+/** What a service close that did not answer leaves behind. */
+const STALLED = Symbol("service.close did not finish");
 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`); };
+    // The report leaves with the process, so the exit is taken in the send
+    // callback: the message has to be flushed first.
+    const report = (message: Record<string, unknown>) => {
+      process.send?.(message, () => {
+        process.disconnect?.();
+        process.exit(0);
+      });
+    };
     step("service.close");
-    await service.close();
+    // The service announces each owner before it awaits it, so the host can
+    // say which owner the shutdown is waiting on. A stall that names its
+    // owner is the difference between "the service close did not finish" and
+    // "the harness port did not finish".
+    let owner = "service.close";
+    const outcome = await new Promise<typeof STALLED | "closed" | unknown>((resolve) => {
+      const timer = setTimeout(() => resolve(STALLED), SERVICE_CLOSE_DEADLINE_MS);
+      service.close((closing) => {
+        owner = closing;
+        step(`owner ${closing}`);
+      }).then(
+        () => { clearTimeout(timer); resolve("closed"); },
+        (error: unknown) => { clearTimeout(timer); resolve(error); },
+      );
+    });
+    if (outcome === STALLED) {
+      // The service close did not answer. The terminals were already killed —
+      // that is the first thing the close does — so the host reports where it
+      // stopped and goes, instead of leaving the suite to time out blind.
+      step(`service.close did not finish (at ${owner})`);
+      report({ closed: false, terminals: 0, exited: exits.length, stalledAt: owner });
+      return;
+    }
+    if (outcome !== "closed") throw outcome;
     step("terminals exited");
     await Promise.all(exits);
     step("apiHost.close");
@@ -89,10 +131,7 @@ async function close() {
     // 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);
-    });
+    report({ closed: true, terminals: 0, exited: exits.length });
   })();
   return closing;
 }