fix: forward signals to the teardown command

lib/flow-control/teardown.spec.ts

@@ -1,4 +1,5 @@
 import { ChildProcess } from 'node:child_process';
+import EventEmitter from 'node:events';
 
 import { afterEach, beforeEach, describe, expect, it, Mock, vi } from 'vitest';
 
@@ -122,3 +123,36 @@ describe('onFinish callback', () => {
         expect(spawn).toHaveBeenLastCalledWith('foo', expect.anything());
     });
 });
+
+describe('signals during teardown', () => {
+    it('forwards a signal to the running teardown command and runs no more', async () => {
+        const fakeProcess = new EventEmitter();
+        const kill = vi.fn();
+        const child = createFakeProcess(1);
+        spawn.mockReturnValue(child);
+
+        const { onFinish } = new Teardown({
+            spawn,
+            logger,
+            commands: [teardown, 'echo after'],
+            process: fakeProcess,
+            kill,
+        }).handle(commands);
+
+        // Listening from the start keeps a signal from killing concurrently, and leaving the
+        // command behind, between the commands' handlers going and teardown starting.
+        expect(fakeProcess.listenerCount('SIGTERM')).toBe(1);
+        fakeProcess.emit('SIGTERM', 'SIGTERM');
+        expect(kill).not.toHaveBeenCalled();
+
+        const result = onFinish();
+        fakeProcess.emit('SIGTERM', 'SIGTERM');
+        expect(kill).toHaveBeenCalledWith(1, 'SIGTERM');
+        expect(spawn).toHaveBeenCalledTimes(1);
+
+        child.emit('close', null, 'SIGTERM');
+        await expect(result).resolves.toBeUndefined();
+        expect(spawn).toHaveBeenCalledTimes(1);
+        expect(fakeProcess.listenerCount('SIGTERM')).toBe(0);
+    });
+});

lib/flow-control/teardown.ts

@@ -1,69 +1,118 @@
+import EventEmitter from 'node:events';
+
 import Rx from 'rxjs';
+import treeKill from 'tree-kill';
 
-import { Command, SpawnCommand } from '../command.js';
+import { Command, KillProcess, SpawnCommand } from '../command.js';
 import { Logger } from '../logger.js';
 import { getSpawnOpts } from '../spawn.js';
 import { FlowController } from './flow-controller.js';
 
+const SIGNALS = ['SIGINT', 'SIGTERM', 'SIGHUP'] as const;
+
 export class Teardown implements FlowController {
     private readonly logger: Logger;
     private readonly spawn: SpawnCommand;
     private readonly teardown: readonly string[];
+    private readonly process: EventEmitter;
+    private readonly kill: KillProcess;
 
     constructor({
         logger,
         spawn,
         commands,
+        process = globalThis.process,
+        kill = treeKill,
     }: {
         logger: Logger;
         /**
          * Which function to use to spawn commands.
          */
         spawn: SpawnCommand;
         commands: readonly string[];
+        /**
+         * The process whose signals go to a running teardown command.
+         */
+        process?: EventEmitter;
+        /**
+         * How to send a signal to a running teardown command. Defaults to `tree-kill`.
+         */
+        kill?: KillProcess;
     }) {
         this.logger = logger;
         this.spawn = spawn;
         this.teardown = commands;
+        this.process = process;
+        this.kill = kill;
     }
 
     handle(commands: Command[]): { commands: Command[]; onFinish: () => Promise<void> } {
-        const { logger, teardown, spawn } = this;
-        const onFinish = async () => {
-            if (!teardown.length) {
+        const { logger, teardown, spawn, process, kill } = this;
+        if (!teardown.length) {
+            return { commands, onFinish: async () => {} };
+        }
+
+        // Listen from the start: the other signal handlers are removed when the commands finish,
+        // and in the moment between that and a listener added later, a signal would kill
+        // concurrently and leave a teardown command running. Until teardown starts, these do
+        // nothing.
+        let tearingDown = false;
+        let caughtSignal: NodeJS.Signals | undefined;
+        let child: ReturnType<SpawnCommand> | undefined;
+        const forwardSignal = (signal: NodeJS.Signals) => {
+            if (!tearingDown) {
                 return;
             }
+            caughtSignal = signal;
+            if (child?.pid) {
+                kill(child.pid, signal);
+            }
+        };
+        SIGNALS.forEach((signal) => process.on(signal, forwardSignal));
 
-            for (const command of teardown) {
-                logger.logGlobalEvent(`Running teardown command "${command}"`);
+        const onFinish = async () => {
+            tearingDown = true;
+            try {
+                for (const command of teardown) {
+                    // A signal during teardown ends it.
+                    if (caughtSignal) {
+                        break;
+                    }
+                    logger.logGlobalEvent(`Running teardown command "${command}"`);
 
-                const child = spawn(command, getSpawnOpts({ stdio: 'raw' }));
-                const error = Rx.fromEvent(child, 'error');
-                const close = Rx.fromEvent(child, 'close');
+                    child = spawn(command, getSpawnOpts({ stdio: 'raw' }));
+                    const error = Rx.fromEvent(child, 'error');
+                    const close = Rx.fromEvent(child, 'close');
 
-                try {
-                    const [exitCode, signal] = await Promise.race([
-                        Rx.firstValueFrom(error).then((event) => {
-                            throw event;
-                        }),
-                        Rx.firstValueFrom(close).then(
-                            (event) => event as [number | null, NodeJS.Signals | null],
-                        ),
-                    ]);
+                    try {
+                        const [exitCode, signal] = await Promise.race([
+                            Rx.firstValueFrom(error).then((event) => {
+                                throw event;
+                            }),
+                            Rx.firstValueFrom(close).then(
+                                (event) => event as [number | null, NodeJS.Signals | null],
+                            ),
+                        ]);
 
-                    logger.logGlobalEvent(
-                        `Teardown command "${command}" exited with code ${exitCode ?? signal}`,
-                    );
+                        logger.logGlobalEvent(
+                            `Teardown command "${command}" exited with code ${exitCode ?? signal}`,
+                        );
 
-                    if (signal === 'SIGINT') {
-                        break;
+                        if (signal === 'SIGINT') {
+                            break;
+                        }
+                    } catch (error) {
+                        const errorText = String(
+                            error instanceof Error ? error.stack || error : error,
+                        );
+                        logger.logGlobalEvent(`Teardown command "${command}" errored:`);
+                        logger.logGlobalEvent(errorText);
+                        return Promise.reject(error);
                     }
-                } catch (error) {
-                    const errorText = String(error instanceof Error ? error.stack || error : error);
-                    logger.logGlobalEvent(`Teardown command "${command}" errored:`);
-                    logger.logGlobalEvent(errorText);
-                    return Promise.reject(error);
                 }
+            } finally {
+                child = undefined;
+                SIGNALS.forEach((signal) => process.off(signal, forwardSignal));
             }
         };