From 48ff1ce08b7e57f703867796c5cae34be8939080 Mon Sep 17 00:00:00 2001 From: probe Date: Wed, 11 Nov 2026 13:04:35 +0800 Subject: [PATCH 1/3] fix(webui): keep the process flags out of node-pty's console-list helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. The failing Windows run of PR #29 shows it: the helper crashed on `AttachConsole` with tsx's transformer in the stack, twice, and the kill then fell back to node-pty's five-second timer. That is the shape microsoft/node-pty#974 describes, and the amplifier is ours because the preload is ours. 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. In a process started without anything unusual — which is the desktop app — this changes nothing at all. --- packages/webui/src/runtime/terminal.ts | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/packages/webui/src/runtime/terminal.ts b/packages/webui/src/runtime/terminal.ts index 7470c271..ecd0fae2 100644 --- a/packages/webui/src/runtime/terminal.ts +++ b/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; } } From 26ef5f4541202cd583926723c0999d326e1acd9d Mon Sep 17 00:00:00 2001 From: probe Date: Wed, 11 Nov 2026 13:04:48 +0800 Subject: [PATCH 2/3] test(webui): the native acceptance says which step it stopped at The native acceptance's teardown hook waits for the host to say it closed. On Windows it sometimes waited the full thirty seconds and then reported a bare timeout, because the host had stopped answering inside its own service close and had nothing to say about it. The one thing the log did carry was the host's step trace, and that is in the hook's error text rather than in the failure itself. The host now gives the service close a deadline of its own, and when that deadline passes it reports the step it stopped at before it goes. The hook then fails with the step in the message and the host's own trace beside it, instead of a timeout that names nothing. Nothing is loosened: the hook's thirty-second budget is unchanged and a stall still fails the suite. It fails sooner, and it says where. The terminals are killed before the service close waits on anything else, so a service close that stalls has already released them and the host can go once it has said where it stopped. --- .../test/browser/native-runtime.spec.mjs | 8 +++- .../browser/support/native-browser-host.ts | 41 ++++++++++++++++--- 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/packages/webui/test/browser/native-runtime.spec.mjs b/packages/webui/test/browser/native-runtime.spec.mjs index 06e8d2cd..08a6ebab 100644 --- a/packages/webui/test/browser/native-runtime.spec.mjs +++ b/packages/webui/test/browser/native-runtime.spec.mjs @@ -52,7 +52,13 @@ test.afterAll(async () => { 30_000, () => new Error(`the native host did not finish closing\n${logs}`), ); - expect(message).toMatchObject({ closed: true, terminals: 0 }); + // 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 }); expect(message.exited).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 diff --git a/packages/webui/test/browser/support/native-browser-host.ts b/packages/webui/test/browser/support/native-browser-host.ts index bb62972e..cab1fb7a 100644 --- a/packages/webui/test/browser/support/native-browser-host.ts +++ b/packages/webui/test/browser/support/native-browser-host.ts @@ -62,6 +62,17 @@ 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 | 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 () => { @@ -69,8 +80,31 @@ async function close() { // 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) => { + process.send?.(message, () => { + process.disconnect?.(); + process.exit(0); + }); + }; step("service.close"); - await service.close(); + const outcome = await new Promise((resolve) => { + const timer = setTimeout(() => resolve(STALLED), SERVICE_CLOSE_DEADLINE_MS); + service.close().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"); + report({ closed: false, terminals: 0, exited: exits.length, stalledAt: "service.close" }); + return; + } + if (outcome !== "closed") throw outcome; step("terminals exited"); await Promise.all(exits); step("apiHost.close"); @@ -89,10 +123,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; } From 976fe12d821b2712a2bcc3a6e55da6575c2f9497 Mon Sep 17 00:00:00 2001 From: probe Date: Wed, 11 Nov 2026 14:09:33 +0800 Subject: [PATCH 3/3] test(webui): the native acceptance names the owner it stopped at MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The service close awaits every owner it owns — the terminal manager, the websocket connections, the websocket server, the browser bridge, the Mini App proxies, the HTTP listener, the harness port — and every one of those awaits is unbounded. The host could therefore only say that "the service close" had not finished, which is one level above the question that matters: which of them is stuck. The service now announces each owner before it awaits it, and the host passes that announcement on. A stall names the owner it stalled at, and the hook's own budget sits below Playwright's hook timeout, so the message that carries the host's trace is the message that is said. The close signature gains an optional callback, so a caller that does not want the progress does not get it. The close order, the owners, and what a failure does are all unchanged. --- packages/webui/src/server/service.ts | 11 ++++++++--- .../webui/test/browser/native-runtime.spec.mjs | 13 +++++++++++-- .../test/browser/support/native-browser-host.ts | 14 +++++++++++--- 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/packages/webui/src/server/service.ts b/packages/webui/src/server/service.ts index 72f7c5af..0b2a9d01 100644 --- a/packages/webui/src/server/service.ts +++ b/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 { - this.closing ??= this.#closeOwners(); + close(onOwner?: (owner: string) => void): Promise { + this.closing ??= this.#closeOwners(onOwner); return this.closing; } - async #closeOwners(): Promise { + async #closeOwners(onOwner?: (owner: string) => void): Promise { /** 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, ): Promise => { + // 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) { diff --git a/packages/webui/test/browser/native-runtime.spec.mjs b/packages/webui/test/browser/native-runtime.spec.mjs index 08a6ebab..a7b1b401 100644 --- a/packages/webui/test/browser/native-runtime.spec.mjs +++ b/packages/webui/test/browser/native-runtime.spec.mjs @@ -47,9 +47,12 @@ 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}`), ); // The host reports the step it stopped at when its own shutdown does not @@ -59,7 +62,13 @@ test.afterAll(async () => { message, `the native host did not close${message?.stalledAt ? ` (stalled at ${message.stalledAt})` : ""}\n${logs}`, ).toMatchObject({ closed: true, terminals: 0 }); - expect(message.exited).toBeGreaterThanOrEqual(6); + // 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 diff --git a/packages/webui/test/browser/support/native-browser-host.ts b/packages/webui/test/browser/support/native-browser-host.ts index cab1fb7a..ec539400 100644 --- a/packages/webui/test/browser/support/native-browser-host.ts +++ b/packages/webui/test/browser/support/native-browser-host.ts @@ -89,9 +89,17 @@ async function close() { }); }; step("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((resolve) => { const timer = setTimeout(() => resolve(STALLED), SERVICE_CLOSE_DEADLINE_MS); - service.close().then( + service.close((closing) => { + owner = closing; + step(`owner ${closing}`); + }).then( () => { clearTimeout(timer); resolve("closed"); }, (error: unknown) => { clearTimeout(timer); resolve(error); }, ); @@ -100,8 +108,8 @@ async function close() { // 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"); - report({ closed: false, terminals: 0, exited: exits.length, stalledAt: "service.close" }); + step(`service.close did not finish (at ${owner})`); + report({ closed: false, terminals: 0, exited: exits.length, stalledAt: owner }); return; } if (outcome !== "closed") throw outcome;