diff --git a/.github/workflows/windows-terminal-restart-e2e.yml b/.github/workflows/windows-terminal-restart-e2e.yml index 65501104281..f206518876d 100644 --- a/.github/workflows/windows-terminal-restart-e2e.yml +++ b/.github/workflows/windows-terminal-restart-e2e.yml @@ -91,8 +91,6 @@ jobs: NODE_PTY_REQUIRE_TEST_FAULTS: '1' run: >- pnpm exec electron - node_modules/mocha/bin/mocha.js - --exit node_modules/node-pty/lib/conptyConnectFailure.test.js - name: Run Windows terminal restart regressions diff --git a/config/patches/node-pty@1.1.0.patch b/config/patches/node-pty@1.1.0.patch index 0b280d4a72c..54adca93d1f 100644 --- a/config/patches/node-pty@1.1.0.patch +++ b/config/patches/node-pty@1.1.0.patch @@ -48,10 +48,10 @@ index 1ac5758bedd8cf54f32280dea4e4aeb5afdee30d..6047590bc9558dabacd3ece5114ed41c 'targets' : [ diff --git a/lib/conptyConnectFailure.test.js b/lib/conptyConnectFailure.test.js new file mode 100644 -index 0000000000000000000000000000000000000000..d6fb0340fbaaf1e76ee0ba56847bcb3b340f9044 +index 0000000000000000000000000000000000000000..fb68c417a5de0e445d5a1c8e0c164a7e03b3fca2 --- /dev/null +++ b/lib/conptyConnectFailure.test.js -@@ -0,0 +1,58 @@ +@@ -0,0 +1,73 @@ +"use strict"; +/** + * Copyright (c) 2026, Stably AI (MIT License). @@ -61,54 +61,69 @@ index 0000000000000000000000000000000000000000..d6fb0340fbaaf1e76ee0ba56847bcb3b +var fs = require("fs"); +var utils_1 = require("./utils"); +if (process.platform === 'win32') { -+ describe('ConPTY failed-connect cleanup', function () { -+ var native = utils_1.loadNativeModule('conpty').module; -+ [ -+ ['assign-job', 'Cannot assign process to Job, error code: 5'], -+ ['resume-thread', 'Cannot resume process, error code: 6'] -+ ].forEach(function (_a) { -+ var failure = _a[0], expectedError = _a[1]; -+ it("releases every native PTY resource after " + failure, function () { -+ if (!native.setTestConnectFailure || !native.getTestResourceState) { -+ if (process.env.NODE_PTY_REQUIRE_TEST_FAULTS === '1') { -+ assert.fail('ConPTY test fault injection was required but is unavailable'); -+ } -+ this.skip(); -+ return; -+ } -+ var pipeName = "node-pty-connect-failure-" + process.pid + "-" + failure; -+ var before = native.getTestResourceState(); -+ native.setTestConnectFailure(failure); ++ var native_1 = utils_1.loadNativeModule('conpty').module; ++ var failureCases_1 = [ ++ ['assign-job', 'Cannot assign process to Job, error code: 5'], ++ ['resume-thread', 'Cannot resume process, error code: 6'] ++ ]; ++ var hasTestHooks_1 = function () { ++ return Boolean(native_1.setTestConnectFailure && native_1.getTestResourceState); ++ }; ++ var runFailureCase_1 = function (failure, expectedError) { ++ assert.ok(native_1.setTestConnectFailure && native_1.getTestResourceState); ++ var pipeName = "node-pty-connect-failure-" + process.pid + "-" + failure; ++ var before = native_1.getTestResourceState(); ++ native_1.setTestConnectFailure(failure); ++ try { ++ // Reusing the same first-instance pipe names proves each failed attempt ++ // closed both server pipes before removing its baton. ++ for (var attempt = 1; attempt <= 3; attempt++) { ++ var term = native_1.startProcess(process.env.ComSpec || 'cmd.exe', 80, 30, false, pipeName, false, false); ++ var inputFd = fs.openSync(term.conin, 'w'); ++ var outputFd = fs.openSync(term.conout, 'r'); + try { -+ // Reusing the same first-instance pipe names proves each failed -+ // attempt closed both server pipes before removing its baton. -+ for (var attempt = 1; attempt <= 3; attempt++) { -+ var term = native.startProcess(process.env.ComSpec || 'cmd.exe', 80, 30, false, pipeName, false, false); -+ var inputFd = fs.openSync(term.conin, 'w'); -+ var outputFd = fs.openSync(term.conout, 'r'); -+ try { -+ assert.throws(function () { return native.connect(term.pty, 'cmd.exe /C exit', process.cwd(), [], false, true, function () { return undefined; }); }, new RegExp(expectedError)); -+ } -+ finally { -+ fs.closeSync(inputFd); -+ fs.closeSync(outputFd); -+ } -+ var resourceState = native.getTestResourceState(); -+ assert.strictEqual(resourceState.ptyCount, before.ptyCount); -+ assert.strictEqual(resourceState.closedPseudoConsoleCount, before.closedPseudoConsoleCount + attempt); -+ assert.strictEqual(resourceState.closedPipeHandleCount, before.closedPipeHandleCount + attempt * 2); -+ assert.strictEqual(resourceState.closedProcessHandleCount, before.closedProcessHandleCount + attempt); -+ assert.strictEqual(resourceState.closedThreadHandleCount, before.closedThreadHandleCount + attempt); -+ assert.strictEqual(resourceState.closedJobHandleCount, before.closedJobHandleCount + attempt); -+ assert.strictEqual(resourceState.drainedFailedProcessCount, before.drainedFailedProcessCount + attempt); -+ } ++ assert.throws(function () { return native_1.connect(term.pty, 'cmd.exe /C exit', process.cwd(), [], false, true, function () { return undefined; }); }, new RegExp(expectedError)); + } + finally { -+ native.setTestConnectFailure('none'); ++ fs.closeSync(inputFd); ++ fs.closeSync(outputFd); + } ++ var resourceState = native_1.getTestResourceState(); ++ assert.strictEqual(resourceState.ptyCount, before.ptyCount); ++ assert.strictEqual(resourceState.closedPseudoConsoleCount, before.closedPseudoConsoleCount + attempt); ++ assert.strictEqual(resourceState.closedPipeHandleCount, before.closedPipeHandleCount + attempt * 2); ++ assert.strictEqual(resourceState.closedProcessHandleCount, before.closedProcessHandleCount + attempt); ++ assert.strictEqual(resourceState.closedThreadHandleCount, before.closedThreadHandleCount + attempt); ++ assert.strictEqual(resourceState.closedJobHandleCount, before.closedJobHandleCount + attempt); ++ assert.strictEqual(resourceState.drainedFailedProcessCount, before.drainedFailedProcessCount + attempt); ++ } ++ } ++ finally { ++ native_1.setTestConnectFailure('none'); ++ } ++ }; ++ if (typeof describe === 'function') { ++ describe('ConPTY failed-connect cleanup', function () { ++ failureCases_1.forEach(function (_a) { ++ var failure = _a[0], expectedError = _a[1]; ++ it("releases every native PTY resource after " + failure, function () { ++ if (!hasTestHooks_1()) { ++ this.skip(); ++ return; ++ } ++ runFailureCase_1(failure, expectedError); ++ }); + }); + }); -+ }); ++ } ++ else { ++ assert.ok(hasTestHooks_1(), 'ConPTY test fault injection was required but is unavailable'); ++ failureCases_1.forEach(function (_a) { ++ var failure = _a[0], expectedError = _a[1]; ++ return runFailureCase_1(failure, expectedError); ++ }); ++ console.log('ConPTY failed-connect cleanup: 2 passed'); ++ } +} diff --git a/lib/conpty_console_list_agent.js b/lib/conpty_console_list_agent.js index 8c4fca9022a6d6f015bca87f61625cde2278f428..0b71eba1d0fdb312bd46e5f47c218bd5330f023a 100644 @@ -344,10 +359,10 @@ index af5f343d17f012f330fe2292083f297768ff3b5e..6b558c789fc77b58c20cd9564d03e93e describe("WindowsTerminal (useConpty = " + useConpty + ", useConptyDll = " + useConptyDll + ")", function () { diff --git a/src/conptyConnectFailure.test.ts b/src/conptyConnectFailure.test.ts new file mode 100644 -index 0000000000000000000000000000000000000000..2cab295e87f01fc44394ccd022bce84c16298c25 +index 0000000000000000000000000000000000000000..70130579acf6c9592266f4834aa4d34f6e95b59f --- /dev/null +++ b/src/conptyConnectFailure.test.ts -@@ -0,0 +1,128 @@ +@@ -0,0 +1,139 @@ +/** + * Copyright (c) 2026, Stably AI (MIT License). + */ @@ -390,91 +405,102 @@ index 0000000000000000000000000000000000000000..2cab295e87f01fc44394ccd022bce84c +} + +if (process.platform === 'win32') { -+ describe('ConPTY failed-connect cleanup', () => { -+ const native = loadNativeModule('conpty').module as ITestConptyNative; ++ const native = loadNativeModule('conpty').module as ITestConptyNative; ++ const failureCases = [ ++ ['assign-job', 'Cannot assign process to Job, error code: 5'], ++ ['resume-thread', 'Cannot resume process, error code: 6'] ++ ]; + -+ [ -+ ['assign-job', 'Cannot assign process to Job, error code: 5'], -+ ['resume-thread', 'Cannot resume process, error code: 6'] -+ ].forEach(([failure, expectedError]) => { -+ it(`releases every native PTY resource after ${failure}`, function() { -+ if (!native.setTestConnectFailure || !native.getTestResourceState) { -+ if (process.env.NODE_PTY_REQUIRE_TEST_FAULTS === '1') { -+ assert.fail('ConPTY test fault injection was required but is unavailable'); -+ } -+ this.skip(); -+ return; -+ } ++ const hasTestHooks = (): boolean => ++ Boolean(native.setTestConnectFailure && native.getTestResourceState); + -+ const pipeName = `node-pty-connect-failure-${process.pid}-${failure}`; -+ const before = native.getTestResourceState(); -+ native.setTestConnectFailure(failure); ++ const runFailureCase = (failure: string, expectedError: string): void => { ++ assert.ok(native.setTestConnectFailure && native.getTestResourceState); ++ const pipeName = `node-pty-connect-failure-${process.pid}-${failure}`; ++ const before = native.getTestResourceState(); ++ native.setTestConnectFailure(failure); ++ try { ++ // Reusing the same first-instance pipe names proves each failed attempt ++ // closed both server pipes before removing its baton. ++ for (let attempt = 1; attempt <= 3; attempt++) { ++ const term = native.startProcess( ++ process.env.ComSpec || 'cmd.exe', ++ 80, ++ 30, ++ false, ++ pipeName, ++ false, ++ false ++ ); ++ const inputFd = fs.openSync(term.conin, 'w'); ++ const outputFd = fs.openSync(term.conout, 'r'); + try { -+ // Reusing the same first-instance pipe names proves each failed -+ // attempt closed both server pipes before removing its baton. -+ for (let attempt = 1; attempt <= 3; attempt++) { -+ const term = native.startProcess( -+ process.env.ComSpec || 'cmd.exe', -+ 80, -+ 30, ++ assert.throws( ++ () => native.connect( ++ term.pty, ++ 'cmd.exe /C exit', ++ process.cwd(), ++ [], + false, -+ pipeName, -+ false, -+ false -+ ); -+ const inputFd = fs.openSync(term.conin, 'w'); -+ const outputFd = fs.openSync(term.conout, 'r'); -+ try { -+ assert.throws( -+ () => native.connect( -+ term.pty, -+ 'cmd.exe /C exit', -+ process.cwd(), -+ [], -+ false, -+ true, -+ () => undefined -+ ), -+ new RegExp(expectedError) -+ ); -+ } finally { -+ fs.closeSync(inputFd); -+ fs.closeSync(outputFd); -+ } -+ -+ const resourceState: ITestResourceState = native.getTestResourceState(); -+ assert.strictEqual(resourceState.ptyCount, before.ptyCount); -+ assert.strictEqual( -+ resourceState.closedPseudoConsoleCount, -+ before.closedPseudoConsoleCount + attempt -+ ); -+ assert.strictEqual( -+ resourceState.closedPipeHandleCount, -+ before.closedPipeHandleCount + attempt * 2 -+ ); -+ assert.strictEqual( -+ resourceState.closedProcessHandleCount, -+ before.closedProcessHandleCount + attempt -+ ); -+ assert.strictEqual( -+ resourceState.closedThreadHandleCount, -+ before.closedThreadHandleCount + attempt -+ ); -+ assert.strictEqual( -+ resourceState.closedJobHandleCount, -+ before.closedJobHandleCount + attempt -+ ); -+ assert.strictEqual( -+ resourceState.drainedFailedProcessCount, -+ before.drainedFailedProcessCount + attempt -+ ); -+ } ++ true, ++ () => undefined ++ ), ++ new RegExp(expectedError) ++ ); + } finally { -+ native.setTestConnectFailure('none'); ++ fs.closeSync(inputFd); ++ fs.closeSync(outputFd); + } ++ ++ const resourceState: ITestResourceState = native.getTestResourceState(); ++ assert.strictEqual(resourceState.ptyCount, before.ptyCount); ++ assert.strictEqual( ++ resourceState.closedPseudoConsoleCount, ++ before.closedPseudoConsoleCount + attempt ++ ); ++ assert.strictEqual( ++ resourceState.closedPipeHandleCount, ++ before.closedPipeHandleCount + attempt * 2 ++ ); ++ assert.strictEqual( ++ resourceState.closedProcessHandleCount, ++ before.closedProcessHandleCount + attempt ++ ); ++ assert.strictEqual( ++ resourceState.closedThreadHandleCount, ++ before.closedThreadHandleCount + attempt ++ ); ++ assert.strictEqual( ++ resourceState.closedJobHandleCount, ++ before.closedJobHandleCount + attempt ++ ); ++ assert.strictEqual( ++ resourceState.drainedFailedProcessCount, ++ before.drainedFailedProcessCount + attempt ++ ); ++ } ++ } finally { ++ native.setTestConnectFailure('none'); ++ } ++ }; ++ ++ if (typeof describe === 'function') { ++ describe('ConPTY failed-connect cleanup', () => { ++ failureCases.forEach(([failure, expectedError]) => { ++ it(`releases every native PTY resource after ${failure}`, function() { ++ if (!hasTestHooks()) { ++ this.skip(); ++ return; ++ } ++ runFailureCase(failure, expectedError); ++ }); + }); + }); -+ }); ++ } else { ++ assert.ok(hasTestHooks(), 'ConPTY test fault injection was required but is unavailable'); ++ failureCases.forEach(([failure, expectedError]) => runFailureCase(failure, expectedError)); ++ console.log('ConPTY failed-connect cleanup: 2 passed'); ++ } +} diff --git a/src/conpty_console_list_agent.ts b/src/conpty_console_list_agent.ts index 181ccabbbe9c4948a9725fb1db907a68e9de01fc..67f31facf85562b67adbfbd04ce28ddd8eeb4a79 100644 diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d1e041dea6b..99c4e4a5e44 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -21,7 +21,7 @@ patchedDependencies: hash: 9c1de9931d86864923ff53bc9d64474a86478085ac81ea4e432648c7e23d702c path: config/patches/@xterm__xterm@6.1.0-beta.287.patch node-pty@1.1.0: - hash: 5dd99f29a42d0e98905d0e89817e815c215416da8e38e6310a4d7a30b01121dd + hash: 2b944fa02cbefd246a847340c68f1c118b3a1823a76c94405cf60b1052c34c27 path: config/patches/node-pty@1.1.0.patch importers: @@ -63,7 +63,7 @@ importers: version: 3.3.1 node-pty: specifier: ^1.1.0 - version: 1.1.0(patch_hash=5dd99f29a42d0e98905d0e89817e815c215416da8e38e6310a4d7a30b01121dd) + version: 1.1.0(patch_hash=2b944fa02cbefd246a847340c68f1c118b3a1823a76c94405cf60b1052c34c27) posthog-node: specifier: ^5.33.3 version: 5.33.3 @@ -12043,7 +12043,7 @@ snapshots: undici: 6.27.0 which: 6.0.1 - node-pty@1.1.0(patch_hash=5dd99f29a42d0e98905d0e89817e815c215416da8e38e6310a4d7a30b01121dd): + node-pty@1.1.0(patch_hash=2b944fa02cbefd246a847340c68f1c118b3a1823a76c94405cf60b1052c34c27): dependencies: node-addon-api: 7.1.1