test(core): cover bundled ConPTY fallback and terminal replies

packages/core/src/services/shellExecutionService.test.ts

@@ -317,7 +317,7 @@ describe('ShellExecutionService', () => {
     simulation: (
       ptyProcess: typeof mockPtyProcess,
       ac: AbortController,
-    ) => void,
+    ) => void | Promise<void>,
     config: ShellExecutionConfig = shellExecutionConfig,
     options: ShellExecuteOptions = {},
   ) => {
@@ -333,7 +333,7 @@ describe('ShellExecutionService', () => {
     );
 
     await new Promise((resolve) => process.nextTick(resolve));
-    simulation(mockPtyProcess, abortController);
+    await simulation(mockPtyProcess, abortController);
     const result = await handle.result;
     return { result, handle, abortController };
   };
@@ -2147,12 +2147,8 @@ describe('ShellExecutionService', () => {
 
     let capturedReplyListener: ((data: string) => void) | undefined;
 
-    // The forwarder under test subscribes through the terminal's public onData
-    // event; capture every listener a spawned terminal hands to it. The
-    // instance field shadows Terminal's prototype getter, so the service's
-    // subscription lands here instead of xterm's core emitter — the listener
-    // is invoked manually, and the returned disposable stands in for
-    // xterm's own so the cleanup path has something to dispose.
+    // Capture the forwarder for the error-containment and platform-gate tests.
+    // The device-attributes test below uses xterm's real parser and emitter.
     class ReplyCapturingTerminal extends pkg.Terminal {
       override onData: pkg.IEvent<string> = (listener) => {
         capturedReplyListener = listener;
@@ -2233,6 +2229,38 @@ describe('ShellExecutionService', () => {
       }
     });
 
+    it('does not run the fallback after the PTY has already spawned', async () => {
+      mockPlatform.mockReturnValue('win32');
+      let pidReads = 0;
+      Object.defineProperty(mockPtyProcess, 'pid', {
+        configurable: true,
+        get: () => {
+          pidReads++;
+          if (pidReads === 1) return 12345;
+          throw new Error('post-spawn handle setup failed');
+        },
+      });
+
+      try {
+        const handle = await ShellExecutionService.execute(
+          'echo hi',
+          '/test/dir',
+          onOutputEventMock,
+          new AbortController().signal,
+          true,
+          shellExecutionConfig,
+        );
+        const result = await handle.result;
+
+        expect(mockPtySpawn).toHaveBeenCalledOnce();
+        expect(mockCpSpawn).not.toHaveBeenCalled();
+        expect(result.executionMethod).toBe('none');
+        expect(result.error?.message).toBe('post-spawn handle setup failed');
+      } finally {
+        ShellExecutionService['activePtys'].delete(12345);
+      }
+    });
+
     it('leaves the inbox ConPTY backend alone off Windows', async () => {
       // beforeEach pins the platform to linux.
       await simulateExecution('echo hi', (pty) => {
@@ -2244,19 +2272,17 @@ describe('ShellExecutionService', () => {
       });
     });
 
-    it('writes the emulated terminal query replies back to the PTY on Windows', async () => {
+    it("writes xterm's device-attributes reply back to the PTY on Windows", async () => {
       // Bundled ConPTY answers no queries itself, so an unanswered DA probe
       // stalls the shell for its full ~2s timeout; the forwarder must carry
-      // the emulated terminal's replies to the PTY.
+      // the reply generated by xterm's real parser back to the PTY.
       mockPlatform.mockReturnValue('win32');
-      mockLoadXtermHeadless.mockResolvedValueOnce({
-        Terminal: ReplyCapturingTerminal,
-      });
 
-      await simulateExecution('echo hi', (pty) => {
-        expect(capturedReplyListener).toBeDefined();
-        capturedReplyListener!('\x1b[?64;1;22c');
-        expect(mockPtyProcess.write).toHaveBeenCalledWith('\x1b[?64;1;22c');
+      await simulateExecution('echo hi', async (pty) => {
+        pty.onData.mock.calls[0][0]('\x1b[c');
+        await vi.waitFor(() => {
+          expect(mockPtyProcess.write).toHaveBeenCalledWith('\x1b[?1;2c');
+        });
         pty.onExit.mock.calls[0][0]({ exitCode: 0, signal: null });
       });
     });

packages/core/src/services/shellExecutionService.ts

@@ -1483,6 +1483,7 @@ export class ShellExecutionService {
       // This should not happen, but as a safeguard...
       throw new Error('PTY implementation not found');
     }
+    const useBundledConpty = os.platform() === 'win32';
     // Records whether pty.spawn returned. The catch at the end of this method
     // needs it to tell a spawn-phase failure — no child exists yet, so handing
     // the command to the child_process fallback cannot run it twice — from
@@ -1528,16 +1529,15 @@ export class ShellExecutionService {
         // Windows: with the inbox ConPTY backend a natural shell exit orphans
         // the `conhost.exe --headless` that backend spawned — the native exit
         // watcher erases the pty baton before JS can reach ClosePseudoConsole
-        // (microsoft/node-pty#965), so hosts accumulate until the CLI exits
-        // (#11303: `+7 conhost for 7 tool commands`). This option makes
-        // node-pty load the conpty.dll shipped with the package instead of the
-        // one built into Windows — that swap is all @lydell/node-pty documents
-        // the option as, and its typings mark it EXPERIMENTAL. #11303 measured
-        // the per-command host growth gone with
-        // @lydell/node-pty-win32-x64 1.2.0-beta.10 on Windows Server 2025 (30
-        // commands: 30 orphaned hosts before, 0 after). Off Windows the option
-        // is inert: `useConptyDll` appears nowhere in the POSIX prebuilds.
-        useConptyDll: os.platform() === 'win32',
+        // (microsoft/node-pty#965), so hosts accumulate until the CLI exits.
+        // The bundled backend calls ConptyReleasePseudoConsole after spawn,
+        // releasing the reference that otherwise keeps its host alive after
+        // the last client exits. Windows verification must count both
+        // `conhost.exe` and `OpenConsole.exe` (or all attributable children),
+        // because the bundled backend normally launches `OpenConsole.exe`. Off
+        // Windows the option is inert: `useConptyDll` appears nowhere in the
+        // POSIX prebuilds.
+        useConptyDll: useBundledConpty,
       });
       ptySpawned = true;
 
@@ -1552,26 +1552,25 @@ export class ShellExecutionService {
         });
         headlessTerminal.scrollToTop();
 
-        // Bundled ConPTY (useConptyDll above) answers no terminal queries
+        // Bundled ConPTY (useBundledConpty above) answers no terminal queries
         // itself, so a shell that probes the terminal — PowerShell's DA query
         // at startup — stalls for its full ~2s timeout unless the emulated
         // terminal's auto-generated reply is written back (measured 3.22s →
         // 0.23s per command). Scoped to Windows to keep the POSIX path
         // byte-identical; the hook dies with headlessTerminal.dispose() in
         // both the foreground and background-promote cleanups.
-        const queryResponseDisposable =
-          os.platform() === 'win32'
-            ? headlessTerminal.onData((data) => {
-                try {
-                  ptyProcess.write(data);
-                } catch (e) {
-                  // A reply racing shell exit finds a dead PTY — drop it.
-                  debugLogger.warn(
-                    `writing terminal query reply to PTY threw: ${e instanceof Error ? e.message : String(e)}`,
-                  );
-                }
-              })
-            : null;
+        const queryResponseDisposable = useBundledConpty
+          ? headlessTerminal.onData((data) => {
+              try {
+                ptyProcess.write(data);
+              } catch (e) {
+                // A reply racing shell exit finds a dead PTY — drop it.
+                debugLogger.warn(
+                  `writing terminal query reply to PTY threw: ${e instanceof Error ? e.message : String(e)}`,
+                );
+              }
+            })
+          : null;
 
         this.activePtys.set(ptyProcess.pid, { ptyProcess, headlessTerminal });
 
@@ -2529,7 +2528,7 @@ export class ShellExecutionService {
       return { pid: ptyProcess.pid, result };
     } catch (e) {
       const error = e as Error;
-      if (!ptySpawned && os.platform() === 'win32') {
+      if (!ptySpawned && useBundledConpty) {
         // The bundled ConPTY backend (useConptyDll above) adds throw sites
         // node-pty reaches synchronously out of spawn — the conpty.dll it
         // ships being missing or unloadable among them — and none of those