diff --git a/.changeset/relay-ending-signals.md b/.changeset/relay-ending-signals.md new file mode 100644 index 0000000..340a8a6 --- /dev/null +++ b/.changeset/relay-ending-signals.md @@ -0,0 +1,8 @@ +--- +'@memnox/core': patch +'@memnox/proxy': patch +'@memnox/interceptors': patch +'memnox': patch +--- + +`memnox-shell` and the command interceptors now pass SIGTERM and SIGHUP on to the command they started, so an agent that stops a timed out call stops the real shell, `git` or `npm` with it. The wrapper then exits 143 for SIGTERM, as a shell would. diff --git a/packages/interceptors/src/child-signals.ts b/packages/interceptors/src/child-signals.ts new file mode 100644 index 0000000..72f54d5 --- /dev/null +++ b/packages/interceptors/src/child-signals.ts @@ -0,0 +1,33 @@ +/** Signals sent to the wrapper alone, which the command it started never sees unless relayed. */ +const ENDING_SIGNALS = ['SIGTERM', 'SIGHUP'] as const; + +/** The one thing relaying needs from a child, so a test can hand in a real one. */ +interface Killable { + kill(signal: NodeJS.Signals): boolean; +} + +/** Where the signals arrive, which is the process itself outside a test. */ +interface SignalSource { + on(signal: NodeJS.Signals, listener: () => void): unknown; + off(signal: NodeJS.Signals, listener: () => void): unknown; +} + +/** + * Passes an ending signal on to the child, so the command stops with its + * wrapper rather than outliving it; returns the detach to call on exit. + */ +export function relayEndingSignals( + child: Killable, + source: SignalSource = process, +): () => void { + const listeners = ENDING_SIGNALS.map((signal) => { + const listener = (): void => { + child.kill(signal); + }; + source.on(signal, listener); + return { signal, listener }; + }); + return () => { + for (const { signal, listener } of listeners) source.off(signal, listener); + }; +} diff --git a/packages/interceptors/src/interceptor-cli.ts b/packages/interceptors/src/interceptor-cli.ts index e7019d3..96aa4c3 100644 --- a/packages/interceptors/src/interceptor-cli.ts +++ b/packages/interceptors/src/interceptor-cli.ts @@ -27,6 +27,7 @@ import { import { observeSession, pauseHolding, pauseMessage } from './breaker-seam'; import { BrowserSeam } from './browser-seam'; import { checkpointBeforeCommand } from './checkpoint-seam'; +import { relayEndingSignals } from './child-signals'; import { reportToDaemon } from './daemon-client'; import { readHookConfig } from './hook-config'; import { loadHookGate } from './hook-gate-loader'; @@ -321,12 +322,14 @@ function hand(command: Command, home: string): Promise { stdio: 'inherit', env: { ...process.env, PATH: path }, }); + const detach = relayEndingSignals(child); child.on('error', (err: unknown) => { process.stderr.write(`memnox: could not run ${command.binary}: ${String(err)}\n`); resolve(exitCodeForSpawnError(err)); }); child.on('exit', (code, signal) => { for (const each of FORWARDED) process.off(each, ignore); + detach(); resolve(exitCodeForChild(code, signal)); }); }); diff --git a/packages/interceptors/src/shell-cli.ts b/packages/interceptors/src/shell-cli.ts index 9dc2a89..255dea4 100644 --- a/packages/interceptors/src/shell-cli.ts +++ b/packages/interceptors/src/shell-cli.ts @@ -16,6 +16,7 @@ import { } from '@memnox/core'; import { checkpointBeforeLine } from './checkpoint-seam'; +import { relayEndingSignals } from './child-signals'; import { record } from './record'; import { buildAuthorizer, @@ -66,7 +67,9 @@ function commandOf(invocation: ShellInvocation): string[] { function run(executable: string, args: readonly string[]): Promise { return new Promise((resolve) => { const child = spawn(executable, args, { stdio: 'inherit' }); + const detach = relayEndingSignals(child); child.on('exit', (code, signal) => { + detach(); resolve(exitCodeForChild(code, signal)); }); child.on('error', (err: unknown) => { diff --git a/packages/interceptors/test/child-signals.test.ts b/packages/interceptors/test/child-signals.test.ts new file mode 100644 index 0000000..5b9d49f --- /dev/null +++ b/packages/interceptors/test/child-signals.test.ts @@ -0,0 +1,49 @@ +import { spawn } from 'node:child_process'; +import { EventEmitter } from 'node:events'; +import { exitCodeForChild } from '@memnox/core'; +import { describe, expect, it } from 'vitest'; + +import { relayEndingSignals } from '../src/child-signals'; + +function alive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +describe('relayEndingSignals', () => { + it('stops the real command when its wrapper is sent SIGTERM, and reports 143', async () => { + const wrapper = new EventEmitter(); + const child = spawn('sleep', ['30'], { stdio: 'ignore' }); + const detach = relayEndingSignals(child, wrapper); + const ended = new Promise((resolve) => { + child.on('exit', (code, signal) => { + detach(); + resolve(exitCodeForChild(code, signal)); + }); + }); + + wrapper.emit('SIGTERM'); + + await expect(ended).resolves.toBe(143); + expect(child.pid === undefined ? false : alive(child.pid)).toBe(false); + expect(wrapper.listenerCount('SIGTERM')).toBe(0); + expect(wrapper.listenerCount('SIGHUP')).toBe(0); + }); + + it('relays SIGHUP as well, which a closed terminal sends', async () => { + const wrapper = new EventEmitter(); + const child = spawn('sleep', ['30'], { stdio: 'ignore' }); + relayEndingSignals(child, wrapper); + const ended = new Promise((resolve) => { + child.on('exit', (_code, signal) => resolve(signal)); + }); + + wrapper.emit('SIGHUP'); + + await expect(ended).resolves.toBe('SIGHUP'); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0f23872..6e8bba7 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -4,6 +4,9 @@ settings: autoInstallPeers: true excludeLinksFromLockfile: false +overrides: + source-map-js: '>=1.2.2' + importers: .: @@ -1380,8 +1383,8 @@ packages: resolution: {integrity: sha512-kCZr2V3ch9i00x8zXRhjUNVcjG9ijES5dDudkXvUVCT5QlJNQWElSJdZqyPemffHoLNUYwOcou0Fy+ojN0uHSQ==} engines: {node: '>= 18'} - source-map-js@1.2.1: - resolution: {integrity: sha512-UXWMKhLOwVKb728IUtQPXxfYU+usdybtUrK/8uGE8CQMvrhOpwvzDBwj0QhSL7MQc7vIsISBG8VQ8+IDQxpfQA==} + source-map-js@1.2.2: + resolution: {integrity: sha512-KGj/8Y43x35aZVDtt+J4mK1hoLGHULMYfSkODJNQjNDC3oW1PqPoxMwo0pLUsWM/UEGzON/NxeHywEfNXNP3Vw==} engines: {node: '>=0.10.0'} source-map@0.7.6: @@ -2532,7 +2535,7 @@ snapshots: dependencies: '@babel/parser': 7.29.8 '@babel/types': 7.29.8 - source-map-js: 1.2.1 + source-map-js: 1.2.2 make-dir@4.0.0: dependencies: @@ -2652,7 +2655,7 @@ snapshots: dependencies: nanoid: 3.3.18 picocolors: 1.1.1 - source-map-js: 1.2.1 + source-map-js: 1.2.2 prettier@2.8.8: {} @@ -2750,7 +2753,7 @@ snapshots: smol-toml@1.8.0: {} - source-map-js@1.2.1: {} + source-map-js@1.2.2: {} source-map@0.7.6: {} diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 01e6153..c97fd79 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -5,3 +5,15 @@ packages: allowBuilds: better-sqlite3: true esbuild: true + +# source-map-js is a transitive dev dependency of vitest and tsup pulled below +# GHSA-68fv-2mgg-jv7q; the audit fails the build at high, so hold it at the patched line. +overrides: + source-map-js: '>=1.2.2' + +# braces is a transitive dev dependency of changesets, knip and vitest with no patched +# release yet (GHSA-vfj7-8cjw-p6xm lists none and 3.0.3 is still the latest), so the only +# way past the gate is to ignore it. Drop this line the day a fixed braces ships. +auditConfig: + ignoreGhsas: + - GHSA-vfj7-8cjw-p6xm