From 118f428167f2f009be005104ad0d2128b7b60ff7 Mon Sep 17 00:00:00 2001 From: Pyseph <45090858+Pyseph@users.noreply.github.com> Date: Wed, 4 Nov 2026 12:54:16 +0100 Subject: [PATCH 1/2] perf(server): report Windows terminal exit about 0.9 s sooner node-pty waited FLUSH_DATA_INTERVAL = 1000 ms after the shell exited before closing the ConPTY output socket and emitting exit, so the terminal showed Process exited a second late. Wait 100 ms instead, and run the cleanup one setImmediate turn after the timer fires so output already buffered in the pipe is read first after a main-thread stall (lowering the constant alone loses cmd's final line when a stall exceeds the interval). --- .../nodePtyFlushDataAndCleanUp.test.ts | 153 ++++++++++++++++++ patches/node-pty@1.2.0-beta.15.patch | 33 +++- pnpm-lock.yaml | 6 +- 3 files changed, 188 insertions(+), 4 deletions(-) create mode 100644 apps/server/src/terminal/nodePtyFlushDataAndCleanUp.test.ts diff --git a/apps/server/src/terminal/nodePtyFlushDataAndCleanUp.test.ts b/apps/server/src/terminal/nodePtyFlushDataAndCleanUp.test.ts new file mode 100644 index 0000000000000..f438e37e58734 --- /dev/null +++ b/apps/server/src/terminal/nodePtyFlushDataAndCleanUp.test.ts @@ -0,0 +1,153 @@ +import * as NodeModule from "node:module"; + +import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; + +const requireCjs = NodeModule.createRequire(import.meta.url); +const { + WindowsPtyAgent, +}: { + WindowsPtyAgent: { + prototype: { + _flushDataAndCleanUp(this: FakeAgent): void; + _cleanUpProcess(this: FakeAgent): void; + }; + }; +} = requireCjs("node-pty/lib/windowsPtyAgent.js"); + +interface FakeAgent { + _useConptyDll: boolean; + _closeTimeout?: unknown; + _inSocket: { readable: boolean }; + _outSocket: { readable: boolean; destroy: () => void }; + _cleanUpProcess: () => void; +} + +// The sockets are stand-ins; the timer logic and _cleanUpProcess are node-pty's own (as patched by T3). +const makeAgent = (): FakeAgent => { + const agent: FakeAgent = { + _useConptyDll: false, + _inSocket: { readable: true }, + _outSocket: { readable: true, destroy: vi.fn() }, + _cleanUpProcess: () => WindowsPtyAgent.prototype._cleanUpProcess.call(agent), + }; + return agent; +}; + +const flush = (agent: FakeAgent) => WindowsPtyAgent.prototype._flushDataAndCleanUp.call(agent); + +// A minimal event loop that mirrors Node's phases: expired timers run first, then immediates (the check phase). +// node-pty calls the global timer functions at call time, so replacing them lets the test place a "poll phase" +// (delivery of already-buffered pipe data) exactly between the two. +interface Timer { + readonly id: number; + readonly dueAt: number; + readonly run: () => void; +} +let now = 0; +let nextId = 0; +let timers: Timer[] = []; +let immediates: Array<() => void> = []; + +const runExpiredTimers = () => { + for (const timer of timers.filter((t) => t.dueAt <= now).toSorted((a, b) => a.dueAt - b.dueAt)) { + timers = timers.filter((t) => t.id !== timer.id); + timer.run(); + } +}; +const runImmediates = () => { + const batch = immediates; + immediates = []; + for (const run of batch) run(); +}; +/** One loop iteration after `ms` of wall time; `poll` runs between the timers and check phases. */ +const loopIteration = (ms: number, poll?: () => void) => { + now += ms; + runExpiredTimers(); + poll?.(); + runImmediates(); +}; + +// ConPTY never closes the output pipe after the shell exits, so node-pty ends the session on a timer. +// T3 shortens it (patches/node-pty@1.2.0-beta.15.patch) so the terminal reports "exited" about 0.9 s sooner. +describe("node-pty Windows exit flush", () => { + beforeEach(() => { + now = 0; + nextId = 0; + timers = []; + immediates = []; + vi.stubGlobal("setTimeout", (run: () => void, ms: number) => { + const id = ++nextId; + timers.push({ id, dueAt: now + ms, run }); + return id; + }); + vi.stubGlobal("clearTimeout", (id: number) => { + timers = timers.filter((t) => t.id !== id); + }); + vi.stubGlobal("setImmediate", (run: () => void) => { + immediates.push(run); + return ++nextId; + }); + }); + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("closes the output socket 100 ms after the exit, not a second later", () => { + const agent = makeAgent(); + flush(agent); + + loopIteration(99); + expect(agent._outSocket.destroy).not.toHaveBeenCalled(); + + loopIteration(1); + expect(agent._outSocket.destroy).toHaveBeenCalledTimes(1); + expect(agent._outSocket.readable).toBe(false); + expect(agent._inSocket.readable).toBe(false); + }); + + it("restarts the window when more output arrives", () => { + const agent = makeAgent(); + flush(agent); + loopIteration(60); + flush(agent); + + loopIteration(99); + expect(agent._outSocket.destroy).not.toHaveBeenCalled(); + + loopIteration(1); + expect(agent._outSocket.destroy).toHaveBeenCalledTimes(1); + }); + + // Timers run before the poll phase. After a main-thread stall longer than the window, output that is already + // buffered in the pipe is read only after the timer has expired, so the cleanup has to wait one loop turn for it. + it("keeps the socket open when buffered output is read right after a stalled timer expires", () => { + const agent = makeAgent(); + flush(agent); + + loopIteration(150, () => flush(agent)); + expect(agent._outSocket.destroy).not.toHaveBeenCalled(); + + loopIteration(99); + expect(agent._outSocket.destroy).not.toHaveBeenCalled(); + + loopIteration(1); + expect(agent._outSocket.destroy).toHaveBeenCalledTimes(1); + }); + + it("closes after a stall when no output was buffered", () => { + const agent = makeAgent(); + flush(agent); + + loopIteration(150); + expect(agent._outSocket.destroy).toHaveBeenCalledTimes(1); + }); + + it("does nothing when node-pty hosts conpty itself", () => { + const agent = makeAgent(); + agent._useConptyDll = true; + flush(agent); + + loopIteration(5000); + expect(agent._outSocket.destroy).not.toHaveBeenCalled(); + }); +}); diff --git a/patches/node-pty@1.2.0-beta.15.patch b/patches/node-pty@1.2.0-beta.15.patch index 125447607990d..0736c118d2ca2 100644 --- a/patches/node-pty@1.2.0-beta.15.patch +++ b/patches/node-pty@1.2.0-beta.15.patch @@ -17,7 +17,18 @@ diff --git a/lib/windowsConoutConnection.js b/lib/windowsConoutConnection.js diff --git a/lib/windowsPtyAgent.js b/lib/windowsPtyAgent.js --- a/lib/windowsPtyAgent.js +++ b/lib/windowsPtyAgent.js -@@ -218,13 +218,31 @@ +@@ -20,7 +20,9 @@ var conptyNative; + * shutting down the socket. The timer will be reset if a new data event comes in after the timer + * has started. + */ +-var FLUSH_DATA_INTERVAL = 1000; ++// ConPTY never closes the output pipe on its own after the shell exits, so wait this long for the last ++// screen update. Trailing output follows the exit within about 20 ms; 1000 ms made every exit report 1 s late. ++var FLUSH_DATA_INTERVAL = 100; + var CONNECTION_TIMEOUT = 5000; + var defaultWindowsPtyAgentOptions = { + connectionTimeout: CONNECTION_TIMEOUT, +@@ -218,13 +220,31 @@ return Promise.resolve([]); } return new Promise(function (resolve) { @@ -53,3 +64,23 @@ diff --git a/lib/windowsPtyAgent.js b/lib/windowsPtyAgent.js - resolve([_this._innerPid]); + finish([_this._innerPid]); }, 5000); +@@ -259,7 +279,18 @@ var WindowsPtyAgent = /** @class */ (function () { + if (this._closeTimeout) { + clearTimeout(this._closeTimeout); + } +- this._closeTimeout = setTimeout(function () { return _this._cleanUpProcess(); }, FLUSH_DATA_INTERVAL); ++ // Timers run before the poll phase. After a main-thread stall longer than the interval, output that is already ++ // buffered in the pipe would be dropped by the cleanup, so give the poll phase one turn to read it first; every ++ // chunk it reads re-arms this timer. ++ this._flushDataSeq = (this._flushDataSeq || 0) + 1; ++ this._closeTimeout = setTimeout(function () { ++ var seq = _this._flushDataSeq; ++ setImmediate(function () { ++ if (_this._flushDataSeq === seq) { ++ _this._cleanUpProcess(); ++ } ++ }); ++ }, FLUSH_DATA_INTERVAL); + }; + WindowsPtyAgent.prototype._cleanUpProcess = function () { + if (this._useConptyDll) { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6ddbc362245c9..a1cb78b9cfb06 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -391,7 +391,7 @@ patchedDependencies: micromark-extension-gfm-table@2.1.1: 9ad23a530e45d9f83cba1da8654fe16d5af8b59d5c73d5c7a7c91fd25b9be1d8 micromark-extension-llm-math@3.1.0: 4eeb9d9e27ebc75e98795e21c0719005483dad7d233055698257f7d10e91603b micromark@4.0.2: cb79edde7eca83bbcaaafb6ceb7b044b81b987314cde4910823b15257ecada67 - node-pty@1.2.0-beta.15: f1e680afec85f650699ea756f91f469844ec4c976f4f168492a45c6b3f67d651 + node-pty@1.2.0-beta.15: ff8117509663fe1d5a07ef2648c324e80a9349b7d90401a00a591702615b8b30 react-native-gesture-handler@3.3.0: abe20e1d1c23549297cbbc818e14fe479b41455930d9663135d6b40fe359b626 react-native-image-viewing@0.2.2: 419f35ee91eac88ced9438058281db5eb09d9c799d8974c1172926068c9448cf react-native-keyboard-controller@1.22.8: 6eb6c847ac693b636ec97c83575acd44b77e92ad0d2c776de0cb4e78e8fb9d4f @@ -927,7 +927,7 @@ importers: version: 6.2.2 node-pty: specifier: 1.2.0-beta.15 - version: 1.2.0-beta.15(patch_hash=f1e680afec85f650699ea756f91f469844ec4c976f4f168492a45c6b3f67d651) + version: 1.2.0-beta.15(patch_hash=ff8117509663fe1d5a07ef2648c324e80a9349b7d90401a00a591702615b8b30) playwright-core: specifier: 1.60.0 version: 1.60.0 @@ -20571,7 +20571,7 @@ snapshots: node-mock-http@1.0.5: {} - node-pty@1.2.0-beta.15(patch_hash=f1e680afec85f650699ea756f91f469844ec4c976f4f168492a45c6b3f67d651): + node-pty@1.2.0-beta.15(patch_hash=ff8117509663fe1d5a07ef2648c324e80a9349b7d90401a00a591702615b8b30): dependencies: node-addon-api: 7.1.1 From 391d864e9ab9d7446ed670c9c259389a21121bf8 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sun, 15 Nov 2026 00:21:13 +0000 Subject: [PATCH 2/2] test(server): keep only the node-pty exit flush cases T3's patch owns Co-Authored-By: Claude Opus 5.5 --- .../nodePtyFlushDataAndCleanUp.test.ts | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/apps/server/src/terminal/nodePtyFlushDataAndCleanUp.test.ts b/apps/server/src/terminal/nodePtyFlushDataAndCleanUp.test.ts index f438e37e58734..7dc0861c7f53c 100644 --- a/apps/server/src/terminal/nodePtyFlushDataAndCleanUp.test.ts +++ b/apps/server/src/terminal/nodePtyFlushDataAndCleanUp.test.ts @@ -105,19 +105,6 @@ describe("node-pty Windows exit flush", () => { expect(agent._inSocket.readable).toBe(false); }); - it("restarts the window when more output arrives", () => { - const agent = makeAgent(); - flush(agent); - loopIteration(60); - flush(agent); - - loopIteration(99); - expect(agent._outSocket.destroy).not.toHaveBeenCalled(); - - loopIteration(1); - expect(agent._outSocket.destroy).toHaveBeenCalledTimes(1); - }); - // Timers run before the poll phase. After a main-thread stall longer than the window, output that is already // buffered in the pipe is read only after the timer has expired, so the cleanup has to wait one loop turn for it. it("keeps the socket open when buffered output is read right after a stalled timer expires", () => { @@ -141,13 +128,4 @@ describe("node-pty Windows exit flush", () => { loopIteration(150); expect(agent._outSocket.destroy).toHaveBeenCalledTimes(1); }); - - it("does nothing when node-pty hosts conpty itself", () => { - const agent = makeAgent(); - agent._useConptyDll = true; - flush(agent); - - loopIteration(5000); - expect(agent._outSocket.destroy).not.toHaveBeenCalled(); - }); });