Browse Source

Merge pull request #3516 from deepseek-harness/turtle/issue-2390-windows-hide-subprocess

fix(subprocess): hide Windows child windows
Turtle 1 month ago
parent
commit
df07f37827

+ 6 - 0
.agents/notes/implemented/bug-fix/2026-09-03-hidden-windows-subprocess-windows.i18n.yaml

@@ -0,0 +1,6 @@
+# Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each
+# side as of the last confirmed-consistent state. Both languages carry equal authority;
+# after editing either side, bring the other along and re-record with:
+#   pnpm run verify-translation-pairing --write .agents/notes/implemented/bug-fix/2026-09-03-hidden-windows-subprocess-windows.md
+2026-09-03-hidden-windows-subprocess-windows.md: 97af84ddc0a51483fa0f1147caee0ea0374c240d
+2026-09-03-hidden-windows-subprocess-windows.zh.md: d8f986278e974342cd24abd6fd3192165762c1f9

+ 29 - 0
.agents/notes/implemented/bug-fix/2026-09-03-hidden-windows-subprocess-windows.md

@@ -0,0 +1,29 @@
+# Agent Note: Suppressing Windows subprocess windows
+
+Status: implemented
+
+English | [中文](2026-09-03-hidden-windows-subprocess-windows.zh.md)
+
+## Problem
+
+The local subprocess provider can run under a GUI or service host with no visible console. Windows creates a new visible window for a child process when the host does not supply one, so an ordinary command or a `taskkill` helper can flash and take focus even though the harness has no user-facing terminal for that process.
+
+## Decision
+
+The provider sets `windowsHide: true` on every non-terminal `spawn` and on both synchronous `taskkill` call sites. The main child uses this option only for the Windows execution path; `taskkill` is itself Windows-only. Terminal processes retain the visibility and console behavior owned by the PTY implementation.
+
+The option hides console windows and GUI windows that honor the Windows process startup visibility setting. Callers do not choose this behavior because the local provider owns whether its background process management creates host windows.
+
+## Alternatives considered
+
+**Hide only the main child.** Rejected because cancellation, timeout escalation, terminal teardown, and host-exit cleanup can still launch `taskkill` and flash a console window.
+
+**Expose a caller option.** Rejected because consumers cannot reliably know whether the local host has a console, and inconsistent choices would reintroduce focus-stealing process-management windows.
+
+**Hide only console programs.** Rejected because Node exposes one Windows startup option rather than a reliable pre-spawn executable classification, and probing the target would add platform-specific races without preserving a useful product behavior.
+
+## Consequences
+
+Background subprocess operations do not create visible Windows child or `taskkill` windows. A directly launched GUI program that honors the startup visibility setting also starts hidden; consumers that need an interactive visible application must use a capability that owns that user interaction instead of the background subprocess provider.
+
+Unit tests inject the process launchers and pin `windowsHide` for the main child and both `taskkill` paths without creating host-global windows or terminating real processes.

+ 29 - 0
.agents/notes/implemented/bug-fix/2026-09-03-hidden-windows-subprocess-windows.zh.md

@@ -0,0 +1,29 @@
+# Agent Note: 隐藏 Windows 子进程窗口
+
+Status: implemented
+
+[English](2026-09-03-hidden-windows-subprocess-windows.md) | 中文
+
+## 问题
+
+本地 subprocess provider 可以在没有可见控制台的 GUI 或服务宿主中运行。宿主未提供控制台时,Windows 会为子进程创建新的可见窗口,因此普通命令或 `taskkill` 辅助进程可能闪现并抢占焦点,即使 harness 并未为该进程提供面向用户的 terminal。
+
+## 决策
+
+provider 对每次非 terminal `spawn` 以及两处同步 `taskkill` 调用都设置 `windowsHide: true`。主子进程只在 Windows 执行路径使用此选项;`taskkill` 本身只用于 Windows。terminal 进程继续采用 PTY 实现拥有的可见性与控制台行为。
+
+该选项会隐藏控制台窗口,以及遵循 Windows 进程启动可见性设置的 GUI 窗口。调用方不能选择此行为,因为本地 provider 负责决定其后台进程管理是否创建宿主窗口。
+
+## 考虑过的替代方案
+
+**只隐藏主子进程。** 不予采纳,因为取消、超时升级、terminal 拆卸与宿主退出清理仍可能启动 `taskkill` 并闪现控制台窗口。
+
+**暴露调用方选项。** 不予采纳,因为消费方无法可靠判断本地宿主是否拥有控制台,不一致的选择会重新引入抢占焦点的进程管理窗口。
+
+**只隐藏控制台程序。** 不予采纳,因为 Node 只暴露一个 Windows 启动选项,无法在 spawn 前可靠区分可执行文件类型;探测目标还会增加平台特定竞态,却不能保留有用的产品行为。
+
+## 后果
+
+后台 subprocess 操作不会创建可见的 Windows 子进程或 `taskkill` 窗口。直接启动且遵循启动可见性设置的 GUI 程序也会以隐藏方式运行;需要交互式可见应用的消费方必须使用拥有该用户交互的能力,而不是后台 subprocess provider。
+
+单元测试注入进程 launcher,固定主子进程和两条 `taskkill` 路径的 `windowsHide`,且不会创建宿主全局窗口或终止真实进程。

+ 2 - 2
packages/subprocess/subprocess-local/README.i18n.yaml

@@ -2,5 +2,5 @@
 # side as of the last confirmed-consistent state. Both languages carry equal authority;
 # after editing either side, bring the other along and re-record with:
 #   pnpm run verify-translation-pairing --write packages/subprocess/subprocess-local/README.md
-README.md: dd9edbc99578f5411fad993cf88f93429dc9cab2
-README.zh.md: 7bb43f7bbce5ef6b128edc9ef2104d35a5887845
+README.md: cacd900729237d60f1bfeab9723ed83be3d9f3aa
+README.zh.md: 559ec64cc3cb0d416934c821090daef3df999c58

+ 1 - 1
packages/subprocess/subprocess-local/README.md

@@ -25,7 +25,7 @@ Mount `dsh-subprocess-local` in any composition that runs child processes on the
 <a id="use-this-package"></a>
 ## Use this package
 
-Mount the provider beside its consumers and start processes exactly as the subprocess service specifies; this package decides only how those processes run on the host.
+Mount the provider beside its consumers and start processes exactly as the subprocess service specifies; this package decides only how those processes run on the host. On Windows, non-terminal children and `taskkill` helpers start with their windows hidden so background operations do not take focus. This also hides GUI windows that honor the process startup visibility setting.
 
 ### Mounting the provider
 

+ 1 - 1
packages/subprocess/subprocess-local/README.zh.md

@@ -25,7 +25,7 @@ kind: "package-reference"
 <a id="use-this-package"></a>
 ## 使用本包
 
-把提供方与它的消费方挂载在同一组合中,并完全按子进程服务的规定启动进程;本包只决定这些进程在宿主机上如何运行。
+把提供方与它的消费方挂载在同一组合中,并完全按子进程服务的规定启动进程;本包只决定这些进程在宿主机上如何运行。在 Windows 上,非终端子进程与 `taskkill` 辅助进程会隐藏窗口,因此后台操作不会抢占焦点。遵循进程启动可见性设置的 GUI 窗口也会被隐藏。
 
 ### 挂载提供方
 

+ 1 - 1
packages/subprocess/subprocess-local/src/index.ts

@@ -39,7 +39,7 @@ export class LocalSubprocessRuntime extends SubprocessRuntime {
   private live = new Set<LocalSubprocessHandle>()
   /** Live terminals retained through normal quiescence or host-exit finalization. */
   private terminals = new Set<LocalTerminalHandle>()
-  /** Test hook: spill and platform knobs forwarded to spawnSubprocess. */
+  /** Test hook: process, spill, and platform operations forwarded to spawnSubprocess. */
   internals: SpawnInternals = {}
   /** Test hook for platform process inspection; production resolves lazily on terminal spawn. */
   terminalInspector: ProcessInspector | undefined

+ 17 - 4
packages/subprocess/subprocess-local/src/spawn.ts

@@ -7,7 +7,7 @@
  * @module dsh-subprocess-local/spawn
  */
 
-import { type ChildProcess, spawn, spawnSync } from 'node:child_process'
+import { type ChildProcess, type SpawnOptions, spawn, spawnSync } from 'node:child_process'
 import type { Readable } from 'node:stream'
 import { randomBytes } from 'node:crypto'
 import { closeSync, mkdtempSync, openSync, unlinkSync, writeSync } from 'node:fs'
@@ -26,6 +26,12 @@ import type {
 } from '@deepseek-ai/dsh-subprocess'
 import { linuxProcessGroupHasLiveMembers } from './process-inspector.ts'
 
+type SpawnProcess = (
+  program: string,
+  args: readonly string[],
+  options: SpawnOptions,
+) => ChildProcess
+
 /**
  * Build a child environment: explicit caller entries override the scrubbed
  * parent base using the target platform's environment-key semantics. A string
@@ -46,8 +52,10 @@ export function childEnv(extra?: Readonly<NodeJS.ProcessEnv>): NodeJS.ProcessEnv
   return Object.fromEntries(entries)
 }
 
-/** Injectable knobs so tests can exercise spill and platform behavior deterministically. */
+/** Injectable process, spill, and platform operations. */
 export interface SpawnInternals {
+  /** Process spawner (defaults to `node:child_process` `spawn`). */
+  spawn?: SpawnProcess
   /** Directory for spill files (defaults to the OS temp dir). */
   spillDir?: string
   /** Windows tree-termination runner (defaults to `taskkill /PID <pid> /T /F`). */
@@ -278,7 +286,10 @@ export function taskkillProcessTree(pid: number): void {
   // Outcome deliberately unchecked: an already-absent tree (status 128), exit
   // races, and a missing taskkill binary (spawnSync reports, never throws) are
   // as tolerable here as ESRCH is for a POSIX group signal.
-  spawnSync('taskkill', ['/PID', String(pid), '/T', '/F'], { stdio: 'ignore' })
+  spawnSync('taskkill', ['/PID', String(pid), '/T', '/F'], {
+    stdio: 'ignore',
+    windowsHide: true,
+  })
 }
 
 /**
@@ -329,6 +340,7 @@ export function spawnSubprocess(spec: SubprocessSpawnSpec, internals: SpawnInter
   }
   const spillDir = internals.spillDir ?? privateSpillDir()
   const platform = internals.platform ?? process.platform
+  const spawnProcess = internals.spawn ?? spawn
   const taskkill = internals.taskkill ?? taskkillProcessTree
   const linuxGroupHasLiveMembers = internals.linuxProcessGroupHasLiveMembers ?? linuxProcessGroupHasLiveMembers
 
@@ -347,7 +359,7 @@ export function spawnSubprocess(spec: SubprocessSpawnSpec, internals: SpawnInter
   const stdinMode = spec.stdio.stdin
 
   const env = childEnv(spec.env)
-  const child = spawn(program, args, {
+  const child = spawnProcess(program, args, {
     cwd: spec.cwd,
     env,
     stdio: [
@@ -358,6 +370,7 @@ export function spawnSubprocess(spec: SubprocessSpawnSpec, internals: SpawnInter
     // `detached` gives teardown a tree root on POSIX (its own process group);
     // Windows terminates by root pid through taskkill /T instead.
     detached: platform !== 'win32',
+    windowsHide: platform === 'win32',
   })
 
   const collectStream = (mode: SubprocessOutputMode, stream: Readable | null, label: string): OutputCollector | undefined => {

+ 4 - 1
packages/subprocess/subprocess-local/src/windows-inspector.ts

@@ -145,7 +145,10 @@ function taskkillTree(pid: number, force: boolean): void {
   if (pid <= 0) return
   // Outcome deliberately unchecked: an already-absent tree, exit races, and a
   // missing taskkill binary are as tolerable here as ESRCH is for POSIX.
-  spawnSync('taskkill', ['/PID', String(pid), '/T', ...(force ? ['/F'] : [])], { stdio: 'ignore' })
+  spawnSync('taskkill', ['/PID', String(pid), '/T', ...(force ? ['/F'] : [])], {
+    stdio: 'ignore',
+    windowsHide: true,
+  })
 }
 
 declare const nativePtr: unique symbol

+ 41 - 0
packages/subprocess/subprocess-local/tests/spawn.spec.ts

@@ -1,3 +1,4 @@
+import { spawn as nodeSpawn, spawnSync as nodeSpawnSync } from 'node:child_process'
 import { mkdtempSync, readFileSync, statSync, unlinkSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { dirname, join } from 'node:path'
@@ -12,6 +13,11 @@ import {
 import type { SubprocessHandle, SubprocessOutputReader } from '@deepseek-ai/dsh-subprocess'
 import { MAX_TIMER_DELAY_MS } from '@deepseek-ai/dsh-timeout'
 
+vi.mock('node:child_process', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('node:child_process')>()
+  return { ...actual, spawnSync: vi.fn(actual.spawnSync) }
+})
+
 /**
  * Translate the suite's POSIX command strings into node one-liners on Windows,
  * where no bash exists; the translated commands keep the same observable
@@ -631,6 +637,30 @@ describe('stdio dispositions', () => {
 })
 
 describe('windows tree semantics (injected platform)', () => {
+  it('hides the child window without changing output, exit, stdio, or tree-root options', async () => {
+    let options: Parameters<typeof nodeSpawn>[2]
+    const result = await finish(spawnSubprocess(spec('echo hello'), {
+      spillDir,
+      platform: 'win32',
+      spawn: (program, args, spawnOptions) => {
+        options = spawnOptions
+        return nodeSpawn(program, args, spawnOptions)
+      },
+    }))
+
+    expect(options!).toMatchObject({
+      windowsHide: true,
+      detached: false,
+      stdio: ['ignore', 'pipe', 'pipe'],
+    })
+    expect(result).toMatchObject({
+      exitCode: 0,
+      signal: null,
+      stdout: { text: 'hello\n', truncated: false },
+      stderr: { text: '', truncated: false },
+    })
+  })
+
   it('host-exit termination routes through taskkill immediately', async () => {
     const killed: number[] = []
     const running = spawnSubprocess(spec('exec sleep 60', { graceMs: 60_000 }), {
@@ -773,6 +803,17 @@ describe.skipIf(process.platform === 'win32')('tree-survivor escalation (termina
 })
 
 describe('coverage seams', () => {
+  it('hides the taskkill helper window', () => {
+    const taskkill = vi.mocked(nodeSpawnSync)
+    taskkill.mockReturnValueOnce({} as never)
+    taskkillProcessTree(77)
+    expect(taskkill).toHaveBeenLastCalledWith(
+      'taskkill',
+      ['/PID', '77', '/T', '/F'],
+      { stdio: 'ignore', windowsHide: true },
+    )
+  })
+
   it('taskkillProcessTree ignores non-positive pids and contains a missing binary', () => {
     expect(() => { taskkillProcessTree(-1) }).not.toThrow()
     expect(() => { taskkillProcessTree(0) }).not.toThrow()

+ 27 - 1
packages/subprocess/subprocess-local/tests/windows-inspector.spec.ts

@@ -1,4 +1,5 @@
-import { describe, expect, it } from 'vitest'
+import { spawnSync as nodeSpawnSync } from 'node:child_process'
+import { describe, expect, it, vi } from 'vitest'
 import {
   createWindowsProcessInspector,
   isInvalidHandle,
@@ -12,6 +13,11 @@ import type {
   WindowsProcessState,
 } from '@deepseek-ai/dsh-subprocess-local/src/windows-inspector.ts'
 
+vi.mock('node:child_process', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('node:child_process')>()
+  return { ...actual, spawnSync: vi.fn(actual.spawnSync) }
+})
+
 function fakeInternals() {
   const entries: ProcessEntry[] = []
   const states = new Map<number, WindowsProcessState>()
@@ -86,6 +92,26 @@ describe('windowsProcessTree', () => {
 })
 
 describe('WindowsProcessInspector (injected internals)', () => {
+  it('hides the default taskkill helper window for both termination tiers', () => {
+    const taskkill = vi.mocked(nodeSpawnSync)
+    taskkill.mockReturnValueOnce({} as never).mockReturnValueOnce({} as never)
+    const inspector = createWindowsProcessInspector()
+    inspector.signalGroup(77, 'SIGKILL')
+    inspector.signalGroup(78, 'SIGTERM')
+    expect(taskkill).toHaveBeenNthCalledWith(
+      1,
+      'taskkill',
+      ['/PID', '77', '/T', '/F'],
+      { stdio: 'ignore', windowsHide: true },
+    )
+    expect(taskkill).toHaveBeenNthCalledWith(
+      2,
+      'taskkill',
+      ['/PID', '78', '/T'],
+      { stdio: 'ignore', windowsHide: true },
+    )
+  })
+
   it('exposes the shell pid as the pseudo foreground group and never proves stdin waits', () => {
     const fake = fakeInternals()
     const inspector = new WindowsProcessInspector(fake.internals)