This repository has no description
Merge PR #36: forward signals from pty wrapper to inner cli
bin/pty was using spawnSync, which doesn't forward signals to the
child process. When systemd (KillMode=process) sent SIGTERM to the
wrapper during a service restart, the wrapper exited but the inner
supervisor cli kept running, holding supervisor.lock. The next
service start then failed with "another supervisor is already
running" — and systemd's Restart=on-failure looped that forever.
Fix: switch to spawn so the wrapper stays event-loop-live, register
forwarders for SIGTERM/SIGINT/SIGHUP/SIGQUIT/SIGUSR1/SIGUSR2, and
drive wrapper exit from the child's exit event so the child has
time to flush. Signal-death maps to 128 + signum per shell
convention. spawn-failure prints a clear stderr message and exits 1.
Reported and fixed by @schickling-assistant. Includes
tests/wrapper-signal-forwarding.test.ts which spawns the real
wrapper running supervisor start, SIGTERMs the wrapper, and asserts
the inner cli pid dies and supervisor.lock is cleared (proving
graceful shutdown rather than wrapper-level SIGKILL). The reporter
verified the test fails when the old spawnSync wrapper is reapplied.
Nathan Herald
(May 3, 2026, 10:12 AM +0200)
0ce84315
d08d20ed
+130
-3
+33
-3
bin/pty
+33
-3
bin/pty
···
1
1
#!/usr/bin/env node
2
2
import { existsSync } from 'node:fs';
3
-
import { spawnSync } from 'node:child_process';
3
+
import { spawn } from 'node:child_process';
4
4
import { fileURLToPath } from 'node:url';
5
5
import { dirname, join } from 'node:path';
6
+
import { constants as osConstants } from 'node:os';
6
7
7
8
const __dirname = dirname(fileURLToPath(import.meta.url));
8
9
const cli = join(__dirname, '..', 'dist', 'cli.js');
···
12
13
process.exit(1);
13
14
}
14
15
15
-
const result = spawnSync(process.execPath, [cli, ...process.argv.slice(2)], {
16
+
// `spawn` (not `spawnSync`) so the wrapper stays event-loop-live and can
17
+
// react to signals it receives. Without this, signals delivered to the
18
+
// wrapper (e.g. systemd's SIGTERM under KillMode=process) terminated the
19
+
// wrapper without ever reaching the child supervisor, leaving an orphaned
20
+
// supervisor that kept holding `supervisor.lock` and made every subsequent
21
+
// service restart fail with "another supervisor is already running".
22
+
const child = spawn(process.execPath, [cli, ...process.argv.slice(2)], {
16
23
stdio: 'inherit',
17
24
env: process.env,
18
25
});
19
26
20
-
process.exit(result.status ?? 1);
27
+
const FORWARDED_SIGNALS = ['SIGTERM', 'SIGINT', 'SIGHUP', 'SIGQUIT', 'SIGUSR1', 'SIGUSR2'];
28
+
for (const sig of FORWARDED_SIGNALS) {
29
+
process.on(sig, () => {
30
+
// Forward and let the child decide when to exit. The wrapper's own
31
+
// exit is driven by the child's 'exit' event so the child has time
32
+
// to flush before we propagate its status.
33
+
try { child.kill(sig); } catch {}
34
+
});
35
+
}
36
+
37
+
child.on('exit', (code, signal) => {
38
+
if (signal) {
39
+
// Mirror shell convention (128 + signum) so callers can distinguish
40
+
// signal death from a nonzero exit code.
41
+
const signum = osConstants.signals[signal] ?? 0;
42
+
process.exit(128 + signum);
43
+
}
44
+
process.exit(code ?? 1);
45
+
});
46
+
47
+
child.on('error', (err) => {
48
+
console.error(`pty: failed to spawn cli: ${err.message}`);
49
+
process.exit(1);
50
+
});
+97
tests/wrapper-signal-forwarding.test.ts
+97
tests/wrapper-signal-forwarding.test.ts
···
1
+
// Verifies that bin/pty forwards SIGTERM/SIGINT to the inner cli.js child.
2
+
// Without forwarding, systemd's KillMode=process leaves the inner supervisor
3
+
// orphaned and still holding supervisor.lock — the new unit invocation then
4
+
// fails with "another supervisor is already running".
5
+
6
+
import { describe, it, expect, afterEach } from "vitest";
7
+
import * as fs from "node:fs";
8
+
import * as os from "node:os";
9
+
import * as path from "node:path";
10
+
import { fileURLToPath } from "node:url";
11
+
import { spawn } from "node:child_process";
12
+
13
+
const __dirname = path.dirname(fileURLToPath(import.meta.url));
14
+
const wrapperPath = path.join(__dirname, "..", "bin", "pty");
15
+
const nodeBin = process.execPath;
16
+
17
+
const sessionDirs: string[] = [];
18
+
const trackedPids: number[] = [];
19
+
afterEach(() => {
20
+
// Belt-and-braces: if a test failed mid-way, kill any inner cli.js it left
21
+
// behind so the next run isn't poisoned by an orphan holding the lock.
22
+
for (const pid of trackedPids) {
23
+
try { process.kill(pid, "SIGKILL"); } catch {}
24
+
}
25
+
trackedPids.length = 0;
26
+
for (const d of sessionDirs) {
27
+
try { fs.rmSync(d, { recursive: true, force: true }); } catch {}
28
+
}
29
+
sessionDirs.length = 0;
30
+
});
31
+
32
+
function makeSessionDir(): string {
33
+
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "pty-wrap-"));
34
+
sessionDirs.push(dir);
35
+
return dir;
36
+
}
37
+
38
+
/** Wait until predicate returns true or `timeoutMs` elapses. */
39
+
async function waitFor(predicate: () => boolean, timeoutMs: number): Promise<boolean> {
40
+
const start = Date.now();
41
+
while (Date.now() - start < timeoutMs) {
42
+
if (predicate()) return true;
43
+
await new Promise((r) => setTimeout(r, 25));
44
+
}
45
+
return predicate();
46
+
}
47
+
48
+
function isAlive(pid: number): boolean {
49
+
try { process.kill(pid, 0); return true; } catch { return false; }
50
+
}
51
+
52
+
describe("bin/pty signal forwarding", () => {
53
+
it("propagates SIGTERM to the inner cli.js so the supervisor releases its lock", async () => {
54
+
const sessionDir = makeSessionDir();
55
+
56
+
// `supervisor start` is the canonical long-lived command and the one the
57
+
// dev3 regression hit. Its SIGTERM handler calls Supervisor.stop() which
58
+
// releases supervisor.lock — so a clean shutdown leaves no lock file.
59
+
const wrapper = spawn(nodeBin, [wrapperPath, "supervisor", "start"], {
60
+
env: { ...process.env, PTY_SESSION_DIR: sessionDir },
61
+
stdio: ["ignore", "pipe", "pipe"],
62
+
});
63
+
64
+
let stdout = "";
65
+
let stderr = "";
66
+
wrapper.stdout?.on("data", (d: Buffer) => { stdout += d.toString(); });
67
+
wrapper.stderr?.on("data", (d: Buffer) => { stderr += d.toString(); });
68
+
69
+
let exitCode: number | null = null;
70
+
let exitSignal: NodeJS.Signals | null = null;
71
+
wrapper.on("exit", (c, s) => { exitCode = c; exitSignal = s; });
72
+
73
+
// Wait for the supervisor to fully start (it writes supervisor.pid).
74
+
const pidPath = path.join(sessionDir, "supervisor", "supervisor.pid");
75
+
const lockPath = path.join(sessionDir, "supervisor.lock");
76
+
const ready = await waitFor(() => fs.existsSync(pidPath) && fs.existsSync(lockPath), 5000);
77
+
expect(ready, `supervisor never started; stdout=${stdout} stderr=${stderr}`).toBe(true);
78
+
79
+
const innerPid = parseInt(fs.readFileSync(pidPath, "utf-8").trim(), 10);
80
+
trackedPids.push(innerPid);
81
+
expect(innerPid).toBeGreaterThan(0);
82
+
expect(innerPid).not.toBe(wrapper.pid);
83
+
expect(isAlive(innerPid)).toBe(true);
84
+
85
+
// The actual test: SIGTERM the wrapper, expect the inner cli.js to also die.
86
+
wrapper.kill("SIGTERM");
87
+
88
+
const wrapperExited = await waitFor(() => exitCode !== null || exitSignal !== null, 5000);
89
+
expect(wrapperExited, "wrapper did not exit after SIGTERM").toBe(true);
90
+
91
+
const innerDied = await waitFor(() => !isAlive(innerPid), 5000);
92
+
expect(innerDied, `inner cli.js (pid ${innerPid}) survived wrapper SIGTERM — signal not forwarded`).toBe(true);
93
+
94
+
// Clean shutdown should release the lock; a SIGKILLed supervisor would leave it.
95
+
expect(fs.existsSync(lockPath), "supervisor.lock not released — child shutdown was not graceful").toBe(false);
96
+
}, 20000);
97
+
});