diff --git a/.agents/docs/cli-effect-ts.md b/.agents/docs/cli-effect-ts.md index 6b058bd2c7..0dfdd65249 100644 --- a/.agents/docs/cli-effect-ts.md +++ b/.agents/docs/cli-effect-ts.md @@ -69,7 +69,7 @@ wait for cleanup. Native Effect callers yield the service directly. A Session owns a container Scope. Each spawn uses a child Scope: failed or interrupted setup closes it before returning; successful setup retains it until whole-group exit and drained stdio. Closing the container Scope terminates its trees and stops its -monitor fibers, then removes cgroup resources. `startProcess` is reserved for +monitor fibers, then removes cgroup resources. `startProcessLegacy` is reserved for legacy synchronous/raw Node handles (including IPC and explicit detach), whose owner must await `terminate`; it is not a scoped Effect API. @@ -79,9 +79,30 @@ The process foundation is `@lody/shared/node/process`. Its Promise entry points run v4 programs at the legacy boundary. CLI's `platform/process-options.ts` only composes services and logging; it executes no program and has no Promise API. `SessionSandbox`, the ACP runner and authentication probes use this foundation. -Other process callers migrate in the next layers. Delete each temporary facade +Other CLI callers use the same core, enforced by `check:cli-process-boundary`. +Electron, supervisor and shared helpers migrate in the final layer. Delete each temporary facade when the corresponding caller itself becomes an Effect service. +Execution facades carry a `Legacy` suffix and `@deprecated`; keep that suffix +visible in imports and calls. New Effect workflows compose core APIs and leave +execution to their owning application entry point. Layer builders and pure error +conversions retain their names because they do not execute a program. Current +shared execution facades are: + +- `runCommandTextLegacy` and `runCommandTextSyncLegacy`: text output for Promise + and blocking callers, respectively. +- `startProcessLegacy`: raw Node handle with manual ownership + (`ProcessHandleLegacy`). +- `terminateChildTreeLegacy` and `signalChildTreeNowLegacy`: awaited tree cleanup + and synchronous exit-hook signalling, respectively. +- `isPidAliveSyncLegacy` and `probePidSyncLegacy`: synchronous process probes. +- `makeProcessRunnerLegacy` (`ProcessRunnerLegacy`) and `runPromiseSquashedLegacy`: + temporary execution and error conversion for Promise entry points. + +`runCommand`, `runCommandOk`, their Effect-returning Sync variants, `spawnProcess`, +`terminateTree`, `childProcessTree`, `isPidAlive` and `probePid` retain their names. +The guard rejects retired facade imports/exports and aliases that hide Legacy. + ## Testing - Use `@effect/vitest` (`it.effect`, `it.live`) with `TestClock` from diff --git a/.agents/notes/implemented/architecture/2026-09-27-effect-process-tree-layer.md b/.agents/notes/implemented/architecture/2026-09-27-effect-process-tree-layer.md index 0412ffd6b5..e2c9923841 100644 --- a/.agents/notes/implemented/architecture/2026-09-27-effect-process-tree-layer.md +++ b/.agents/notes/implemented/architecture/2026-09-27-effect-process-tree-layer.md @@ -84,3 +84,31 @@ The tests are local evidence. No real Windows host, Linux cgroup hierarchy or signed desktop installer has been tested. taskkill cannot retain descendants whose Windows parent has already exited; [#429](https://github.com/LodyAI/Lody/issues/429) remains partly addressed until the separate Job Object work. + +## Remaining CLI consumers (#1069) + +Git, worktree setup/GC, daemon commands, MCP, preview, file scanning, resource +probes and PTY termination use the same process facades. The CLI guard rejects +raw process APIs and process-spawning dependencies, including dynamic imports +and direct child kill forms. Standalone generated scripts and node-pty spawning +are named exceptions; PTY termination still uses the core. + +The shared login-shell probe is introduced with its CLI caller. All fallback +shells share one deadline, printf delimiters preserve PATH, probe-only tmux and +update variables are restored to their original values, and multiline values +remain intact. CLI startup still unblocks after 3 seconds while the bounded +probe finishes in the background. shell-env is removed from CLI dependencies. +PTY hangup retains a bounded grace period before force-killing its group. + +Current-main integration also migrates the new Simulator worker, native server, +guest helper, fixed xcrun commands and memory-provider CLI calls in #1069. +IPC ownership and guest EOF release remain with their owners; shared tree +termination replaces local signal loops. Global-only host Git identity and the +verified upgrade-installation handoff from main are preserved. + +The timeout regressions for the login shell and daemon runner now wait for explicit +startup and advance virtual time. They still assert that roots and descendants +stop, and that a survivor reports failure. Disabling command grouping fails the +shell timeout case. The browser command-builder test only repeated its literal +mapping; it and the test-only export were removed. Platform opener behavior and +URL argument passing remain unchanged. diff --git a/.agents/notes/implemented/architecture/2026-09-27-effect-process-tree-layer.zh.md b/.agents/notes/implemented/architecture/2026-09-27-effect-process-tree-layer.zh.md index 886c259ebf..cdde1ba796 100644 --- a/.agents/notes/implemented/architecture/2026-09-27-effect-process-tree-layer.zh.md +++ b/.agents/notes/implemented/architecture/2026-09-27-effect-process-tree-layer.zh.md @@ -61,3 +61,22 @@ daemon 级运行时留给后续会话资源阶段。此 stack 不引入 v3 进 这些是本地证据；未验证真实 Windows、Linux cgroup 或签名安装包。taskkill 无法保留 Windows 父进程已退出后的后代身份，因此 [#429](https://github.com/LodyAI/Lody/issues/429) 仍待单独的 Job Object 工作补齐。 + +## 其余 CLI 调用方（#1069） + +Git、worktree setup/GC、daemon 命令、MCP、preview、文件扫描、资源探测与 PTY 终止统一使用进程 +门面。CLI 守卫拦截直接进程 API、启动进程的第三方依赖、动态导入和对子进程直接 kill。 +独立生成脚本与 node-pty 启动是写明理由的例外；PTY 终止仍使用核心。 + +共享登录 shell 探测随 CLI 调用方引入。fallback shell 共用一个截止时间，printf 分隔符保留 PATH， +仅供探测的 tmux/update 变量恢复原值，多行环境值保持完整。CLI 启动仍在 3 秒后先放行，后台有上限 +地完成探测。删除 CLI 的 shell-env 依赖；PTY hangup 后保留有上限的宽限再强杀进程组。 + +合并当前 main 后，#1069 也迁移新加入的 Simulator worker、原生服务、guest helper、 +固定 xcrun 命令和 memory provider 命令。IPC 所有权及 guest EOF 释放仍由原模块负责， +进程树终止复用共享层；保留 main 的全局 Git 身份读取和已验证升级安装路径交接。 + +登录 shell 和 daemon runner 的超时回归测试改为确认启动后推进虚拟时钟，继续检查根进程与 +后代退出，以及幸存进程必须报告失败。禁用命令进程组会使 shell 超时案例失败。 +浏览器命令构造器测试仅重复其字面量映射，已连同仅供测试的导出删除；平台打开方式与 URL +参数传递不变。 diff --git a/.agents/notes/implemented/architecture/2026-10-09-effect-official-process-service.md b/.agents/notes/implemented/architecture/2026-10-09-effect-official-process-service.md index 15e1ff0ceb..eff827c0b4 100644 --- a/.agents/notes/implemented/architecture/2026-10-09-effect-official-process-service.md +++ b/.agents/notes/implemented/architecture/2026-10-09-effect-official-process-service.md @@ -91,7 +91,7 @@ The shared Promise runner forwards an explicit AbortSignal to runPromiseExit. It still starts a separate root fiber: wrapping it in tryPromise without forwarding that signal cannot propagate cancellation, and even forwarding a signal does not make an outer Promise wrapper await the nested finalizers automatically. Effect -callers must yield the service. Synchronous legacy startProcess remains manually +callers must yield the service. Synchronous legacy startProcessLegacy remains manually owned and supports an explicit signal; callers needing proof of termination await its terminate method. It is not represented as a scoped Effect acquisition. @@ -104,6 +104,19 @@ ACP callers, containers and this decision belong to #1065. Remaining CLI import changes and guard guidance belong to #1069; cross-runtime consumers inherit the service through #1348 without another implementation. +After #1065 merged, [#1069](https://github.com/LodyAI/Lody/pull/1069) names all +nine shared execution facades with a Legacy suffix and marks them deprecated. +The runner and manually owned handle types also carry that suffix. Text and Sync +still describe output and blocking behavior; Legacy identifies temporary +compatibility, not another process implementation. Old exports are removed and +callers use the full names without aliases hiding the suffix. New Effect programs +compose the core and leave execution to their owning entry point. Layer builders, +pure conversion helpers and Effect-returning core APIs keep their names. The +process guard rejects retired entry names and aliases hiding Legacy. Cross-runtime +consumers are updated in #1348. This changes names and documentation only; explicit +cancellation, typed failures, bounded tree termination and Scope retirement retain +the existing implementation and behavioral tests. + ## Validation and limits Behavioral tests cover official output helpers and a pipeline, replacement diff --git a/.agents/notes/implemented/architecture/2026-10-09-effect-official-process-service.zh.md b/.agents/notes/implemented/architecture/2026-10-09-effect-official-process-service.zh.md index c582c63128..deedec7de8 100644 --- a/.agents/notes/implemented/architecture/2026-10-09-effect-official-process-service.zh.md +++ b/.agents/notes/implemented/architecture/2026-10-09-effect-official-process-service.zh.md @@ -61,7 +61,7 @@ cgroup.events 成为初始化必需能力。best-effort cleanup 记录失败， shared Promise runner 把明确的 AbortSignal 交给 runPromiseExit，但仍启动独立根 fiber。 用 tryPromise 包装且不传递信号，无法传播取消；即使传递信号，也不会自动让外层 Promise -包装等待内层 finalizer 完成。Effect 调用方应直接 yield 服务。同步兼容 startProcess 仍由 +包装等待内层 finalizer 完成。Effect 调用方应直接 yield 服务。同步兼容 startProcessLegacy 仍由 旧调用方手动持有，支持明确的取消信号；需要证明终止的调用方等待 terminate。它不被描述为 具有 Scope 所有权的 Effect 获取。 @@ -75,6 +75,15 @@ container 和 Scope；已删除的 cgroup 不会自动重建。 container 和这份决定属于 #1065；其它 CLI 调用方的导入调整及守卫指引属于 #1069； 跨运行时调用方通过 #1348 继承服务，不增加另一份实现。 +#1065 合入后，[#1069](https://github.com/LodyAI/Lody/pull/1069) 为九个 shared 执行兼容入口 +统一加上 Legacy 后缀和 deprecated 标记，runner 与手动持有的 handle 类型也带此后缀。 +Text/Sync 仍表达输出和阻塞方式，Legacy 表明临时兼容用途，不代表另一套进程实现。 +删除旧导出，调用方使用完整名称，不通过别名隐藏后缀。新 Effect 工作流组合核心 API， +由所属入口统一执行；Layer 构造器、纯转换函数和返回 Effect 的核心 API 保持原名。 +进程守卫拒绝旧入口名称及隐藏 Legacy 的别名，跨运行时调用方在 #1348 同步。 +此次只改命名和说明：显式取消、有类型的错误、有界进程树终止和 Scope 释放保持原实现， +继续使用已有行为测试验证。 + ## 验证与限制 行为测试覆盖官方输出方法及管道、替换环境变量、Scope 中断、孤儿后代、owner hook 失败、 diff --git a/apps/cli/AGENTS.md b/apps/cli/AGENTS.md index 641582d519..101ab29824 100644 --- a/apps/cli/AGENTS.md +++ b/apps/cli/AGENTS.md @@ -45,6 +45,8 @@ Root `AGENTS.md` applies; this file adds CLI context. Build, PR-poller, and adap ## Coding rules - Prefer Effect v4 for new/refactored CLI code; follow the [Effect guide](../../.agents/docs/cli-effect-ts.md). +- Processes use `@lody/shared/node/process` (CLI wiring: `src/platform/process-options`); + [rules](../../packages/shared/src/node/AGENTS.md), enforced by `check:cli-process-boundary`. - Keep the strict tsconfig, no `any` or non-null assertions, and Zod at every foreign boundary: context/cli-type-safety.md. - After a remote prompt arrives, only correctness-critical setup may block before ACP @@ -108,8 +110,5 @@ execution/consent rules. These rules also bind CLI callers outside that director - Pi extension scanning runs only the pinned runtime's read-only listing entry under a frozen default or saved-profile environment — never caller-supplied launch fields. Selections require the pinned extension-aware runtime (`piExtensionsProtocolVersion`), not a fallback. -- `src/lib/pr-poller/` compensates for a broken hosted GitHub webhook → Streams fan-out. Keep - policy in its pure modules with a thin scheduler, keep priority driven by presence and - `lastMessageAt` rather than a turn-end hook, and keep only scheduling state (never PR status) in - `~/.lody/pr-poller-state.json`. Spec: `specs/pr-status-reconciler.md`; invariants: - `src/lib/pr-poller/AGENTS.md`. +- `src/lib/pr-poller/` compensates for the hosted GitHub webhook fan-out. Spec: + `specs/pr-status-reconciler.md`; invariants: [pr-poller](src/lib/pr-poller/AGENTS.md). diff --git a/apps/cli/package.json b/apps/cli/package.json index 98a2cfe151..4fefd2f921 100644 --- a/apps/cli/package.json +++ b/apps/cli/package.json @@ -130,7 +130,6 @@ "@napi-rs/keyring": "2.1.0", "better-sqlite3": "catalog:", "loro-crdt": "1.16.3", - "shell-env": "^4.0.3", "tinypool": "^1.0.0", "typescript": "^5.9.3" } diff --git a/apps/cli/src/agent/acp-authentication.ts b/apps/cli/src/agent/acp-authentication.ts index 280fd452b2..d6e02fbe2e 100644 --- a/apps/cli/src/agent/acp-authentication.ts +++ b/apps/cli/src/agent/acp-authentication.ts @@ -36,7 +36,7 @@ import { BuiltinAuthenticationOutputParser, } from './acp-authentication-output'; import { shutdownLocalAcpAgent, spawnAcpProcess, terminateAcpProcessTree } from './acp-runner'; -import { startProcess } from '@lody/shared/node/process'; +import { startProcessLegacy } from '@lody/shared/node/process'; import type { NodeProcessApi } from '@lody/shared/node/process'; import type { ManagedRuntimeProgressEvent } from './managed-agent-runtime'; @@ -440,7 +440,7 @@ export async function probeBuiltinAuthentication( } // Its own group, so a timeout or cancel ends whatever the status command // started, not just the command itself. - const { child } = startProcess( + const { child } = startProcessLegacy( { command: launch.command, args: launch.args, @@ -699,7 +699,7 @@ export class AcpAuthenticationManager { if (preparationInterruption) return preparationInterruption; options.onProgress?.({ status: 'starting' }); - const { child } = startProcess( + const { child } = startProcessLegacy( { command: launch.command, args: options.codexProfile diff --git a/apps/cli/src/agent/acp-runner.ts b/apps/cli/src/agent/acp-runner.ts index cf1bfbc2df..3a0bad7f35 100644 --- a/apps/cli/src/agent/acp-runner.ts +++ b/apps/cli/src/agent/acp-runner.ts @@ -14,7 +14,7 @@ import { z } from 'zod'; import type { Logger } from '@/utils/logger'; import { formatErrorMessage } from '@/utils/format-error'; -import { startProcess, terminateChildTree } from '@lody/shared/node/process'; +import { startProcessLegacy, terminateChildTreeLegacy } from '@lody/shared/node/process'; import { withSpawn } from '@/platform/process-options'; import type { NodeProcessApi } from '@lody/shared/node/process'; @@ -191,7 +191,7 @@ export async function terminateAcpProcessTree( nodeProcess?: NodeProcessApi; } ): Promise { - await terminateChildTree( + await terminateChildTreeLegacy( child, { graceMs: options.force ? 0 : options.exitTimeoutMs, @@ -244,7 +244,7 @@ export const spawnAcpProcess = (options: SpawnAcpProcessOptions): ChildProcess = // Its own process group on POSIX (a tree rooted at it on Windows), so // `terminateAcpProcessTree` reaches everything the agent starts. - return startProcess( + return startProcessLegacy( { command: executable.command, args: executable.args, diff --git a/apps/cli/src/agent/codex-profile-process-usage.ts b/apps/cli/src/agent/codex-profile-process-usage.ts index f4fbd0712c..2cf74d8800 100644 --- a/apps/cli/src/agent/codex-profile-process-usage.ts +++ b/apps/cli/src/agent/codex-profile-process-usage.ts @@ -1,8 +1,11 @@ +import { toShared } from '@/platform/process-options'; import { randomUUID } from 'node:crypto'; import { writeFileSync } from 'node:fs'; import { lstat, mkdir, readFile, readdir, rm, writeFile } from 'node:fs/promises'; import path from 'node:path'; import { z } from 'zod'; +import { isPidAliveSyncLegacy } from '@lody/shared/node/process'; + import type { ResolvedCodexProfile } from './codex-profile-store'; const UsageRecordSchema = z.object({ version: z.literal(1), token: z.uuid() }).strict(); @@ -15,14 +18,10 @@ export type CodexProfileProcessUsage = (() => Promise) & { abandonBeforeSpawn(): Promise; }; +/** A pid owned by another user (EPERM) still counts as running. */ function nativeProcessIsGone(pid: number | undefined): boolean { if (pid === undefined) return false; - try { - process.kill(pid, 0); - return false; - } catch (error) { - return (error as NodeJS.ErrnoException).code === 'ESRCH'; - } + return !isPidAliveSyncLegacy(pid, toShared()); } async function readProof(file: string) { diff --git a/apps/cli/src/agent/codex-profile-store.ts b/apps/cli/src/agent/codex-profile-store.ts index 4e0d7426b9..c7722abf26 100644 --- a/apps/cli/src/agent/codex-profile-store.ts +++ b/apps/cli/src/agent/codex-profile-store.ts @@ -1,3 +1,4 @@ +import { toShared } from '@/platform/process-options'; import { createHash, randomUUID } from 'node:crypto'; import { lstat, mkdir, readFile, readdir, realpath, rename, rm, writeFile } from 'node:fs/promises'; import path from 'node:path'; @@ -10,7 +11,8 @@ import { } from '@lody/shared'; import { getLodyDataDir } from '@lody/shared/node/installation-profile'; import { SystemCodexCredentialVault, type CodexCredentialVault } from './codex-credential-vault'; -import { execFile } from 'node:child_process'; +import { runCommandTextLegacy } from '@lody/shared/node/process'; + import { resolveBuiltinAuthenticationProcessLaunch } from './setting'; import { registerCodexProfileProcess, @@ -356,21 +358,26 @@ async function logoutNativeProfile( if (/^(CODEX_|OPENAI_|LODY_CODEX_|DYLD_|NODE_OPTIONS$|LD_PRELOAD$|LD_LIBRARY_PATH$)/i.test(key)) delete env[key]; env.CODEX_HOME = profile.home; - await new Promise((resolve, reject) => { - const child = execFile( - launch.command, - [ - ...(profile.authStore === 'keyring' ? ['-c', 'cli_auth_credentials_store="keyring"'] : []), - 'logout', - ], - { env, timeout: 20_000, windowsHide: true }, - (error) => { - if (error) reject(new Error('Codex account cleanup could not complete')); - else resolve(); - } + try { + await runCommandTextLegacy( + { + command: launch.command, + args: [ + ...(profile.authStore === 'keyring' + ? ['-c', 'cli_auth_credentials_store="keyring"'] + : []), + 'logout', + ], + env, + timeout: 20_000, + check: 'exit-0', + onSpawned: (child) => lease.recordNativePid(child.pid), + }, + toShared() ); - lease.recordNativePid(child.pid); - }); + } catch { + throw new Error('Codex account cleanup could not complete'); + } } let defaultStore: CodexProfileStore | undefined; diff --git a/apps/cli/src/agent/login-shell-env.ts b/apps/cli/src/agent/login-shell-env.ts index 59c381d2d5..dad4a937a2 100644 --- a/apps/cli/src/agent/login-shell-env.ts +++ b/apps/cli/src/agent/login-shell-env.ts @@ -1,7 +1,13 @@ -import { shellEnv } from 'shell-env'; +import { probeLoginShellEnv } from '@lody/shared/node/login-shell-env'; -// Match the Electron probe's budget (cli-service.ts SHELL_ENV_TIMEOUT_MS). -const SHELL_ENV_TIMEOUT_MS = 3000; +import { toShared } from '@/platform/process-options'; + +/** + * How long an ACP spawn waits for the probe before going ahead without it. The + * probe itself keeps its own bound (shared with the desktop) and ends the + * shell's whole process tree when it runs out, so a hung rc file leaks nothing. + */ +const SHELL_ENV_WAIT_MS = 3000; /** * Resolving the login-shell env spawns the user's shell with `-ilc` so it sources @@ -12,13 +18,12 @@ let cachedShellEnvPromise: Promise | null = null; /** Last successfully resolved env, exposed to synchronous callers. */ let resolvedShellEnv: NodeJS.ProcessEnv = {}; -const shouldSkip = (): boolean => - // shell-env just returns process.env on win32, so skip the subprocess there. - process.platform === 'win32' || process.env.LODY_DISABLE_SHELL_ENV === '1'; +const shouldSkip = (): boolean => process.env.LODY_DISABLE_SHELL_ENV === '1'; const resolveOnce = (): Promise => { - const probe = shellEnv() - .then((env) => { + const probe = probeLoginShellEnv({ processOptions: toShared() }) + .then((probed) => { + const env = probed ?? {}; resolvedShellEnv = env; // The race below may have already resolved the memoized promise to {} via // the timeout. Replace it so *later* awaiters (acp-runner aux sessions, @@ -31,14 +36,12 @@ const resolveOnce = (): Promise => { }) .catch((): NodeJS.ProcessEnv => ({})); - // shell-env has no built-in timeout; a hanging dotfile would otherwise block - // every awaiting ACP spawn forever. Fail open to the empty overlay so the - // withDefaultAcpPathEntries fallback still applies. We cannot reap the spawned - // login shell here (shell-env exposes no child handle), but execa's default - // cleanup kills it on process exit, so at worst a hung dotfile leaks one idle - // shell for the daemon's lifetime — not per spawn. + // A slow rc file must not hold every awaiting ACP spawn for the probe's + // whole bound. Fail open to the empty overlay so the + // withDefaultAcpPathEntries fallback still applies; the probe keeps running + // and replaces the cached value when it finishes. const timeout = new Promise((resolve) => { - const timer = setTimeout(() => resolve({}), SHELL_ENV_TIMEOUT_MS); + const timer = setTimeout(() => resolve({}), SHELL_ENV_WAIT_MS); timer.unref(); }); @@ -47,22 +50,21 @@ const resolveOnce = (): Promise => { /** * Read the environment (most importantly `PATH`) from the user's login shell - * profile via the `shell-env` library. + * profile. * * GUI/daemon launches (Electron Dock, systemd, npx, IDE terminals) inherit a * minimal PATH that omits the dirs users actually install tools into * (`~/.local/bin`, homebrew, cargo, volta, asdf, ...). Spawning an agent binary - * such as `opencode acp` then fails with `spawn opencode ENOENT`. `shell-env` - * sources the login shell, so we pick up tools wherever they live instead of - * guessing a fixed set of directories. + * such as `opencode acp` then fails with `spawn opencode ENOENT`. The shared + * probe (`@lody/shared/node/login-shell-env`, also used by the desktop) runs the + * login shell, so we pick up tools wherever they live instead of guessing a + * fixed set of directories. * - * Never throws: on a clean failure the `.catch` yields `{}` (an empty overlay). - * Note `shell-env` itself, when every candidate shell fails, falls back to - * returning the inherited `process.env` rather than throwing — so the overlay may - * be the full process env, not `{}`. That stays safe because `mergeLoginShellEnv` - * is base-wins for non-PATH vars (a no-op for vars the base already has) and the - * caller scrubs inherited auth/routing vars *after* overlaying (see session.ts). - * Disable entirely via `LODY_DISABLE_SHELL_ENV=1`. + * Never throws: when no shell yields an environment the overlay is `{}`. The + * overlay is the login shell's whole environment, which stays safe because + * `mergeLoginShellEnv` is base-wins for non-PATH vars (a no-op for vars the + * base already has) and the caller scrubs inherited auth/routing vars *after* + * overlaying (see session.ts). Disable entirely via `LODY_DISABLE_SHELL_ENV=1`. */ export const getLoginShellEnv = async (): Promise => { if (shouldSkip()) { diff --git a/apps/cli/src/agent/pi-extensions.ts b/apps/cli/src/agent/pi-extensions.ts index 9fce392c80..d710015048 100644 --- a/apps/cli/src/agent/pi-extensions.ts +++ b/apps/cli/src/agent/pi-extensions.ts @@ -1,9 +1,8 @@ -import { execFile } from 'node:child_process'; -import { promisify } from 'node:util'; +import { toShared } from '@/platform/process-options'; import { PiExtensionDiscoverySchema, type PiExtensionDiscovery } from '@lody/shared'; -import { getManagedAgentRuntimeManager, PI_EXTENSIONS_SUPPORTED } from './managed-agent-runtime'; +import { runCommandTextLegacy } from '@lody/shared/node/process'; -const execFileAsync = promisify(execFile); +import { getManagedAgentRuntimeManager, PI_EXTENSIONS_SUPPORTED } from './managed-agent-runtime'; const SCAN_ENV_KEYS = [ 'PATH', @@ -55,16 +54,16 @@ export async function discoverManagedPiExtensions( } } try { - const { stdout } = await execFileAsync( - process.execPath, - [runtime.command, '--list-extensions'], + const { stdout } = await runCommandTextLegacy( { - encoding: 'utf8', + command: process.execPath, + args: [runtime.command, '--list-extensions'], timeout: 30_000, - maxBuffer: 2 * 1024 * 1024, - windowsHide: true, + maxOutputBytes: 2 * 1024 * 1024, env, - } + check: 'exit-0', + }, + toShared() ); return PiExtensionDiscoverySchema.parse(JSON.parse(stdout)); } catch { diff --git a/apps/cli/src/agent/title-generator.ts b/apps/cli/src/agent/title-generator.ts index c04bb80d01..2871735e88 100644 --- a/apps/cli/src/agent/title-generator.ts +++ b/apps/cli/src/agent/title-generator.ts @@ -1,8 +1,7 @@ -import { execFile } from 'child_process'; +import { toShared } from '@/platform/process-options'; import path from 'path'; import os from 'os'; import * as fs from 'fs'; -import { promisify } from 'util'; import { type AcpSessionNotification, @@ -29,11 +28,12 @@ import type { } from '@agentclientprotocol/sdk'; import { extractTextFromAgentResponse } from './response-utils'; import { formatErrorMessage } from '@/utils/format-error'; +import { runCommandTextLegacy } from '@lody/shared/node/process'; + import { normalizeConfigOptions } from './acp-capabilities'; import { readLegacySessionModelState } from './acp-capability-normalization'; import { parseLodyMessagePhase } from './lody-acp-extension'; -const execFileAsync = promisify(execFile); const TITLE_GIT_TIMEOUT_MS = 5_000; export const TITLE_TASK_PROMPT_MAX_CHARS = 4_000; @@ -235,12 +235,17 @@ export const sanitizeGeneratedTitle = (candidate?: string | null): string | null const ensureWorkdirIsGitRepo = async (workdir: string, logger: Logger): Promise => { const isGitRepo = async (): Promise => { try { - const result = await execFileAsync('git', ['rev-parse', '--is-inside-work-tree'], { - cwd: workdir, - encoding: 'utf8', - timeout: TITLE_GIT_TIMEOUT_MS, - }); - return String(result.stdout ?? '').trim() === 'true'; + const result = await runCommandTextLegacy( + { + command: 'git', + args: ['rev-parse', '--is-inside-work-tree'], + cwd: workdir, + timeout: TITLE_GIT_TIMEOUT_MS, + check: 'exit-0', + }, + toShared() + ); + return result.stdout.trim() === 'true'; } catch { return false; } @@ -251,11 +256,16 @@ const ensureWorkdirIsGitRepo = async (workdir: string, logger: Logger): Promise< } try { - await execFileAsync('git', ['init', '--quiet'], { - cwd: workdir, - encoding: 'utf8', - timeout: TITLE_GIT_TIMEOUT_MS, - }); + await runCommandTextLegacy( + { + command: 'git', + args: ['init', '--quiet'], + cwd: workdir, + timeout: TITLE_GIT_TIMEOUT_MS, + check: 'exit-0', + }, + toShared() + ); } catch (error) { logger.debug( `[title-generator] Failed to init git repo in ${workdir}: ${formatErrorMessage(error)}` diff --git a/apps/cli/src/commands/daemon-runner.ts b/apps/cli/src/commands/daemon-runner.ts index 91e7dd6b3d..56eba81a69 100644 --- a/apps/cli/src/commands/daemon-runner.ts +++ b/apps/cli/src/commands/daemon-runner.ts @@ -1,5 +1,5 @@ +import { toShared } from '@/platform/process-options'; import { Command } from 'commander'; -import { spawn } from 'node:child_process'; import { randomUUID } from 'node:crypto'; import { version } from '@/pkg'; import { @@ -37,6 +37,8 @@ import { normalizeCurrentProcessResourceProfile } from '@/utils/process-resource import { flushTelemetry } from '@/instrument'; import { captureSupervisorEvent } from './analytics-events'; import { getRuntimeDiagnostics } from '@/utils/runtime-diagnostics'; +import { startProcessLegacy } from '@lody/shared/node/process'; + import { EXIT_CODE_REMOTE_RESTART, EXIT_CODE_REMOTE_UPGRADE, @@ -75,11 +77,21 @@ function launchLodyStart( delete env.LODY_ELECTRON_BOOTSTRAP; delete env.ELECTRON_RUN_AS_NODE; delete env[DAEMON_RUNNER_READY_FD_ENV]; - const child = spawn(process.execPath, args, { - stdio: ['ignore', 'pipe', 'pipe', 'ipc'], - env, - windowsHide: true, - }); + // The Worker stays in the watchdog's process group: the Supervisor owns its + // shutdown through `requestShutdown` and signals to the exact child. + const { child } = startProcessLegacy( + { + command: process.execPath, + args, + options: { + stdio: ['ignore', 'pipe', 'pipe', 'ipc'], + env, + windowsHide: true, + }, + processGroup: false, + }, + toShared() + ); const result = new Promise((resolve, reject) => { let stdout = ''; diff --git a/apps/cli/src/commands/daemon-shared.test.ts b/apps/cli/src/commands/daemon-shared.test.ts index 7a93a2d045..afc01681ff 100644 --- a/apps/cli/src/commands/daemon-shared.test.ts +++ b/apps/cli/src/commands/daemon-shared.test.ts @@ -1,13 +1,18 @@ +import { toShared } from '@/platform/process-options'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { EventEmitter } from 'node:events'; -import type { ChildProcess } from 'node:child_process'; -import { afterEach, describe, expect, it, vi } from 'vitest'; +import { PassThrough } from 'node:stream'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { startProcessLegacy } from '@lody/shared/node/process'; + +import type { NodeProcessApi } from '@lody/shared/node/process'; +import { FakeProcessTable } from '@lody/shared/node/process-testing'; import { interpretDaemonRunnerLaunchOutcome, readPidFileRecord, removePidFile, + spawnDaemonRunnerAndAwaitReady, terminateSpawnedDaemonRunner, writePidFile, } from './daemon-shared'; @@ -21,29 +26,32 @@ function createPidPath(): string { } afterEach(() => { - vi.useRealTimers(); for (const dir of tempDirs.splice(0)) fs.rmSync(dir, { recursive: true, force: true }); }); -class FakeRunnerChild extends EventEmitter { - exitCode: number | null = null; - signalCode: NodeJS.Signals | null = null; - readonly signals: NodeJS.Signals[] = []; - onKill: (signal: NodeJS.Signals) => void = () => {}; - - kill(signal: NodeJS.Signals = 'SIGTERM'): boolean { - this.signals.push(signal); - this.onKill(signal); - return true; - } - - finish(signal: NodeJS.Signals): void { - this.signalCode = signal; - this.emit('exit', null, signal); - this.emit('close', null, signal); - } +/** + * The fake table's runner, given the fd 3 readiness pipe and `unref` a real + * detached spawn has. Each spawn's pipe is handed to the test to report on. + */ +function runnerProcessTable(platform: NodeJS.Platform = 'linux') { + const table = new FakeProcessTable(platform); + const readyPipes: PassThrough[] = []; + const api: NodeProcessApi = { + ...table.api, + spawn: (command, args, options) => { + const child = table.api.spawn(command, args, options); + if (command === 'taskkill') return child; + const readyPipe = new PassThrough(); + readyPipes.push(readyPipe); + return Object.assign(child, { stdio: [null, null, null, readyPipe], unref: () => child }); + }, + }; + return { table, api, readyPipes }; } +const runnerSpawns = (table: FakeProcessTable) => + table.spawned.filter((call) => call.command !== 'taskkill'); + describe('daemon PID ownership', () => { it('atomically replaces stale diagnostics after Host ownership is acquired', () => { const filePath = createPidPath(); @@ -74,6 +82,8 @@ describe('daemon PID ownership', () => { }); describe('daemon runner launch cleanup', () => { + beforeEach(() => vi.useFakeTimers()); + afterEach(() => vi.useRealTimers()); it.each(['1.2.2', undefined])( 'rejects and drains a replacement reporting version %s', (cliVersion) => { @@ -131,30 +141,98 @@ describe('daemon runner launch cleanup', () => { }); }); - it('waits for a timed-out spawned runner to stop gracefully', async () => { - const child = new FakeRunnerChild(); - child.onKill = (signal) => child.finish(signal); + it('leaves a ready runner running, detached from the launching terminal', async () => { + const { table, api, readyPipes } = runnerProcessTable(); + const launch = spawnDaemonRunnerAndAwaitReady([], { + nodeProcess: api, + pidFilePath: createPidPath(), + }); + const [runnerSpawn] = runnerSpawns(table); + readyPipes[0]?.write( + `${JSON.stringify({ status: 'ready', pid: 1, instanceId: 'runner-a' })}\n` + ); + + const result = await launch; + + expect(result).toMatchObject({ status: 'ready', instanceId: 'runner-a' }); + const runnerPid = result.status === 'ready' ? result.pid : -1; + expect(table.isAlive(runnerPid)).toBe(true); + expect(runnerSpawn?.options).toMatchObject({ + detached: true, + windowsHide: true, + stdio: ['ignore', 'ignore', 'ignore', 'pipe'], + }); + }); - await expect( - terminateSpawnedDaemonRunner(child as unknown as ChildProcess, 999_001) - ).resolves.toBe(true); - expect(child.signals).toEqual(['SIGTERM']); + it('ends the runner and its Worker when the readiness report is invalid', async () => { + const { table, api, readyPipes } = runnerProcessTable(); + const launch = spawnDaemonRunnerAndAwaitReady([], { + nodeProcess: api, + pidFilePath: createPidPath(), + }); + const runnerPid = 1000; + const workerPid = table.addDescendant(runnerPid); + readyPipes[0]?.write('not json\n'); + + const result = await launch; + + expect(result).toEqual({ status: 'error', runnerPid, message: 'invalid readiness report' }); + expect(table.isAlive(runnerPid)).toBe(false); + expect(table.isAlive(workerPid)).toBe(false); }); - it('force-kills the spawned runner when graceful shutdown does not finish', async () => { - vi.useFakeTimers(); - const child = new FakeRunnerChild(); - child.onKill = (signal) => { - if (signal === 'SIGKILL') child.finish(signal); - }; + it('detaches the runner on Windows too and ends its tree after a failed launch', async () => { + const { table, api, readyPipes } = runnerProcessTable('win32'); + const launch = spawnDaemonRunnerAndAwaitReady([], { + nodeProcess: api, + pidFilePath: createPidPath(), + }); + readyPipes[0]?.end(); + + const result = await launch; + + expect(result).toEqual({ status: 'runner_exited', runnerPid: 1000 }); + expect(runnerSpawns(table)[0]?.options.detached).toBe(true); + expect(table.isAlive(1000)).toBe(false); + }); - const stopped = terminateSpawnedDaemonRunner(child as unknown as ChildProcess, 999_002, { + it('force-kills the runner tree when graceful shutdown does not finish', async () => { + const table = new FakeProcessTable(); + table.queueSpawn({ ignores: ['SIGTERM'] }); + const runner = startProcessLegacy( + { command: 'runner', args: [], options: {}, processGroup: true }, + toShared({ nodeProcess: table.api }) + ); + const runnerPid = runner.child.pid ?? -1; + const workerPid = table.addDescendant(runnerPid, { ignores: ['SIGTERM'] }); + + const stopped = terminateSpawnedDaemonRunner(runner, runnerPid, { shutdownGraceMs: 10, - forceKillWaitMs: 10, + forceKillWaitMs: 1_000, + pidFilePath: createPidPath(), }); - await vi.advanceTimersByTimeAsync(10); + await vi.advanceTimersByTimeAsync(1_020); + expect(await stopped).toBe(true); - await expect(stopped).resolves.toBe(true); - expect(child.signals).toEqual(['SIGTERM', 'SIGKILL']); + expect(table.isAlive(runnerPid)).toBe(false); + expect(table.isAlive(workerPid)).toBe(false); + expect(table.delivered.map((delivery) => delivery.signal)).toEqual(['SIGTERM', 'SIGKILL']); + }); + + it('reports a runner tree that survives SIGKILL instead of claiming it stopped', async () => { + const table = new FakeProcessTable(); + table.queueSpawn({ ignores: ['SIGTERM', 'SIGKILL'] }); + const runner = startProcessLegacy( + { command: 'runner', args: [], options: {}, processGroup: true }, + toShared({ nodeProcess: table.api }) + ); + + const stopped = terminateSpawnedDaemonRunner(runner, runner.child.pid ?? -1, { + shutdownGraceMs: 0, + forceKillWaitMs: 10, + pidFilePath: createPidPath(), + }); + await vi.advanceTimersByTimeAsync(20); + expect(await stopped).toBe(false); }); }); diff --git a/apps/cli/src/commands/daemon-shared.ts b/apps/cli/src/commands/daemon-shared.ts index f348b50962..766181bd42 100644 --- a/apps/cli/src/commands/daemon-shared.ts +++ b/apps/cli/src/commands/daemon-shared.ts @@ -1,6 +1,7 @@ +import { toShared } from '@/platform/process-options'; import fs from 'node:fs'; import path from 'node:path'; -import { spawn, type ChildProcess } from 'node:child_process'; +import type { ChildProcess } from 'node:child_process'; import { randomUUID } from 'node:crypto'; import type { Readable } from 'node:stream'; import os from 'node:os'; @@ -14,6 +15,9 @@ import { import { getLodyDataDir } from '@lody/shared/node/installation-profile'; import { requestLocalCliHostShutdown } from '@lody/shared/node/local-cli-host-lease'; import { calculateWorkerMaxOldSpaceMiB } from '@lody/cli-supervisor'; +import { startProcessLegacy, type ProcessHandleLegacy } from '@lody/shared/node/process'; + +import type { NodeProcessApi } from '@lody/shared/node/process'; export { LODY_LOG_DIR } from '@/utils/log-retention'; export const DAEMON_PID_FILE = path.join(getLodyDataDir(), 'daemon.pid'); @@ -232,74 +236,61 @@ function isChildProcessRunning(child: ChildProcess): boolean { return child.exitCode === null && child.signalCode === null; } -async function waitForChildProcessExit(child: ChildProcess, timeoutMs: number): Promise { - if (!isChildProcessRunning(child)) return true; - return await new Promise((resolve) => { - let settled = false; - let timeout: ReturnType | undefined; - let onExit: () => void; - const finish = (exited: boolean) => { - if (settled) return; - settled = true; - if (timeout !== undefined) clearTimeout(timeout); - child.off('exit', onExit); - child.off('close', onExit); - resolve(exited); - }; - onExit = () => finish(true); - timeout = setTimeout(() => finish(!isChildProcessRunning(child)), timeoutMs); - child.once('exit', onExit); - child.once('close', onExit); - if (!isChildProcessRunning(child)) finish(true); - }); +/** Whether the runner's root process exited within `timeoutMs`. */ +async function waitForRunnerExit(runner: ProcessHandleLegacy, timeoutMs: number): Promise { + if (!isChildProcessRunning(runner.child)) return true; + let timeout: ReturnType | undefined; + try { + return await Promise.race([ + runner.exited.then(() => true), + new Promise((resolve) => { + timeout = setTimeout(() => resolve(!isChildProcessRunning(runner.child)), timeoutMs); + }), + ]); + } finally { + if (timeout !== undefined) clearTimeout(timeout); + } } /** - * A failed foreground launch must not leave its exact detached child running. - * Ask that runner's authenticated Host endpoint to drain its Worker; the exact - * ChildProcess handle we spawned is the fallback and force-stop authority. + * A failed foreground launch must not leave its exact detached runner, or the + * Worker in its process tree, running. Ask that runner's authenticated Host + * endpoint to drain its Worker; the process tree we spawned is the fallback + * and force-stop authority. */ export async function terminateSpawnedDaemonRunner( - child: ChildProcess, + runner: ProcessHandleLegacy, runnerPid: number, - options: { shutdownGraceMs?: number; forceKillWaitMs?: number } = {} + options: { shutdownGraceMs?: number; forceKillWaitMs?: number; pidFilePath?: string } = {} ): Promise { - if (!isChildProcessRunning(child)) return true; - - // Prefer the runner's authenticated cross-platform shutdown channel so its - // Worker drains even where OS signals do not reach Node handlers (Windows). - const pidRecord = readPidFileRecord(); - let gracefulShutdownRequested = false; - if (pidRecord?.pid === runnerPid) { - const requested = await requestLocalCliHostShutdown({ - instanceId: pidRecord.instanceId, - token: pidRecord.controlToken, - expectedPid: runnerPid, - expectedMode: 'daemon', - }); - gracefulShutdownRequested = requested.ok; - } - if (!gracefulShutdownRequested) { - try { - child.kill('SIGTERM'); - } catch { - // Exit state below remains authoritative. + const graceMs = options.shutdownGraceMs ?? DAEMON_RUNNER_SHUTDOWN_GRACE_MS; + const killWaitMs = options.forceKillWaitMs ?? DAEMON_RUNNER_FORCE_KILL_WAIT_MS; + let signalGraceMs = graceMs; + if (isChildProcessRunning(runner.child)) { + // Prefer the runner's authenticated cross-platform shutdown channel so its + // Worker drains even where OS signals do not reach Node handlers (Windows). + const pidRecord = readPidFileRecord(options.pidFilePath); + if (pidRecord?.pid === runnerPid) { + const requested = await requestLocalCliHostShutdown({ + instanceId: pidRecord.instanceId, + token: pidRecord.controlToken, + expectedPid: runnerPid, + expectedMode: 'daemon', + }); + if (requested.ok) { + // The drain already had its grace period; whatever outlives it is killed. + await waitForRunnerExit(runner, graceMs); + signalGraceMs = 0; + } } } - if ( - await waitForChildProcessExit(child, options.shutdownGraceMs ?? DAEMON_RUNNER_SHUTDOWN_GRACE_MS) - ) { - return true; - } try { - child.kill('SIGKILL'); + await runner.terminate({ graceMs: signalGraceMs, killWaitMs }); + return true; } catch { - // The final wait reports whether the exact child actually exited. + // TerminationFailed: the tree could not be proven gone. + return false; } - return await waitForChildProcessExit( - child, - options.forceKillWaitMs ?? DAEMON_RUNNER_FORCE_KILL_WAIT_MS - ); } /** @@ -309,7 +300,13 @@ export async function terminateSpawnedDaemonRunner( */ export async function spawnDaemonRunnerAndAwaitReady( passthroughArgs: string[], - options: { timeoutMs?: number; installation?: { bin: string; version: string } } = {} + options: { + timeoutMs?: number; + installation?: { bin: string; version: string }; + /** Test seams: the OS process table and the daemon PID record location. */ + nodeProcess?: NodeProcessApi; + pidFilePath?: string; + } = {} ): Promise { const bin = options.installation?.bin ?? resolveLodyBin(); const env: NodeJS.ProcessEnv = { @@ -324,23 +321,30 @@ export async function spawnDaemonRunnerAndAwaitReady( delete env[LODY_SUPERVISOR_TOKEN_ENV]; delete env[LODY_SUPERVISOR_CONTRACT_ENV]; - const child = spawn( - process.execPath, - [ - `--max-old-space-size=${DAEMON_WATCHDOG_MAX_OLD_SPACE_MB}`, - bin, - 'daemon-runner', - ...passthroughArgs, - ], + // Detached everywhere: its own process group on POSIX, its own console on + // Windows, so the runner outlives the terminal that started it. + const runner = startProcessLegacy( { - detached: true, - // On Windows a detached child would otherwise get its own visible - // console window. - windowsHide: true, - stdio: ['ignore', 'ignore', 'ignore', 'pipe'], - env, - } + command: process.execPath, + args: [ + `--max-old-space-size=${DAEMON_WATCHDOG_MAX_OLD_SPACE_MB}`, + bin, + 'daemon-runner', + ...passthroughArgs, + ], + options: { + // On Windows a detached child would otherwise get its own visible + // console window. + windowsHide: true, + stdio: ['ignore', 'ignore', 'ignore', 'pipe'], + env, + }, + processGroup: true, + windowsDetached: true, + }, + toShared({ nodeProcess: options.nodeProcess }) ); + const { child } = runner; const runnerPid = child.pid; if (runnerPid === undefined) { child.unref(); @@ -403,7 +407,10 @@ export async function spawnDaemonRunnerAndAwaitReady( readyPipe?.removeAllListeners(); readyPipe?.destroy(); let result = waitResult.outcome; - if (waitResult.cancelRunner && !(await terminateSpawnedDaemonRunner(child, runnerPid))) { + if ( + waitResult.cancelRunner && + !(await terminateSpawnedDaemonRunner(runner, runnerPid, { pidFilePath: options.pidFilePath })) + ) { result = { status: 'error', runnerPid, diff --git a/apps/cli/src/ios-simulator/AGENTS.md b/apps/cli/src/ios-simulator/AGENTS.md index 5758bbe382..b3a0ef7e08 100644 --- a/apps/cli/src/ios-simulator/AGENTS.md +++ b/apps/cli/src/ios-simulator/AGENTS.md @@ -21,10 +21,9 @@ commands, join teardown. Never restart SpringBoard. Prewarm failure preserves video. - `device-controls.ts` maps the shared control union to fixed loopback routes. Text uses `host-controls.ts` to write the device clipboard, then sends acknowledged Cmd-V. - The IPC worker directly owns fixed simctl/devicectl commands for text, appearance, + The IPC worker owns fixed simctl/devicectl commands for text, appearance, shake and deep links; abort joins child close before release. Do not delegate these to - Baguette's Foundation.Process paths: those children create separate process groups - and can outlive the native server. Control bodies/errors never enter logs or RPC. + Baguette's subprocess paths: they can outlive the native server. Control bodies/errors never enter logs or RPC. iPhone/iPad preparation disables device-local `AutomaticMinimizationEnabled` and notifies keyboard preferences so the guest software keyboard remains available. Never rewrite host-global Simulator preferences or reboot to change keyboards. @@ -74,6 +73,7 @@ release; the gateway validates start band and rejects mid-gesture edge changes. Authenticated init chooses mobile upright or desktop-following display rotation; layout/input/capture share that angle, independent of native rotation. +- Use `@lody/shared/node/process`; keep EOF release and join bounded tree cleanup. - `baguette-worker.ts` owns the native process through an IPC lease. Owner loss must reap it and join pending host controls; never terminate the worker as normal cleanup. All build compositions emit the same sibling worker entry. No user simulator is shut down during cleanup. @@ -88,7 +88,7 @@ key. Never put a viewer URL in workspace-readable Streams. Revocation fences proof verification as well as startup; owner/machine reassignment closes existing viewers. Browser and Simulator have independent service/proxy owners and share only transport - primitives. There is no simulator sharing route or anonymous viewer grant. + primitives. No simulator sharing or anonymous viewer grant. - One operation owns one native process and gateway, with lazy local and remote endpoints. Return only the requesting plane's URL/status, including while connecting. The remote gateway path selects remote flow budgets without reinitializing controls. diff --git a/apps/cli/src/ios-simulator/README.md b/apps/cli/src/ios-simulator/README.md index 99f8fb8534..8bb0262e83 100644 --- a/apps/cli/src/ios-simulator/README.md +++ b/apps/cli/src/ios-simulator/README.md @@ -14,7 +14,9 @@ failure does not interrupt local viewing; explicit start retries that endpoint. Stop ends both endpoints. Remote revocation conservatively ends any operation with a remote attachment, including its local endpoint. Device shutdown is never implied. -The pinned Baguette executable runs in an IPC-owned worker. The native HTTP API +The pinned Baguette executable runs in an IPC-owned worker. Worker, native server, +simctl and guest-helper processes start through `@lody/shared/node/process`; +cancellation joins the shared whole-tree termination policy. The native HTTP API stays on loopback; only the bound device's MJPEG stream and validated one/two-finger input cross `gateway.ts`, along with typed device controls on the private preview connection. `device-controls.ts` maps these controls to fixed native endpoints; diff --git a/apps/cli/src/ios-simulator/baguette-process.test.ts b/apps/cli/src/ios-simulator/baguette-process.test.ts index 34cd122ba1..9631bfef56 100644 --- a/apps/cli/src/ios-simulator/baguette-process.test.ts +++ b/apps/cli/src/ios-simulator/baguette-process.test.ts @@ -18,6 +18,9 @@ it('joins native cleanup when its IPC lifecycle lease ends', async () => { bundle: true, platform: 'node', format: 'esm', + banner: { + js: "import { createRequire as __lodyCreateRequire } from 'node:module'; const require = __lodyCreateRequire(import.meta.url);", + }, outfile: worker, logLevel: 'silent', }); @@ -51,6 +54,9 @@ it('joins descendants even when the native parent exits first', async () => { bundle: true, platform: 'node', format: 'esm', + banner: { + js: "import { createRequire as __lodyCreateRequire } from 'node:module'; const require = __lodyCreateRequire(import.meta.url);", + }, outfile: worker, logLevel: 'silent', }); @@ -116,6 +122,9 @@ it('joins an active host command before resolving worker closure on owner discon bundle: true, platform: 'node', format: 'esm', + banner: { + js: "import { createRequire as __lodyCreateRequire } from 'node:module'; const require = __lodyCreateRequire(import.meta.url);", + }, outfile: worker, logLevel: 'silent', plugins: [ diff --git a/apps/cli/src/ios-simulator/baguette-process.ts b/apps/cli/src/ios-simulator/baguette-process.ts index f01f44ab31..450eb0f2c3 100644 --- a/apps/cli/src/ios-simulator/baguette-process.ts +++ b/apps/cli/src/ios-simulator/baguette-process.ts @@ -1,4 +1,4 @@ -import { spawn } from 'node:child_process'; +import { startProcessLegacy } from '@lody/shared/node/process'; import { fileURLToPath } from 'node:url'; import { z } from 'zod'; import type { SimulatorHostControl } from './host-controls'; @@ -18,9 +18,11 @@ export async function startBaguetteProcess( const env: NodeJS.ProcessEnv = {}; for (const key of ['HOME', 'PATH', 'TMPDIR', 'DEVELOPER_DIR', 'ELECTRON_RUN_AS_NODE']) if (process.env[key]) env[key] = process.env[key]; - const worker = spawn(process.execPath, [workerPath, binary], { - env, - stdio: ['ignore', 'ignore', 'ignore', 'ipc'], + const { child: worker } = startProcessLegacy({ + command: process.execPath, + args: [workerPath, binary], + processGroup: false, + options: { env, stdio: ['ignore', 'ignore', 'ignore', 'ipc'] }, }); const stop = () => { if (worker.connected) worker.disconnect(); diff --git a/apps/cli/src/ios-simulator/baguette-worker.ts b/apps/cli/src/ios-simulator/baguette-worker.ts index e4ad9cd036..1e1ac7730a 100644 --- a/apps/cli/src/ios-simulator/baguette-worker.ts +++ b/apps/cli/src/ios-simulator/baguette-worker.ts @@ -1,4 +1,4 @@ -import { spawn } from 'node:child_process'; +import { startProcessLegacy, type ProcessHandleLegacy } from '@lody/shared/node/process'; import { createServer } from 'node:net'; import { setTimeout as delay } from 'node:timers/promises'; import { z } from 'zod'; @@ -68,7 +68,7 @@ const onMessage = (raw: unknown) => { process.on('message', onMessage); process.once('SIGTERM', stop); process.once('SIGINT', stop); -let child: ReturnType | undefined; +let owned: ProcessHandleLegacy | undefined; let exited: Promise | undefined; try { if (!process.connected) throw new Error('Missing lifecycle owner'); @@ -85,12 +85,13 @@ try { reservation.close((error) => (error ? reject(error) : resolve())) ); abort.signal.throwIfAborted(); - child = spawn(binary, ['serve', '--host', '127.0.0.1', '--port', String(port), '--no-plugins'], { - // Isolate the native server and its xcrun descendants in our own process group. - detached: true, - stdio: 'ignore', - env: process.env, + owned = startProcessLegacy({ + command: binary, + args: ['serve', '--host', '127.0.0.1', '--port', String(port), '--no-plugins'], + processGroup: true, + options: { stdio: 'ignore', env: process.env }, }); + const child = owned.child; exited = new Promise((resolve) => { child?.once('error', () => { stop(); @@ -128,26 +129,9 @@ try { await buttons.close().catch(() => { process.exitCode = 1; }); - if (child?.pid !== undefined) { - const group = -child.pid; - const signalGroup = (signal: NodeJS.Signals | 0): boolean => { - try { - process.kill(group, signal); - return true; - } catch (error) { - if (error instanceof Error && 'code' in error && error.code === 'ESRCH') return false; - throw error; - } - }; - // Parent exit alone is insufficient: an in-flight simctl can outlive Baguette. - // Keep IPC ownership until the whole group is gone, including forced cleanup. - if (signalGroup('SIGTERM')) { - const killAt = Date.now() + 3000; - while (signalGroup(0)) { - if (Date.now() >= killAt) signalGroup('SIGKILL'); - await delay(25); - } - } + if (owned) { + // Keep IPC ownership through whole-tree cleanup, even if the leader exited. + await owned.terminate({ graceMs: 3000, killWaitMs: 2000 }); await exited; } process.removeListener('disconnect', stop); diff --git a/apps/cli/src/ios-simulator/devices.ts b/apps/cli/src/ios-simulator/devices.ts index 2576162692..51d19aaf7a 100644 --- a/apps/cli/src/ios-simulator/devices.ts +++ b/apps/cli/src/ios-simulator/devices.ts @@ -1,8 +1,7 @@ -import { execFile } from 'node:child_process'; -import { promisify } from 'node:util'; +import { Effect } from 'effect'; +import { processLayer, runCommandOk } from '@lody/shared/node/process'; import { z } from 'zod'; import type { IosSimulatorDevice } from '@lody/shared'; -const exec = promisify(execFile); const Devices = z.object({ devices: z.record( z.string(), @@ -36,18 +35,32 @@ export function parseSimulatorDevices(value: unknown): IosSimulatorDevice[] { } export async function listSimulatorDevices(signal?: AbortSignal): Promise { if (process.platform !== 'darwin') throw new Error('iOS Simulator requires macOS.'); - const { stdout } = await exec('/usr/bin/xcrun', ['simctl', 'list', 'devices', '--json'], { - signal, - timeout: 15000, - maxBuffer: 4 * 1024 * 1024, - }); - return parseSimulatorDevices(JSON.parse(stdout)); + const { stdout } = await Effect.runPromise( + Effect.provide( + runCommandOk({ + command: '/usr/bin/xcrun', + args: ['simctl', 'list', 'devices', '--json'], + timeout: 15000, + maxOutputBytes: 4 * 1024 * 1024, + }), + processLayer({}) + ), + { signal } + ); + return parseSimulatorDevices(JSON.parse(stdout.toString('utf8'))); } export async function bootSimulator(udid: string, signal: AbortSignal): Promise { // bootstatus -b starts a stopped device and joins an already-running boot. - await exec('/usr/bin/xcrun', ['simctl', 'bootstatus', z.string().uuid().parse(udid), '-b'], { - signal, - timeout: 180000, - maxBuffer: 2 * 1024 * 1024, - }); + await Effect.runPromise( + Effect.provide( + runCommandOk({ + command: '/usr/bin/xcrun', + args: ['simctl', 'bootstatus', z.string().uuid().parse(udid), '-b'], + timeout: 180000, + maxOutputBytes: 2 * 1024 * 1024, + }), + processLayer({}) + ), + { signal } + ); } diff --git a/apps/cli/src/ios-simulator/exterior.ts b/apps/cli/src/ios-simulator/exterior.ts index 57c11483db..10fbef23fb 100644 --- a/apps/cli/src/ios-simulator/exterior.ts +++ b/apps/cli/src/ios-simulator/exterior.ts @@ -1,5 +1,5 @@ -import { execFile } from 'node:child_process'; -import { promisify } from 'node:util'; +import { Effect } from 'effect'; +import { processLayer, runCommandOk } from '@lody/shared/node/process'; import { z } from 'zod'; import { IosSimulatorExteriorSchema, @@ -107,21 +107,18 @@ export async function readSimulatorExterior(options: { /** Read immutable DeviceKit artwork without booting, streaming, leasing or opening a server. */ export async function readIdleSimulatorExterior(binary: string, udid: string, signal: AbortSignal) { - const exec = promisify(execFile); const target = z.string().uuid().parse(udid); - const { stdout: layout } = await exec(binary, ['chrome', 'layout', '--udid', target], { - signal, - timeout: 10000, - maxBuffer: 65536, - encoding: 'utf8', - }); - const { stdout: png } = await exec(binary, ['chrome', 'composite', '--udid', target], { - signal, - timeout: 10000, - maxBuffer: 256 * 1024, - encoding: 'buffer', - }); - return parseIdleSimulatorExterior(layout, png); + const run = (args: string[], maxOutputBytes: number) => + Effect.runPromise( + Effect.provide( + runCommandOk({ command: binary, args, timeout: 10000, maxOutputBytes }), + processLayer({}) + ), + { signal } + ); + const { stdout: layout } = await run(['chrome', 'layout', '--udid', target], 65536); + const { stdout: png } = await run(['chrome', 'composite', '--udid', target], 256 * 1024); + return parseIdleSimulatorExterior(layout.toString('utf8'), png); } export function parseIdleSimulatorExterior(layout: string, png: Buffer) { diff --git a/apps/cli/src/ios-simulator/guest-buttons.ts b/apps/cli/src/ios-simulator/guest-buttons.ts index 9bb672be66..0a46b205d0 100644 --- a/apps/cli/src/ios-simulator/guest-buttons.ts +++ b/apps/cli/src/ios-simulator/guest-buttons.ts @@ -1,9 +1,10 @@ -import { spawn, type ChildProcessByStdio } from 'node:child_process'; +import type { ChildProcessByStdio } from 'node:child_process'; +import { Effect } from 'effect'; +import { startProcessLegacy, terminateChildTreeLegacy } from '@lody/shared/node/process'; import type { Readable, Writable } from 'node:stream'; import { mkdtemp, writeFile, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { setTimeout as delay } from 'node:timers/promises'; import { guestButtonsSource } from './guest-buttons-source'; type Button = 'home' | 'app-switcher' | 'lock'; @@ -32,31 +33,15 @@ export function createGuestButtons(signal: AbortSignal, executable = '/usr/bin/x const closed = childClosed; terminating = (async () => { owned.stdin.end(); - const send = (terminationSignal: NodeJS.Signals | 0): boolean => { - if (owned.pid) { - try { - process.kill(-owned.pid, terminationSignal); - return true; - } catch (e) { - if (!(e instanceof Error && 'code' in e && e.code === 'ESRCH')) throw e; - } - } - return false; - }; - // EOF lets the guest release an in-progress key and cancel its service. - const term = setTimeout(() => send('SIGTERM'), 1000); - const kill = setTimeout(() => send('SIGKILL'), 2000); + // EOF gives the guest one second to release a key and cancel its service. + if (closed) + await Effect.runPromise(Effect.raceAll([Effect.promise(() => closed), Effect.sleep(1000)])); + await terminateChildTreeLegacy(owned, { + processGroup: true, + graceMs: 1000, + killWaitMs: 2000, + }); await closed; - clearTimeout(term); - clearTimeout(kill); - // Parent close is not a join of compiler/simctl descendants with separate stdio. - if (send('SIGTERM')) { - const killAt = Date.now() + 1000; - while (send(0)) { - if (Date.now() >= killAt) send('SIGKILL'); - await delay(25); - } - } if (child === owned) child = undefined; })(); return terminating; @@ -64,7 +49,13 @@ export function createGuestButtons(signal: AbortSignal, executable = '/usr/bin/x function launch(args: string[], protocol: boolean): Promise { signal.throwIfAborted(); if (failed) throw error(); - const process = spawn(executable, args, { detached: true, stdio: ['pipe', 'pipe', 'ignore'] }); + const { child: started } = startProcessLegacy({ + command: executable, + args, + processGroup: true, + options: { stdio: ['pipe', 'pipe', 'ignore'] }, + }); + const process = started as ChildProcessByStdio; child = process; terminating = undefined; childClosed = new Promise((resolve) => process.once('close', resolve)); diff --git a/apps/cli/src/ios-simulator/host-controls.ts b/apps/cli/src/ios-simulator/host-controls.ts index 81b85d34f4..1d393b7fc0 100644 --- a/apps/cli/src/ios-simulator/host-controls.ts +++ b/apps/cli/src/ios-simulator/host-controls.ts @@ -1,4 +1,5 @@ -import { spawn } from 'node:child_process'; +import { Effect } from 'effect'; +import { processLayer, runCommand } from '@lody/shared/node/process'; import type { IosSimulatorDeviceControl } from '@lody/shared'; export type SimulatorHostControl = @@ -16,34 +17,26 @@ export async function runSimulatorHostControl( ): Promise { async function run(args: string[], input?: string): Promise { signal.throwIfAborted(); - return new Promise((resolve, reject) => { - const child = spawn(executable, args, { - stdio: ['pipe', 'ignore', 'ignore'], - // The IPC worker has a filtered environment. simctl decodes stdin with - // the locale, so an unset locale rejects non-ASCII pasteboard contents. - env: { ...process.env, LANG: 'en_US.UTF-8', LC_ALL: 'en_US.UTF-8' }, - }); - let kill: ReturnType | undefined; - const cancel = () => { - child.kill('SIGTERM'); - kill ??= setTimeout(() => child.kill('SIGKILL'), 1000); - }; - signal.addEventListener('abort', cancel, { once: true }); - if (signal.aborted) cancel(); - // EPIPE is reported by the child's exit status, never with the input contents. - child.stdin.on('error', () => {}); - child.stdin.end(input); - let failed = false; - child.once('error', () => { - failed = true; - }); - child.once('close', (code) => { - clearTimeout(kill); - signal.removeEventListener('abort', cancel); - if (signal.aborted || failed) reject(new Error('Simulator control failed.')); - else resolve(code); - }); - }); + try { + const result = await Effect.runPromise( + Effect.provide( + runCommand({ + command: executable, + args, + input: input ?? '', + // simctl decodes stdin with the locale; keep Unicode clipboard input. + env: { ...process.env, LANG: 'en_US.UTF-8', LC_ALL: 'en_US.UTF-8' }, + abandonPolicy: { graceMs: 1000, killWaitMs: 2000 }, + }), + processLayer({}) + ), + { signal } + ); + return result.code; + } catch { + // Keep command input and subprocess diagnostics out of RPC and logs. + throw new Error('Simulator control failed.'); + } } let code: number | null; switch (control.kind) { diff --git a/apps/cli/src/lib/README.md b/apps/cli/src/lib/README.md index bc1fca97a0..2acb300ac5 100644 --- a/apps/cli/src/lib/README.md +++ b/apps/cli/src/lib/README.md @@ -31,7 +31,7 @@ subdirectory; this file is the navigation index. Cross-module explanations live WS control-plane listener, and serializes remote bridge attach/detach/revoke through `runBridgeTransition`. - `machine-lifecycle.ts` — remote lifecycle verification and upgrade intents; runs the - fixed npm install through `cross-spawn` so Windows `npm.cmd` shims use the command + fixed npm install through the shared process layer’s `startProcessLegacy` so Windows `npm.cmd` shims use the command interpreter. It resolves the installed package through that npm's global root and verifies its entry/version. `../commands/daemon-runner.ts` owns restart and handoff to that explicit entry, including matching-version readiness. diff --git a/apps/cli/src/lib/code-collab/code-collab-v2-service.ts b/apps/cli/src/lib/code-collab/code-collab-v2-service.ts index 47965dbf28..4788ea2bad 100644 --- a/apps/cli/src/lib/code-collab/code-collab-v2-service.ts +++ b/apps/cli/src/lib/code-collab/code-collab-v2-service.ts @@ -1,5 +1,5 @@ +import { toShared } from '@/platform/process-options'; import { createHash, randomUUID } from 'node:crypto'; -import { execFile } from 'node:child_process'; import { chmod, lstat, @@ -55,6 +55,9 @@ import { } from '@lody/shared'; import { formatErrorMessage } from '@/utils/format-error'; import { getLogger } from '@/utils/logger'; +import { runCommandOk } from '@lody/shared/node/process'; +import { makeProcessRunnerLegacy, runCommandTextLegacy } from '@lody/shared/node/process'; + import { mapWithConcurrency } from '@/lib/bounded-concurrency'; import { CodeCollabFileIndexChangedPublishError } from './code-collab-flock-publish'; import type { CodeCollabV2DiffStore } from './code-collab-v2-diff-store'; @@ -76,7 +79,6 @@ import type { WorkspaceWatchSubscription, } from './workspace-watch-coordinator'; -const execFileAsync = promisify(execFile); // gzip/gunzip on libuv's threadpool instead of the sync variants: a large file's // (de)compression must not block the single Node event loop, which would stall every // other concurrent Machine RPC handler (open-file/refresh/diff) and the request loop. @@ -2984,25 +2986,32 @@ async function runGit( args: readonly string[] ): Promise<{ ok: true; stdout: string } | { ok: false }> { try { - const { stdout } = await execFileAsync('git', ['-C', cwd, ...args], { - maxBuffer: 64 * 1024 * 1024, - }); + const { stdout } = await runCommandTextLegacy( + { + command: 'git', + args: ['-C', cwd, ...args], + maxOutputBytes: 64 * 1024 * 1024, + check: 'exit-0', + }, + toShared() + ); return { ok: true, stdout }; } catch { return { ok: false }; } } +const runPlatformCommand = makeProcessRunnerLegacy(toShared({})); + async function runGitBuffer( cwd: string, args: readonly string[], maxBuffer: number ): Promise<{ ok: true; stdout: Buffer } | { ok: false }> { try { - const { stdout } = (await execFileAsync('git', ['-C', cwd, ...args], { - encoding: 'buffer', - maxBuffer, - })) as { stdout: Buffer }; + const { stdout } = await runPlatformCommand( + runCommandOk({ command: 'git', args: ['-C', cwd, ...args], maxOutputBytes: maxBuffer }) + ); return { ok: true, stdout }; } catch { return { ok: false }; diff --git a/apps/cli/src/lib/code-collab/diff-line-counts.test.ts b/apps/cli/src/lib/code-collab/diff-line-counts.test.ts index 3b3502de8c..679e7d7b67 100644 --- a/apps/cli/src/lib/code-collab/diff-line-counts.test.ts +++ b/apps/cli/src/lib/code-collab/diff-line-counts.test.ts @@ -68,6 +68,18 @@ describe('computeTextLineCounts', () => { expect(computeTextLineCounts(oldText, newText)).toEqual(gitNumstat(oldText, newText)); } }); + + it('asks git for exact counts once jsdiff bails on scattered edits', () => { + // Every other line changes: far past jsdiff's edit cap, and the prefix/suffix + // estimate would call nearly the whole file changed. + const oldText = Array.from({ length: 2_000 }, (_, i) => `line ${i}\n`).join(''); + const newText = Array.from({ length: 2_000 }, (_, i) => + i % 2 === 0 ? `line ${i}\n` : `edited ${i}\n` + ).join(''); + + expect(computeTextLineCounts(oldText, newText)).toEqual([1_000, 1_000]); + expect(prefixSuffixLineCounts(oldText, newText)).not.toEqual([1_000, 1_000]); + }); }); describe('computeLineCounts (nullable sides)', () => { diff --git a/apps/cli/src/lib/code-collab/diff-line-counts.ts b/apps/cli/src/lib/code-collab/diff-line-counts.ts index 990802082b..5aaaf46166 100644 --- a/apps/cli/src/lib/code-collab/diff-line-counts.ts +++ b/apps/cli/src/lib/code-collab/diff-line-counts.ts @@ -1,10 +1,12 @@ -import { spawnSync } from 'node:child_process'; import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { diffLines } from 'diff'; +// The shared facade directly: the CLI one loads the daemon's winston logger. +import { runCommandTextSyncLegacy } from '@lody/shared/node/process'; + /** * Added/deleted line counts for a single file change, as `[add, del]`. * @@ -21,8 +23,8 @@ import { diffLines } from 'diff'; * git numbers (when git is available); * 3. otherwise the cheap prefix/suffix estimate as a last resort. * - * This module is intentionally dependency-light (only `diff` + node builtins) so - * it can be bundled into the standalone diff worker entry. + * This module is intentionally dependency-light (`diff`, node builtins and the + * shared process facade) so it can be bundled into the standalone diff worker entry. */ /** @@ -43,6 +45,13 @@ const MAX_LINES_FOR_JSDIFF = 200_000; /** `git diff --no-index` capture cap; turn snapshots are already <= ~1MiB. */ const GIT_NUMSTAT_MAX_BUFFER = 64 * 1024 * 1024; +/** + * Both git calls block the worker thread, so each is bounded. A timed-out numstat + * falls through to the prefix/suffix estimate like any other git failure. + */ +const GIT_VERSION_TIMEOUT_MS = 10_000; +const GIT_NUMSTAT_TIMEOUT_MS = 30_000; + export function countTextLines(text: string): number { if (text.length === 0) return 0; return text.endsWith('\n') ? text.split('\n').length - 1 : text.split('\n').length; @@ -112,7 +121,12 @@ function isGitAvailable(): boolean { if (gitAvailable === undefined) { try { gitAvailable = - spawnSync('git', ['--version'], { stdio: 'ignore', windowsHide: true }).status === 0; + runCommandTextSyncLegacy({ + command: 'git', + args: ['--version'], + timeout: GIT_VERSION_TIMEOUT_MS, + check: 'none', + }).code === 0; } catch { gitAvailable = false; } @@ -137,19 +151,21 @@ function gitNumstatLineCounts(oldText: string, newText: string): [number, number const newPath = path.join(dir, 'new'); writeFileSync(oldPath, oldText); writeFileSync(newPath, newText); - const result = spawnSync('git', ['diff', '--no-index', '--numstat', '--', oldPath, newPath], { - encoding: 'utf8', - maxBuffer: GIT_NUMSTAT_MAX_BUFFER, - windowsHide: true, + const result = runCommandTextSyncLegacy({ + command: 'git', + args: ['diff', '--no-index', '--numstat', '--', oldPath, newPath], + maxOutputBytes: GIT_NUMSTAT_MAX_BUFFER, + timeout: GIT_NUMSTAT_TIMEOUT_MS, + check: 'none', }); // Exit 0 = identical, 1 = differences (expected), >1 = real error. - if (result.status !== 0 && result.status !== 1) { + if (result.code !== 0 && result.code !== 1) { return null; } - const match = NUMSTAT_LINE.exec(result.stdout ?? ''); + const match = NUMSTAT_LINE.exec(result.stdout); if (!match) { // No diff line: identical content. - return result.status === 0 ? [0, 0] : null; + return result.code === 0 ? [0, 0] : null; } if (match[1] === '-' || match[2] === '-') { // Binary diff: git reports no line counts. diff --git a/apps/cli/src/lib/code-collab/file-index-scan-core.ts b/apps/cli/src/lib/code-collab/file-index-scan-core.ts index d2672a0e9e..2d5e0394db 100644 --- a/apps/cli/src/lib/code-collab/file-index-scan-core.ts +++ b/apps/cli/src/lib/code-collab/file-index-scan-core.ts @@ -1,7 +1,5 @@ -import { execFile } from 'node:child_process'; import { readFile } from 'node:fs/promises'; import path from 'node:path'; -import { promisify } from 'node:util'; import type { CodeCollabV2AllChangesState, @@ -9,18 +7,19 @@ import type { CodeCollabV2FileTreeValue, } from '@lody/shared'; +// The shared facade directly: the CLI one loads the daemon's winston logger. +import { runCommandTextLegacy } from '@lody/shared/node/process'; + import { countTextLines } from './diff-line-counts'; import { gitDiffBaseRefCandidates } from '../git/git-diff-base'; // Pure Git-backed scanning + All Changes computation shared by the file-index // Tinypool worker (`file-index-scan-worker.ts`) and the main-thread fallback in -// `code-collab-v2-service.ts`. Keep this module dependency-light (node builtins + -// `@lody/shared` types only) so the worker bundle stays free of wasm/top-level-await -// imports. The filesystem (`opendir`) directory-scan fallback is intentionally NOT +// `code-collab-v2-service.ts`. Keep this module dependency-light (node builtins, +// `@lody/shared` types and the shared process facade only) so the worker bundle stays +// free of wasm/top-level-await imports. The filesystem (`opendir`) directory-scan fallback is intentionally NOT // here: its error classification differs between the worker and the service. -const execFileAsync = promisify(execFile); - const GIT_MAX_BUFFER_BYTES = 64 * 1024 * 1024; export type GitDirectoryScanInput = { @@ -58,9 +57,9 @@ async function runGitLsFiles( ): Promise<{ readonly ok: true; readonly paths: readonly string[] } | { readonly ok: false }> { try { const [{ stdout }, deleted] = await Promise.all([ - execFileAsync( - 'git', - [ + runCommandTextLegacy({ + command: 'git', + args: [ '-C', cwd, 'ls-files', @@ -72,8 +71,9 @@ async function runGitLsFiles( '--', '.', ], - { maxBuffer: GIT_MAX_BUFFER_BYTES } - ), + maxOutputBytes: GIT_MAX_BUFFER_BYTES, + check: 'exit-0', + }), runGit(cwd, ['ls-files', '--deleted', '-z', '--', '.']), ]); const deletedPaths = deleted.ok @@ -246,8 +246,11 @@ async function runGit( args: readonly string[] ): Promise<{ readonly ok: true; readonly stdout: string } | { readonly ok: false }> { try { - const { stdout } = await execFileAsync('git', ['-C', cwd, ...args], { - maxBuffer: GIT_MAX_BUFFER_BYTES, + const { stdout } = await runCommandTextLegacy({ + command: 'git', + args: ['-C', cwd, ...args], + maxOutputBytes: GIT_MAX_BUFFER_BYTES, + check: 'exit-0', }); return { ok: true, stdout }; } catch { diff --git a/apps/cli/src/lib/code-collab/workspace-watch-coordinator.test.ts b/apps/cli/src/lib/code-collab/workspace-watch-coordinator.test.ts index 55426fedad..354502ae1e 100644 --- a/apps/cli/src/lib/code-collab/workspace-watch-coordinator.test.ts +++ b/apps/cli/src/lib/code-collab/workspace-watch-coordinator.test.ts @@ -1,6 +1,10 @@ +import { toShared } from '@/platform/process-options'; import { EventEmitter } from 'node:events'; import { describe, expect, it, vi } from 'vitest'; import { getLogger } from '@/utils/logger'; +import { startProcessLegacy } from '@lody/shared/node/process'; + +import { FakeProcessTable } from '@lody/shared/node/process-testing'; import { WorkspaceWatchCoordinator, buildWorkspaceWatchWorkerEnvironment, @@ -24,9 +28,6 @@ class FakeChild extends EventEmitter { } return true; } - kill(): boolean { - return true; - } } describe('WorkspaceWatchCoordinator', () => { @@ -39,7 +40,7 @@ describe('WorkspaceWatchCoordinator', () => { childLauncher: () => { const child = new FakeChild(); children.push(child); - return child as never; + return { child: child as never, terminate: async () => {} }; }, restartDelayMs: () => 1_000, }); @@ -93,6 +94,36 @@ describe('WorkspaceWatchCoordinator', () => { await coordinator.dispose(); }); + it('ends a worker that ignores the shutdown request and SIGTERM', async () => { + const table = new FakeProcessTable(); + table.queueSpawn({ ignores: ['SIGTERM'] }); + const coordinator = new WorkspaceWatchCoordinator(getLogger('workspace-watch-test'), { + realpath: async () => '/canonical/workspace', + childLauncher: () => + startProcessLegacy( + { command: 'watch-worker', args: [], options: {}, processGroup: false }, + toShared({ nodeProcess: table.api }) + ), + gracefulShutdownMs: 0, + sigtermWaitMs: 10, + }); + await coordinator.subscribe({ + workspaceId: 'workspace-a', + ownerSessionId: 'owner-a', + workspaceRoot: '/workspace', + onDirty: () => {}, + }); + const workerPid = coordinator.getSnapshot().pid ?? -1; + + await coordinator.dispose(); + + expect(table.isAlive(workerPid)).toBe(false); + expect(table.delivered).toEqual([ + { target: workerPid, signal: 'SIGTERM' }, + { target: workerPid, signal: 'SIGKILL' }, + ]); + }); + it('does not forward representative credentials or NODE_OPTIONS', () => { expect( buildWorkspaceWatchWorkerEnvironment({ diff --git a/apps/cli/src/lib/code-collab/workspace-watch-coordinator.ts b/apps/cli/src/lib/code-collab/workspace-watch-coordinator.ts index 87d3a71f28..5fd5327fd4 100644 --- a/apps/cli/src/lib/code-collab/workspace-watch-coordinator.ts +++ b/apps/cli/src/lib/code-collab/workspace-watch-coordinator.ts @@ -1,9 +1,13 @@ -import { spawn, type ChildProcess } from 'node:child_process'; +import { toShared } from '@/platform/process-options'; +import type { ChildProcess } from 'node:child_process'; import { existsSync } from 'node:fs'; import { realpath } from 'node:fs/promises'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; import type { Logger } from '@/utils/logger'; +import { formatErrorMessage } from '@/utils/format-error'; +import { startProcessLegacy, type ProcessHandleLegacy } from '@lody/shared/node/process'; + import { parseWorkspaceWatchChildMessage, type WorkspaceWatchParentMessage, @@ -53,17 +57,24 @@ type RootState = { recycledForResourceError: boolean; }; -type ChildLike = Pick; +type ChildLike = Pick; + +/** A launched watch worker: its IPC channel plus whole-tree termination. */ +export type WorkspaceWatchChild = { + readonly child: ChildLike; + readonly terminate: ProcessHandleLegacy['terminate']; +}; export type WorkspaceWatchCoordinatorOptions = { mode?: 'broker' | 'off'; - childLauncher?: (generation: number) => ChildLike; + childLauncher?: (generation: number) => WorkspaceWatchChild; realpath?: (value: string) => Promise; restartDelayMs?: (attempt: number) => number; noRootLingerMs?: number; cooldownMs?: number; gracefulShutdownMs?: number; sigtermWaitMs?: number; + sigkillWaitMs?: number; maxReconfigurations?: number; maxChildAgeMs?: number; maxRssBytes?: number; @@ -103,7 +114,7 @@ export class WorkspaceWatchCoordinator implements WorkspaceWatchCoordinatorApi { { root: string; onDirty: (reason: WorkspaceWatchDirtyReason) => void } >(); private readonly roots = new Map(); - private child: ChildLike | null = null; + private worker: WorkspaceWatchChild | null = null; private generation = 0; private revision = 0; private restartCount = 0; @@ -176,7 +187,7 @@ export class WorkspaceWatchCoordinator implements WorkspaceWatchCoordinatorApi { getSnapshot(): WorkspaceWatchCoordinatorSnapshot { return { generation: this.generation, - pid: this.child?.pid ?? null, + pid: this.worker?.child.pid ?? null, phase: this.phase, restartReason: this.restartReason, restartCount: this.restartCount, @@ -229,7 +240,7 @@ export class WorkspaceWatchCoordinator implements WorkspaceWatchCoordinatorApi { private reconcile(reason: string): void { if (this.disposed || this.phase === 'off') return; if (this.roots.size === 0) { - if (!this.child || this.lingerTimer) return; + if (!this.worker || this.lingerTimer) return; this.sendDesiredRoots(); this.lingerTimer = setTimeout(() => { this.lingerTimer = null; @@ -238,7 +249,7 @@ export class WorkspaceWatchCoordinator implements WorkspaceWatchCoordinatorApi { this.lingerTimer.unref?.(); return; } - if (!this.child) { + if (!this.worker) { this.startChild(reason); return; } @@ -246,20 +257,21 @@ export class WorkspaceWatchCoordinator implements WorkspaceWatchCoordinatorApi { } private startChild(reason: string): void { - if (this.disposed || this.child || this.restartTimer || this.roots.size === 0) return; + if (this.disposed || this.worker || this.restartTimer || this.roots.size === 0) return; this.generation += 1; this.phase = 'starting'; try { - const child = (this.options.childLauncher ?? launchWorkspaceWatchChild)(this.generation); - this.child = child; + const worker = (this.options.childLauncher ?? launchWorkspaceWatchChild)(this.generation); + this.worker = worker; + const { child } = worker; const generation = this.generation; - child.on('message', (raw: unknown) => this.handleMessage(child, generation, raw)); + child.on('message', (raw: unknown) => this.handleMessage(worker, generation, raw)); child.on('error', (error: Error) => { this.logger.warn(`[code-collab-watch] child error code=${errorCode(error)}`); }); child.on('exit', (code: number | null, signal: NodeJS.Signals | null) => { - if (this.child !== child || generation !== this.generation) return; - this.child = null; + if (this.worker !== worker || generation !== this.generation) return; + this.worker = null; this.actualWatcherCount = 0; if (this.disposed) return; this.markAllDirty('broker-gap'); @@ -271,7 +283,7 @@ export class WorkspaceWatchCoordinator implements WorkspaceWatchCoordinatorApi { this.sendDesiredRoots(); } catch (error) { this.logger.warn(`[code-collab-watch] spawn failed code=${errorCode(error)}`); - this.child = null; + this.worker = null; if (errorCode(error) === 'WATCH_WORKER_MISSING') { this.phase = 'off'; return; @@ -281,7 +293,7 @@ export class WorkspaceWatchCoordinator implements WorkspaceWatchCoordinatorApi { } private sendDesiredRoots(): void { - const child = this.child; + const child = this.worker?.child; if (!child?.connected) return; const now = this.now(); const roots = Array.from(this.roots, ([root, state]) => @@ -300,8 +312,8 @@ export class WorkspaceWatchCoordinator implements WorkspaceWatchCoordinatorApi { }); } - private handleMessage(child: ChildLike, generation: number, raw: unknown): void { - if (child !== this.child || generation !== this.generation) return; + private handleMessage(worker: WorkspaceWatchChild, generation: number, raw: unknown): void { + if (worker !== this.worker || generation !== this.generation) return; const message = parseWorkspaceWatchChildMessage(raw); if (!message || message.generation !== generation) return; if (message.type === 'code-collab-watch/dirty') { @@ -360,7 +372,7 @@ export class WorkspaceWatchCoordinator implements WorkspaceWatchCoordinatorApi { } private async recycle(reason: string): Promise { - if (!this.child || this.disposed) return; + if (!this.worker || this.disposed) return; this.recycleCount += 1; this.markAllDirty('broker-gap'); await this.stopChild(); @@ -412,18 +424,25 @@ export class WorkspaceWatchCoordinator implements WorkspaceWatchCoordinatorApi { } private async stopChild(): Promise { - const child = this.child; - if (!child) return; - this.child = null; + const worker = this.worker; + if (!worker) return; + this.worker = null; + const { child } = worker; const exited = new Promise((resolve) => child.on('exit', () => resolve())); if (child.connected) { child.send?.({ type: 'code-collab-watch/shutdown', generation: this.generation }); } if (await waitFor(exited, this.options.gracefulShutdownMs ?? 1_000)) return; - child.kill('SIGTERM'); - if (await waitFor(exited, this.options.sigtermWaitMs ?? 2_000)) return; - child.kill('SIGKILL'); - await exited; + try { + await worker.terminate({ + graceMs: this.options.sigtermWaitMs ?? 2_000, + killWaitMs: this.options.sigkillWaitMs ?? 5_000, + }); + } catch (error) { + this.logger.warn( + `[code-collab-watch] worker pid=${child.pid ?? 'unknown'} did not stop: ${formatErrorMessage(error)}` + ); + } } private cancelLinger(): void { @@ -445,16 +464,24 @@ export class WorkspaceWatchCoordinator implements WorkspaceWatchCoordinatorApi { } } -function launchWorkspaceWatchChild(_generation: number): ChildLike { +function launchWorkspaceWatchChild(_generation: number): WorkspaceWatchChild { const moduleDirectory = path.dirname(fileURLToPath(import.meta.url)); const productionEntry = path.join(moduleDirectory, 'code-collab-watch-worker.js'); const env = buildWorkspaceWatchWorkerEnvironment(); if (existsSync(productionEntry)) { - return spawn(process.execPath, [productionEntry], { - env, - stdio: ['ignore', 'ignore', 'ignore', 'ipc'], - windowsHide: true, - }); + return startProcessLegacy( + { + command: process.execPath, + args: [productionEntry], + options: { + env, + stdio: ['ignore', 'ignore', 'ignore', 'ipc'], + windowsHide: true, + }, + processGroup: false, + }, + toShared() + ); } throw Object.assign(new Error('Code Collab watch worker bundle is missing'), { code: 'WATCH_WORKER_MISSING', diff --git a/apps/cli/src/lib/local-ipc-socket-server.ts b/apps/cli/src/lib/local-ipc-socket-server.ts index b2ed896c7a..7fde9fd1af 100644 --- a/apps/cli/src/lib/local-ipc-socket-server.ts +++ b/apps/cli/src/lib/local-ipc-socket-server.ts @@ -1,3 +1,4 @@ +import { toShared } from '@/platform/process-options'; import fs from 'node:fs'; import http from 'node:http'; import net from 'node:net'; @@ -19,6 +20,7 @@ import { type LocalSessionControlConfig, } from '@/lib/local-session-control'; import { removeStaleUnixSocket } from '@/lib/stale-unix-socket'; +import { isPidAliveSyncLegacy } from '@lody/shared/node/process'; type LocalIpcSocketServerPaths = { probeSocketPath?: string; @@ -57,16 +59,6 @@ function unlinkIfExists(filePath: string): void { } } -function isProcessAlive(pid: number): boolean { - try { - process.kill(pid, 0); - return true; - } catch (error) { - const code = (error as NodeJS.ErrnoException).code; - return code !== 'ESRCH'; - } -} - function readLockOwnerPid(lockFilePath: string): number | null { try { const raw = fs.readFileSync(lockFilePath, 'utf8').trim(); @@ -96,7 +88,7 @@ function acquireStartupLock(lockFilePath: string): number { } const ownerPid = readLockOwnerPid(lockFilePath); - if (ownerPid !== null && isProcessAlive(ownerPid)) { + if (ownerPid !== null && isPidAliveSyncLegacy(ownerPid, toShared())) { throw new Error(`local_ipc_lock_in_use:${lockFilePath}`, { cause: error }); } diff --git a/apps/cli/src/lib/local-project-control-service.ts b/apps/cli/src/lib/local-project-control-service.ts index 0fc92dcf1d..6c99fab689 100644 --- a/apps/cli/src/lib/local-project-control-service.ts +++ b/apps/cli/src/lib/local-project-control-service.ts @@ -1,9 +1,8 @@ +import { toShared } from '@/platform/process-options'; import fs from 'node:fs'; import path, { isAbsolute, relative, resolve as resolvePath } from 'node:path'; import os from 'node:os'; import { createHash } from 'node:crypto'; -import { execFile } from 'node:child_process'; -import { promisify } from 'node:util'; import type { LocalProjectBrowseDirectoryResult, LocalProjectBrowseRootsResult, @@ -41,6 +40,7 @@ import { import { getLodyDataDir } from '@lody/shared/node/installation-profile'; import type { Logger } from '@/utils/logger'; import { formatErrorMessage } from '@/utils/format-error'; +import { runCommandTextLegacy } from '@lody/shared/node/process'; const DEFAULT_LOCAL_PROJECT_MAX_FILES = 80_000; const HARD_LOCAL_PROJECT_MAX_FILES = 200_000; @@ -58,7 +58,6 @@ const HARD_LOCAL_PROJECT_READ_MAX_BYTES = 5 * 1024 * 1024; const GIT_COMMAND_MAX_BUFFER_BYTES = 64 * 1024 * 1024; const LOCAL_PROJECT_GIT_COMMAND_TIMEOUT_MS = 5_000; const LOCAL_PROJECT_WALK_YIELD_EVERY_ENTRIES = 1_000; -const execFileAsync = promisify(execFile); const LOCAL_REPO_ID_RE = /^local---[0-9a-f]{12}$/; const yieldToEventLoop = (): Promise => @@ -256,33 +255,26 @@ type GitCommandResult = { async function runGitCommand(rootPath: string, args: string[]): Promise { try { - const result = await execFileAsync('git', args, { - cwd: rootPath, - encoding: 'utf8', - maxBuffer: GIT_COMMAND_MAX_BUFFER_BYTES, - timeout: LOCAL_PROJECT_GIT_COMMAND_TIMEOUT_MS, - killSignal: 'SIGTERM', - env: { - ...process.env, - GIT_TERMINAL_PROMPT: '0', - GIT_OPTIONAL_LOCKS: '0', + const result = await runCommandTextLegacy( + { + command: 'git', + args, + cwd: rootPath, + maxOutputBytes: GIT_COMMAND_MAX_BUFFER_BYTES, + timeout: LOCAL_PROJECT_GIT_COMMAND_TIMEOUT_MS, + env: { + ...process.env, + GIT_TERMINAL_PROMPT: '0', + GIT_OPTIONAL_LOCKS: '0', + }, + check: 'none', }, - }); - return { - status: 0, - stdout: String(result.stdout ?? ''), - stderr: String(result.stderr ?? ''), - }; + toShared() + ); + return { status: result.code, stdout: result.stdout, stderr: result.stderr }; } catch (error) { - const withOutput = error as - | (Error & { code?: number | string; stdout?: string; stderr?: string }) - | undefined; - const message = formatErrorMessage(error); - return { - status: typeof withOutput?.code === 'number' ? withOutput.code : null, - stdout: String(withOutput?.stdout ?? ''), - stderr: String(withOutput?.stderr ?? message), - }; + // Spawn failure, timeout or output overflow: no exit status to report. + return { status: null, stdout: '', stderr: formatErrorMessage(error) }; } } diff --git a/apps/cli/src/lib/machine-lifecycle-upgrade.test.ts b/apps/cli/src/lib/machine-lifecycle-upgrade.test.ts index ad773a47a1..ab8a4eb70a 100644 --- a/apps/cli/src/lib/machine-lifecycle-upgrade.test.ts +++ b/apps/cli/src/lib/machine-lifecycle-upgrade.test.ts @@ -1,6 +1,7 @@ import { mkdir, mkdtemp, readFile, realpath, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import path from 'node:path'; +import { FakeProcessTable } from '@lody/shared/node/process-testing'; import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; const fixture = vi.hoisted(() => ({ home: '' })); @@ -220,4 +221,39 @@ describe('daemon upgrade command execution', () => { expect(await lifecycle.runDaemonUpgradeFromIntent({ logger: {} })).toBeNull(); await expect(readFile(argsFile)).rejects.toMatchObject({ code: 'ENOENT' }); }); + + it('cancels a running install by ending its whole process tree', async () => { + await lifecycle.writeDaemonUpgradeIntent({ + action: 'upgrade', + requestId: 'synthetic-cancel', + requesterUserId: 'synthetic-user', + targetVersion: '1.2.3', + requestedAtMs: 0, + }); + const table = new FakeProcessTable('linux'); + const spawned = Promise.withResolvers(); + const controller = new AbortController(); + + const upgrade = lifecycle.runDaemonUpgradeFromIntent({ + logger: {}, + signal: controller.signal, + nodeProcess: { + ...table.api, + spawn: (command, args, options) => { + const child = table.api.spawn(command, args, options); + if (typeof child.pid === 'number') spawned.resolve(child.pid); + return child; + }, + }, + }); + const npmPid = await spawned.promise; + // A lifecycle script npm started. + const scriptPid = table.addDescendant(npmPid); + controller.abort(); + + await expect(upgrade).rejects.toMatchObject({ name: 'AbortError' }); + expect(table.isAlive(npmPid)).toBe(false); + expect(table.isAlive(scriptPid)).toBe(false); + expect(await lifecycle.readDaemonUpgradeIntent()).toBeNull(); + }); }); diff --git a/apps/cli/src/lib/machine-lifecycle.ts b/apps/cli/src/lib/machine-lifecycle.ts index 5aa7ebebae..8f27014ed3 100644 --- a/apps/cli/src/lib/machine-lifecycle.ts +++ b/apps/cli/src/lib/machine-lifecycle.ts @@ -1,5 +1,4 @@ -import type { ChildProcess, SpawnOptions } from 'node:child_process'; -import spawn from 'cross-spawn'; +import { toShared } from '@/platform/process-options'; import * as fs from 'node:fs/promises'; import * as path from 'node:path'; import { z } from 'zod'; @@ -19,6 +18,10 @@ import { } from '@lody/shared/node/local-cli-supervisor'; import { LODY_AUTH_SITE_URL, LODY_AUTH_URL } from '@/utils/const'; import { getLodyDataDir } from '@lody/shared/node/installation-profile'; +import { startProcessLegacy } from '@lody/shared/node/process'; + +import type { NodeProcessApi } from '@lody/shared/node/process'; +import type { TerminationPolicy } from '@lody/shared/node/process'; // The reserved Worker exit codes are part of the shared Supervisor<->Worker // contract; Electron consumes the same values from @lody/shared. @@ -224,13 +227,17 @@ export const buildLodyUpgradeInstallArgs = (targetVersion: string): string[] => `--registry=${NPM_REGISTRY_URL}`, ]; -type SpawnLike = (command: string, args: readonly string[], options: SpawnOptions) => ChildProcess; +/** + * A timed-out or cancelled install is ended as a tree: npm's lifecycle scripts + * and their children go with it. SIGKILL after 2 s, exit proven within 5 s more. + */ +const UPGRADE_TERMINATION_POLICY: TerminationPolicy = { graceMs: 2_000, killWaitMs: 5_000 }; const runCommand = async (args: { command: string; commandArgs: readonly string[]; timeoutMs: number; - spawnImpl: SpawnLike; + nodeProcess?: NodeProcessApi; signal?: AbortSignal; }): Promise<{ code: number | null; @@ -242,39 +249,51 @@ const runCommand = async (args: { if (args.signal?.aborted) { throw new DOMException('Daemon upgrade canceled', 'AbortError'); } - const child = args.spawnImpl(args.command, args.commandArgs, { - stdio: ['ignore', 'pipe', 'pipe'], - env: process.env, - }); let stdout = ''; let stderr = ''; const append = (current: string, chunk: Buffer): string => `${current}${chunk.toString()}`.slice(-64 * 1024); - child.stdout?.on('data', (chunk: Buffer) => { - stdout = append(stdout, chunk); - }); - child.stderr?.on('data', (chunk: Buffer) => { - stderr = append(stderr, chunk); - }); + const install = startProcessLegacy( + { + command: args.command, + args: args.commandArgs, + options: { stdio: ['ignore', 'pipe', 'pipe'], env: process.env }, + processGroup: true, + onSpawned: (child) => { + child.stdout?.on('data', (chunk: Buffer) => { + stdout = append(stdout, chunk); + }); + child.stderr?.on('data', (chunk: Buffer) => { + stderr = append(stderr, chunk); + }); + }, + }, + toShared({ nodeProcess: args.nodeProcess }) + ); + const child = install.child; return await new Promise((resolve, reject) => { let timedOut = false; let aborted = false; let processError: Error | null = null; let terminationStarted = false; - let forceKillTimer: ReturnType | null = null; - let exitConfirmationTimer: ReturnType | null = null; const requestTermination = () => { if (terminationStarted) return; terminationStarted = true; - child.kill('SIGTERM'); - forceKillTimer = setTimeout(() => child.kill('SIGKILL'), 2_000); - forceKillTimer.unref?.(); - exitConfirmationTimer = setTimeout(() => { - cleanup(); - reject(new Error('Upgrade process did not confirm exit after SIGKILL')); - }, 7_000); - exitConfirmationTimer.unref?.(); + void install.terminate(UPGRADE_TERMINATION_POLICY).then( + // The whole tree is gone; do not wait on a pipe some escaped process + // may still hold open. + () => + install.exited.then(({ code }) => { + cleanup(); + if (processError) reject(processError); + else resolve({ code, stdout, stderr, timedOut, aborted }); + }), + (error: unknown) => { + cleanup(); + reject(new Error('Upgrade process did not confirm exit after SIGKILL', { cause: error })); + } + ); }; const onAbort = () => { if (aborted) return; @@ -291,14 +310,15 @@ const runCommand = async (args: { const cleanup = () => { clearTimeout(timeout); - if (forceKillTimer) clearTimeout(forceKillTimer); - if (exitConfirmationTimer) clearTimeout(exitConfirmationTimer); args.signal?.removeEventListener('abort', onAbort); }; child.once('error', (error) => { processError = error; }); child.once('close', (code) => { + // Once termination starts it owns the result: the root can close while + // the rest of its tree is still being ended. + if (terminationStarted) return; cleanup(); if (processError) { reject(processError); @@ -322,7 +342,7 @@ async function resolveUpgradedInstallation(args: { npmExecutable: string; targetVersion: string; timeoutMs: number; - spawnImpl: SpawnLike; + nodeProcess?: NodeProcessApi; signal?: AbortSignal; }): Promise { const run = async (command: string, commandArgs: string[]) => { @@ -374,7 +394,8 @@ async function resolveUpgradedInstallation(args: { export const runDaemonUpgradeFromIntent = async (args: { logger: LifecycleLogger; timeoutMs?: number; - spawnImpl?: SpawnLike; + /** OS process seam of the process layer; tests substitute a fake. */ + nodeProcess?: NodeProcessApi; signal?: AbortSignal; }): Promise => { const intent = await readDaemonUpgradeIntent(); @@ -394,7 +415,7 @@ export const runDaemonUpgradeFromIntent = async (args: { command: npmExecutable, commandArgs: installArgs, timeoutMs: args.timeoutMs ?? MACHINE_UPGRADE_TIMEOUT_MS, - spawnImpl: args.spawnImpl ?? spawn, + nodeProcess: args.nodeProcess, signal: args.signal, }); if (result.aborted) { @@ -421,7 +442,7 @@ export const runDaemonUpgradeFromIntent = async (args: { npmExecutable, targetVersion, timeoutMs: args.timeoutMs ?? MACHINE_UPGRADE_TIMEOUT_MS, - spawnImpl: args.spawnImpl ?? spawn, + nodeProcess: args.nodeProcess, signal: args.signal, }); args.logger.info?.( diff --git a/apps/cli/src/lib/memory-providers.ts b/apps/cli/src/lib/memory-providers.ts index 9878c01e06..9876562b10 100644 --- a/apps/cli/src/lib/memory-providers.ts +++ b/apps/cli/src/lib/memory-providers.ts @@ -1,5 +1,6 @@ -import { execFile } from 'node:child_process'; -import { promisify } from 'node:util'; +import { toShared } from '@/platform/process-options'; +import { runCommandTextLegacy } from '@lody/shared/node/process'; + import { z } from 'zod'; import { MemoryBindingSchema, @@ -12,19 +13,25 @@ import { } from '@lody/shared'; import { getLoginShellEnv } from '@/agent/login-shell-env'; -const execFileAsync = promisify(execFile); type CommandResult = { stdout: string; code?: string | number }; export type MemoryCommandRunner = (args: string[]) => Promise; const runNmem: MemoryCommandRunner = async (args) => { try { - const result = await execFileAsync('nmem', args, { - env: { ...process.env, ...(await getLoginShellEnv()) }, - timeout: 15_000, - maxBuffer: 1024 * 1024, - encoding: 'utf8', - windowsHide: true, - }); - return { stdout: result.stdout }; + const result = await runCommandTextLegacy( + { + command: 'nmem', + args, + check: 'none', + env: { ...process.env, ...(await getLoginShellEnv()) }, + timeout: 15_000, + maxOutputBytes: 1024 * 1024, + }, + toShared() + ); + return { + stdout: result.stdout, + code: result.code === 0 ? undefined : (result.code ?? 'failed'), + }; } catch (error) { const parsed = z .object({ code: z.union([z.string(), z.number()]).optional(), stdout: z.string().optional() }) diff --git a/apps/cli/src/lib/pr-poller/github-credential-resolver.ts b/apps/cli/src/lib/pr-poller/github-credential-resolver.ts index e789ef9632..a9ab2ebfa2 100644 --- a/apps/cli/src/lib/pr-poller/github-credential-resolver.ts +++ b/apps/cli/src/lib/pr-poller/github-credential-resolver.ts @@ -1,7 +1,8 @@ -import { execFile } from 'node:child_process'; +import { toShared } from '@/platform/process-options'; import type { CloudGithubTokenManager, CloudGithubWriteTokenContext } from '@lody/platform'; import type { Logger } from '@/utils/logger'; import { formatErrorMessage } from '@/utils/format-error'; +import { runCommandTextLegacy } from '@lody/shared/node/process'; /** * Per-repo GitHub credential resolution for the poller (plan §3). @@ -39,40 +40,42 @@ type GhHarvest = | { outcome: 'gh-missing' } | { outcome: 'not-authed' }; -const defaultHarvestGhToken = (): Promise => - new Promise((resolve) => { - execFile( - 'gh', - ['auth', 'token', '--hostname', 'github.com'], - { timeout: 5000, windowsHide: true }, - (error, stdout) => { - if (error) { - const code = (error as NodeJS.ErrnoException).code; - resolve({ outcome: code === 'ENOENT' ? 'gh-missing' : 'not-authed' }); - return; - } - const token = stdout.trim(); - resolve(token ? { outcome: 'token', token } : { outcome: 'not-authed' }); - } +const defaultHarvestGhToken = async (): Promise => { + try { + const { stdout } = await runCommandTextLegacy( + { + command: 'gh', + args: ['auth', 'token', '--hostname', 'github.com'], + timeout: 5000, + check: 'exit-0', + }, + toShared() ); - }); + const token = stdout.trim(); + return token ? { outcome: 'token', token } : { outcome: 'not-authed' }; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + return { outcome: code === 'ENOENT' ? 'gh-missing' : 'not-authed' }; + } +}; -const defaultFetchGhUserId = (): Promise => - new Promise((resolve) => { - execFile( - 'gh', - ['api', '--hostname', 'github.com', 'user', '--jq', '.id'], - { timeout: 5000, windowsHide: true }, - (error, stdout) => { - if (error) { - resolve(null); - return; - } - const id = stdout.trim(); - resolve(id ? id : null); - } +const defaultFetchGhUserId = async (): Promise => { + try { + const { stdout } = await runCommandTextLegacy( + { + command: 'gh', + args: ['api', '--hostname', 'github.com', 'user', '--jq', '.id'], + timeout: 5000, + check: 'exit-0', + }, + toShared() ); - }); + const id = stdout.trim(); + return id ? id : null; + } catch { + return null; + } +}; export type GitHubCredentialResolverDeps = { /** Workspace-bound token manager; null disables the managed tier entirely. */ diff --git a/apps/cli/src/lib/review-automation/review-automation-github.ts b/apps/cli/src/lib/review-automation/review-automation-github.ts index 7a8a34ef2a..3513c7197a 100644 --- a/apps/cli/src/lib/review-automation/review-automation-github.ts +++ b/apps/cli/src/lib/review-automation/review-automation-github.ts @@ -1,6 +1,8 @@ -import { execFile } from 'node:child_process'; +import { toShared } from '@/platform/process-options'; import { z } from 'zod'; +import { runCommandTextLegacy } from '@lody/shared/node/process'; + /** * GitHub access for the review engine, over the `gh` CLI. * @@ -38,24 +40,26 @@ export const createGhRunner = if (!token) { return { stdout: 'no GitHub credential available', exitCode: 1 }; } - return await new Promise((resolve) => { - execFile( - 'gh', - [...args], + try { + const result = await runCommandTextLegacy( { + command: 'gh', + args, timeout: GH_TIMEOUT_MS, env: { ...process.env, GH_TOKEN: token, GH_PROMPT: 'disabled' }, - maxBuffer: 8 * 1024 * 1024, + maxOutputBytes: 8 * 1024 * 1024, + check: 'none', }, - (error, stdout, stderr) => { - if (error) { - resolve({ stdout: `${stdout}${stderr}`.trim(), exitCode: 1 }); - return; - } - resolve({ stdout, exitCode: 0 }); - } + toShared() ); - }); + if (result.code !== 0 || result.signal !== null) { + return { stdout: `${result.stdout}${result.stderr}`.trim(), exitCode: 1 }; + } + return { stdout: result.stdout, exitCode: 0 }; + } catch { + // Spawn failure, timeout or output overflow: `gh` produced no usable output. + return { stdout: '', exitCode: 1 }; + } }; const PullRequestFactsSchema = z.object({ diff --git a/apps/cli/src/lib/session-file-attachments.ts b/apps/cli/src/lib/session-file-attachments.ts index 887af89abc..7e3502c9a5 100644 --- a/apps/cli/src/lib/session-file-attachments.ts +++ b/apps/cli/src/lib/session-file-attachments.ts @@ -1,10 +1,9 @@ +import { toShared } from '@/platform/process-options'; import * as crypto from 'crypto'; -import { execFile } from 'child_process'; import * as fs from 'fs'; import * as path from 'path'; -import { promisify } from 'util'; -const execFileAsync = promisify(execFile); +import { runCommandTextLegacy } from '@lody/shared/node/process'; /** * Directory (relative to the session workspace root) where human→agent file @@ -124,11 +123,15 @@ export const computeExcludeFileContent = (existingContent: string): string | nul export const resolveGitInfoExcludePath = async (workspaceRoot: string): Promise => { try { // `--git-path info/exclude` resolves correctly for worktrees/submodules. - const { stdout } = await execFileAsync('git', ['rev-parse', '--git-path', 'info/exclude'], { - cwd: workspaceRoot, - encoding: 'utf8', - windowsHide: true, - }); + const { stdout } = await runCommandTextLegacy( + { + command: 'git', + args: ['rev-parse', '--git-path', 'info/exclude'], + cwd: workspaceRoot, + check: 'exit-0', + }, + toShared() + ); const raw = stdout.trim(); if (!raw) { return null; diff --git a/apps/cli/src/lib/terminal-pty-service.test.ts b/apps/cli/src/lib/terminal-pty-service.test.ts new file mode 100644 index 0000000000..99998b4ade --- /dev/null +++ b/apps/cli/src/lib/terminal-pty-service.test.ts @@ -0,0 +1,65 @@ +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import type { TerminalExitEvent } from '@lody/shared'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { Logger } from '@/utils/logger'; +import { makeTerminalPtyService } from './terminal-pty-service'; + +const logger = { + debug: () => {}, + trace: () => {}, + info: () => {}, + warn: () => {}, + error: () => {}, +} as unknown as Logger; + +// A real PTY: the process-group semantics under test are the kernel's, not a model's. +describe.skipIf(process.platform === 'win32')('terminal PTY termination', () => { + let root: string; + + beforeEach(async () => { + root = await mkdtemp(path.join(tmpdir(), 'lody-terminal-pty-')); + vi.stubEnv('LODY_DISABLE_SHELL_ENV', '1'); + }); + + afterEach(async () => { + vi.unstubAllEnvs(); + await rm(root, { recursive: true, force: true }); + }); + + it('ends a shell that ignores the hangup instead of leaking it', async () => { + const shell = path.join(root, 'stubborn-shell'); + await writeFile( + shell, + "#!/bin/sh\ntrap '' HUP TERM\necho ready\nwhile read line; do :; done\n", + { mode: 0o700 } + ); + vi.stubEnv('SHELL', shell); + const service = makeTerminalPtyService({ logger, resolveSessionWorkdir: async () => root }); + const ready = Promise.withResolvers(); + const exited = Promise.withResolvers(); + let output = ''; + service.onEvent((event) => { + if (event.type === 'data') { + output += event.data; + if (output.includes('ready')) ready.resolve(); + } + if (event.type === 'exit') exited.resolve(event); + }); + + const { terminalId } = await service.open({ + sessionId: 'session-pty', + cols: 80, + rows: 24, + }); + await ready.promise; + service.close(terminalId); + + const exit = await exited.promise; + expect(exit.terminalId).toBe(terminalId); + // The hangup did not end it; the process layer's SIGKILL did. + expect(exit.signal).toBe('9'); + expect(service.list('session-pty')).toEqual([]); + }); +}); diff --git a/apps/cli/src/lib/terminal-pty-service.ts b/apps/cli/src/lib/terminal-pty-service.ts index 351eea7f1f..8fc05a807a 100644 --- a/apps/cli/src/lib/terminal-pty-service.ts +++ b/apps/cli/src/lib/terminal-pty-service.ts @@ -1,7 +1,9 @@ +import { toShared } from '@/platform/process-options'; import { randomUUID } from 'node:crypto'; import { createRequire } from 'node:module'; import path from 'node:path'; import type { IPty } from '@lydell/node-pty'; +import { Duration, Effect } from 'effect'; import { type SessionId, type TerminalDataEvent, @@ -19,6 +21,11 @@ import { LODY_GIT_CRED_CONTEXT_TOKEN_ENV } from '@/lib/git-credential-broker'; import { clearManagedGhTokenEnv, LODY_MANAGED_GH_TOKEN_SHA256_ENV } from '@/lib/gh-token-env'; import type { Logger } from '@/utils/logger'; import { formatErrorMessage } from '@/utils/format-error'; +import { makeProcessRunnerLegacy } from '@lody/shared/node/process'; + +import { NodeProcess } from '@lody/shared/node/process'; +import { posixGroupTree, terminateTree, waitUntilGone } from '@lody/shared/node/process'; +import type { TerminationPolicy } from '@lody/shared/node/process'; const SCROLLBACK_MAX_CHARS = 512 * 1024; const TITLE_PARSE_BUFFER_MAX_CHARS = 4096; @@ -37,6 +44,13 @@ const TERMINAL_ENV_BLOCKLIST = new Set([ LODY_MANAGED_GH_TOKEN_SHA256_ENV, ]); const require = createRequire(import.meta.url); +/** + * The hangup is the polite signal: a shell's SIGHUP path forwards the hangup to + * its jobs (fish sends no SIGHUP to them when SIGTERM ends it first). A shell + * still running after the grace is SIGKILLed. + */ +const PTY_HANGUP_GRACE = Duration.seconds(2); +const PTY_KILL_POLICY: TerminationPolicy = { graceMs: 0, killWaitMs: 2_000 }; // @lydell/node-pty ships its binding through per-platform optional dependencies and // never compiles from source, so hosts it has no prebuild for (musl, armv7, …) resolve @@ -141,6 +155,24 @@ function extractLatestTitle(record: TerminalRecord, data: string): string | null return latest; } +/** + * TEMPORARY facade: ends a PTY's process group through the process layer. + * node-pty's POSIX child calls setsid(), so the shell leads its own session and + * process group (pgid == pid) and `posixGroupTree` reaches it plus anything + * running in that group. An interactive shell puts each job in a group of its + * own; those are reached by the hangup `end()` sends first, which the shell + * forwards to its jobs, and by the kernel's SIGHUP to the terminal's foreground + * group when the session leader exits. + */ +const terminatePtyProcessGroup = (pid: number, logger: Logger): Promise => + makeProcessRunnerLegacy(toShared({ logger }))( + Effect.gen(function* () { + const tree = posixGroupTree(yield* NodeProcess, pid); + if (yield* waitUntilGone(tree, PTY_HANGUP_GRACE)) return; + yield* terminateTree(tree, PTY_KILL_POLICY); + }) + ); + class TerminalPtyServiceImpl implements TerminalPtyServiceApi { private readonly logger: Logger; private readonly resolveSessionWorkdir: (sessionId: SessionId) => Promise; @@ -262,8 +294,7 @@ class TerminalPtyServiceImpl implements TerminalPtyServiceApi { } close(terminalId: string): void { - const record = this.requireRecord(terminalId); - record.pty.kill(); + this.end(this.requireRecord(terminalId)); } closeSession(sessionId: string): void { @@ -299,7 +330,7 @@ class TerminalPtyServiceImpl implements TerminalPtyServiceApi { const record = this.records.get(terminalId); if (!record) return; try { - record.pty.kill(); + this.end(record); } catch (error) { this.logger.debug( `[terminal] failed to close terminalId=${terminalId}: ${formatErrorMessage(error)}` @@ -308,6 +339,26 @@ class TerminalPtyServiceImpl implements TerminalPtyServiceApi { } } + /** + * Hang up the terminal, then make sure its process group is gone. The + * hangup is synchronous, so a daemon exiting right after `closeAll()` has + * still delivered it; the bounded escalation runs in the background. + */ + private end(record: TerminalRecord): void { + // node-pty's kill() is SIGHUP on POSIX, the signal a closing terminal sends. + // On Windows it closes the ConPTY pseudoconsole, which ends the processes + // attached to it; POSIX signals and process groups do not exist there, so + // the process layer has no better tree to end. + record.pty.kill(); + if (process.platform === 'win32') return; + const { pid } = record.pty; + void terminatePtyProcessGroup(pid, this.logger).catch((error: unknown) => { + this.logger.warn( + `[terminal] process group ${pid} of terminalId=${record.terminalId} did not exit: ${formatErrorMessage(error)}` + ); + }); + } + private removeRecord(terminalId: string): void { const record = this.records.get(terminalId); if (!record) return; diff --git a/apps/cli/src/mcp/lody-mcp-http-server.ts b/apps/cli/src/mcp/lody-mcp-http-server.ts index 0ae1b589d3..1b83e826df 100644 --- a/apps/cli/src/mcp/lody-mcp-http-server.ts +++ b/apps/cli/src/mcp/lody-mcp-http-server.ts @@ -1,9 +1,12 @@ -import { spawn, type ChildProcess } from 'node:child_process'; +import { toShared } from '@/platform/process-options'; +import type { ChildProcess } from 'node:child_process'; import { randomBytes } from 'node:crypto'; import { once } from 'node:events'; import type { Readable } from 'node:stream'; import { Logger } from '@/utils/logger'; import { formatErrorMessage } from '@/utils/format-error'; +import { startProcessLegacy, type ProcessHandleLegacy } from '@lody/shared/node/process'; + import { MCP_HTTP_PREFERRED_PORT_ENV, MCP_HTTP_TOKEN_ENV, @@ -36,6 +39,8 @@ const MAX_RESTART_ATTEMPTS = 10; /** An exit after this much uptime resets the restart budget. */ const STABLE_UPTIME_MS = 30_000; const SIGTERM_GRACE_MS = 5_000; +/** Bound on waiting for a SIGKILLed host to be reaped. */ +const SIGKILL_WAIT_MS = 5_000; /** * Host exit codes that mean "this environment can never serve HTTP MCP" * (missing token wiring, unreadable /proc/net/tcp). Restarting cannot help. @@ -44,7 +49,7 @@ const PERMANENT_EXIT_CODES = new Set([2, 3]); class McpHttpHostSupervisor { private readonly token = randomBytes(32).toString('base64url'); - private child: ChildProcess | null = null; + private host: ProcessHandleLegacy | null = null; private endpoint: LodyMcpHttpEndpoint | null = null; private lastPort = 0; private attempts = 0; @@ -70,8 +75,9 @@ class McpHttpHostSupervisor { let exitSignal: NodeJS.Signals | null = null; let hostPid: number | undefined; try { - const child = this.spawnChild(cliEntrypoint); - this.child = child; + const host = this.spawnHost(cliEntrypoint); + this.host = host; + const { child } = host; hostPid = child.pid; const port = await this.readHandshake(child); handshaked = true; @@ -88,16 +94,16 @@ class McpHttpHostSupervisor { if (!this.stopping) { this.logger.warn(`[mcp-http] host start failed: ${formatErrorMessage(error)}`); } - const failed = this.child; - // A handshake timeout or invalid handshake SIGKILLs the host and rejects - // at once: wait for its real exit, so the trace below reports how it - // ended and the next host never overlaps a dying one. A child that never - // spawned (no pid) emits no 'exit'. - if (failed?.pid !== undefined && failed.exitCode === null && failed.signalCode === null) { - await once(failed, 'exit'); + const failed = this.host; + // A handshake timeout or invalid handshake rejects at once: SIGKILL the + // host and wait for it to be gone, so the trace below reports how it + // ended and the next host never overlaps a dying one. A host that + // already exited (or never spawned) has nothing left to end. + if (failed) { + await this.forceStop(failed); } - exitCode = failed?.exitCode ?? null; - exitSignal = failed?.signalCode ?? null; + exitCode = failed?.child.exitCode ?? null; + exitSignal = failed?.child.signalCode ?? null; } if (!this.stopping) { this.logger.warn( @@ -107,7 +113,7 @@ class McpHttpHostSupervisor { ); } this.endpoint = null; - this.child = null; + this.host = null; if (this.stopping) { return; } @@ -135,43 +141,60 @@ class McpHttpHostSupervisor { async stop(): Promise { this.stopping = true; this.wakeBackoffSleep?.(); - const child = this.child; - if (!child || child.exitCode !== null) { + const host = this.host; + if (!host) { return; } - const exited = once(child, 'exit'); - child.kill('SIGTERM'); - const outcome = await Promise.race([ - exited.then(() => 'exited' as const), - new Promise<'timeout'>((resolve) => setTimeout(() => resolve('timeout'), SIGTERM_GRACE_MS)), - ]); - if (outcome === 'timeout' && child.exitCode === null) { - child.kill('SIGKILL'); - await exited; + await this.endHost(host, { graceMs: SIGTERM_GRACE_MS, killWaitMs: SIGKILL_WAIT_MS }); + } + + private async forceStop(host: ProcessHandleLegacy): Promise { + await this.endHost(host, { graceMs: 0, killWaitMs: SIGKILL_WAIT_MS }); + } + + private async endHost( + host: ProcessHandleLegacy, + policy: Parameters[0] + ): Promise { + try { + await host.terminate(policy); + } catch (error) { + this.logger.warn( + `[mcp-http] host pid=${host.child.pid ?? '?'} did not stop: ${formatErrorMessage(error)}` + ); } } - private spawnChild(cliEntrypoint: string): ChildProcess { - const child = spawn(process.execPath, [cliEntrypoint, '__internal', 'lody-mcp-http-host'], { - // stdin held open as the daemon-death signal; fd 3 is the handshake - // pipe. The token goes through the environment (owner-readable only), - // never through argv. - stdio: ['pipe', 'ignore', 'pipe', 'pipe'], - env: { - ...process.env, - [MCP_HTTP_TOKEN_ENV]: this.token, - [MCP_HTTP_PREFERRED_PORT_ENV]: String(this.lastPort), + private spawnHost(cliEntrypoint: string): ProcessHandleLegacy { + return startProcessLegacy( + { + command: process.execPath, + args: [cliEntrypoint, '__internal', 'lody-mcp-http-host'], + options: { + // stdin held open as the daemon-death signal; fd 3 is the handshake + // pipe. The token goes through the environment (owner-readable only), + // never through argv. + stdio: ['pipe', 'ignore', 'pipe', 'pipe'], + env: { + ...process.env, + [MCP_HTTP_TOKEN_ENV]: this.token, + [MCP_HTTP_PREFERRED_PORT_ENV]: String(this.lastPort), + }, + windowsHide: true, + }, + processGroup: false, + onSpawned: (child) => { + child.stderr?.setEncoding('utf8'); + child.stderr?.on('data', (chunk: string) => { + const text = chunk.trim(); + if (text) { + this.logger.debug(`[mcp-http] host stderr: ${text.slice(0, 2_000)}`); + } + }); + }, }, - windowsHide: true, - }); - child.stderr?.setEncoding('utf8'); - child.stderr?.on('data', (chunk: string) => { - const text = chunk.trim(); - if (text) { - this.logger.debug(`[mcp-http] host stderr: ${text.slice(0, 2_000)}`); - } - }); - return child; + toShared() + ); } private async readHandshake(child: ChildProcess): Promise { @@ -197,13 +220,12 @@ class McpHttpHostSupervisor { // have answered. Record the lateness so a stalled parent is not misread // as a broken host. const elapsedMs = Date.now() - armedAtMs; - child.kill('SIGKILL'); settle(() => rejectPromise( new Error( `host handshake timed out (pid=${child.pid ?? '?'} elapsedMs=${elapsedMs} timeoutMs=${HANDSHAKE_TIMEOUT_MS} timerLateMs=${ elapsedMs - HANDSHAKE_TIMEOUT_MS - } partialHandshakeBytes=${buffer.length}); killed with SIGKILL` + } partialHandshakeBytes=${buffer.length}); killing with SIGKILL` ) ) ); @@ -223,7 +245,6 @@ class McpHttpHostSupervisor { const parsed = McpHttpHostHandshakeSchema.parse(JSON.parse(buffer.slice(0, newline))); settle(() => resolve(parsed.port)); } catch (error) { - child.kill('SIGKILL'); settle(() => rejectPromise(new Error(`invalid host handshake: ${formatErrorMessage(error)}`)) ); diff --git a/apps/cli/src/mcp/lody-mcp-server.ts b/apps/cli/src/mcp/lody-mcp-server.ts index bf991aef76..fb795f62ac 100644 --- a/apps/cli/src/mcp/lody-mcp-server.ts +++ b/apps/cli/src/mcp/lody-mcp-server.ts @@ -1,6 +1,6 @@ +import { toShared } from '@/platform/process-options'; import { snapshotAgentRole, readMessageAuthor, type AgentMessageAuthor } from '@lody/shared'; import { resolveSessionMessageAuthor } from '@/session/message-author'; -import { spawn } from 'child_process'; import { resolveSessionLinkId } from '@lody/shared/session-link'; import { AsyncLocalStorage } from 'node:async_hooks'; import { createHash, randomUUID } from 'node:crypto'; @@ -10,6 +10,9 @@ import { StdioServerTransport } from '@modelcontextprotocol/server/stdio'; import { Effect } from 'effect'; import { z } from 'zod'; import { requestSessionShare } from '@/lib/session-share-delivery'; +import { startProcessLegacy } from '@lody/shared/node/process'; + +import { formatErrorMessage } from '@/utils/format-error'; import { ACP_CAPABILITY_ROW_FAMILIES, getAcpCapabilityCacheKey, @@ -1121,30 +1124,57 @@ const resolveCliEntrypoint = (): string => { return entrypoint; }; +/** A timed-out `lody` subcommand gets SIGTERM, then SIGKILL if it lingers. */ +const LODY_CLI_TIMEOUT_TERMINATION = { graceMs: 2_000, killWaitMs: 2_000 }; + const runLodyCli = async ( args: string[], timeoutMs = LODY_CLI_DEFAULT_TIMEOUT_MS -): Promise<{ stdout: string; stderr: string }> => - await new Promise((resolve, reject) => { - const child = spawn(process.execPath, [resolveCliEntrypoint(), ...args], { - env: process.env, - stdio: ['ignore', 'pipe', 'pipe'], - windowsHide: true, - }); - const stdout: Buffer[] = []; - const stderr: Buffer[] = []; +): Promise<{ stdout: string; stderr: string }> => { + const stdout: Buffer[] = []; + const stderr: Buffer[] = []; + const cli = startProcessLegacy( + { + command: process.execPath, + args: [resolveCliEntrypoint(), ...args], + options: { + env: process.env, + stdio: ['ignore', 'pipe', 'pipe'], + windowsHide: true, + }, + // Stays in this server's process group: ending the agent tree that runs + // the MCP server must end its in-flight `lody` subcommands too. + processGroup: false, + onSpawned: (child) => { + child.stdout?.on('data', (chunk: Buffer) => stdout.push(chunk)); + child.stderr?.on('data', (chunk: Buffer) => stderr.push(chunk)); + }, + }, + toShared() + ); + return await new Promise((resolve, reject) => { + let timedOut = false; const timeout = setTimeout(() => { - child.kill('SIGTERM'); - reject(new Error(`lody ${args.join(' ')} timed out after ${timeoutMs}ms`)); + // The timeout owns the outcome: the exit it causes is not a CLI failure. + timedOut = true; + void cli.terminate(LODY_CLI_TIMEOUT_TERMINATION).then( + () => reject(new Error(`lody ${args.join(' ')} timed out after ${timeoutMs}ms`)), + (error: unknown) => + reject( + new Error( + `lody ${args.join(' ')} timed out after ${timeoutMs}ms and could not be stopped: ${formatErrorMessage(error)}` + ) + ) + ); }, timeoutMs); - child.stdout.on('data', (chunk: Buffer) => stdout.push(chunk)); - child.stderr.on('data', (chunk: Buffer) => stderr.push(chunk)); - child.on('error', (error) => { + cli.child.on('error', (error) => { + if (timedOut) return; clearTimeout(timeout); reject(error); }); - child.on('close', (code, signal) => { + cli.child.on('close', (code, signal) => { + if (timedOut) return; clearTimeout(timeout); const stdoutText = Buffer.concat(stdout).toString('utf8'); const stderrText = Buffer.concat(stderr).toString('utf8'); @@ -1159,6 +1189,7 @@ const runLodyCli = async ( resolve({ stdout: stdoutText, stderr: stderrText }); }); }); +}; const parseJsonCliOutput = (stdout: string): unknown => { const line = stdout diff --git a/apps/cli/src/monitor/process-table.ts b/apps/cli/src/monitor/process-table.ts index 339e0143b3..8da10d8dc5 100644 --- a/apps/cli/src/monitor/process-table.ts +++ b/apps/cli/src/monitor/process-table.ts @@ -1,9 +1,17 @@ +import { toShared } from '@/platform/process-options'; import os from 'os'; -import spawn from 'cross-spawn'; import { z } from 'zod'; +import { + CommandOutputTooLarge, + CommandTimedOut, + READ_ONLY_ABANDON_POLICY, +} from '@lody/shared/node/process'; +import { runCommandTextLegacy } from '@lody/shared/node/process'; const PROCESS_TABLE_TIMEOUT_MS = 2_000; const MAX_PROCESS_TABLE_BYTES = 8 * 1024 * 1024; +/** Only the start of a failing probe's stderr goes into its error. */ +const MAX_PROBE_STDERR_CHARS = 64 * 1024; export type ProcessTableEntry = { pid: number; @@ -177,57 +185,34 @@ function parseDarwinMemoryBytes(value: string | undefined): number | null { } async function runProbe(command: string, args: string[], env: NodeJS.ProcessEnv): Promise { - return await new Promise((resolve, reject) => { - const child = spawn(command, args, { - env, - windowsHide: true, - stdio: ['ignore', 'pipe', 'pipe'], - }); - const stdoutChunks: Buffer[] = []; - const stderrChunks: Buffer[] = []; - let stdoutBytes = 0; - let settled = false; - const timer = setTimeout(() => { - child.kill('SIGKILL'); - finish(new Error(`${command} process-table probe timed out`)); - }, PROCESS_TABLE_TIMEOUT_MS); - timer.unref?.(); - - const finish = (error?: Error): void => { - if (settled) return; - settled = true; - clearTimeout(timer); - if (error) { - reject(error); - return; - } - resolve(Buffer.concat(stdoutChunks).toString('utf8')); - }; - - child.stdout?.on('data', (chunk: Buffer) => { - stdoutBytes += chunk.length; - if (stdoutBytes > MAX_PROCESS_TABLE_BYTES) { - child.kill('SIGKILL'); - finish(new Error(`${command} process-table probe exceeded output limit`)); - return; - } - stdoutChunks.push(chunk); - }); - child.stderr?.on('data', (chunk: Buffer) => { - if (stderrChunks.reduce((total, item) => total + item.length, 0) < 64 * 1024) { - stderrChunks.push(chunk); - } - }); - child.once('error', (error) => finish(error)); - child.once('close', (code) => { - if (code !== 0) { - const stderr = Buffer.concat(stderrChunks).toString('utf8').trim(); - finish(new Error(`${command} process-table probe exited with ${code}: ${stderr}`)); - return; - } - finish(); - }); - }); + let result: Awaited>; + try { + result = await runCommandTextLegacy( + { + command, + args, + env, + timeout: PROCESS_TABLE_TIMEOUT_MS, + abandonPolicy: READ_ONLY_ABANDON_POLICY, + maxOutputBytes: MAX_PROCESS_TABLE_BYTES, + check: 'none', + }, + toShared() + ); + } catch (error) { + if (error instanceof CommandTimedOut) { + throw new Error(`${command} process-table probe timed out`, { cause: error }); + } + if (error instanceof CommandOutputTooLarge) { + throw new Error(`${command} process-table probe exceeded output limit`, { cause: error }); + } + throw error; + } + if (result.code !== 0) { + const stderr = result.stderr.slice(0, MAX_PROBE_STDERR_CHARS).trim(); + throw new Error(`${command} process-table probe exited with ${result.code}: ${stderr}`); + } + return result.stdout; } function parseNonNegativeInteger(value: string | undefined): number | null { diff --git a/apps/cli/src/preview/cloudflared-native.test.ts b/apps/cli/src/preview/cloudflared-native.test.ts index 50e2129fb5..726747051c 100644 --- a/apps/cli/src/preview/cloudflared-native.test.ts +++ b/apps/cli/src/preview/cloudflared-native.test.ts @@ -1,6 +1,7 @@ import { ChildProcess } from 'node:child_process'; import { PassThrough } from 'node:stream'; import { afterEach, describe, expect, it, vi } from 'vitest'; +import type { NodeProcessApi } from '@lody/shared/node/process'; import { startCloudflaredNative, type CloudflaredProcess } from './cloudflared-native'; class ControlledChild extends ChildProcess { @@ -14,13 +15,22 @@ class ControlledChild extends ChildProcess { null, null, ]; + // A started child always has a pid; `launch` routes the process layer's + // signals for it back to `kill` below. + override pid: number | undefined = 4242; signals: Array = []; exitOnSignal = true; override kill(signal: NodeJS.Signals | number = 'SIGTERM'): boolean { this.signals.push(signal); - if (this.exitOnSignal) this.emit('close', 0, signal); + if (this.exitOnSignal) this.exit(null, typeof signal === 'string' ? signal : null); return true; } + /** What Node does when the OS reports the exit: set the exit fields, then emit. */ + exit(code: number | null, signal: NodeJS.Signals | null) { + Object.assign(this, { exitCode: code, signalCode: signal }); + this.emit('exit', code, signal); + this.emit('close', code, signal); + } log(message: string, level = 'info', error?: string) { this.stderr.write(`${JSON.stringify({ message, level, error })}\n`); } @@ -38,15 +48,35 @@ afterEach(async () => { function launch() { const spawned = Promise.withResolvers(); + let current: ControlledChild | undefined; const result = startCloudflaredNative({ binary: '/managed/cloudflared', proxyOrigin: 'http://127.0.0.1:5173', signal: new AbortController().signal, - spawn: () => { - const child = new ControlledChild(); - spawned.resolve(child); - return child; - }, + nodeProcess: { + platform: 'linux', + spawn: () => { + const child = new ControlledChild(); + current = child; + spawned.resolve(child); + return child; + }, + spawnSync: () => { + throw new Error('cloudflared is never run synchronously'); + }, + kill: (target, signal) => { + const child = current; + if ( + !child || + Math.abs(target) !== child.pid || + child.exitCode !== null || + child.signalCode !== null + ) { + throw Object.assign(new Error('kill ESRCH'), { code: 'ESRCH' }); + } + if (signal !== 0) child.kill(signal); + }, + } satisfies NodeProcessApi, }); launches.push(result); return { result, spawned: spawned.promise }; @@ -82,7 +112,7 @@ describe('cloudflared process ownership', () => { const registrationFailure = handle.registered.catch((error: unknown) => error); child.stderr.write(`${line}\n`); child.log('Failed to initialize DNS local resolver', 'error', 'operation was canceled'); - child.emit('close', 0, null); + child.exit(0, null); const failure = await handle.closed; expect(failure?.message).toBe( _label === 'oversized diagnostic' @@ -111,7 +141,7 @@ describe('cloudflared process ownership', () => { 'error', 'dial timeout https://proxy.test/?credential=secret' ); - child.emit('close', 1, null); + child.exit(1, null); const failure = await handle.closed; expect(failure?.message).toContain('Unable to reach edge [url]'); expect(failure?.message).toContain('dial timeout [url]'); @@ -127,7 +157,7 @@ describe('cloudflared process ownership', () => { void handle.registered.catch((error: unknown) => { registrationFailure = error; }); - child.emit('close', 1, null); + child.exit(1, null); const failure = await handle.closed; expect(failure?.message).toContain('cloudflared exited'); expect(registrationFailure).toBe(failure); @@ -152,7 +182,9 @@ describe('cloudflared process ownership', () => { expect(child.signals).toEqual(['SIGTERM']); await vi.advanceTimersByTimeAsync(3_000); expect(child.signals).toEqual(['SIGTERM', 'SIGKILL']); - child.emit('close', null, 'SIGKILL'); + child.exit(null, 'SIGKILL'); + // Exit is confirmed on the termination's next liveness poll. + await vi.advanceTimersByTimeAsync(5_000); await rejected; }); }); diff --git a/apps/cli/src/preview/cloudflared-native.ts b/apps/cli/src/preview/cloudflared-native.ts index 628caa75b0..3c1daaeeb7 100644 --- a/apps/cli/src/preview/cloudflared-native.ts +++ b/apps/cli/src/preview/cloudflared-native.ts @@ -1,12 +1,13 @@ -import { - spawn, - type ChildProcessWithoutNullStreams, - type SpawnOptionsWithoutStdio, -} from 'node:child_process'; +import { toShared } from '@/platform/process-options'; +import type { ChildProcess } from 'node:child_process'; import { mkdtemp, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { z } from 'zod'; +import { startProcessLegacy, type ProcessHandleLegacy } from '@lody/shared/node/process'; + +import type { NodeProcessApi } from '@lody/shared/node/process'; +import type { TerminationPolicy } from '@lody/shared/node/process'; export class CloudflaredError extends Error { constructor( @@ -29,12 +30,6 @@ export type CloudflaredProcess = { stop(): Promise; }; -type SpawnCloudflared = ( - binary: string, - args: string[], - options: SpawnOptionsWithoutStdio -) => ChildProcessWithoutNullStreams; - const LogLine = z.object({ message: z.string(), level: z.string().optional(), @@ -43,7 +38,8 @@ const LogLine = z.object({ }); const QUICK_ORIGIN = /^https:\/\/[a-z0-9]+(?:-[a-z0-9]+)*\.trycloudflare\.com$/; const START_TIMEOUT_MS = 30_000; -const STOP_GRACE_MS = 3_000; +/** SIGTERM, then SIGKILL after 3 s; the exit wait after SIGKILL is bounded too. */ +const STOP_POLICY: TerminationPolicy = { graceMs: 3_000, killWaitMs: 5_000 }; const MAX_LOG_LINE_BYTES = 64 * 1024; export function parseQuickTunnelOrigin(message: string): string | undefined { @@ -87,7 +83,8 @@ export async function startCloudflaredNative(options: { binary: string; proxyOrigin: string; signal: AbortSignal; - spawn?: SpawnCloudflared; + /** OS process seam of the process layer; tests substitute a fake. */ + nodeProcess?: NodeProcessApi; env?: NodeJS.ProcessEnv; onDiagnostic?: (message: string) => void; }): Promise { @@ -107,33 +104,47 @@ export async function startCloudflaredNative(options: { } const configDir = await mkdtemp(join(tmpdir(), 'lody-cloudflared-')); const configFile = join(configDir, 'config.yaml'); - let child: ChildProcessWithoutNullStreams; + let native: ProcessHandleLegacy; try { await writeFile(configFile, '{}\n', { mode: 0o600 }); options.signal.throwIfAborted(); - child = (options.spawn ?? spawn)( - options.binary, - [ - 'tunnel', - '--config', - configFile, - '--no-autoupdate', - '--output', - 'json', - '--metrics', - '127.0.0.1:0', - '--protocol', - 'http2', - '--url', - proxy.origin, - ], - { cwd: configDir, env: childEnvironment(options.env ?? process.env), windowsHide: true } + native = startProcessLegacy( + { + command: options.binary, + args: [ + 'tunnel', + '--config', + configFile, + '--no-autoupdate', + '--output', + 'json', + '--metrics', + '127.0.0.1:0', + '--protocol', + 'http2', + '--url', + proxy.origin, + ], + options: { + cwd: configDir, + env: childEnvironment(options.env ?? process.env), + stdio: ['pipe', 'pipe', 'pipe'], + }, + // A single native binary with no children of its own. + processGroup: false, + }, + toShared({ nodeProcess: options.nodeProcess }) ); } catch (error) { await rm(configDir, { recursive: true }); throw new CloudflaredError('start', 'Unable to start cloudflared', error); } + const child: ChildProcess = native.child; + let termination: Promise | undefined; + // Every path that ends cloudflared shares one bounded termination, so a + // failure path that only signals and the owner's stop() converge on it. + const terminate = () => (termination ??= native.terminate(STOP_POLICY)); let stopping = false; let exited = false; let exitError: CloudflaredError | null = null; @@ -167,26 +178,26 @@ export async function startCloudflaredNative(options: { const stop = (): Promise => { stopPromise ??= (async () => { stopping = true; + let stopFailure: unknown; if (!exited) { - child.kill('SIGTERM'); - const force = setTimeout(() => { - if (!exited) child.kill('SIGKILL'); - }, STOP_GRACE_MS); - force.unref(); try { + await terminate(); await closed; - } finally { - clearTimeout(force); + } catch (error) { + stopFailure = error; } } await rm(configDir, { recursive: true }); + if (stopFailure !== undefined) { + throw new CloudflaredError('stop', 'cloudflared did not exit', stopFailure); + } })(); return stopPromise; }; const abort = () => { rejectOrigin(options.signal.reason); - // The caller's failure path owns awaiting cleanup; this listener only signals. - if (!exited) child.kill('SIGTERM'); + // The caller's failure path owns awaiting cleanup; this listener only starts it. + if (!exited) void terminate().catch(() => {}); }; options.signal.addEventListener('abort', abort, { once: true }); child.once('error', (error) => { @@ -216,7 +227,7 @@ export async function startCloudflaredNative(options: { if (Buffer.byteLength(pending) > MAX_LOG_LINE_BYTES) { exitError ??= new CloudflaredError('start', 'cloudflared log line exceeded its size limit'); rejectOrigin(exitError); - child.kill('SIGTERM'); + void terminate().catch(() => {}); pending = ''; return; } @@ -258,13 +269,13 @@ export async function startCloudflaredNative(options: { // for closed as well, before SIGTERM produces secondary shutdown errors. exitError ??= new CloudflaredError('start', 'Invalid cloudflared JSON output', error); rejectOrigin(exitError); - child.kill('SIGTERM'); + void terminate().catch(() => {}); } } }); }; - consume(child.stdout); - consume(child.stderr); + if (child.stdout) consume(child.stdout); + if (child.stderr) consume(child.stderr); const timer = setTimeout( () => rejectOrigin( diff --git a/apps/cli/src/preview/cloudflared-process.test.ts b/apps/cli/src/preview/cloudflared-process.test.ts index e1bdb86349..23f7c7bdca 100644 --- a/apps/cli/src/preview/cloudflared-process.test.ts +++ b/apps/cli/src/preview/cloudflared-process.test.ts @@ -18,6 +18,13 @@ const NativeStarted = z.object({ electron: z.string().optional(), }); +// Bundled CommonJS dependencies (cross-spawn in the process layer) require Node +// built-ins; an ESM bundle needs a real `require` for that, as Vite provides in +// the production build. +const cjsRequireBanner = { + js: "import { createRequire as __lodyCreateRequire } from 'node:module'; const require = __lodyCreateRequire(import.meta.url);", +}; + // Real process/IPC tests, no public network. POSIX shebang fixture stands in for // the native binary; Windows native execution belongs to platform acceptance. describe.skipIf(process.platform === 'win32')('cloudflared lifecycle IPC', () => { @@ -37,6 +44,7 @@ describe.skipIf(process.platform === 'win32')('cloudflared lifecycle IPC', () => platform: 'node', format: 'esm', target: 'node22', + banner: cjsRequireBanner, }); await build({ stdin: { @@ -54,6 +62,7 @@ describe.skipIf(process.platform === 'win32')('cloudflared lifecycle IPC', () => platform: 'node', format: 'esm', target: 'node22', + banner: cjsRequireBanner, }); await writeFile( binary, diff --git a/apps/cli/src/preview/cloudflared-process.ts b/apps/cli/src/preview/cloudflared-process.ts index cc25574e07..8c6c41b39e 100644 --- a/apps/cli/src/preview/cloudflared-process.ts +++ b/apps/cli/src/preview/cloudflared-process.ts @@ -1,6 +1,8 @@ -import { spawn } from 'node:child_process'; +import { toShared } from '@/platform/process-options'; import { fileURLToPath } from 'node:url'; import { z } from 'zod'; +import { startProcessLegacy } from '@lody/shared/node/process'; + import { CloudflaredError, type CloudflaredProcess } from './cloudflared-native'; export { CloudflaredError, type CloudflaredProcess } from './cloudflared-native'; @@ -55,19 +57,24 @@ export async function startCloudflaredProcess(options: { onDiagnostic?: (message: string) => void; }): Promise { options.signal.throwIfAborted(); - const worker = spawn( - process.execPath, - [ - options.workerPath ?? fileURLToPath(new URL('./cloudflared-worker.js', import.meta.url)), - options.binary, - options.proxyOrigin, - ], + // Never terminated from here: IPC stop/disconnect is its only shutdown path, + // so the worker can reap cloudflared and remove its config first. + const worker = startProcessLegacy( { - env: workerEnvironment(options.env ?? process.env), - stdio: ['ignore', 'ignore', 'ignore', 'ipc'], - windowsHide: true, - } - ); + command: process.execPath, + args: [ + options.workerPath ?? fileURLToPath(new URL('./cloudflared-worker.js', import.meta.url)), + options.binary, + options.proxyOrigin, + ], + options: { + env: workerEnvironment(options.env ?? process.env), + stdio: ['ignore', 'ignore', 'ignore', 'ipc'], + }, + processGroup: false, + }, + toShared() + ).child; let failure: CloudflaredError | null = null; let diagnostic: string | undefined; let stopping = false; diff --git a/apps/cli/src/session/git-identity.ts b/apps/cli/src/session/git-identity.ts index 4a294986b2..3ca6d7528d 100644 --- a/apps/cli/src/session/git-identity.ts +++ b/apps/cli/src/session/git-identity.ts @@ -1,7 +1,8 @@ -import { execFileSync } from 'node:child_process'; - +import { toShared } from '@/platform/process-options'; import { isMissingEmail } from '@lody/shared'; +import { runCommandTextSyncLegacy } from '@lody/shared/node/process'; + export const DEFAULT_AI_GIT_AUTHOR_NAME = 'LodyAI'; export const DEFAULT_AI_GIT_AUTHOR_EMAIL = 'agent@lody.ai'; @@ -64,17 +65,22 @@ const normalizeName = (name: string | undefined, email: string): string => { return email; }; -// Global scope only. Lody-managed bare repositories share one config across all -// worktrees, so a repository-level `user.*` there is whatever some earlier agent -// wrote (e.g. a test fixture identity) and would leak into every other session. +/** `git config` blocks the caller's event loop, so a wedged git cannot stall it for long. */ +const GIT_CONFIG_TIMEOUT_MS = 5_000; + +// Only global identity is a host default; shared bare-repo config belongs to agents. const readGitConfig = (key: 'user.name' | 'user.email'): string | undefined => { try { - const output = execFileSync('git', ['config', '--global', key], { - encoding: 'utf8', - stdio: ['ignore', 'pipe', 'ignore'], - windowsHide: true, - }); - return trimNonEmpty(output); + const { stdout } = runCommandTextSyncLegacy( + { + command: 'git', + args: ['config', '--global', key], + timeout: GIT_CONFIG_TIMEOUT_MS, + check: 'exit-0', + }, + toShared() + ); + return trimNonEmpty(stdout); } catch { return undefined; } diff --git a/apps/cli/src/session/session-fork-service.ts b/apps/cli/src/session/session-fork-service.ts index ed47402082..d91e6ff05f 100644 --- a/apps/cli/src/session/session-fork-service.ts +++ b/apps/cli/src/session/session-fork-service.ts @@ -1,6 +1,5 @@ +import { toShared } from '@/platform/process-options'; import { resolveSessionConversationConfig } from '@lody/shared'; -import { execFile } from 'node:child_process'; -import { promisify } from 'node:util'; import { getServerNow, resolveSessionAcpTargetId, @@ -26,6 +25,8 @@ import { } from '@lody/shared'; import type { Logger } from '@/utils/logger'; import { formatErrorMessage } from '@/utils/format-error'; +import { runCommandTextLegacy } from '@lody/shared/node/process'; + import { mapWithConcurrency } from '@/lib/bounded-concurrency'; import type { LoroDocumentManager } from '@/lib/loro/doc'; import type { SessionManager } from './session-manager'; @@ -39,7 +40,6 @@ import { import { createSessionBackend, type SessionBackendForkSnapshot } from './session-backend'; type ForkWarning = SessionForkResponse['warnings'][number]; -const execFileAsync = promisify(execFile); /** Recovery opens only store-listed docs; keep even that small fan-out bounded. */ const FORK_RECOVERY_CONCURRENCY = 4; @@ -964,16 +964,26 @@ export class SessionForkService { } private async inspectGitWorkdir(workdir: string): Promise<{ dirty: boolean; headSha: string }> { - const status = await execFileAsync('git', ['status', '--porcelain'], { - cwd: workdir, - windowsHide: true, - timeout: 10_000, - }); - const head = await execFileAsync('git', ['rev-parse', '--verify', 'HEAD^{commit}'], { - cwd: workdir, - windowsHide: true, - timeout: 10_000, - }); + const status = await runCommandTextLegacy( + { + command: 'git', + args: ['status', '--porcelain'], + cwd: workdir, + timeout: 10_000, + check: 'exit-0', + }, + toShared() + ); + const head = await runCommandTextLegacy( + { + command: 'git', + args: ['rev-parse', '--verify', 'HEAD^{commit}'], + cwd: workdir, + timeout: 10_000, + check: 'exit-0', + }, + toShared() + ); return { dirty: status.stdout.trim().length > 0, headSha: head.stdout.trim() }; } @@ -1038,11 +1048,16 @@ export class SessionForkService { const resolvedBranch = this.deps.resolveGitBranch ? await this.deps.resolveGitBranch(sessionWorkdir) : ( - await execFileAsync('git', ['branch', '--show-current'], { - cwd: sessionWorkdir, - windowsHide: true, - timeout: 10_000, - }) + await runCommandTextLegacy( + { + command: 'git', + args: ['branch', '--show-current'], + cwd: sessionWorkdir, + timeout: 10_000, + check: 'exit-0', + }, + toShared() + ) ).stdout.trim() || undefined; const targetAcpSessionId = targetSession.acpSessionId; const branchName = resolvedBranch ?? targetMeta.baseBranch; diff --git a/apps/cli/src/session/session-sandbox.ts b/apps/cli/src/session/session-sandbox.ts index 483a1de8ce..0ae789b51b 100644 --- a/apps/cli/src/session/session-sandbox.ts +++ b/apps/cli/src/session/session-sandbox.ts @@ -5,10 +5,7 @@ import * as fs from 'fs/promises'; import { Effect, Exit, Scope } from 'effect'; import { type SessionId } from '@lody/shared'; -import { - makeProcessRunner as makePlatformRunner, - type ProcessRunner as PlatformRunner, -} from '@lody/shared/node/process'; +import { makeProcessRunnerLegacy, type ProcessRunnerLegacy } from '@lody/shared/node/process'; import { platformLayer } from '@/platform/process-options'; import { unwrapSpawnFailure } from '@lody/shared/node/process'; @@ -191,7 +188,7 @@ export function createSessionSandboxFactory( ...(options.deps ?? {}), }; const facade = { logger: options.logger, nodeProcess: toNodeProcess(deps) }; - const run = makePlatformRunner(toShared(facade)); + const run = makeProcessRunnerLegacy(toShared(facade)); let warnedUnsupportedPlatform = false; return async (sessionId: SessionId): Promise => { @@ -242,12 +239,12 @@ export function createNoopSessionSandbox( ): SessionSandbox { const deps: SessionSandboxDeps = { ...defaultSandboxDeps(), spawnProcess }; const facade = { nodeProcess: toNodeProcess(deps) }; - return createNoopSandbox(facade, makePlatformRunner(toShared(facade)), deps, description); + return createNoopSandbox(facade, makeProcessRunnerLegacy(toShared(facade)), deps, description); } function createNoopSandbox( facade: { logger?: Logger; nodeProcess: NodeProcessApi }, - run: PlatformRunner, + run: ProcessRunnerLegacy, deps: SessionSandboxDeps, description: string ): SessionSandbox { @@ -273,7 +270,7 @@ class ContainerSessionSandbox implements SessionSandbox { constructor( private container: ProcessContainer, private scope: Scope.Closeable, - private readonly run: PlatformRunner, + private readonly run: ProcessRunnerLegacy, private readonly logger: Logger | undefined, // The legacy noop sandbox is reusable; each new generation gets a fresh // Scope. A removed cgroup remains closed and never reopens implicitly. diff --git a/apps/cli/src/session/workspace-git-service.ts b/apps/cli/src/session/workspace-git-service.ts index 418b93d9db..62e7c8ff8f 100644 --- a/apps/cli/src/session/workspace-git-service.ts +++ b/apps/cli/src/session/workspace-git-service.ts @@ -1,15 +1,14 @@ -import { execFile } from 'node:child_process'; -import { promisify } from 'node:util'; +import { toShared } from '@/platform/process-options'; import { getLocalProjectGitHubRepoAtRootPath } from '@lody/shared/node/local-project'; import type { SessionId } from '@lody/shared'; import { resolveGitBranch, type SessionExec } from '@/lib/git/resolve-git-branch-name'; import type { LoroDocumentManager } from '@/lib/loro/doc'; +import { runCommandTextLegacy } from '@lody/shared/node/process'; + import { formatErrorMessage } from '@/utils/format-error'; import type { Logger } from '@/utils/logger'; import type { ISession } from './session-manager'; -const execFileAsync = promisify(execFile); - /** Git checkout facts, without provider API calls or credentials. */ export class WorkspaceGitService { private readonly pending = new Map>(); @@ -41,12 +40,17 @@ export class WorkspaceGitService { ownerSessionId, workspaceRoot, async (command, args, cwd) => { - const { stdout } = await execFileAsync(command, args, { - cwd, - encoding: 'utf8', - timeout: 10_000, - maxBuffer: 64 * 1024, - }); + const { stdout } = await runCommandTextLegacy( + { + command, + args, + cwd, + timeout: 10_000, + maxOutputBytes: 64 * 1024, + check: 'exit-0', + }, + toShared() + ); return stdout; }, true diff --git a/apps/cli/src/session/worktree/worktree-gc.ts b/apps/cli/src/session/worktree/worktree-gc.ts index 512411e248..701a3ac32c 100644 --- a/apps/cli/src/session/worktree/worktree-gc.ts +++ b/apps/cli/src/session/worktree/worktree-gc.ts @@ -1,16 +1,16 @@ -import { execFile } from 'child_process'; +import { toShared } from '@/platform/process-options'; import fs from 'fs'; import path from 'path'; -import { promisify } from 'util'; import type { RepoId, SessionId, SessionMeta } from '@lody/shared'; +import { runCommandTextLegacy } from '@lody/shared/node/process'; + import { formatErrorMessage } from '@/utils/format-error'; import type { Logger } from '@/utils/logger'; import { getWorktreeManager } from './worktree-manager'; -const execFileAsync = promisify(execFile); const GIT_PROBE_TIMEOUT_MS = 30_000; /** @@ -248,11 +248,16 @@ export class WorktreeGarbageCollector { private async isInsideGitWorkTree(dir: string): Promise { if (!fs.existsSync(dir)) return false; try { - const { stdout } = await execFileAsync('git', ['rev-parse', '--is-inside-work-tree'], { - cwd: dir, - timeout: GIT_PROBE_TIMEOUT_MS, - encoding: 'utf8', - }); + const { stdout } = await runCommandTextLegacy( + { + command: 'git', + args: ['rev-parse', '--is-inside-work-tree'], + cwd: dir, + timeout: GIT_PROBE_TIMEOUT_MS, + check: 'exit-0', + }, + toShared() + ); return stdout.trim() === 'true'; } catch { return false; diff --git a/apps/cli/src/session/worktree/worktree-manager-broker-auth.test.ts b/apps/cli/src/session/worktree/worktree-manager-broker-auth.test.ts index 290fb33c27..e4a6d1a40d 100644 --- a/apps/cli/src/session/worktree/worktree-manager-broker-auth.test.ts +++ b/apps/cli/src/session/worktree/worktree-manager-broker-auth.test.ts @@ -1,12 +1,13 @@ import { EventEmitter } from 'node:events'; import { Readable } from 'node:stream'; import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; -import { execFileSync, spawn } from 'node:child_process'; +import { execFileSync, spawn, type ChildProcess, type SpawnOptions } from 'node:child_process'; import os from 'node:os'; import path from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { MachineId, RepoId, SessionId, WorkspaceId } from '@lody/shared'; import type { Logger } from '@/utils/logger'; +import type { NodeProcessApi } from '@lody/shared/node/process'; import { SessionManager } from '../session-manager'; import type { SessionConfig } from '../types'; import type { LoroDocumentManager } from '@/lib/loro/doc'; @@ -14,34 +15,68 @@ import { createTestCloudPort } from '../../../tests/test-cloud-port'; import { materializeSpeculativeWorktree } from './speculative-worktree'; import { ensureGitHubGitTransport } from '@/lib/github-git-transport'; import type { GitCredentialBrokerAuth } from './worktree-manager'; +import { isGitExecutableNotFoundError } from './git-process-error'; -const spawnMock = vi.hoisted(() => vi.fn()); - -vi.mock('cross-spawn', () => ({ default: spawnMock })); vi.mock('@/utils/file-lock', () => ({ withFileLock: async (_name: string, fn: () => Promise): Promise => fn(), })); +type SpawnCall = { command: string; args: readonly string[]; options: SpawnOptions }; +type SpawnImpl = (command: string, args: readonly string[], options: SpawnOptions) => ChildProcess; + +/** Pids no OS hands out, so scripted children never collide with real ones. */ +const FAKE_PID_BASE = 2 ** 30; +let nextFakePid = FAKE_PID_BASE; +let spawnImpl: SpawnImpl; +const spawnCalls: SpawnCall[] = []; + +/** + * The process layer's OS seam. Scripted git children have fake pids that + * no signal can reach; children the native fixture really spawns get real + * signals, so their process groups are proven gone like in production. + */ +const nodeProcess: NodeProcessApi = { + platform: process.platform, + spawn: (command, args, options) => { + spawnCalls.push({ command, args, options }); + return spawnImpl(command, args, options); + }, + spawnSync: () => { + throw new Error('host git never runs synchronously'); + }, + kill: (pid, signal) => { + if (Math.abs(pid) >= FAKE_PID_BASE) { + throw Object.assign(new Error('kill ESRCH'), { code: 'ESRCH' }); + } + process.kill(pid, signal); + }, +}; + /** * Minimal stand-in for a git child process that exits successfully. * `stdout` is scripted per invocation so callers that parse output (fetchspec * probing, rev-parse) take their normal branches without a real repository. */ -function makeChild(stdout: string, stderr = '', code = 0) { +function makeChild(stdout: string, stderr = '', code = 0): ChildProcess { const child = new EventEmitter() as EventEmitter & { + pid: number; stdout: Readable; stderr: Readable; }; - child.stdout = Readable.from([stdout]); - child.stderr = Readable.from([stderr]); + child.pid = nextFakePid++; + child.stdout = Readable.from([Buffer.from(stdout)]); + child.stderr = Readable.from([Buffer.from(stderr)]); // Match child_process: close follows both output streams, including errors. let remainingStreams = 2; const ended = () => { - if (--remainingStreams === 0) child.emit('close', code); + if (--remainingStreams === 0) { + child.emit('exit', code, null); + child.emit('close', code, null); + } }; child.stdout.once('end', ended); child.stderr.once('end', ended); - return child; + return child as unknown as ChildProcess; } function createLogger(): Logger { @@ -64,15 +99,15 @@ const REPO_URL = 'https://github.com/owner/repo.git'; /** Env of the git invocation whose argv contains `verb`. */ function envOfGitCall(verb: string): NodeJS.ProcessEnv { - const call = spawnMock.mock.calls.find(([, args]) => (args as string[]).includes(verb)); + const call = spawnCalls.find(({ args }) => args.includes(verb)); if (!call) { throw new Error( - `no git invocation with "${verb}"; saw: ${spawnMock.mock.calls - .map(([, args]) => (args as string[]).join(' ')) + `no git invocation with "${verb}"; saw: ${spawnCalls + .map(({ args }) => args.join(' ')) .join(' | ')}` ); } - return (call[2] as { env: NodeJS.ProcessEnv }).env; + return call.options.env ?? {}; } describe('WorktreeManager host git credential broker routing', () => { @@ -80,15 +115,15 @@ describe('WorktreeManager host git credential broker routing', () => { let previousDataDir: string | undefined; beforeEach(() => { - spawnMock.mockReset(); - spawnMock.mockImplementation((_cmd: string, args: string[]) => { + spawnCalls.length = 0; + spawnImpl = (_cmd, args) => { // `remote.origin.fetch` already configured -> no `config --add` detour. if (args.includes('--get-all')) { return makeChild('+refs/heads/*:refs/remotes/origin/*\n'); } if (args.includes('rev-parse')) return makeChild('deadbeef\n'); return makeChild(''); - }); + }; previousDataDir = process.env.LODY_DATA_DIR; dataDir = mkdtempSync(path.join(os.tmpdir(), 'lody-broker-auth-')); @@ -203,27 +238,26 @@ process.stdout.write(input); ); let networkCalls = 0; let checkoutFailed = false; - spawnMock.mockImplementation( - (_command: string, args: string[], options: { cwd: string; env: NodeJS.ProcessEnv }) => { - if (retryCheckout && !checkoutFailed && args[0] === 'worktree' && args[1] === 'add') { - checkoutFailed = true; - return makeChild('', 'fatal: missing stale worktree registration', 1); - } - const verb = args.find((arg) => arg === 'fetch' || arg === 'clone'); - if (!verb) - return spawn( - checkoutCredentials ? process.execPath : git, - checkoutCredentials ? [wrapper, ...args] : args, - { - ...options, - env: { ...options.env, NODE_OPTIONS: `--require ${JSON.stringify(preload)}` }, - stdio: ['ignore', 'pipe', 'pipe'], - } - ); - networkCalls++; - // Ask actual Git to run the generated helper with EXACTLY the supplied - // auth env before serving pack data from the local fixture repository. - const script = ` + spawnImpl = (_command, args, options) => { + if (retryCheckout && !checkoutFailed && args[0] === 'worktree' && args[1] === 'add') { + checkoutFailed = true; + return makeChild('', 'fatal: missing stale worktree registration', 1); + } + const verb = args.find((arg) => arg === 'fetch' || arg === 'clone'); + if (!verb) + return spawn( + checkoutCredentials ? process.execPath : git, + checkoutCredentials ? [wrapper, ...args] : args, + { + ...options, + env: { ...options.env, NODE_OPTIONS: `--require ${JSON.stringify(preload)}` }, + stdio: ['ignore', 'pipe', 'pipe'], + } + ); + networkCalls++; + // Ask actual Git to run the generated helper with EXACTLY the supplied + // auth env before serving pack data from the local fixture repository. + const script = ` const { spawnSync } = require('node:child_process'); const args = ${JSON.stringify(args)}; const index = args.indexOf(${JSON.stringify(verb)}); @@ -238,13 +272,12 @@ const operation = ${JSON.stringify(verb)} === 'clone' const result = spawnSync(${JSON.stringify(git)}, operation, { env: process.env, stdio: 'inherit' }); process.exit(result.status ?? 1); `; - return spawn(process.execPath, ['-e', script], { - ...options, - env: { ...options.env, NODE_OPTIONS: `--require ${JSON.stringify(preload)}` }, - stdio: ['ignore', 'pipe', 'pipe'], - }); - } - ); + return spawn(process.execPath, ['-e', script], { + ...options, + env: { ...options.env, NODE_OPTIONS: `--require ${JSON.stringify(preload)}` }, + stdio: ['ignore', 'pipe', 'pipe'], + }); + }; const auth = async (context: string): Promise => { const stateFilePath = path.join(dataDir, context + '-broker.json'); mkdirSync(stateFilePath + '.contexts', { recursive: true }); @@ -453,6 +486,7 @@ process.exit(result.status ?? 1); repoId: REPO_ID, source: { kind: 'github', repoUrl: REPO_URL }, logger: createLogger(), + nodeProcess, }); } @@ -506,13 +540,31 @@ process.exit(result.status ?? 1); LODY_GIT_CRED_CONTEXT_TOKEN: 'frozen-requester', LODY_GIT_CRED_CONTEXT_FILE: undefined, }); - const args = spawnMock.mock.calls.find(([, invocationArgs]) => - (invocationArgs as string[]).includes(verb) - )?.[1] as string[]; + const args = spawnCalls.find((call) => call.args.includes(verb))?.args ?? []; expect(args.some((arg) => arg.includes('credential.https://github.com.helper'))).toBe(false); } ); + it('reports a missing git executable through the process layer', async () => { + rmSync(path.join(dataDir, 'repos', REPO_ID, 'bare.git'), { recursive: true }); + spawnImpl = () => { + // What Node does for a missing executable: no pid, then an async ENOENT. + const child = new EventEmitter(); + queueMicrotask(() => + child.emit('error', Object.assign(new Error('spawn git ENOENT'), { code: 'ENOENT' })) + ); + return child as unknown as ChildProcess; + }; + const manager = await newManager(); + + const failure = await manager.ensureRepo().then( + () => undefined, + (error: unknown) => error + ); + + expect(isGitExecutableNotFoundError(failure)).toBe(true); + }); + it('uses native Git without installing managed credentials or borrowing an ambient broker', async () => { // Local platform has no token manager and therefore no broker; host git must // keep working off whatever the environment already provides. @@ -525,8 +577,10 @@ process.exit(result.status ?? 1); const env = envOfGitCall('fetch'); expect(env.LODY_GIT_CRED_BROKER_URL).toBeUndefined(); expect(env.LODY_GIT_CRED_BROKER_TOKEN).toBeUndefined(); - expect( - spawnMock.mock.calls.find(([, args]) => (args as string[]).includes('fetch'))?.[1] - ).toEqual(['fetch', 'origin', '--prune']); + expect(spawnCalls.find(({ args }) => args.includes('fetch'))?.args).toEqual([ + 'fetch', + 'origin', + '--prune', + ]); }); }); diff --git a/apps/cli/src/session/worktree/worktree-manager.ts b/apps/cli/src/session/worktree/worktree-manager.ts index 7dba0738e5..ecefd6d15c 100644 --- a/apps/cli/src/session/worktree/worktree-manager.ts +++ b/apps/cli/src/session/worktree/worktree-manager.ts @@ -1,7 +1,7 @@ +import { toShared } from '@/platform/process-options'; import { GITHUB_CREDENTIAL_ENV_KEYS } from '@/lib/gh-token-env'; import { RepoId, SessionId } from '@lody/shared'; import { resolveLocalProjectBranchAtRootPath } from '@lody/shared/node/local-project'; -import spawn from 'cross-spawn'; import * as fs from 'fs'; import * as path from 'path'; import { Logger } from '@/utils/logger'; @@ -9,6 +9,10 @@ import { withFileLock } from '@/utils/file-lock'; import { redactUrlAuth } from '@/utils/github'; import { getCredentialHelperHostPath } from '@/lib/git-credential-helper-script'; import { formatErrorMessage } from '@/utils/format-error'; +import { runCommandTextLegacy } from '@lody/shared/node/process'; + +import { CommandTimedOut } from '@lody/shared/node/process'; +import type { NodeProcessApi } from '@lody/shared/node/process'; import { ensureLodyDataDir, getLodyDataDir } from '@lody/shared/node/installation-profile'; import { mapGitSpawnError } from './git-process-error'; import { @@ -90,6 +94,8 @@ export interface WorktreeManagerConfig { source?: WorktreeManagerSource; repoUrl?: string; logger: Logger; + /** OS process seam of the process layer; tests substitute a fake. */ + nodeProcess?: NodeProcessApi; } type RepoFetchMode = 'skip' | 'best-effort' | 'required'; @@ -162,6 +168,10 @@ const DEFAULT_ARCHIVE_BACKUP_AUTHOR_EMAIL = 'archive@lody.ai'; // staleness window (30 min) so a stalled git process releases the repo lock // by failing instead of looking like a live holder. const GIT_OPERATION_TIMEOUT_MS = 10 * 60 * 1000; +// Git output used to be collected without a ceiling. `status --porcelain` or +// `for-each-ref` in a large repository can exceed the process layer's 1 MiB +// default, so git keeps a ceiling that only a runaway command reaches. +const GIT_MAX_OUTPUT_BYTES = 64 * 1024 * 1024; /** * Per-repo file lock for git operations (cross-process safe) @@ -287,6 +297,7 @@ export class WorktreeManager { private source: WorktreeManagerSource; private repoUrl?: string; private readonly logger: Logger; + private readonly nodeProcess?: NodeProcessApi; /** Base directory on host: /repos */ private readonly baseDir: string; @@ -304,6 +315,7 @@ export class WorktreeManager { this.source = config.source ?? { kind: 'github', repoUrl: config.repoUrl }; this.repoUrl = this.source.kind === 'github' ? this.source.repoUrl : undefined; this.logger = config.logger; + this.nodeProcess = config.nodeProcess; this.baseDir = path.join(getLodyDataDir(), 'repos'); this.repoDir = path.join(this.baseDir, this.repoId); @@ -337,72 +349,44 @@ export class WorktreeManager { } } - private runGit(args: string[], cwd: string, env?: NodeJS.ProcessEnv): Promise { + private async runGit(args: string[], cwd: string, env?: NodeJS.ProcessEnv): Promise { const mergedEnv: NodeJS.ProcessEnv = { ...process.env, ...env, GIT_TERMINAL_PROMPT: '0', }; this.logger.debug(`[${this.repoId}] Running git ${args.join(' ')}`); - return new Promise((resolve, reject) => { - let child: ReturnType; - try { - child = spawn('git', args, { + let result: { code: number | null; stdout: string; stderr: string }; + try { + // Git has no deadline of its own for stalled network operations; without this a + // hung fetch/clone would pin the per-repo worktree lock until the file-lock + // staleness window frees it. The timeout ends git's whole process tree + // (remote helpers, credential helpers), not just git itself. + result = await runCommandTextLegacy( + { + command: 'git', + args, cwd, env: mergedEnv, - stdio: ['ignore', 'pipe', 'pipe'], - windowsHide: true, + timeout: GIT_OPERATION_TIMEOUT_MS, + maxOutputBytes: GIT_MAX_OUTPUT_BYTES, + check: 'none', + }, + toShared({ nodeProcess: this.nodeProcess }) + ); + } catch (error) { + if (error instanceof CommandTimedOut) { + throw new Error(`git ${args.join(' ')} timed out after ${GIT_OPERATION_TIMEOUT_MS}ms`, { + cause: error, }); - } catch (error) { - reject(mapGitSpawnError(error, cwd)); - return; } - - let stdout = ''; - let stderr = ''; - let timedOut = false; - - // Git has no deadline of its own for stalled network operations; without this a - // hung fetch/clone would pin the per-repo worktree lock until the file-lock - // staleness window frees it. - const timeoutTimer = setTimeout(() => { - timedOut = true; - child.kill('SIGKILL'); - }, GIT_OPERATION_TIMEOUT_MS); - timeoutTimer.unref(); - - // Use setEncoding to handle UTF-8 multibyte boundaries correctly - // (e.g., non-ASCII branch names, file paths, user.name) - child.stdout?.setEncoding('utf8'); - child.stderr?.setEncoding('utf8'); - - child.stdout?.on('data', (chunk: string) => { - stdout += chunk; - }); - - child.stderr?.on('data', (chunk: string) => { - stderr += chunk; - }); - - child.on('error', (error) => { - clearTimeout(timeoutTimer); - reject(mapGitSpawnError(error, cwd)); - }); - - child.on('close', (code) => { - clearTimeout(timeoutTimer); - if (timedOut) { - reject(new Error(`git ${args.join(' ')} timed out after ${GIT_OPERATION_TIMEOUT_MS}ms`)); - return; - } - if (code !== 0) { - const details = (stderr || stdout || '').trim(); - reject(new Error(details ? details : `git exited with code ${code}`)); - return; - } - resolve(stdout.trim()); - }); - }); + throw mapGitSpawnError(error, cwd); + } + if (result.code !== 0) { + const details = (result.stderr || result.stdout || '').trim(); + throw new Error(details ? details : `git exited with code ${result.code}`); + } + return result.stdout.trim(); } private isLocalSharedSource(): boolean { @@ -615,39 +599,19 @@ export class WorktreeManager { env: NodeJS.ProcessEnv; }): Promise<{ exitCode: number | null; returnedCredentials: boolean; stderrNonEmpty: boolean }> { const input = `protocol=https\nhost=${options.host}\npath=/${options.repoFullName}.git\n\n`; - return await new Promise((resolve, reject) => { - const child = spawn('node', [options.helperPath, 'get'], { + const { code, stdout, stderr } = await runCommandTextLegacy( + { + command: 'node', + args: [options.helperPath, 'get'], env: options.env, - stdio: ['pipe', 'pipe', 'pipe'], - windowsHide: true, - }); - - let stdout = ''; - let stderr = ''; - - // Use setEncoding to handle UTF-8 multibyte boundaries correctly - child.stdout?.setEncoding('utf8'); - child.stderr?.setEncoding('utf8'); - - child.stdout?.on('data', (chunk: string) => { - stdout += chunk; - }); - child.stderr?.on('data', (chunk: string) => { - stderr += chunk; - }); - - child.on('error', (error) => reject(error)); - child.on('close', (code) => { - const returnedCredentials = - stdout.includes('username=') && stdout.includes('\npassword=') && stdout.includes('\n\n'); - resolve({ exitCode: code, returnedCredentials, stderrNonEmpty: stderr.trim().length > 0 }); - }); - - if (child.stdin) { - child.stdin.write(input); - child.stdin.end(); - } - }); + input, + check: 'none', + }, + toShared({ nodeProcess: this.nodeProcess }) + ); + const returnedCredentials = + stdout.includes('username=') && stdout.includes('\npassword=') && stdout.includes('\n\n'); + return { exitCode: code, returnedCredentials, stderrNonEmpty: stderr.trim().length > 0 }; } /** diff --git a/apps/cli/src/session/worktree/worktree-setup-runner.ts b/apps/cli/src/session/worktree/worktree-setup-runner.ts index d25c440c88..f63b39a38d 100644 --- a/apps/cli/src/session/worktree/worktree-setup-runner.ts +++ b/apps/cli/src/session/worktree/worktree-setup-runner.ts @@ -1,4 +1,4 @@ -import { spawn } from 'node:child_process'; +import { toShared } from '@/platform/process-options'; import { randomUUID } from 'node:crypto'; import { DEFAULT_WORKTREE_CLEANUP_TIMEOUT_MS, @@ -12,9 +12,18 @@ import { type WorktreeSetupShell, type WorkspaceId, } from '@lody/shared'; +import { startProcessLegacy } from '@lody/shared/node/process'; + +import type { TerminationPolicy } from '@lody/shared/node/process'; import type { Logger } from '@/utils/logger'; +import { formatErrorMessage } from '@/utils/format-error'; const OUTPUT_TAIL_MAX_CHARS = 16_000; +/** + * How a failed or timed-out script is ended: SIGTERM to the shell's whole + * process group (every command the script started), then SIGKILL. + */ +const SCRIPT_TERMINATION_POLICY: TerminationPolicy = { graceMs: 2_000, killWaitMs: 2_000 }; export type WorktreeScriptOutputStream = 'stdout' | 'stderr'; @@ -227,21 +236,31 @@ async function runWorktreeScript(options: { let childClosed = false; let childExitStatus: WorktreeScriptEndEvent['exitStatus']; let waitForCloseResolve: (() => void) | null = null; - const child = spawn(command, args, { - cwd: options.workdir, - windowsHide: true, - env: { - ...process.env, - LODY_WORKSPACE_ID: options.workspaceId, - LODY_SESSION_ID: options.sessionId, - LODY_WORKTREE_PATH: options.workdir, - LODY_WORKTREE_BRANCH: options.branch, - LODY_WORKTREE_SCRIPT_PHASE: options.phase, - ...(options.repoFullName ? { LODY_GITHUB_REPO: options.repoFullName } : {}), - ...(options.localProjectId ? { LODY_LOCAL_PROJECT_ID: options.localProjectId } : {}), + // The shell leads its own process group so a failure or timeout ends every + // command the script started, not just the shell. + const shellProcess = startProcessLegacy( + { + command, + args, + options: { + cwd: options.workdir, + env: { + ...process.env, + LODY_WORKSPACE_ID: options.workspaceId, + LODY_SESSION_ID: options.sessionId, + LODY_WORKTREE_PATH: options.workdir, + LODY_WORKTREE_BRANCH: options.branch, + LODY_WORKTREE_SCRIPT_PHASE: options.phase, + ...(options.repoFullName ? { LODY_GITHUB_REPO: options.repoFullName } : {}), + ...(options.localProjectId ? { LODY_LOCAL_PROJECT_ID: options.localProjectId } : {}), + }, + stdio: ['pipe', 'pipe', 'pipe'], + }, + processGroup: true, }, - stdio: ['pipe', 'pipe', 'pipe'], - }); + toShared() + ); + const child = shellProcess.child; const emitOutput = (step: RunningStep, stream: WorktreeScriptOutputStream, chunk: string) => { if (stream === 'stdout') { @@ -412,17 +431,24 @@ async function runWorktreeScript(options: { } settled = true; clearTimeout(timeout); - if (error && !child.killed && !childClosed) { - child.kill('SIGTERM'); - } - void Promise.resolve( - options.events?.onEnd?.({ - phase: options.phase, - status, - exitStatus, - error, - }) - ) + // A successful script may leave background services running on purpose; + // a failed one must not leave anything behind, even after its shell exited. + const terminated = error + ? shellProcess.terminate(SCRIPT_TERMINATION_POLICY).catch((terminationError: unknown) => { + options.logger.warn( + `[${options.sessionId}] Worktree ${options.phase} processes did not exit: ${formatErrorMessage(terminationError)}` + ); + }) + : Promise.resolve(); + void terminated + .then(() => + options.events?.onEnd?.({ + phase: options.phase, + status, + exitStatus, + error, + }) + ) .then(() => { if (error) { reject(error); @@ -461,14 +487,14 @@ async function runWorktreeScript(options: { settle('failed', error, { signal: 'SIGTERM' }); }, timeoutMs); - child.stdout.on('data', (chunk: Buffer) => { + child.stdout?.on('data', (chunk: Buffer) => { const text = chunk.toString('utf8'); if (!activeStep) { return; } consumeStepStream(activeStep, 'stdout', text); }); - child.stderr.on('data', (chunk: Buffer) => { + child.stderr?.on('data', (chunk: Buffer) => { const text = chunk.toString('utf8'); if (!activeStep) { return; diff --git a/apps/cli/src/utils/memory.ts b/apps/cli/src/utils/memory.ts index b16f8cd03e..500b1f4321 100644 --- a/apps/cli/src/utils/memory.ts +++ b/apps/cli/src/utils/memory.ts @@ -1,9 +1,11 @@ -import { execFile } from 'child_process'; +import { toShared } from '@/platform/process-options'; import { readFileSync } from 'fs'; import os from 'os'; -import { promisify } from 'util'; -const execFileAsync = promisify(execFile); +import { READ_ONLY_ABANDON_POLICY } from '@lody/shared/node/process'; + +import { runCommandTextLegacy } from '@lody/shared/node/process'; + const DARWIN_MEMORY_PROBE_TIMEOUT_MS = 1_000; /** @@ -260,16 +262,17 @@ async function probeWindowsMemoryStatus(): Promise { ].join('; '); try { - const { stdout } = await execFileAsync( - 'powershell.exe', - ['-NoProfile', '-NonInteractive', '-ExecutionPolicy', 'Bypass', '-Command', script], + const { stdout } = await runCommandTextLegacy( { - encoding: 'utf8', + command: 'powershell.exe', + args: ['-NoProfile', '-NonInteractive', '-ExecutionPolicy', 'Bypass', '-Command', script], timeout: WINDOWS_MEMORY_PROBE_TIMEOUT_MS, - windowsHide: true, - } + abandonPolicy: READ_ONLY_ABANDON_POLICY, + check: 'exit-0', + }, + toShared() ); - return parseWindowsMemoryStatus(String(stdout ?? ''), os.totalmem()); + return parseWindowsMemoryStatus(stdout, os.totalmem()); } catch { return null; } @@ -523,15 +526,17 @@ async function getDarwinMemoryPressureLevel(): Promise { } try { - const { stdout } = await execFileAsync('vm_stat', [], { - encoding: 'utf8', - timeout: DARWIN_MEMORY_PROBE_TIMEOUT_MS, - }); - const parsed = parseDarwinAvailableMemoryBytes(String(stdout ?? '')); + const { stdout } = await runCommandTextLegacy( + { + command: 'vm_stat', + args: [], + timeout: DARWIN_MEMORY_PROBE_TIMEOUT_MS, + abandonPolicy: READ_ONLY_ABANDON_POLICY, + check: 'exit-0', + }, + toShared() + ); + const parsed = parseDarwinAvailableMemoryBytes(stdout); if (parsed !== null) { return Math.max(parsed, os.freemem()); } diff --git a/apps/cli/src/utils/open-browser.ts b/apps/cli/src/utils/open-browser.ts index 99a23bab21..9a2f299c46 100644 --- a/apps/cli/src/utils/open-browser.ts +++ b/apps/cli/src/utils/open-browser.ts @@ -1,30 +1,52 @@ -import { exec } from 'child_process'; +import { toShared } from '@/platform/process-options'; +import { startProcessLegacy } from '@lody/shared/node/process'; -function buildCommand(url: string): string { - const quotedUrl = `"${url.replace(/"/g, '\\"')}"`; - switch (process.platform) { +type OpenCommand = { readonly command: string; readonly args: readonly string[] }; + +/** + * The platform's URL opener as an explicit command. Windows uses the URL + * protocol handler directly (as Go's `pkg/browser` does) instead of cmd's + * `start`, so the URL never passes through cmd's metacharacter parsing. + */ +function buildOpenBrowserCommand(url: string, platform: NodeJS.Platform): OpenCommand { + switch (platform) { case 'darwin': - return `open ${quotedUrl}`; + return { command: 'open', args: [url] }; case 'win32': - // Use cmd to ensure the built-in START command is available. - return `cmd /c start "" ${quotedUrl.replace(/&/g, '^&')}`; + return { command: 'rundll32', args: ['url.dll,FileProtocolHandler', url] }; default: - return `xdg-open ${quotedUrl}`; + return { command: 'xdg-open', args: [url] }; } } +/** + * Resolves once the opener exits 0. The opener is never terminated: on Linux a + * browser it starts can stay in its process tree, and must outlive this call. + */ export async function openBrowser(url: string): Promise { - const command = buildCommand(url); - - await new Promise((resolve, reject) => { - const child = exec(command, { windowsHide: true }, (error) => { - if (error) { - reject(error); - } else { - resolve(); - } - }); - - child.on('error', reject); + const { command, args } = buildOpenBrowserCommand(url, process.platform); + const handle = startProcessLegacy( + { + command, + args, + // rundll32 passes its show state on to ShellExecute: hidden, a browser or + // the desktop app cold-started for the URL would open without a window. + options: { stdio: 'ignore', windowsHide: false }, + processGroup: false, + }, + toShared() + ); + let spawnError: unknown; + handle.child.once('error', (error) => { + spawnError = error; }); + const exit = await handle.exited; + if (spawnError !== undefined) { + throw spawnError; + } + if (exit.code !== 0) { + throw new Error( + `${command} failed to open the browser (${exit.signal ?? `exit ${exit.code}`})` + ); + } } diff --git a/apps/cli/tests/e2e/quick-tunnel.e2e.test.ts b/apps/cli/tests/e2e/quick-tunnel.e2e.test.ts index 74a3cdf53d..cadde995fc 100644 --- a/apps/cli/tests/e2e/quick-tunnel.e2e.test.ts +++ b/apps/cli/tests/e2e/quick-tunnel.e2e.test.ts @@ -47,6 +47,10 @@ describe.skipIf(process.env.LODY_QUICK_TUNNEL_E2E !== '1')('real Quick Tunnel tr platform: 'node', format: 'esm', target: 'node22', + // Bundled CommonJS dependencies (cross-spawn) require Node built-ins. + banner: { + js: "import { createRequire as __lodyCreateRequire } from 'node:module'; const require = __lodyCreateRequire(import.meta.url);", + }, }); server.listen(0, '127.0.0.1'); await once(server, 'listening'); diff --git a/apps/cli/tests/login-shell-env.test.ts b/apps/cli/tests/login-shell-env.test.ts index 7f8285aa03..3cc2e57be0 100644 --- a/apps/cli/tests/login-shell-env.test.ts +++ b/apps/cli/tests/login-shell-env.test.ts @@ -3,9 +3,9 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; // Mock the underlying probe so we can drive its timing deterministically. The // opt-out test never reaches it (shouldSkip short-circuits), so the default // vi.fn() is harmless there. -vi.mock('shell-env', () => ({ shellEnv: vi.fn() })); +vi.mock('@lody/shared/node/login-shell-env', () => ({ probeLoginShellEnv: vi.fn() })); -import { shellEnv } from 'shell-env'; +import { probeLoginShellEnv } from '@lody/shared/node/login-shell-env'; import { getCachedLoginShellEnvSync, @@ -33,7 +33,7 @@ describe('login-shell-env slow-probe recovery', () => { beforeEach(() => { delete process.env.LODY_DISABLE_SHELL_ENV; resetLoginShellEnvCache(); - vi.mocked(shellEnv).mockReset(); + vi.mocked(probeLoginShellEnv).mockReset(); }); afterEach(() => { @@ -43,9 +43,9 @@ describe('login-shell-env slow-probe recovery', () => { it('serves the real env to later awaiters once a slow probe lands after the timeout', async () => { vi.useFakeTimers(); - let resolveProbe!: (env: NodeJS.ProcessEnv) => void; - vi.mocked(shellEnv).mockReturnValue( - new Promise((resolve) => { + let resolveProbe!: (env: NodeJS.ProcessEnv | null) => void; + vi.mocked(probeLoginShellEnv).mockReturnValue( + new Promise((resolve) => { resolveProbe = resolve; }) ); diff --git a/apps/cli/tests/worktree-setup-runner.test.ts b/apps/cli/tests/worktree-setup-runner.test.ts index 47d06de7a8..6b660eec7a 100644 --- a/apps/cli/tests/worktree-setup-runner.test.ts +++ b/apps/cli/tests/worktree-setup-runner.test.ts @@ -140,4 +140,43 @@ describe('worktree setup runner', () => { expect(outputs).toHaveLength(0); expect(ends).toHaveLength(0); }); + + it.skipIf(process.platform === 'win32')( + 'ends every process a failed script started, not just its shell', + async () => { + const workdir = await fs.mkdtemp(path.join(os.tmpdir(), 'lody-worktree-setup-')); + tempDirs.push(workdir); + const pidFile = path.join(workdir, 'background.pid'); + + // The first step leaves a background job in the shell's process group; + // the second fails, which must take that job down with the shell. + const failure = await runWorktreeSetup({ + config: { + scripts: { bash: `sleep 600 &\necho $! > ${JSON.stringify(pidFile)}\nfalse` }, + }, + sessionId: 'session-1' as SessionId, + workspaceId: 'workspace-1' as WorkspaceId, + workdir, + branch: 'feature/worktree-setup-failure', + logger: testLogger, + }).then( + () => undefined, + (error: unknown) => error + ); + + expect(failure).toBeInstanceOf(Error); + const backgroundPid = Number((await fs.readFile(pidFile, 'utf8')).trim()); + expect(backgroundPid).toBeGreaterThan(0); + expect(isAlive(backgroundPid)).toBe(false); + } + ); }); + +function isAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch (error) { + return (error as NodeJS.ErrnoException).code === 'EPERM'; + } +} diff --git a/package.json b/package.json index 048db69175..23912b9aca 100644 --- a/package.json +++ b/package.json @@ -40,13 +40,14 @@ "lint:i18n": "node scripts/check-i18n.mjs", "check:code-collab-imports": "node scripts/check-code-collab-imports.mjs", "check:platform-boundaries": "node scripts/check-platform-boundaries.mjs", + "check:cli-process-boundary": "node scripts/check-cli-process-boundary.mjs", "check:public-boundary": "node scripts/check-public-boundary.mjs", - "check:quick": "corepack pnpm lint && corepack pnpm lint:i18n && corepack pnpm check:code-collab-imports && corepack pnpm check:platform-boundaries && corepack pnpm check:public-boundary", + "check:quick": "corepack pnpm lint && corepack pnpm lint:i18n && corepack pnpm check:code-collab-imports && corepack pnpm check:platform-boundaries && corepack pnpm check:cli-process-boundary && corepack pnpm check:public-boundary", "check:heavy": "corepack pnpm check", "generate:acp-registry": "node scripts/generate-acp-registry.mjs", "generate:open-source-attributions": "node scripts/generate-open-source-attributions.mjs", "package:kimi-runtime": "node scripts/package-kimi-runtime.mjs", - "check": "corepack pnpm typecheck && corepack pnpm lint && corepack pnpm test:ci && corepack pnpm lint:i18n && corepack pnpm check:code-collab-imports && corepack pnpm check:platform-boundaries && corepack pnpm check:public-boundary", + "check": "corepack pnpm typecheck && corepack pnpm lint && corepack pnpm test:ci && corepack pnpm lint:i18n && corepack pnpm check:code-collab-imports && corepack pnpm check:platform-boundaries && corepack pnpm check:cli-process-boundary && corepack pnpm check:public-boundary", "install:all": "corepack pnpm install", "build:electron": "corepack pnpm --dir apps/electron build", "build:electron:mac": "corepack pnpm --dir apps/electron build:mac", diff --git a/packages/shared/src/node/AGENTS.md b/packages/shared/src/node/AGENTS.md index 6885770d08..eee7fc9c40 100644 --- a/packages/shared/src/node/AGENTS.md +++ b/packages/shared/src/node/AGENTS.md @@ -15,14 +15,17 @@ record: [process tree layer](../../../../.agents/notes/implemented/architecture/ - Effect callers use `effect/process` commands and `ChildProcessSpawner`, provided by `ProcessSpawnerLive` with Lody's bounded backend. Process acquisition requires Scope; only legacy Promise entry points use raw handle compatibility. + Temporary execution entry points carry the `Legacy` suffix and `@deprecated`; + keep it visible in imports and calls. New Effect workflows compose core APIs + and execute once at their owning application entry point. Use the workspace Effect v4 catalog and `Context.Service` / `Layer.effect`; follow the pinned-version APIs in cli-effect-ts, not v3 compatibility helpers. - New or refactored process callers use the official Effect service or shared legacy Promise entry points; never add raw spawn or kill paths, and never put an Effect through a Promise facade and wrap it back into Effect. - ACP callers are migrated in this layer. Existing CLI callers, Node git/lock - helpers, Electron and supervisor migrate in subsequent PRs; the automated - guard is introduced with those consumers. + ACP and remaining CLI callers use this layer, enforced by the CLI process guard. + Existing Node git/lock helpers, Electron and supervisor migrate in the next PR; + the guard expands with those consumers. - `process.ts` stays one module with no relative imports: Electron's `node --test` cannot resolve extensionless relative imports. - Missing a capability (a new spawn shape, a pid-only kill)? Add it to diff --git a/packages/shared/src/node/login-shell-env.ts b/packages/shared/src/node/login-shell-env.ts new file mode 100644 index 0000000000..643a580bd7 --- /dev/null +++ b/packages/shared/src/node/login-shell-env.ts @@ -0,0 +1,139 @@ +/** + * The user's login-shell environment, probed once through the process layer. + * + * GUI and daemon launches (macOS launchd, Linux .desktop, systemd, npx) inherit + * a minimal PATH without the directories users install tools into (Homebrew, + * nvm, volta, `~/.local/bin`, editor CLIs). Both the CLI and the desktop read + * the login shell's environment to recover them; this is their one probe. + */ +import { userInfo } from 'node:os'; + +import { Duration } from 'effect'; + +import { + CommandTimedOut, + READ_ONLY_ABANDON_POLICY, + runCommandTextLegacy, + type ProcessFacadeOptions, +} from './process'; + +const DELIMITER = '_LODY_SHELL_ENV_DELIMITER_'; +/** Verbose rc files (`set -x`) write to stderr; that must not fail the probe. */ +const MAX_OUTPUT_BYTES = 8 * 1024 * 1024; +/** + * Rc files (nvm, conda, oh-my-zsh) can take seconds on a cold login; a shell + * past this is stuck. One budget for the whole probe, fallbacks included. + */ +const DEFAULT_TIMEOUT = Duration.seconds(15); +/** POSIX shells to try when the default one fails (for example Nushell). */ +const FALLBACK_SHELLS = ['/bin/zsh', '/bin/bash']; + +/** + * The script each shell runs. `command env` skips aliases and functions named + * `env`; `-0` keeps values with newlines intact, with plain `env` for one + * without it (BusyBox); the delimiters separate the environment from whatever + * the rc files print. They go through `printf`: macOS `/bin/sh` prints + * `echo -n X` as `-n X` plus a newline. Interactive login bash reads + * `~/.bash_profile` but not `~/.bashrc`, where most PATH edits live. + */ +const probeScript = (shell: string): string => + `${shell.endsWith('/bash') ? 'source ~/.bashrc >/dev/null 2>&1 || true; ' : ''}` + + `printf '%s' "${DELIMITER}"; command env -0 2>/dev/null || command env; printf '%s' "${DELIMITER}"; exit`; + +/** + * Keep rc files from blocking the probe (oh-my-zsh auto-update, tmux + * autostart). The shell prints them back; they are not the user's settings. + */ +const PROBE_ENV: Record = { + DISABLE_AUTO_UPDATE: 'true', + ZSH_TMUX_AUTOSTARTED: 'true', + ZSH_TMUX_AUTOSTART: 'false', +}; + +export const parseLoginShellEnvOutput = (stdout: string): NodeJS.ProcessEnv | null => { + const section = stdout.split(DELIMITER)[1]; + if (section === undefined) return null; + const parsed: NodeJS.ProcessEnv = {}; + for (const entry of section.split(section.includes('\0') ? '\0' : '\n')) { + const separator = entry.indexOf('='); + if (separator <= 0) continue; + parsed[entry.slice(0, separator)] = entry.slice(separator + 1); + } + return Object.keys(parsed).length > 0 ? parsed : null; +}; + +/** Undo what the probe itself injected: the base value, or nothing. */ +const withoutProbeEnv = ( + parsed: NodeJS.ProcessEnv, + baseEnv: NodeJS.ProcessEnv +): NodeJS.ProcessEnv => { + const result = { ...parsed }; + for (const key of Object.keys(PROBE_ENV)) { + if (baseEnv[key] === undefined) delete result[key]; + else result[key] = baseEnv[key]; + } + return result; +}; + +const defaultShell = (env: NodeJS.ProcessEnv): string => { + try { + const { shell } = userInfo(); + if (shell) return shell; + } catch { + // No passwd entry (a container user); fall back to the environment. + } + return env.SHELL || (process.platform === 'darwin' ? '/bin/zsh' : '/bin/sh'); +}; + +export interface LoginShellEnvOptions { + /** + * The whole probe's budget, fallback shells included (15 s by default). A + * shell still running at the deadline is ended with its process tree. + */ + readonly timeout?: Duration.Input; + /** The environment the shell starts from; defaults to `process.env`. */ + readonly env?: NodeJS.ProcessEnv; + /** Defaults to the user's login shell. */ + readonly shell?: string; + readonly processOptions?: ProcessFacadeOptions; +} + +/** + * Run the login shell interactively and return the environment it ends with, + * or null (Windows, or no shell produced one). A shell that fails to start or + * exits unsuccessfully falls back to zsh, then bash; a shell that times out + * does not, because its rc files are what hung. + */ +export const probeLoginShellEnv = async ( + options: LoginShellEnvOptions +): Promise => { + if (process.platform === 'win32') return null; + const baseEnv = options.env ?? process.env; + const first = options.shell ?? defaultShell(baseEnv); + const shells = [first, ...FALLBACK_SHELLS.filter((shell) => shell !== first)]; + const deadline = Date.now() + Duration.toMillis(options.timeout ?? DEFAULT_TIMEOUT); + for (const shell of shells) { + const remainingMs = deadline - Date.now(); + if (remainingMs <= 0) return null; + try { + const output = await runCommandTextLegacy( + { + command: shell, + args: ['-ilc', probeScript(shell)], + env: { ...baseEnv, ...PROBE_ENV }, + timeout: remainingMs, + // An interactive shell ignores SIGTERM; a grace period only delays. + abandonPolicy: READ_ONLY_ABANDON_POLICY, + maxOutputBytes: MAX_OUTPUT_BYTES, + check: 'exit-0', + }, + options.processOptions + ); + const parsed = parseLoginShellEnvOutput(output.stdout); + if (parsed) return withoutProbeEnv(parsed, baseEnv); + } catch (error) { + if (error instanceof CommandTimedOut) return null; + } + } + return null; +}; diff --git a/packages/shared/src/node/process.ts b/packages/shared/src/node/process.ts index 7caa02b9b3..055453646f 100644 --- a/packages/shared/src/node/process.ts +++ b/packages/shared/src/node/process.ts @@ -1018,19 +1018,17 @@ export const runCommand = ( spec.abandonPolicy ?? ABANDONED_COMMAND_POLICY, true ); - const handle = yield* spawner - .spawn(command) - .pipe( - Effect.mapError((error) => - error.cause instanceof SpawnFailed - ? error.cause - : new SpawnFailed({ - command: spec.command, - message: error.message, - cause: error.cause, - }) - ) - ); + const handle = yield* spawner.spawn(command).pipe( + Effect.mapError((error) => + error.cause instanceof SpawnFailed + ? error.cause + : new SpawnFailed({ + command: spec.command, + message: error.message, + cause: error.cause, + }) + ) + ); const collect = (stream: Stream.Stream) => Effect.gen(function* () { let bytes = 0; @@ -1238,7 +1236,8 @@ export interface ProcessFacadeOptions { readonly signal?: AbortSignal; } -export type ProcessRunner = ( +/** @deprecated The temporary legacy execution contract; native Effect callers use the core API. */ +export type ProcessRunnerLegacy = ( effect: Effect.Effect ) => Promise; @@ -1253,7 +1252,11 @@ export const processLayer = ( ) ); -export const runPromiseSquashed = ( +/** + * @deprecated Only for unmigrated Promise callers. New Effect workflows compose + * core Effects and leave execution to their owning application entry point. + */ +export const runPromiseSquashedLegacy = ( effect: Effect.Effect, options?: Effect.RunOptions ): Promise => @@ -1272,9 +1275,14 @@ const runSyncSquashed = (effect: Effect.Effect): A => { export const unwrapSpawnFailure = (error: unknown): unknown => error instanceof SpawnFailed && error.cause instanceof Error ? error.cause : error; -export const makeProcessRunner = (options: ProcessFacadeOptions): ProcessRunner => { +/** + * @deprecated Only for unmigrated Promise callers. New Effect workflows compose + * core process APIs and leave execution to their owning application entry point. + */ +export const makeProcessRunnerLegacy = (options: ProcessFacadeOptions): ProcessRunnerLegacy => { const layer = processLayer(options); - return (effect) => runPromiseSquashed(Effect.provide(effect, layer), { signal: options.signal }); + return (effect) => + runPromiseSquashedLegacy(Effect.provide(effect, layer), { signal: options.signal }); }; export interface CommandText { @@ -1295,12 +1303,14 @@ const toText = (output: CommandOutput): CommandText => ({ * `runCommand` / `runCommandOk` for Promise callers. `check: 'exit-0'` rejects * with `CommandFailed` on a non-zero exit, like `execFile`; `check: 'none'` * resolves with any exit status. + * @deprecated Only for unmigrated Promise callers. Compose runCommand/runCommandOk + * in new Effect workflows and execute once at the owning application entry point. */ -export const runCommandText = async ( +export const runCommandTextLegacy = async ( spec: CommandSpec & { readonly check: 'exit-0' | 'none' }, options: ProcessFacadeOptions = {} ): Promise => { - const run = makeProcessRunner(options); + const run = makeProcessRunnerLegacy(options); try { return toText(await run(spec.check === 'exit-0' ? runCommandOk(spec) : runCommand(spec))); } catch (error) { @@ -1308,8 +1318,12 @@ export const runCommandText = async ( } }; -/** `runCommandSync` for synchronous callers; blocks the event loop. */ -export const runCommandTextSync = ( +/** + * `runCommandSync` for synchronous callers; blocks the event loop. + * @deprecated Only for unmigrated synchronous callers. Compose runCommandSync + * or runCommandSyncOk in new Effect workflows; execution belongs at the entry point. + */ +export const runCommandTextSyncLegacy = ( spec: CommandSpec & { readonly timeout: Duration.Input; readonly check: 'exit-0' | 'none'; @@ -1324,7 +1338,8 @@ export const runCommandTextSync = ( } }; -export interface ProcessHandle { +/** @deprecated The temporary legacy execution contract; native Effect callers use the core API. */ +export interface ProcessHandleLegacy { readonly child: ChildProcess; /** Resolves with the root's exit; never rejects. */ readonly exited: Promise; @@ -1336,11 +1351,13 @@ export interface ProcessHandle { * `spawnProcess` for long-lived children owned by Promise code. Synchronous * like `spawn`: an asynchronous start failure arrives on `child`'s `error` * event, and `exited` then resolves with nulls. + * @deprecated Only for unmigrated callers that manually own raw Node handles. + * New Effect workflows acquire spawnProcess in Scope and run at their entry point. */ -export const startProcess = ( +export const startProcessLegacy = ( spec: SpawnSpec, options: ProcessFacadeOptions = {} -): ProcessHandle => { +): ProcessHandleLegacy => { options.signal?.throwIfAborted(); let managed: ManagedProcess; try { @@ -1349,7 +1366,7 @@ export const startProcess = ( throw unwrapSpawnFailure(error); } // Cleanup must still run after the caller's cancellation signal is aborted. - const run = makeProcessRunner({ ...options, signal: undefined }); + const run = makeProcessRunnerLegacy({ ...options, signal: undefined }); const abort = () => { void run(managed.terminate(READ_ONLY_ABANDON_POLICY)).catch((error) => run( @@ -1360,14 +1377,19 @@ export const startProcess = ( ); }; if (managed.setupFailure) { - signalChildTreeNow(managed.child, 'SIGKILL', { processGroup: spec.processGroup }, options); + signalChildTreeNowLegacy( + managed.child, + 'SIGKILL', + { processGroup: spec.processGroup }, + options + ); throw unwrapSpawnFailure(managed.setupFailure); } options.signal?.addEventListener('abort', abort, { once: true }); if (options.signal?.aborted) abort(); return { child: managed.child, - exited: runPromiseSquashed(managed.exited), + exited: runPromiseSquashedLegacy(managed.exited), terminate: (policy) => run(managed.terminate(policy)).then(() => { options.signal?.removeEventListener('abort', abort); @@ -1375,13 +1397,17 @@ export const startProcess = ( }; }; -/** End an existing child's whole tree (its group when it was started detached). */ -export const terminateChildTree = ( +/** + * End an existing child's whole tree (its group when it was started detached). + * @deprecated Only for unmigrated Promise callers. Compose childProcessTree and + * terminateTree in new Effect workflows and execute at the owning entry point. + */ +export const terminateChildTreeLegacy = ( child: ChildProcess, policy: TerminationPolicy & { readonly processGroup: boolean }, options: ProcessFacadeOptions = {} ): Promise => - makeProcessRunner(options)( + makeProcessRunnerLegacy(options)( Effect.flatMap(childProcessTree(child, { processGroup: policy.processGroup }), (tree) => terminateTree(tree, policy) ) @@ -1391,9 +1417,11 @@ export const terminateChildTree = ( * Send one signal to a child's tree without waiting, for `process.on('exit')` * handlers that cannot await. The signal (and a Windows `taskkill`) starts * before this returns; nothing confirms the tree is gone, so prefer - * `terminateChildTree` wherever the caller can wait. + * `terminateChildTreeLegacy` wherever the caller can wait. + * @deprecated Only for existing synchronous exit hooks that cannot await. + * New Effect workflows compose tree termination under their owning Scope. */ -export const signalChildTreeNow = ( +export const signalChildTreeNowLegacy = ( child: ChildProcess, signal: TreeSignal, target: { readonly processGroup: boolean }, @@ -1412,8 +1440,16 @@ export const signalChildTreeNow = ( ); }; -export const isPidAliveSync = (pid: number, options: ProcessFacadeOptions = {}): boolean => +/** + * @deprecated Only for unmigrated synchronous callers. Compose isPidAlive in + * new Effect workflows and execute at the owning application entry point. + */ +export const isPidAliveSyncLegacy = (pid: number, options: ProcessFacadeOptions = {}): boolean => runSyncSquashed(Effect.provide(isPidAlive(pid), processLayer(options))); -export const probePidSync = (pid: number, options: ProcessFacadeOptions = {}): PidState => +/** + * @deprecated Only for unmigrated synchronous callers. Compose probePid in + * new Effect workflows and execute at the owning application entry point. + */ +export const probePidSyncLegacy = (pid: number, options: ProcessFacadeOptions = {}): PidState => runSyncSquashed(Effect.provide(probePid(pid), processLayer(options))); diff --git a/packages/shared/tests/fixtures/terminate-failed-spawn.mjs b/packages/shared/tests/fixtures/terminate-failed-spawn.mjs index 1c44352a46..d6fe8112ff 100644 --- a/packages/shared/tests/fixtures/terminate-failed-spawn.mjs +++ b/packages/shared/tests/fixtures/terminate-failed-spawn.mjs @@ -1,14 +1,14 @@ // Starts a command that cannot spawn and terminates it in the same tick, before // Node reports the failure. Run in its own process group: a regression signals // that whole group (pid 0), which this script observes as its own SIGTERM. -const { startProcess } = await import(process.argv[2]); +const { startProcessLegacy } = await import(process.argv[2]); process.on('SIGTERM', () => { console.log('own-group-signalled'); process.exit(3); }); await Promise.resolve(); -const handle = startProcess({ +const handle = startProcessLegacy({ command: '/nonexistent/lody-no-such-binary', args: [], options: { stdio: 'ignore' }, diff --git a/packages/shared/tests/fixtures/unref-process.mjs b/packages/shared/tests/fixtures/unref-process.mjs index d56c043ab4..bad841dc09 100644 --- a/packages/shared/tests/fixtures/unref-process.mjs +++ b/packages/shared/tests/fixtures/unref-process.mjs @@ -1,8 +1,8 @@ // A long-lived child is unrefed: merely awaiting its exit through the facade // must not hold the launching Node process open. It stays in the launcher's // process group so the test can always clean up the group. -const { startProcess } = await import(process.argv[2]); -const handle = startProcess({ +const { startProcessLegacy } = await import(process.argv[2]); +const handle = startProcessLegacy({ command: process.execPath, args: ['-e', 'setInterval(() => {}, 10000)'], options: { stdio: 'ignore' }, diff --git a/packages/shared/tests/login-shell-env.test.ts b/packages/shared/tests/login-shell-env.test.ts new file mode 100644 index 0000000000..66cddf55e6 --- /dev/null +++ b/packages/shared/tests/login-shell-env.test.ts @@ -0,0 +1,132 @@ +import { existsSync } from 'node:fs'; +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { parseLoginShellEnvOutput, probeLoginShellEnv } from '../src/node/login-shell-env'; +import { FakeProcessTable } from '../src/node/process-testing'; + +// Real shells under a throwaway HOME: the probe's contract is what a login +// shell's rc files leave in its environment. +describe.skipIf(process.platform === 'win32')('probeLoginShellEnv', () => { + let home: string; + const env = () => ({ HOME: home, PATH: '/usr/bin:/bin', ZDOTDIR: home }); + + beforeEach(async () => { + home = await mkdtemp(path.join(tmpdir(), 'lody-login-shell-')); + }); + + afterEach(async () => { + await rm(home, { recursive: true, force: true }); + }); + + it('recovers what the login profile exports, whatever the profile prints', async () => { + await writeFile( + path.join(home, '.profile'), + 'echo "welcome banner"\nexport LODY_PROBE_VALUE="$(printf \'one\\ntwo=three\')"\n' + ); + + const result = await probeLoginShellEnv({ + shell: '/bin/sh', + env: env(), + timeout: '10 seconds', + }); + + expect(result?.LODY_PROBE_VALUE).toBe('one\ntwo=three'); + expect(result?.HOME).toBe(home); + // Every variable keeps its name, including the first one after the + // delimiter (macOS /bin/sh prints `echo -n` literally). + expect(result?.PATH).toBeDefined(); + expect(Object.keys(result ?? {}).filter((key) => /\s/u.test(key))).toEqual([]); + }); + + // The probe disables oh-my-zsh auto-update and tmux autostart for itself; + // passing them on would disable them in every Lody terminal too. + it('returns none of the variables the probe injected into its own shell', async () => { + const result = await probeLoginShellEnv({ + shell: '/bin/sh', + env: { ...env(), ZSH_TMUX_AUTOSTART: 'true' }, + timeout: '10 seconds', + }); + + expect(result?.DISABLE_AUTO_UPDATE).toBeUndefined(); + expect(result?.ZSH_TMUX_AUTOSTARTED).toBeUndefined(); + expect(result?.ZSH_TMUX_AUTOSTART).toBe('true'); + }); + + // shell-env could not reap a hung shell; the daemon kept it until exit. + it('ends a shell whose profile hangs and reports no environment', async () => { + vi.useFakeTimers(); + try { + const table = new FakeProcessTable(); + table.queueSpawn({ ignores: ['SIGTERM'] }); + let started!: () => void; + const ready = new Promise((resolve) => { + started = resolve; + }); + let pid = -1; + let descendant = -1; + const result = probeLoginShellEnv({ + shell: '/bin/sh', + env: env(), + timeout: '1 second', + processOptions: { + nodeProcess: { + ...table.api, + spawn: (command, args, options) => { + const child = table.api.spawn(command, args, options); + pid = child.pid!; + descendant = table.addDescendant(pid, { ignores: ['SIGTERM'] }); + started(); + return child; + }, + }, + }, + }); + await ready; + await vi.advanceTimersByTimeAsync(999); + expect(table.isAlive(pid)).toBe(true); + expect(table.isAlive(descendant)).toBe(true); + await vi.advanceTimersByTimeAsync(1); + expect(await result).toBeNull(); + expect(table.isAlive(pid)).toBe(false); + expect(table.isAlive(descendant)).toBe(false); + } finally { + vi.useRealTimers(); + } + }); + + it.skipIf(!existsSync('/bin/zsh') && !existsSync('/bin/bash'))( + 'falls back to zsh or bash when the login shell cannot start', + async () => { + const exportLine = 'export LODY_PROBE_VALUE=fallback\n'; + await writeFile(path.join(home, '.zshenv'), exportLine); + await writeFile(path.join(home, '.bash_profile'), exportLine); + + const result = await probeLoginShellEnv({ + shell: '/nonexistent/lody-shell', + env: env(), + timeout: '10 seconds', + }); + + expect(result?.LODY_PROBE_VALUE).toBe('fallback'); + } + ); +}); + +describe('parseLoginShellEnvOutput', () => { + // BusyBox `env` has no -0; the probe then prints one variable per line. + it('reads newline-separated output from an env without -0', () => { + expect( + parseLoginShellEnvOutput( + 'banner_LODY_SHELL_ENV_DELIMITER_PATH=/usr/bin\nHOME=/root\n_LODY_SHELL_ENV_DELIMITER_' + ) + ).toEqual({ PATH: '/usr/bin', HOME: '/root' }); + }); + + it('reports nothing when the shell never reached the probe', () => { + expect(parseLoginShellEnvOutput('command not found: env')).toBeNull(); + }); +}); diff --git a/packages/shared/tests/process.test.ts b/packages/shared/tests/process.test.ts index e18d7073e3..174cb71916 100644 --- a/packages/shared/tests/process.test.ts +++ b/packages/shared/tests/process.test.ts @@ -12,12 +12,12 @@ import { runCommand, runCommandOk, type CommandSpec, - runCommandText, - startProcess, + runCommandTextLegacy, + startProcessLegacy, } from '../src/node/process'; -import { signalChildTreeNow, SpawnFailed, TerminationFailed } from '../src/node/process'; +import { signalChildTreeNowLegacy, SpawnFailed, TerminationFailed } from '../src/node/process'; import { spawnProcess as acquireProcess, type SpawnSpec } from '../src/node/process'; -import { processLayer, isPidAliveSync } from '../src/node/process'; +import { processLayer, isPidAliveSyncLegacy } from '../src/node/process'; import { READ_ONLY_ABANDON_POLICY, resolveWindowsCommand, @@ -220,7 +220,7 @@ describe('process tree termination (Windows)', () => { const child = table.api.spawn('cli', [], {}); table.exitOnItsOwn(child.pid ?? -1); - signalChildTreeNow(child, 'SIGKILL', { processGroup: false }, { nodeProcess: table.api }); + signalChildTreeNowLegacy(child, 'SIGKILL', { processGroup: false }, { nodeProcess: table.api }); expect(table.spawned.filter((call) => call.command === 'taskkill')).toEqual([]); }); @@ -265,11 +265,11 @@ describe('process tree termination (real processes)', () => { }); const grandchild = yield* readPidLine(managed.child.stdout); yield* managed.exited; - expect(isPidAliveSync(grandchild)).toBe(true); + expect(isPidAliveSyncLegacy(grandchild)).toBe(true); yield* managed.terminate(FORCED); - expect(isPidAliveSync(grandchild)).toBe(false); + expect(isPidAliveSyncLegacy(grandchild)).toBe(false); }).pipe(Effect.scoped, Effect.provide(processLayer({}))) ); @@ -294,9 +294,9 @@ describe('process tree termination (real processes)', () => { const childPid = yield* readPidLine(managed.child.stdout); const exit = yield* managed.exited.pipe(Effect.timeout('5 seconds')); expect(exit.code).toBe(0); - expect(isPidAliveSync(childPid)).toBe(true); + expect(isPidAliveSyncLegacy(childPid)).toBe(true); yield* managed.terminate(FORCED); - expect(isPidAliveSync(childPid)).toBe(false); + expect(isPidAliveSyncLegacy(childPid)).toBe(false); }).pipe(Effect.scoped, Effect.provide(processLayer({}))) ); @@ -468,14 +468,14 @@ describe('runCommand process-tree ownership', () => { }); }); -describe('signalChildTreeNow', () => { +describe('signalChildTreeNowLegacy', () => { it('signals the whole group before returning, for exit handlers that cannot wait', () => { const table = new FakeProcessTable('linux'); const child = table.api.spawn('cli', [], { detached: true }); const leader = child.pid ?? -1; const descendant = table.addDescendant(leader); - signalChildTreeNow(child, 'SIGTERM', { processGroup: true }, { nodeProcess: table.api }); + signalChildTreeNowLegacy(child, 'SIGTERM', { processGroup: true }, { nodeProcess: table.api }); expect(table.delivered).toEqual([{ target: -leader, signal: 'SIGTERM' }]); expect(table.isAlive(descendant)).toBe(false); @@ -608,7 +608,10 @@ describe('official process service and interruption', () => { const table = new FakeProcessTable('linux'); return Effect.gen(function* () { const controller = new AbortController(); - const handle = startProcess(agentSpec, { nodeProcess: table.api, signal: controller.signal }); + const handle = startProcessLegacy(agentSpec, { + nodeProcess: table.api, + signal: controller.signal, + }); const descendant = table.addDescendant(1000); controller.abort(); yield* Effect.promise(() => handle.terminate({ graceMs: 0, killWaitMs: 0 })); @@ -622,7 +625,7 @@ describe('official process service and interruption', () => { return Effect.gen(function* () { const ready = yield* Deferred.make(); const controller = new AbortController(); - const result = runCommandText( + const result = runCommandTextLegacy( { command: 'git', args: [], diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index bfdd16e91d..d2f7b09e35 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -264,9 +264,6 @@ importers: loro-crdt: specifier: 1.16.3 version: 1.16.3 - shell-env: - specifier: ^4.0.3 - version: 4.0.3 tinypool: specifier: ^1.0.0 version: 1.1.1 @@ -8738,10 +8735,6 @@ packages: resolution: {integrity: sha512-m1pAzaJgZ/gssEqlOhJkPJp8Xly7QyW6xcrkUa2KKcDeDSEMP7X8xipU3snUcfisTQx0w1AGae+9UtJSfVnXGw==} engines: {node: '>=18'} - default-shell@2.2.0: - resolution: {integrity: sha512-sPpMZcVhRQ0nEMDtuMJ+RtCxt7iHPAMBU+I4tAlo5dU1sjRpNax0crj6nR3qKpvVnckaQ9U38enXcwW9nZJeCw==} - engines: {node: ^12.20.0 || ^14.13.1 || >=16.0.0} - defer-to-connect@2.0.1: resolution: {integrity: sha512-4tvttepXG1VaYGrRibk5EwJd1t4udunSOVMdLSAL6mId1ix438oPwPZMALY41FCijukO1L0twNcGsdzS7dHgDg==} engines: {node: '>=10'} @@ -9250,10 +9243,6 @@ packages: resolution: {integrity: sha512-CRT1WTyuQoD771GW56XEZFQ/ZoSfWid1alKGDYMmkt2yl8UXrVR4pspqWNEcqKvVIzg6PAltWjxcSSPrboA4iA==} engines: {node: '>=18.0.0'} - execa@5.1.1: - resolution: {integrity: sha512-8uSpZZocAZRBAPIEINJj3Lo9HyGitllczc27Eh5YYojjMFMn8yHMDMaUHE2Jqfq05D/wucwI4JGURyXt1vchyg==} - engines: {node: '>=10'} - expand-template@2.0.3: resolution: {integrity: sha512-XYfuKMvj4O35f/pOXLObndIRvyQ+/+6AhODh+OKWj9S9498pHHn/IMszH+gt0fBCRWMNfk1ZSp5x3AifmnI2vg==} engines: {node: '>=6'} @@ -9658,10 +9647,6 @@ packages: resolution: {integrity: sha512-nBF+F1rAZVCu/p7rjzgA+Yb4lfYXrpl7a6VmJrU8wF9I1CKvP/QwPNZHnOlwbTkY6dvtFIzFMSyQXbLoTQPRpA==} engines: {node: '>=8'} - get-stream@6.0.1: - resolution: {integrity: sha512-ts6Wi+2j3jQjqi70w5AlN8DFnkSwC+MqmxEzdEALB2qXZYV3X/b1CTfgPLGJNMeAWxdPfU8FO1ms3NUfaHCPYg==} - engines: {node: '>=10'} - get-symbol-description@1.1.0: resolution: {integrity: sha512-w9UMqWwJxHNOvoNzSJ2oPF5wvYcvP7jUvYzhp67yEhTi17ZDBBC1z9pTdGuzjD+EFIqLSYRweZjqfiPzQ06Ebg==} engines: {node: '>= 0.4'} @@ -9953,10 +9938,6 @@ packages: resolution: {integrity: sha512-vK9P5/iUfdl95AI+JVyUuIcVtd4ofvtrOr3HNtM2yxC9bnMbEdp3x01OhQNnjb8IJYi38VlTE3mBXwcfvywuSw==} engines: {node: '>= 14'} - human-signals@2.1.0: - resolution: {integrity: sha512-B4FFZ6q/T2jhhksgkbEW3HBvWIfDW85snkQgawt07S7J5QXTk6BkNV+0yAeZrM5QpMAdYlocGoljn0sJ/WQkFw==} - engines: {node: '>=10.17.0'} - i18next@25.8.4: resolution: {integrity: sha512-a9A0MnUjKvzjEN/26ZY1okpra9kA8MEwzYEz1BNm+IyxUKPRH6ihf0p7vj8YvULwZHKHl3zkJ6KOt4hewxBecQ==} peerDependencies: @@ -10916,9 +10897,6 @@ packages: resolution: {integrity: sha512-Snk314V5ayFLhp3fkUREub6WtjBfPdCPY1Ln8/8munuLuiYhsABgBVWsozAG+MWMbVEvcdcpbi9R7ww22l9Q3g==} engines: {node: '>=18'} - merge-stream@2.0.0: - resolution: {integrity: sha512-abv/qOcuPfk3URPfDzmZU1LKmuw8kT+0nIHvKrKgFrwifol/doWcdA4ZqsWQ8ENrFKkd67Mfpo/LovbIUsbt3w==} - merge2@1.4.1: resolution: {integrity: sha512-8q7VEgMJW4J8tcfVPy8g09NcQwZdbwFEqhe/WZkoIzjn/3TGDwtOCYtXGxA3O8tPzpczCCDgv+P2P5y00ZJOOg==} engines: {node: '>= 8'} @@ -11076,10 +11054,6 @@ packages: engines: {node: '>=16'} hasBin: true - mimic-fn@2.1.0: - resolution: {integrity: sha512-OqbOk5oEQeAZ8WXWydlu9HJjz9WVdEIvamMCcXmuqUYjTknH/sqsWvhQ3vgwKFRR1HpjvNBKQ37nbJgYzGqGcg==} - engines: {node: '>=6'} - mimic-function@5.0.1: resolution: {integrity: sha512-VP79XUPxV2CigYP3jWwAUFSku2aKqBH7uTAapFWCBqutsbmDo96KY5o8uh6U+/YSIn5OxJnXp73beVkpqMIGhA==} engines: {node: '>=18'} @@ -11347,10 +11321,6 @@ packages: nostics@1.2.0: resolution: {integrity: sha512-FGqEfhQjrvo1lL8KFifdTQiNwwQHJxC1jtYE1Rc54qF/jxONUNL+kC9gS1krX8Q65PgrQ5fCqH/I4NhWBvdSqg==} - npm-run-path@4.0.1: - resolution: {integrity: sha512-S48WzZW777zhNIrn7gxOlISNAqi9ZC/uQFnRdbeIHhZhCA6UqpkOT8T1G7BvfdgP4Er8gF4sUbaS0i7QvIfCWw==} - engines: {node: '>=8'} - number-flow@0.6.2: resolution: {integrity: sha512-MCnImG4Q5vPwhSXnov56nOuyyKn6LC+Qd7II1UiKc+ACRtug5iAtn0+CwXNxM38AC5lSowEY+oYEtZX2qMnUyw==} @@ -11415,10 +11385,6 @@ packages: one-time@1.0.0: resolution: {integrity: sha512-5DXOiRKwuSEcQ/l0kGCF6Q3jcADFv5tSmRaJck/OqkVFcOzutB134KRSfF0xDrL39MNnqxbHBbUUcjZIhTgb2g==} - onetime@5.1.2: - resolution: {integrity: sha512-kbpaSSGJTWdAY5KPVeMOKXSrPtr8C8C7wodJbcsd51jRnmD+GZu8Y0VoU6Dm5Z4vWr0Ig/1NKuWRKf7j5aaYSg==} - engines: {node: '>=6'} - onetime@7.0.0: resolution: {integrity: sha512-VXJjc87FScF88uafS3JllDgvAm+c/Slfz06lorj2uAY34rlUu0Nt+v8wreiImcrgAjjIHp1rXpTDlLOGw29WwQ==} engines: {node: '>=18'} @@ -12590,10 +12556,6 @@ packages: resolution: {integrity: sha512-7++dFhtcx3353uBaq8DDR4NuxBetBzC7ZQOhmTQInHEd6bSrXdiEyzCvG07Z44UYdLShWUyXt5M/yhz8ekcb1A==} engines: {node: '>=8'} - shell-env@4.0.3: - resolution: {integrity: sha512-Ioe5h+hCDZ7pKL5+JGzbtPvZ5ESMHePZ8nLxohlDL+twmlcmutttMhRkrQOed8DeLT8mkYBgbwZfohe8pqaA3g==} - engines: {node: ^12.20.0 || ^14.13.1 || >=16.0.0} - shiki@3.22.0: resolution: {integrity: sha512-LBnhsoYEe0Eou4e1VgJACes+O6S6QC0w71fCSp5Oya79inkwkm15gQ1UF6VtQ8j/taMDh79hAB49WUk8ALQW3g==} @@ -12823,10 +12785,6 @@ packages: resolution: {integrity: sha512-vavAMRXOgBVNF6nyEEmL3DBK19iRpDcoIwW+swQ+CbGiu7lju6t+JklA1MHweoWtadgt4ISVUsXLyDq34ddcwA==} engines: {node: '>=4'} - strip-final-newline@2.0.0: - resolution: {integrity: sha512-BrpvfNAE3dcvq7ll3xVumzjKjZQ5tI1sEUIKr3Uoks0XUl45St3FlatVqef9prk4jRDzhW6WZg+3bk93y6pLjA==} - engines: {node: '>=6'} - strip-indent@3.0.0: resolution: {integrity: sha512-laJTa3Jb+VQpaC6DseHhF7dXVqHTfJPCRDaEbid/drOhgitgYku/letMUqOXFoWV0zIIUbjpdH2t+tYj4bQMRQ==} engines: {node: '>=8'} @@ -20530,7 +20488,7 @@ snapshots: '@vitest/spy': 4.1.11 '@vitest/utils': 4.1.11 chai: 6.2.2 - tinyrainbow: 3.1.0 + tinyrainbow: 3.2.0 optional: true '@vitest/istanbul-lib-coverage@1.0.2': {} @@ -20626,7 +20584,7 @@ snapshots: '@vitest/pretty-format@4.1.11': dependencies: - tinyrainbow: 3.1.0 + tinyrainbow: 3.2.0 optional: true '@vitest/runner@3.2.4': @@ -20696,7 +20654,7 @@ snapshots: dependencies: '@vitest/pretty-format': 4.1.11 convert-source-map: 2.0.0 - tinyrainbow: 3.1.0 + tinyrainbow: 3.2.0 optional: true '@volar/language-core@2.4.28': @@ -22093,8 +22051,6 @@ snapshots: bundle-name: 4.1.0 default-browser-id: 5.0.1 - default-shell@2.2.0: {} - defer-to-connect@2.0.1: {} define-data-property@1.1.4: @@ -22827,18 +22783,6 @@ snapshots: dependencies: eventsource-parser: 3.0.7 - execa@5.1.1: - dependencies: - cross-spawn: 7.0.6 - get-stream: 6.0.1 - human-signals: 2.1.0 - is-stream: 2.0.1 - merge-stream: 2.0.0 - npm-run-path: 4.0.1 - onetime: 5.1.2 - signal-exit: 3.0.7 - strip-final-newline: 2.0.0 - expand-template@2.0.3: {} expect-type@1.4.0: {} @@ -23275,8 +23219,6 @@ snapshots: dependencies: pump: 3.0.3 - get-stream@6.0.1: {} - get-symbol-description@1.1.0: dependencies: call-bound: 1.0.4 @@ -23714,8 +23656,6 @@ snapshots: transitivePeerDependencies: - supports-color - human-signals@2.1.0: {} - i18next@25.8.4(typescript@5.9.3): dependencies: '@babel/runtime': 7.28.6 @@ -24683,8 +24623,6 @@ snapshots: merge-descriptors@2.0.0: {} - merge-stream@2.0.0: {} - merge2@1.4.1: {} mermaid@11.15.0: @@ -25012,8 +24950,6 @@ snapshots: mime@4.1.0: {} - mimic-fn@2.1.0: {} - mimic-function@5.0.1: {} mimic-response@1.0.1: {} @@ -25241,10 +25177,6 @@ snapshots: nostics@1.2.0: {} - npm-run-path@4.0.1: - dependencies: - path-key: 3.1.1 - number-flow@0.6.2: dependencies: esm-env: 1.2.2 @@ -25315,10 +25247,6 @@ snapshots: dependencies: fn.name: 1.1.0 - onetime@5.1.2: - dependencies: - mimic-fn: 2.1.0 - onetime@7.0.0: dependencies: mimic-function: 5.0.1 @@ -26843,12 +26771,6 @@ snapshots: shebang-regex@3.0.0: {} - shell-env@4.0.3: - dependencies: - default-shell: 2.2.0 - execa: 5.1.1 - strip-ansi: 7.1.2 - shiki@3.22.0: dependencies: '@shikijs/core': 3.22.0 @@ -27134,8 +27056,6 @@ snapshots: strip-bom@3.0.0: {} - strip-final-newline@2.0.0: {} - strip-indent@3.0.0: dependencies: min-indent: 1.0.1 diff --git a/scripts/check-cli-process-boundary.mjs b/scripts/check-cli-process-boundary.mjs new file mode 100644 index 0000000000..4aa38c1cad --- /dev/null +++ b/scripts/check-cli-process-boundary.mjs @@ -0,0 +1,185 @@ +#!/usr/bin/env node + +// Every OS process the CLI starts, waits for or signals goes +// through the Effect process layer (`@lody/shared/node/process`). This guard +// fails when their source reaches for child_process, a process-spawning library +// (cross-spawn, execa, shell-env, node-pty, ...) or a direct kill, so a second +// process implementation cannot creep back in. +// Rules: packages/shared/src/node/AGENTS.md. + +import { execFile } from 'node:child_process'; +import { readFile } from 'node:fs/promises'; +import path from 'node:path'; +import { promisify } from 'node:util'; +import { fileURLToPath } from 'node:url'; +import ts from 'typescript'; + +const execFileAsync = promisify(execFile); +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); +const sourceRoots = [ + 'apps/cli/src/', + 'packages/shared/src/node/process.ts', + 'packages/shared/src/node/login-shell-env.ts', +]; + +/** Files allowed to reach the OS directly, each with the reason it is not a second implementation. */ +const allowlist = new Map([ + ['packages/shared/src/node/process.ts', 'the process layer and its single OS boundary'], + [ + 'packages/shared/src/node/process-testing.ts', + 'test support that models the OS process table; imported only by tests', + ], + [ + 'apps/cli/src/lib/terminal-pty-service.ts', + 'loads node-pty, the only PTY spawner; PTY trees end through the process layer', + ], + [ + 'apps/cli/src/lib/github-git-transport.ts', + 'source text of a standalone git wrapper script that runs in its own process', + ], + [ + 'apps/cli/src/lib/gh-shim-script.ts', + 'source text of a standalone gh shim script that runs in its own process', + ], + [ + 'apps/cli/src/lib/git-credential-helper-script.ts', + 'source text of a standalone credential helper that runs in its own process', + ], + [ + 'apps/cli/src/agent/deepseek-harness-runtime.ts', + 'source text injected into the DeepSeek Harness child, not daemon code', + ], +]); + +// A module specifier position: `from`, a side-effect `import`, dynamic +// `import(...)`, `require(...)` and `createRequire(...)(...)`. A string that +// only names a module (Tinypool's `runtime: 'child_process'`) is not one. +const MODULE_POSITION = String.raw`(?:\bfrom|\bimport\s*\(?|(?:\brequire|\))\s*\()\s*`; +const PROCESS_MODULES = String.raw`(?:node:)?child_process|cross-spawn|execa|shell-env|tree-kill|ps-tree|find-process|pidusage|(?:@lydell/)?node-pty`; +/** The statement before a specifier is a type-only import or export, possibly multi-line. */ +const TYPE_ONLY_STATEMENT = /\b(?:import|export)\s+type\b[^;'"]*$/u; + +const forbidden = [ + { + // `child_process` and the libraries that start or signal processes on + // their own, in any import form. Type-only imports carry no behaviour. + pattern: new RegExp(`${MODULE_POSITION}['"](?:${PROCESS_MODULES})['"]`, 'gu'), + label: 'process module reference', + skip: (match, text) => TYPE_ONLY_STATEMENT.test(text.slice(0, match.index)), + }, + { pattern: /\bprocess\.kill\s*\(/gu, label: 'process.kill call' }, + // Signalling a ChildProcess (or PTY) directly is a hand-written termination + // path; `terminateTree` owns escalation and bounded waits. The `NodeProcess` + // service's own `kill` is the sanctioned door. Covers `child?.kill(`, + // `(child as T).kill(` and `children[i].kill(` too. `yield* handle.kill(...)` + // is the official Effect handle operation; yielding Node's boolean kill result + // is rejected by TypeScript and must not be mistaken for an OS bypass. + { + pattern: /([\w$]+|[)\]])\s*\??\.kill\s*\(/gu, + label: 'direct child signal', + skip: (match, text) => + ['process', 'np', 'nodeProcess', 'nodeProcessLive'].includes(match[1] ?? '') || + /\byield\s*\*\s*$/u.test(text.slice(0, match.index)), + }, +]; + +async function listSources() { + const { stdout } = await execFileAsync( + 'git', + ['ls-files', '-co', '--exclude-standard', ...sourceRoots], + { + cwd: repoRoot, + maxBuffer: 20 * 1024 * 1024, + } + ); + return stdout + .split('\n') + .filter((file) => /\.(?:ts|tsx|mts|cts|js|mjs|cjs)$/u.test(file)) + .filter( + (file) => !/\.(?:test|spec)\.[cm]?[jt]sx?$/u.test(file) && !file.includes('/__tests__/') + ); +} + +const lineOf = (text, index) => text.slice(0, index).split('\n').length; + +// Migration-only execution doors keep Legacy visible; core Effect APIs keep +// their original names. Parse bindings so comments and unrelated identifiers +// cannot be mistaken for an import, and an alias cannot hide the boundary. +const retiredFacades = new Set([ + 'runCommandText', + 'runCommandTextSync', + 'startProcess', + 'terminateChildTree', + 'signalChildTreeNow', + 'isPidAliveSync', + 'probePidSync', + 'makeProcessRunner', + 'runPromiseSquashed', + 'ProcessRunner', + 'ProcessHandle', +]); +const legacyFacades = new Set([...retiredFacades].map((name) => `${name}Legacy`)); + +function checkFacadeBindings(file, text) { + const source = ts.createSourceFile(file, text, ts.ScriptTarget.Latest, true); + const report = (node, label) => + violations.push(`${file}:${lineOf(text, node.getStart(source))} ${label}`); + for (const statement of source.statements) { + if (ts.isImportDeclaration(statement) || ts.isExportDeclaration(statement)) { + const module = statement.moduleSpecifier; + if (!module || !ts.isStringLiteral(module)) continue; + if ( + module.text !== '@lody/shared/node/process' && + !(file.startsWith('packages/shared/src/node/') && module.text === './process') + ) + continue; + const bindings = ts.isImportDeclaration(statement) + ? statement.importClause?.namedBindings + : statement.exportClause; + if (!bindings || !(ts.isNamedImports(bindings) || ts.isNamedExports(bindings))) continue; + for (const binding of bindings.elements) { + const imported = (binding.propertyName ?? binding.name).text; + if (retiredFacades.has(imported)) report(binding, `retired process facade ${imported}`); + if (legacyFacades.has(imported) && !binding.name.text.endsWith('Legacy')) + report(binding, `process facade alias hides Legacy: ${binding.name.text}`); + } + } + if ( + file === 'packages/shared/src/node/process.ts' && + statement.modifiers?.some((modifier) => modifier.kind === ts.SyntaxKind.ExportKeyword) + ) { + const names = ts.isVariableStatement(statement) + ? statement.declarationList.declarations.map((declaration) => declaration.name) + : [statement.name]; + for (const name of names) { + if (name && ts.isIdentifier(name) && retiredFacades.has(name.text)) + report(name, `retired process facade export ${name.text}`); + } + } + } +} + +const violations = []; +for (const file of await listSources()) { + let text; + try { + text = await readFile(path.join(repoRoot, file), 'utf8'); + } catch { + continue; + } + checkFacadeBindings(file, text); + if (allowlist.has(file)) continue; + for (const { pattern, label, skip } of forbidden) { + for (const match of text.matchAll(pattern)) { + if (skip?.(match, text)) continue; + violations.push(`${file}:${lineOf(text, match.index ?? 0)} ${label}`); + } + } +} + +if (violations.length > 0) { + console.error('Process boundary violations (use @lody/shared/node/process instead):'); + for (const violation of violations) console.error(` ${violation}`); + process.exit(1); +} +console.log('Process boundary guard passed.');