diff --git a/src/cli.ts b/src/cli.ts index 76b081bf1..267833047 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -93,6 +93,7 @@ import { dispatchDeferredTopicSend, reusableDeferredTopicRoot, type DeferredSche import { readDeferredTopicBinding } from './core/deferred-topic-binding.js'; import { resolveDaemonEnv } from './cli/daemon-lifecycle-env.js'; import { buildPm2SpawnCommand } from './cli/pm2-command.js'; +import { pm2ManagedExitConfig } from './pm2-graceful-exit.js'; import { callDashboard, type DashboardEndpoint, type DashboardResult } from './cli/dashboard-endpoint.js'; import { globalInstallUpdateLockTargetIn, installLatestBotmuxSync } from './core/maintenance.js'; import { withFileLockSync } from './utils/file-lock.js'; @@ -506,6 +507,7 @@ function ecosystemConfig(activationAppId?: string): string { process.env, existsSync(ENV_FILE) ? readFileSync(ENV_FILE, 'utf-8') : undefined, ); + const managedExit = pm2ManagedExitConfig(); const baseApp = { script: daemonScript, @@ -517,14 +519,14 @@ function ecosystemConfig(activationAppId?: string): string { autorestart: true, max_restarts: 10, restart_delay: 3000, - // A graceful daemon shutdown exits 0 (SIGTERM/SIGINT → drain → process.exit(0)). - // Tell pm2 that exit 0 is intentional so it does NOT autorestart the daemon + // A graceful PM2-managed shutdown exits with a dedicated sentinel code. + // Tell pm2 that only this code is intentional so it does NOT autorestart the daemon // while `botmux restart` is tearing the fleet down — otherwise pm2 revives // each daemon (after restart_delay) the instant our parallel SIGTERM drains // it, and re-deleting those revivals one-by-one re-serializes the teardown - // (~13s of churn for 31 bots). Crashes (non-zero exit / killed by signal) - // are NOT in this list, so genuine crash-autorestart is preserved. - stop_exit_codes: [0], + // (~13s of churn for 31 bots). Do not use 0 here: PM2 normalizes a signal + // exit's null code to 0, which would suppress autorestart after SIGKILL. + stop_exit_codes: managedExit.stopExitCodes, // pm2's default kill_timeout (1.6s) is SHORTER than the daemon's own // SHUTDOWN_GRACE_MS (3s), so any daemon pm2 has to signal directly gets // SIGKILL'd mid-drain → orphaned (ppid=1) workers. Give pm2 headroom past @@ -585,6 +587,7 @@ function ecosystemConfig(activationAppId?: string): string { out_file: join(LOG_DIR, `daemon-${i}-out.log`), env: { ...daemonEnv, + ...managedExit.env, SESSION_DATA_DIR: DATA_DIR, BOTMUX_BOT_INDEX: String(i), BOTMUX_LARK_APP_ID: appId, @@ -610,15 +613,16 @@ function ecosystemConfig(activationAppId?: string): string { autorestart: true, max_restarts: 10, restart_delay: 3000, - // Same rationale as the bot daemons: don't let pm2 revive on graceful exit-0 + // Same rationale as the bot daemons: don't let pm2 revive on graceful exit // during a fleet teardown, and don't SIGKILL mid-shutdown. (See baseApp.) - stop_exit_codes: [0], + stop_exit_codes: managedExit.stopExitCodes, kill_timeout: 3500, error_file: join(LOG_DIR, 'dashboard-error.log'), out_file: join(LOG_DIR, 'dashboard-out.log'), merge_logs: true, env: { ...daemonEnv, + ...managedExit.env, // MUST match the bot daemons' SESSION_DATA_DIR: the dashboard shares // pairings/federations/memberships with them via {dataDir}/*.json. Without // it the dashboard falls back to an install-relative ../data and reads a diff --git a/src/core/local-terminal-opener.ts b/src/core/local-terminal-opener.ts index 857b7221e..84f566dd5 100644 --- a/src/core/local-terminal-opener.ts +++ b/src/core/local-terminal-opener.ts @@ -6,6 +6,7 @@ import { getBot } from '../bot-registry.js'; import { createCliAdapterSync } from '../adapters/cli/registry.js'; import type { CliId } from '../adapters/cli/types.js'; import { buildWrappedLaunch, decorateResumeForWrapper, parseWrapperCli } from '../setup/cli-selection.js'; +import { stripPm2GracefulExitMarker } from '../pm2-graceful-exit.js'; type LocalTerminalBackend = 'cli' | 'app'; @@ -146,7 +147,15 @@ export function localCliCommandForSession(ds: DaemonSession): LocalCliCommandRes function spawnDetached(command: string, args: string[]): { ok: true } | { ok: false; error: string } { try { - const child = spawn(command, args, { detached: true, stdio: 'ignore' }); + // Strip the daemon's graceful-exit sentinel: the launched terminal runs a + // login shell → local AI CLI that could itself start a foreground botmux, + // which would then exit 90 on a clean stop (a supervisor reads that as a + // crash). Only the PM2-managed cores may carry the marker. + const child = spawn(command, args, { + detached: true, + stdio: 'ignore', + env: stripPm2GracefulExitMarker(process.env), + }); child.unref(); return { ok: true }; } catch (err) { diff --git a/src/core/plugins/pm2.ts b/src/core/plugins/pm2.ts index b62a30039..8287bce4a 100644 --- a/src/core/plugins/pm2.ts +++ b/src/core/plugins/pm2.ts @@ -4,6 +4,7 @@ import { join } from 'node:path'; import { createRequire } from 'node:module'; import { spawnSync } from 'node:child_process'; import { buildPm2SpawnCommand } from '../../cli/pm2-command.js'; +import { stripPm2GracefulExitMarker } from '../../pm2-graceful-exit.js'; const require = createRequire(import.meta.url); const BOTMUX_HOME = join(homedir(), '.botmux'); @@ -28,7 +29,12 @@ function pm2Bin(): string { function pm2Env(extra?: Record): NodeJS.ProcessEnv { mkdirSync(PLUGIN_PM2_HOME, { recursive: true }); - const inherited = { ...process.env }; + // Strip the daemon/dashboard graceful-exit sentinel before it rides + // process.env into a plugin PM2 app (esp. with `pm2 start --update-env`): + // the plugin service is an arbitrary long-lived process that could launch a + // foreground botmux, which would then exit 90 on a clean stop. See + // stripPm2GracefulExitMarker. + const inherited = stripPm2GracefulExitMarker(process.env); delete inherited.kill_timeout; return { ...inherited, ...(extra ?? {}), PM2_HOME: PLUGIN_PM2_HOME }; } diff --git a/src/core/worker-pool.ts b/src/core/worker-pool.ts index 60231031a..ba210b530 100644 --- a/src/core/worker-pool.ts +++ b/src/core/worker-pool.ts @@ -312,7 +312,13 @@ export const TRANSFER_DETACH_FENCE_PICKER_MS = 8_000; // worker exits within a few ms of the ACK. const TRANSFER_DETACH_POST_ACK_KILL_MS = 300; const TRANSFER_FORCE_EXIT_MS = 500; -const WORKER_REDACTED_ENV_KEYS = ['GITHUB_TOKEN', 'GH_TOKEN'] as const; +// Keys the daemon must NOT propagate into a forked worker. GH tokens are the +// bot's, not the agent's. BOTMUX_PM2_GRACEFUL_EXIT_CODE is pm2's private +// graceful-exit sentinel for the daemon/dashboard cores only (see +// pm2-graceful-exit.ts): a worker (or the CLI child it forks — redactChildEnv +// strips it there too) that inherited it would exit 90 instead of 0 on a +// clean foreground stop, which a supervisor reads as a crash. +const WORKER_REDACTED_ENV_KEYS = ['GITHUB_TOKEN', 'GH_TOKEN', 'BOTMUX_PM2_GRACEFUL_EXIT_CODE'] as const; function workerForkEnv(base: NodeJS.ProcessEnv): NodeJS.ProcessEnv { const env: NodeJS.ProcessEnv = { ...base }; diff --git a/src/daemon.ts b/src/daemon.ts index 38eec1575..874db963c 100644 --- a/src/daemon.ts +++ b/src/daemon.ts @@ -87,6 +87,7 @@ import { bindResourcesToMessage, composeForwardFollowupContent, mergeMessageMent import { buildQuoteHint } from './im/lark/quote-hint.js'; import { buildTopicThreadContext } from './im/lark/topic-root-context.js'; import { logger } from './utils/logger.js'; +import { gracefulProcessExitCode } from './pm2-graceful-exit.js'; import { applyAllowedUsersResolve } from './utils/allowed-users-apply.js'; import { withFileLock, withFileLockSync } from './utils/file-lock.js'; import { delay } from './utils/timing.js'; @@ -19543,7 +19544,7 @@ export async function startDaemon(botIndex?: number): Promise { flushIdentityCacheSync(); removePidFile(); - process.exit(0); + process.exit(gracefulProcessExitCode()); }; process.on('SIGTERM', () => { shutdown().catch(err => { logger.error(`shutdown failed: ${err?.message ?? err}`); process.exit(1); }); }); diff --git a/src/dashboard.ts b/src/dashboard.ts index 072f57bf2..f28eb35ea 100644 --- a/src/dashboard.ts +++ b/src/dashboard.ts @@ -11,6 +11,7 @@ import { homedir } from 'node:os'; import { fileURLToPath } from 'node:url'; import { randomBytes } from 'node:crypto'; import { logger } from './utils/logger.js'; +import { gracefulProcessExitCode } from './pm2-graceful-exit.js'; import { config, isWildcardBindHost } from './config.js'; import { listenWithProbe } from './utils/listen-with-probe.js'; import { @@ -5632,9 +5633,9 @@ function shutdown(): void { resourceMonitor.stop(); platformTunnel?.stop(); debugTerminalManager.shutdown(); - server.close(() => process.exit(0)); + server.close(() => process.exit(gracefulProcessExitCode())); // Hard-exit fallback after 5s - setTimeout(() => process.exit(0), 5_000).unref(); + setTimeout(() => process.exit(gracefulProcessExitCode()), 5_000).unref(); } process.on('SIGTERM', shutdown); process.on('SIGINT', shutdown); diff --git a/src/pm2-graceful-exit.ts b/src/pm2-graceful-exit.ts new file mode 100644 index 000000000..526f572e6 --- /dev/null +++ b/src/pm2-graceful-exit.ts @@ -0,0 +1,44 @@ +/** + * PM2 only receives an exit code plus a signal. For signal exits Node reports + * a null code, which PM2 normalizes to 0 before applying `stop_exit_codes`. + * Therefore 0 cannot safely mean "intentional shutdown" for a PM2-managed + * process: SIGKILL would be mistaken for a clean stop and never restarted. + */ +// Keep the sentinel outside POSIX sysexits (64-78) and signal-derived 128+N. +export const PM2_GRACEFUL_EXIT_CODE = 90; +export const PM2_GRACEFUL_EXIT_CODE_ENV = 'BOTMUX_PM2_GRACEFUL_EXIT_CODE'; + +export function pm2ManagedExitConfig(): { + stopExitCodes: number[]; + env: Record; +} { + return { + stopExitCodes: [PM2_GRACEFUL_EXIT_CODE], + env: { [PM2_GRACEFUL_EXIT_CODE_ENV]: String(PM2_GRACEFUL_EXIT_CODE) }, + }; +} + +/** Keep direct/foreground launches on the conventional successful exit code. */ +export function gracefulProcessExitCode(env: NodeJS.ProcessEnv = process.env): number { + return env[PM2_GRACEFUL_EXIT_CODE_ENV] === String(PM2_GRACEFUL_EXIT_CODE) + ? PM2_GRACEFUL_EXIT_CODE + : 0; +} + +/** + * Strip the graceful-exit sentinel from an env destined for a child/spawned + * process, returning a fresh shallow copy (never mutates the input). Only the + * two PM2-managed cores (daemon.ts, dashboard.ts) may see this marker; + * anything they fork/spawn that inherits it — a worker, a CLI child, a plugin + * PM2 app, a local terminal — could re-launch a foreground `botmux` and then + * exit 90 on a clean stop, which a supervisor misreads as a crash. Boundaries + * that copy raw `process.env` (pm2Env, spawnDetached) call this; boundaries + * with a key deny-list (redactChildEnv, workerForkEnv) list the key there + * instead. Always returns a new object so callers can safely mutate the result + * (e.g. delete other keys) without touching process.env. + */ +export function stripPm2GracefulExitMarker(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv { + const next = { ...env }; + delete next[PM2_GRACEFUL_EXIT_CODE_ENV]; + return next; +} diff --git a/src/utils/child-env.ts b/src/utils/child-env.ts index 4a8c9fb0a..b05656a4c 100644 --- a/src/utils/child-env.ts +++ b/src/utils/child-env.ts @@ -84,6 +84,19 @@ export const REDACTED_CHILD_ENV_KEYS = [ // the source. Non-tmux CLIs are unaffected (they don't read TMUX). 'TMUX', 'TMUX_PANE', + // PM2 graceful-exit sentinel (BOTMUX_PM2_GRACEFUL_EXIT_CODE, defined as + // PM2_GRACEFUL_EXIT_CODE_ENV in pm2-graceful-exit.ts). pm2 bakes it into the + // daemon/dashboard env so ONLY those two managed cores exit with the sentinel + // code (90) on graceful stop instead of 0 — otherwise a signal-killed daemon's + // null→0 code would match stop_exit_codes and suppress crash-autorestart. But + // it must never reach a session's CLI child: a foreground `botmux serve + // --api-only` / `daemon` / `dashboard` launched from inside a bot session would + // inherit the marker and, per gracefulProcessExitCode(), exit 90 on a clean + // Ctrl+C — a supervisor/launcher then misreads that non-zero code as a crash. + // Only daemon.ts + dashboard.ts read this key, so stripping it at the child + // boundary is safe. Kept as a string literal (like the keys above) with a + // drift-guard test pinning it to PM2_GRACEFUL_EXIT_CODE_ENV. + 'BOTMUX_PM2_GRACEFUL_EXIT_CODE', ] as const; /** diff --git a/src/workflows/v3/ephemeral-pool.ts b/src/workflows/v3/ephemeral-pool.ts index 4a09b0f7a..1f077a683 100644 --- a/src/workflows/v3/ephemeral-pool.ts +++ b/src/workflows/v3/ephemeral-pool.ts @@ -28,6 +28,7 @@ import { type WorkerSessionInfo, } from './contract.js'; import { workflowSandboxInitFields } from '../shared/sandbox-policy.js'; +import { stripPm2GracefulExitMarker } from '../../pm2-graceful-exit.js'; import { armV3AttemptWorkerFence, bindV3AttemptWorkerFence, @@ -140,7 +141,12 @@ async function runNodeImpl( workerPath: deps.workerPath, cwd, env: { - ...process.env, + // v3 ephemeral workers fork straight from the daemon here (not via + // workerForkEnv), so strip the PM2 graceful-exit sentinel to keep the + // "only daemon/dashboard carry the marker" invariant — harmless today + // (the worker doesn't read the graceful helper and its CLI children go + // through redactChildEnv), but this keeps the boundary honest. + ...stripPm2GracefulExitMarker(process.env), ...req.env, [GOAL_ENV.V3_MARKER]: '1', BOTMUX_WORKFLOW: '1', diff --git a/test/child-env.test.ts b/test/child-env.test.ts index 7fb7ee9d3..15e0a5c74 100644 --- a/test/child-env.test.ts +++ b/test/child-env.test.ts @@ -4,10 +4,12 @@ import { BOTMUX_INJECTED_ENV_KEYS, CLAUDE_SESSION_MARKER_ENV_KEYS, redactChildEnv, + REDACTED_CHILD_ENV_KEYS, scrubClaudeSessionMarkerEnv, scrubSessionCliHomeEnv, SESSION_CLI_HOME_ENV_KEYS, } from '../src/utils/child-env.js'; +import { PM2_GRACEFUL_EXIT_CODE_ENV } from '../src/pm2-graceful-exit.js'; describe('redactChildEnv()', () => { it('truly removes leaked keys — absent, not present-with-"undefined"', () => { @@ -64,6 +66,28 @@ describe('redactChildEnv()', () => { expect(out.KEEP).toBe('v'); }); + it('removes the PM2 graceful-exit sentinel so a foreground CLI child exits 0, not 90', () => { + // pm2 bakes BOTMUX_PM2_GRACEFUL_EXIT_CODE=90 into the daemon env so ONLY the + // daemon/dashboard cores exit with the sentinel on graceful stop. Left in a + // session's CLI-child env, a foreground `botmux serve --api-only` / `daemon` + // launched from inside that session would exit 90 on a clean Ctrl+C + // (gracefulProcessExitCode reads this key) — a supervisor reads non-zero as + // a crash. redactChildEnv must strip it at the child boundary. + const out = redactChildEnv({ + [PM2_GRACEFUL_EXIT_CODE_ENV]: '90', + KEEP: 'v', + }); + expect(PM2_GRACEFUL_EXIT_CODE_ENV in out).toBe(false); + expect(out.KEEP).toBe('v'); + }); + + it('pins the redacted sentinel key to PM2_GRACEFUL_EXIT_CODE_ENV (drift guard)', () => { + // The key is a string literal in REDACTED_CHILD_ENV_KEYS (matching its + // neighbors) rather than an import, so guard against the two definitions + // drifting apart if the env var is ever renamed. + expect(REDACTED_CHILD_ENV_KEYS).toContain(PM2_GRACEFUL_EXIT_CODE_ENV); + }); + it('real node-pty child does NOT inherit a redacted var (not the string "undefined")', async () => { // End-to-end guard for the actual leak vector Codex found: a spawned child // must see the redacted var as genuinely UNSET. `${VAR+x}` expands to empty @@ -71,11 +95,16 @@ describe('redactChildEnv()', () => { // 'undefined'". Run against the real bundled node-pty + /bin/sh. const pty = await import('node-pty'); const prev = process.env.LARK_APP_ID; + const prevSentinel = process.env[PM2_GRACEFUL_EXIT_CODE_ENV]; process.env.LARK_APP_ID = 'cli_parent_must_not_leak'; + // Simulate a PM2-managed daemon's env carrying the graceful-exit sentinel, + // which must not survive into the forked CLI child. + process.env[PM2_GRACEFUL_EXIT_CODE_ENV] = '90'; try { const env = redactChildEnv(process.env) as { [k: string]: string }; const script = - 'if [ -z "${LARK_APP_ID+x}" ]; then echo "R=UNSET"; else echo "R=SET[$LARK_APP_ID]"; fi'; + 'if [ -z "${LARK_APP_ID+x}" ]; then echo "R=UNSET"; else echo "R=SET[$LARK_APP_ID]"; fi; ' + + `if [ -z "\${${PM2_GRACEFUL_EXIT_CODE_ENV}+x}" ]; then echo "S=UNSET"; else echo "S=SET[\$${PM2_GRACEFUL_EXIT_CODE_ENV}]"; fi`; const out: string = await new Promise((resolve) => { const p = pty.spawn('/bin/sh', ['-c', script], { name: 'xterm-256color', cols: 80, rows: 24, cwd: '/tmp', env, @@ -85,10 +114,13 @@ describe('redactChildEnv()', () => { p.onExit(() => resolve(buf)); }); expect(out).toContain('R=UNSET'); + expect(out).toContain('S=UNSET'); expect(out).not.toContain('undefined'); } finally { if (prev === undefined) delete process.env.LARK_APP_ID; else process.env.LARK_APP_ID = prev; + if (prevSentinel === undefined) delete process.env[PM2_GRACEFUL_EXIT_CODE_ENV]; + else process.env[PM2_GRACEFUL_EXIT_CODE_ENV] = prevSentinel; } }); }); @@ -185,6 +217,16 @@ describe('session CLI home scrub call sites', () => { expect(read('index-daemon.ts')).toContain('scrubClaudeSessionMarkerEnv(process.env)'); expect(read('worker.ts')).toContain('scrubClaudeSessionMarkerEnv(process.env)'); }); + + it('worker-pool strips the PM2 sentinel when forking a worker (source pin)', () => { + // WORKER_REDACTED_ENV_KEYS is a private const in worker-pool.ts (worker fork + // boundary, not importable without side effects), so pin at the source that + // the sentinel is in the strip list. redactChildEnv covers the CLI child; + // this covers the worker process itself so it also never exits 90. + const src = read('core/worker-pool.ts'); + const decl = src.slice(src.indexOf('const WORKER_REDACTED_ENV_KEYS')); + expect(decl.slice(0, decl.indexOf('\n'))).toContain(PM2_GRACEFUL_EXIT_CODE_ENV); + }); }); // ─── read-isolation markers must reach the child ────────────────────────── diff --git a/test/local-terminal-opener-spawn-env.test.ts b/test/local-terminal-opener-spawn-env.test.ts new file mode 100644 index 000000000..80a54427f --- /dev/null +++ b/test/local-terminal-opener-spawn-env.test.ts @@ -0,0 +1,80 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { DaemonSession } from '../src/core/types.js'; + +// Mock child_process so we can (a) make a terminal resolve on PATH via +// spawnSync (onPath) and (b) capture the detached spawn's env. Isolated in its +// own file so the real-spawnSync tests in local-terminal-opener.test.ts stay +// untouched. +const cp = vi.hoisted(() => ({ + spawn: vi.fn(() => ({ unref: () => {} })), + spawnSync: vi.fn(), +})); + +vi.mock('node:child_process', () => ({ + spawn: cp.spawn, + spawnSync: cp.spawnSync, +})); + +function session(overrides: Partial = {}): DaemonSession { + return { + session: { + sessionId: '1234567890abcdef', + chatId: 'oc_1', + rootMessageId: 'om_1', + title: 'test', + status: 'active', + createdAt: new Date(0).toISOString(), + backendType: 'tmux', + workingDir: '/tmp/project', + cliId: 'codex', + cliPathOverride: '/bin/echo', + cliSessionId: 'codex-native-session', + ...overrides, + }, + worker: null, + larkAppId: 'cli_app', + chatId: 'oc_1', + chatType: 'group', + scope: 'thread', + spawnedAt: 0, + cliVersion: 'test', + lastMessageAt: 0, + hasHistory: false, + } as DaemonSession; +} + +describe.skipIf(process.platform !== 'linux')('openLocalTerminalForSession spawn env (linux)', () => { + beforeEach(() => { + cp.spawn.mockClear(); + cp.spawnSync.mockReset(); + // Two spawnSync consumers in openLocalTerminalForSession's path, both via + // onPath(): (1) the CLI executable check `test -x /bin/echo`, and (2) the + // terminal-candidate probes `test -x /`. Report the CLI bin and + // the first terminal candidate (xdg-terminal-exec) as present so the flow + // reaches spawn(). + cp.spawnSync.mockImplementation((cmd: string, args: string[]) => { + const target = String(args?.[args.length - 1] ?? ''); + if (cmd === 'test' && target === '/bin/echo') return { status: 0, stdout: '', stderr: '' }; + const found = target.endsWith('/xdg-terminal-exec') || target === 'xdg-terminal-exec'; + return { status: found ? 0 : 1, stdout: '', stderr: '' }; + }); + vi.stubEnv('DISPLAY', ':0'); + }); + afterEach(() => vi.unstubAllEnvs()); + + it('does NOT pass the PM2 graceful-exit sentinel to the launched terminal', async () => { + const { PM2_GRACEFUL_EXIT_CODE_ENV } = await import('../src/pm2-graceful-exit.js'); + vi.stubEnv(PM2_GRACEFUL_EXIT_CODE_ENV, '90'); + const { openLocalTerminalForSession } = await import('../src/core/local-terminal-opener.js'); + + const result = openLocalTerminalForSession(session()); + expect(result.ok).toBe(true); + expect(cp.spawn).toHaveBeenCalledOnce(); + const opts = cp.spawn.mock.calls[0]?.[2] as { env: NodeJS.ProcessEnv }; + // The detached terminal (login shell → local AI CLI) must not inherit the + // marker, or a foreground botmux it launches would exit 90 on clean stop. + expect(opts.env[PM2_GRACEFUL_EXIT_CODE_ENV]).toBeUndefined(); + // Sanity: unrelated env still flows through (we didn't hand it an empty env). + expect(opts.env.DISPLAY).toBe(':0'); + }); +}); diff --git a/test/plugin-pm2-env.test.ts b/test/plugin-pm2-env.test.ts index 21078ac94..0368e2273 100644 --- a/test/plugin-pm2-env.test.ts +++ b/test/plugin-pm2-env.test.ts @@ -42,4 +42,21 @@ describe('plugin PM2 environment', () => { expect(options.env.PLUGIN_VALUE).toBe('preserved'); expect(options.env.PM2_HOME).toBe(join(home, '.botmux', 'pm2')); }); + + it('does not leak the daemon PM2 graceful-exit sentinel into plugin PM2 apps', async () => { + // The sentinel (BOTMUX_PM2_GRACEFUL_EXIT_CODE) is baked into the + // daemon/dashboard env. Dashboard starts plugin services via `pm2 start + // --update-env`, so pm2Env's raw process.env copy would otherwise write 90 + // into the plugin app's env — and a plugin service that later launches a + // foreground botmux would exit 90 on a clean stop. pm2Env must strip it. + const { PM2_GRACEFUL_EXIT_CODE_ENV } = await import('../src/pm2-graceful-exit.js'); + vi.stubEnv(PM2_GRACEFUL_EXIT_CODE_ENV, '90'); + vi.resetModules(); + const { runPluginPm2 } = await import('../src/core/plugins/pm2.js'); + + runPluginPm2(['start', 'fixture'], { inherit: false }); + + const options = childProcess.spawnSync.mock.calls[0]?.[2] as { env: NodeJS.ProcessEnv }; + expect(options.env[PM2_GRACEFUL_EXIT_CODE_ENV]).toBeUndefined(); + }); }); diff --git a/test/pm2-graceful-exit.test.ts b/test/pm2-graceful-exit.test.ts new file mode 100644 index 000000000..e36a1495f --- /dev/null +++ b/test/pm2-graceful-exit.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from 'vitest'; +import { + PM2_GRACEFUL_EXIT_CODE, + PM2_GRACEFUL_EXIT_CODE_ENV, + gracefulProcessExitCode, + pm2ManagedExitConfig, + stripPm2GracefulExitMarker, +} from '../src/pm2-graceful-exit.js'; + +describe('PM2 graceful exit sentinel', () => { + it('uses a dedicated stop code instead of 0 for PM2-managed processes', () => { + expect(pm2ManagedExitConfig()).toEqual({ + stopExitCodes: [PM2_GRACEFUL_EXIT_CODE], + env: { [PM2_GRACEFUL_EXIT_CODE_ENV]: String(PM2_GRACEFUL_EXIT_CODE) }, + }); + expect(pm2ManagedExitConfig().stopExitCodes).not.toContain(0); + }); + + it('returns the sentinel only for an explicitly marked PM2 process', () => { + expect(gracefulProcessExitCode({ + [PM2_GRACEFUL_EXIT_CODE_ENV]: String(PM2_GRACEFUL_EXIT_CODE), + })).toBe(PM2_GRACEFUL_EXIT_CODE); + expect(gracefulProcessExitCode({})).toBe(0); + expect(gracefulProcessExitCode({ [PM2_GRACEFUL_EXIT_CODE_ENV]: '0' })).toBe(0); + }); +}); + +describe('stripPm2GracefulExitMarker', () => { + it('removes the sentinel and returns a fresh copy (never mutates input)', () => { + const input = { [PM2_GRACEFUL_EXIT_CODE_ENV]: '90', KEEP: 'v' }; + const out = stripPm2GracefulExitMarker(input); + expect(PM2_GRACEFUL_EXIT_CODE_ENV in out).toBe(false); + expect(out.KEEP).toBe('v'); + // Input untouched — callers pass process.env and must not mutate it. + expect(input[PM2_GRACEFUL_EXIT_CODE_ENV]).toBe('90'); + expect(out).not.toBe(input); + }); + + it('returns a fresh copy even when the marker is absent (safe to mutate)', () => { + const input = { KEEP: 'v' }; + const out = stripPm2GracefulExitMarker(input); + expect(out).not.toBe(input); + expect(out.KEEP).toBe('v'); + // A caller that deletes another key from the result must not hit process.env. + delete (out as Record).KEEP; + expect(input.KEEP).toBe('v'); + }); +}); diff --git a/test/workflow-v3-ephemeral-pool.test.ts b/test/workflow-v3-ephemeral-pool.test.ts index f25d3a5ba..a7e6ba24a 100644 --- a/test/workflow-v3-ephemeral-pool.test.ts +++ b/test/workflow-v3-ephemeral-pool.test.ts @@ -103,6 +103,41 @@ describe('v3 ephemeral pool', () => { expect(worker.rawInputs).toEqual([buildGoalCommand(req)]); }); + it('does not carry the PM2 graceful-exit sentinel into the ephemeral worker env', async () => { + // The v3 ephemeral worker forks straight from the daemon (not via + // workerForkEnv), so the daemon's PM2 graceful-exit sentinel would ride + // process.env into it unless stripped. Harmless today (the worker doesn't + // read the graceful helper) but the "only daemon/dashboard carry it" + // invariant must hold — pin it so a refactor can't silently reintroduce it. + const { PM2_GRACEFUL_EXIT_CODE_ENV } = await import('../src/pm2-graceful-exit.js'); + const prev = process.env[PM2_GRACEFUL_EXIT_CODE_ENV]; + process.env[PM2_GRACEFUL_EXIT_CODE_ENV] = '90'; + try { + const worker = new ScriptedWorker(); + const factory = factoryFor(worker); + const pool = createEphemeralPool({ + factory, + workerPath: '/tmp/worker.js', + quiesceMs: 1, + resolveLarkAppSecret: () => 'secret', + }); + const promise = pool.runNode(request()); + await waitFor(() => factory.lastOpts !== undefined); + expect(factory.lastOpts?.env[PM2_GRACEFUL_EXIT_CODE_ENV]).toBeUndefined(); + // Teardown so the run promise resolves and doesn't leak a timer. + await worker.waitForInit(); + worker.emitMessage({ type: 'ready', port: 3001, token: 'tok' }); + worker.emitMessage({ type: 'prompt_ready' }); + worker.emitMessage({ type: 'final_output', content: 'done', lastUuid: 'u', turnId: 't' }); + await waitFor(() => worker.kills.includes('SIGTERM')); + worker.emitExit(0); + await promise; + } finally { + if (prev === undefined) delete process.env[PM2_GRACEFUL_EXIT_CODE_ENV]; + else process.env[PM2_GRACEFUL_EXIT_CODE_ENV] = prev; + } + }); + it('threads the run chat binding into worker init so CLI children get real BOTMUX_* identity env', async () => { const worker = new ScriptedWorker(); const factory = factoryFor(worker);