From 52d1a04652c3ff4779a0e2babfb0c7d15ea112af Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 26 Jun 2026 13:54:10 +0000 Subject: [PATCH] fix(mcp): reused attached windows console for stdio wrappers Detected whether the OMP host already owns an inheritable Windows console before resolving stdio MCP spawn flags. Skipped CREATE_NO_WINDOW for console-attached MCP wrapper chains so cmd.exe and PowerShell grandchildren reuse the existing terminal instead of allocating visible conhost windows. Fixes #3567 --- packages/coding-agent/CHANGELOG.md | 1 + .../src/mcp/transports/stdio.test.ts | 22 +++++++--- .../coding-agent/src/mcp/transports/stdio.ts | 44 +++++++++++-------- .../test/mcp-stdio-transport.test.ts | 26 +++++------ 4 files changed, 55 insertions(+), 38 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 1334b25e0..9e4a51a6e 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixed +- Fixed Windows stdio MCP wrapper chains spawning visible PowerShell/cmd windows on startup after the #3544 fix. `StdioTransport.connect()` now probes whether OMP already has an inheritable console and `resolveStdioSpawnCommand` skips `windowsHide`/`CREATE_NO_WINDOW` in that case, so `cmd.exe`/PowerShell grandchildren reuse the terminal console instead of allocating visible conhosts during MCP startup or reconnects. ([#3567](https://github.com/can1357/oh-my-pi/issues/3567)) - Fixed Kimi-family models defaulting to hashline edit mode; they now fall back to `replace` unless `edit.modelVariants`, `PI_EDIT_VARIANT`, or `PI_STRICT_EDIT_MODE` explicitly opts into hashline. - Fixed MCP OAuth discovery rejecting Atlassian-style cross-host issuer metadata during the resource-server fallback probe; issuer matching now remains enforced for advertised auth-server candidates but no longer blocks fallback metadata where the resource host and authorization-server issuer differ. ([#3551](https://github.com/can1357/oh-my-pi/issues/3551)) - Fixed plan approval applying the wrong execution model when the model-tier slider sat on the model that exit would restore. The match check now compares the selected role's effective thinking level against the pre-plan thinking level, so picking the active planning tier is retained and picking a same-model tier with an explicit thinking suffix (e.g. `default = sonnet:off` while plan-mode raised thinking to `high`) goes through `applyRoleModel` instead of silently restoring the pre-plan level. ([#3554](https://github.com/can1357/oh-my-pi/issues/3554)) diff --git a/packages/coding-agent/src/mcp/transports/stdio.test.ts b/packages/coding-agent/src/mcp/transports/stdio.test.ts index 0b8a2783d..ff2f20f6b 100644 --- a/packages/coding-agent/src/mcp/transports/stdio.test.ts +++ b/packages/coding-agent/src/mcp/transports/stdio.test.ts @@ -3,14 +3,13 @@ import { describe, expect, it } from "bun:test"; import { resolveStdioSpawnCommand } from "./stdio"; describe("resolveStdioSpawnCommand", () => { - it("hides AND stays attached to direct Windows executable MCP servers", async () => { - // Hidden so the direct .exe does not pop a console (#3536); attached so - // nested console grandchildren do not allocate a new visible conhost - // that strips their stdout from our pipe (#3544). + it("hides Windows executable MCP servers when the host has no console", async () => { + // Hidden so a console-app child does not allocate a visible window when + // OMP is launched without a terminal console (#3536). await expect( resolveStdioSpawnCommand( { command: "server.exe", args: ["--stdio"] }, - { cwd: process.cwd(), env: {}, platform: "win32" }, + { cwd: process.cwd(), env: {}, platform: "win32", hostHasInheritableConsole: false }, ), ).resolves.toEqual({ cmd: ["server.exe", "--stdio"], @@ -19,6 +18,19 @@ describe("resolveStdioSpawnCommand", () => { }); }); + it("inherits an attached Windows console instead of forcing CREATE_NO_WINDOW", async () => { + await expect( + resolveStdioSpawnCommand( + { command: "server.exe", args: ["--stdio"] }, + { cwd: process.cwd(), env: {}, platform: "win32", hostHasInheritableConsole: true }, + ), + ).resolves.toEqual({ + cmd: ["server.exe", "--stdio"], + windowsHide: false, + detached: false, + }); + }); + it("detaches off-Windows MCP servers so terminal job-control signals cannot stop them", async () => { await expect( resolveStdioSpawnCommand( diff --git a/packages/coding-agent/src/mcp/transports/stdio.ts b/packages/coding-agent/src/mcp/transports/stdio.ts index e5a46717b..0bfe71547 100644 --- a/packages/coding-agent/src/mcp/transports/stdio.ts +++ b/packages/coding-agent/src/mcp/transports/stdio.ts @@ -7,9 +7,9 @@ import * as fs from "node:fs/promises"; import * as path from "node:path"; - import { getProjectDir, readJsonl, Snowflake } from "@oh-my-pi/pi-utils"; import { type Subprocess, spawn } from "bun"; +import { hostHasInheritableConsole } from "../../eval/py/spawn-options"; import type { JsonRpcError, JsonRpcMessage, @@ -25,6 +25,16 @@ import { isMCPTimeoutEnabled, resolveMCPTimeoutMs } from "../timeout"; /** Subprocess argv and platform-derived spawn flags for an MCP stdio server. */ export interface StdioSpawnCommand { cmd: string[]; + /** + * Hide the Windows console window for the direct child. + * + * Windows uses this only when the OMP host has no console to share. When + * the host is running inside a terminal, `windowsHide: true` maps to + * `CREATE_NO_WINDOW`, which strips that inheritable console from hidden + * `cmd.exe` / PowerShell wrapper chains. Their console grandchildren then + * allocate fresh visible conhost windows during startup or reconnects + * (#3567). + */ windowsHide?: boolean; /** * Run the subprocess in its own session. @@ -34,15 +44,9 @@ export interface StdioSpawnCommand { * background-read SIGTTIN) cannot stop stdio servers such as * `chrome-devtools-mcp` and leave our read loop blocked on silent pipes. * - * Windows: `false`. There is no SIGTSTP/SIGTTIN to escape, but detaching - * passes `DETACHED_PROCESS` to `CreateProcess`, which strips the parent's - * inherited console. A hidden `cmd.exe` wrapper whose grandchild is a - * console app (`node`, `npx.cmd`, `mcp-remote`) then allocates a brand-new - * visible conhost for that grandchild, and its stdout no longer routes - * back through our pipe — the proxy reports it is up while OMP times out - * waiting for the MCP `initialize` response (#3544). `windowsHide` only - * hides the direct child's console window (#3536); without `detached: - * false` the nested grandchild still pops. + * Windows: `false`. There is no SIGTSTP/SIGTTIN to escape, and Windows + * wrapper chains must stay in the OMP console session so nested console + * grandchildren keep stdout routed through our pipe (#3544). */ detached: boolean; } @@ -51,6 +55,7 @@ export interface StdioSpawnCommand { export interface ResolveStdioSpawnOptions { cwd: string; env: Record; + hostHasInheritableConsole?: boolean; platform?: NodeJS.Platform; } @@ -161,6 +166,7 @@ async function resolveWindowsNpmShimCommand( command: string, args: readonly string[], cwd: string, + windowsHide: boolean, ): Promise { if (!isWindowsBatchCommand(command)) return null; if (!hasPathSegment(command)) return null; @@ -195,7 +201,7 @@ async function resolveWindowsNpmShimCommand( const nodeCommand = (await fileExists(siblingNode)) ? siblingNode : "node"; return { cmd: [nodeCommand, target, ...args], - windowsHide: true, + windowsHide, detached: false, }; } @@ -251,18 +257,18 @@ export async function resolveStdioSpawnCommand( const args = config.args ?? []; if (options.platform !== "win32") return { cmd: [config.command, ...args], detached: true }; + const windowsHide = options.hostHasInheritableConsole === undefined ? true : !options.hostHasInheritableConsole; const resolved = await resolveWindowsCommandPath(config.command, options.cwd, options.env); const resolvedCommand = resolved ?? config.command; - const npmShimCommand = await resolveWindowsNpmShimCommand(resolvedCommand, args, options.cwd); + const npmShimCommand = await resolveWindowsNpmShimCommand(resolvedCommand, args, options.cwd, windowsHide); if (npmShimCommand) return npmShimCommand; // Direct-spawn only when we resolved to a concrete file AND its extension // is not a batch script. Everything else (resolved .cmd/.bat, or an // unresolved extensionless command) goes through cmd.exe so PATHEXT runs. - // Every Windows stdio server launch hides its console window AND stays - // attached: detaching on Windows breaks nested console grandchildren - // (see `StdioSpawnCommand.detached`). - const windowsHide = true; + // Windows stdio servers stay attached so wrapper grandchildren inherit the + // same console session. Only hide the child when OMP itself has no console + // to share; CREATE_NO_WINDOW breaks console inheritance for nested wrappers. const detached = false; const needsCmdExe = resolved === null || isWindowsBatchCommand(resolvedCommand); if (!needsCmdExe) return { cmd: [resolvedCommand, ...args], windowsHide, detached }; @@ -364,14 +370,14 @@ export class StdioTransport implements MCPTransport { cwd, env, platform: process.platform, + hostHasInheritableConsole: hostHasInheritableConsole(), }); // Platform-derived session and console-window handling come from // `resolveStdioSpawnCommand`: POSIX detaches into its own session to // escape terminal job-control signals (SIGTSTP, SIGTTIN); Windows stays - // attached and hidden so nested console grandchildren do not allocate a - // brand-new conhost that strips their stdout from our pipe. See - // `StdioSpawnCommand.detached`. + // attached, and only hides the child when the host has no console to + // share. See `StdioSpawnCommand`. this.#process = spawn({ cmd: spawnCommand.cmd, cwd, diff --git a/packages/coding-agent/test/mcp-stdio-transport.test.ts b/packages/coding-agent/test/mcp-stdio-transport.test.ts index ebe145bdc..86ec04bdc 100644 --- a/packages/coding-agent/test/mcp-stdio-transport.test.ts +++ b/packages/coding-agent/test/mcp-stdio-transport.test.ts @@ -109,11 +109,12 @@ describe("resolveStdioSpawnCommand", () => { PATHEXT: ".cmd", }, platform: "win32", + hostHasInheritableConsole: true, }, ); expect(result.cmd).toEqual(["node", entry, "serve", "--mcp"]); - expect(result.windowsHide).toBe(true); + expect(result.windowsHide).toBe(false); expect(result.detached).toBe(false); } finally { await fs.rm(tempDir, { recursive: true, force: true }); @@ -339,18 +340,14 @@ describe("resolveStdioSpawnCommand", () => { expect(result.detached).toBe(true); }); - it("never detaches Windows stdio launches so nested cmd.exe wrappers keep stdout routed back to the parent pipe (#3544)", async () => { - // The reporter's failing shape is `cmd.exe` → `node wrapper` → - // `cmd.exe /C npx.cmd -y mcp-remote`. Detaching the direct hidden - // `cmd.exe` strips its inherited console; the nested console - // grandchildren (`node`, `npx.cmd`, `mcp-remote`) then allocate a - // brand-new visible conhost whose stdout no longer routes back through - // our pipe — the proxy reports the bridge is up while OMP times out on - // the MCP `initialize` response. `windowsHide` only hides the direct - // child's window (#3536); the only fix that keeps the grandchild - // attached to the same console session is `detached: false`. Pin every - // Windows return shape here so the contract cannot regress for the - // direct-`cmd.exe` launcher the reporter used. + it("keeps console-attached Windows cmd.exe wrapper chains out of CREATE_NO_WINDOW (#3567)", async () => { + // The #3544 shape is `cmd.exe` → `node wrapper` → another console + // launcher (`cmd.exe /C npx.cmd`, PowerShell, similar). If the OMP host + // already owns a terminal console, `windowsHide: true` maps to + // CREATE_NO_WINDOW and strips that inheritable console from the direct + // hidden wrapper. Grandchildren then allocate fresh visible conhost + // windows during startup or reconnect loops (#3567). Keep the tree + // attached to OMP's console instead. const result = await resolveStdioSpawnCommand( { type: "stdio", command: "cmd.exe", args: ["/C", "node .codex\\mcp-wrapper.js"] }, { @@ -361,11 +358,12 @@ describe("resolveStdioSpawnCommand", () => { PATHEXT: ".COM;.EXE;.BAT;.CMD", }, platform: "win32", + hostHasInheritableConsole: true, }, ); expect(result.detached).toBe(false); - expect(result.windowsHide).toBe(true); + expect(result.windowsHide).toBe(false); }); });