|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * A signal to `os start` takes its `serve` child down with it (#21114). |
| 5 | + * |
| 6 | + * ## The defect, as measured on `origin/main` before the repair |
| 7 | + * |
| 8 | + * `os start` is a supervisor: it spawns `os serve` as a SEPARATE process and |
| 9 | + * then waits. It listened for the child's `exit` and nothing else, so a SIGTERM |
| 10 | + * sent to the `start` pid alone (a plain `kill`, a CI step, a process manager) |
| 11 | + * ended the parent and left the child running, reparented to init, with its |
| 12 | + * port still bound and `/health` still answering 200. `os dev` never had the |
| 13 | + * defect: its `ServeRestartCoordinator` forwards SIGINT / SIGTERM to the child |
| 14 | + * and reaps the child when the parent exits. |
| 15 | + * |
| 16 | + * The repair is that `start` supervises its child through that SAME |
| 17 | + * coordinator, so there is one forwarding mechanism in the CLI, not two. |
| 18 | + * |
| 19 | + * ## What is asserted |
| 20 | + * |
| 21 | + * The ruling's pin, literally: a SIGTERM (and a SIGINT) to the PARENT leaves no |
| 22 | + * child process and a free port. `os dev`, booted on the same artifact and sent |
| 23 | + * the same signal, is the control: it states that the instrument reads the |
| 24 | + * shipped mechanism correctly, and that `dev`'s behaviour did not move. |
| 25 | + * |
| 26 | + * Both readings carry a positive control taken on the same boot before the |
| 27 | + * signal: the probe sees the child ALIVE and the port BOUND. Without that, "no |
| 28 | + * child" passes on a probe that cannot see children at all, and "port free" |
| 29 | + * passes on a child that bound somewhere else. |
| 30 | + * |
| 31 | + * ⛔ The signal goes to the parent's pid ALONE, never to its process group. A |
| 32 | + * group signal reaches the child directly and passes whether or not the parent |
| 33 | + * forwards anything — it is the shape that HID this defect (the neighbouring |
| 34 | + * `start-port-banner-agreement.e2e.test.ts` documents why it kills the group, |
| 35 | + * and for its purpose it is right to). |
| 36 | + * |
| 37 | + * ## Spawn shape |
| 38 | + * |
| 39 | + * `bin/run.js` with `NODE_ENV` unset (hence `requireBuiltCli`), the entrypoint |
| 40 | + * an operator runs. Each child gets its own process group (`detached: true`) |
| 41 | + * so `afterEach` can still SIGKILL a survivor — which is exactly what the |
| 42 | + * unrepaired command leaves behind — instead of leaking it into the container. |
| 43 | + */ |
| 44 | + |
| 45 | +import { describe, it, expect, beforeAll, afterEach } from 'vitest'; |
| 46 | +import { execFileSync, spawn, type ChildProcess } from 'node:child_process'; |
| 47 | +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; |
| 48 | +import { tmpdir } from 'node:os'; |
| 49 | +import { join, resolve, dirname } from 'node:path'; |
| 50 | +import { fileURLToPath } from 'node:url'; |
| 51 | +import { PROTOCOL_MAJOR } from '@objectstack/spec/kernel'; |
| 52 | +import { |
| 53 | + childEnv, |
| 54 | + boundPortFromBanner, |
| 55 | + portIsFree, |
| 56 | + requireBuiltCli, |
| 57 | + reservePort, |
| 58 | + RUN_JS_RESOLVES_FROM_DIST, |
| 59 | +} from './helpers/serve-process.js'; |
| 60 | + |
| 61 | +const HERE = dirname(fileURLToPath(import.meta.url)); |
| 62 | +const RUN_JS = resolve(HERE, '../bin/run.js'); |
| 63 | + |
| 64 | +/** The banner tail `printServerReady` ends with — the boot is fully printed. */ |
| 65 | +const BANNER_TAIL = /Press Ctrl\+C to stop/; |
| 66 | + |
| 67 | +const BOOT_TIMEOUT_MS = 180_000; |
| 68 | +/** The kernel's graceful shutdown, plus event-loop delivery of the exit. */ |
| 69 | +const EXIT_TIMEOUT_MS = 60_000; |
| 70 | + |
| 71 | +/** A minimal but real compiled artifact, booted identically by both commands. */ |
| 72 | +const ARTIFACT = JSON.stringify({ |
| 73 | + manifest: { |
| 74 | + id: 'com.example.signals', |
| 75 | + name: 'signals', |
| 76 | + version: '1.0.0', |
| 77 | + type: 'app', |
| 78 | + engines: { protocol: `^${PROTOCOL_MAJOR}` }, |
| 79 | + }, |
| 80 | + objects: [ |
| 81 | + { name: 'sig_note', label: 'Note', fields: { name: { type: 'text', label: 'Name' } } }, |
| 82 | + ], |
| 83 | + views: [], |
| 84 | + apps: [], |
| 85 | + flows: [], |
| 86 | + requires: [], |
| 87 | +}); |
| 88 | + |
| 89 | +type Command = 'start' | 'dev'; |
| 90 | + |
| 91 | +/** The argv that boots `command` on `artifact` at `port`, with no Console. */ |
| 92 | +function argvFor(command: Command, artifact: string, port: number): string[] { |
| 93 | + // `start` declares `--ui` with `allowNo` (default on); `dev` declares it |
| 94 | + // default-off and rejects `--no-ui`. Both therefore boot without a Console. |
| 95 | + return command === 'start' |
| 96 | + ? ['start', '--artifact', artifact, '--port', String(port), '--no-ui'] |
| 97 | + : ['dev', '--artifact', artifact, '--port', String(port)]; |
| 98 | +} |
| 99 | + |
| 100 | +/** Every pid whose parent is `ppid`, read from the process table. */ |
| 101 | +function childPidsOf(ppid: number): number[] { |
| 102 | + const table = execFileSync('ps', ['-A', '-o', 'pid=,ppid='], { |
| 103 | + encoding: 'utf8', |
| 104 | + env: childEnv(), |
| 105 | + }); |
| 106 | + return table |
| 107 | + .split('\n') |
| 108 | + .map((line) => line.trim().split(/\s+/).map(Number)) |
| 109 | + .filter(([pid, parent]) => Number.isInteger(pid) && parent === ppid) |
| 110 | + .map(([pid]) => pid); |
| 111 | +} |
| 112 | + |
| 113 | +/** Does `pid` name a live process? (`EPERM` = alive, owned by someone else.) */ |
| 114 | +function isAlive(pid: number): boolean { |
| 115 | + try { |
| 116 | + process.kill(pid, 0); |
| 117 | + return true; |
| 118 | + } catch (err) { |
| 119 | + return (err as NodeJS.ErrnoException).code === 'EPERM'; |
| 120 | + } |
| 121 | +} |
| 122 | + |
| 123 | +/** SIGKILL a whole process group — the survivor sweep, never the measurement. */ |
| 124 | +const killGroup = (child: ChildProcess | undefined): void => { |
| 125 | + if (child?.pid === undefined) return; |
| 126 | + try { process.kill(-child.pid, 'SIGKILL'); } catch { /* group already gone */ } |
| 127 | +}; |
| 128 | + |
| 129 | +interface Exit { code: number | null; signal: NodeJS.Signals | null } |
| 130 | + |
| 131 | +interface Booted { |
| 132 | + parent: ChildProcess; |
| 133 | + port: number; |
| 134 | + output: () => string; |
| 135 | + exited: Promise<Exit>; |
| 136 | +} |
| 137 | + |
| 138 | +let running: ChildProcess | undefined; |
| 139 | +let workdir: string | undefined; |
| 140 | + |
| 141 | +afterEach(() => { |
| 142 | + killGroup(running); |
| 143 | + running = undefined; |
| 144 | + if (workdir) rmSync(workdir, { recursive: true, force: true }); |
| 145 | + workdir = undefined; |
| 146 | +}); |
| 147 | + |
| 148 | +/** Boot a real `os <command>` and resolve once its child printed the ready banner. */ |
| 149 | +function boot(command: Command): Promise<Booted> { |
| 150 | + workdir = mkdtempSync(join(tmpdir(), `os-${command}-signal-`)); |
| 151 | + const artifact = join(workdir, 'objectstack.json'); |
| 152 | + writeFileSync(artifact, ARTIFACT); |
| 153 | + const port = reservePort(); |
| 154 | + |
| 155 | + return new Promise((resolveBoot, rejectBoot) => { |
| 156 | + const parent = spawn(process.execPath, [RUN_JS, ...argvFor(command, artifact, port)], { |
| 157 | + cwd: workdir, |
| 158 | + // `NODE_ENV: undefined` is required by the built entrypoint (#11464): |
| 159 | + // `development`/`test` sends oclif's command lookup back to `src/`. |
| 160 | + env: childEnv({ |
| 161 | + NODE_ENV: undefined, |
| 162 | + NO_COLOR: '1', |
| 163 | + OS_HOME: join(workdir, 'home'), |
| 164 | + OS_LOG_LEVEL: 'error', |
| 165 | + }), |
| 166 | + stdio: ['ignore', 'pipe', 'pipe'], |
| 167 | + detached: true, |
| 168 | + }); |
| 169 | + running = parent; |
| 170 | + |
| 171 | + let output = ''; |
| 172 | + let ready = false; |
| 173 | + const exited = new Promise<Exit>((settle) => { |
| 174 | + parent.once('exit', (code, signal) => settle({ code, signal })); |
| 175 | + }); |
| 176 | + |
| 177 | + const timer = setTimeout(() => { |
| 178 | + if (ready) return; |
| 179 | + rejectBoot(new Error(`os ${command} never printed a complete banner.\n--- output ---\n${output}`)); |
| 180 | + }, BOOT_TIMEOUT_MS); |
| 181 | + |
| 182 | + const onData = (d: unknown) => { |
| 183 | + output += String(d); |
| 184 | + if (!ready && BANNER_TAIL.test(output)) { |
| 185 | + ready = true; |
| 186 | + clearTimeout(timer); |
| 187 | + const readback = boundPortFromBanner(output); |
| 188 | + if (readback.state !== 'bound') { |
| 189 | + rejectBoot(new Error(`os ${command}: banner unreadable (${readback.state}).\n--- output ---\n${output}`)); |
| 190 | + return; |
| 191 | + } |
| 192 | + resolveBoot({ parent, port: readback.port, output: () => output, exited }); |
| 193 | + } |
| 194 | + }; |
| 195 | + parent.stdout?.on('data', onData); |
| 196 | + parent.stderr?.on('data', onData); |
| 197 | + parent.on('error', (err) => { clearTimeout(timer); rejectBoot(err); }); |
| 198 | + void exited.then(({ code, signal }) => { |
| 199 | + if (ready) return; |
| 200 | + clearTimeout(timer); |
| 201 | + rejectBoot(new Error( |
| 202 | + `os ${command} exited (${signal ?? code}) before its banner.\n--- output ---\n${output}`, |
| 203 | + )); |
| 204 | + }); |
| 205 | + }); |
| 206 | +} |
| 207 | + |
| 208 | +/** Signal the parent alone, then read what is left of its child and its port. */ |
| 209 | +async function signalParent(command: Command, signal: NodeJS.Signals): Promise<void> { |
| 210 | + const { parent, port, output, exited } = await boot(command); |
| 211 | + const parentPid = parent.pid!; |
| 212 | + |
| 213 | + // ── Positive controls, on the same boot, before the signal ───────────── |
| 214 | + const children = childPidsOf(parentPid); |
| 215 | + expect(children.length, `os ${command}: the probe sees no child of the parent.\n${output()}`) |
| 216 | + .toBeGreaterThan(0); |
| 217 | + expect(children.every(isAlive), `os ${command}: a child reads dead before the signal`).toBe(true); |
| 218 | + expect(portIsFree(port), `os ${command}: port ${port} reads free while the server is up`).toBe(false); |
| 219 | + |
| 220 | + // ── The measurement: the PARENT's pid alone, never its group ─────────── |
| 221 | + process.kill(parentPid, signal); |
| 222 | + |
| 223 | + let timer: ReturnType<typeof setTimeout> | undefined; |
| 224 | + const fate = await Promise.race([ |
| 225 | + exited, |
| 226 | + new Promise<'timeout'>((settle) => { timer = setTimeout(() => settle('timeout'), EXIT_TIMEOUT_MS); }), |
| 227 | + ]); |
| 228 | + clearTimeout(timer); |
| 229 | + expect(fate, `os ${command} did not exit within ${EXIT_TIMEOUT_MS}ms of ${signal}.\n${output()}`) |
| 230 | + .not.toBe('timeout'); |
| 231 | + |
| 232 | + expect( |
| 233 | + children.filter(isAlive), |
| 234 | + `os ${command}: ${signal} to the parent left its serve child running (orphaned, ` + |
| 235 | + `reparented to init).\n--- output ---\n${output()}`, |
| 236 | + ).toEqual([]); |
| 237 | + expect( |
| 238 | + portIsFree(port), |
| 239 | + `os ${command}: ${signal} to the parent left port ${port} bound.\n--- output ---\n${output()}`, |
| 240 | + ).toBe(true); |
| 241 | +} |
| 242 | + |
| 243 | +describe('a signal to the CLI parent takes its `serve` child down with it', () => { |
| 244 | + beforeAll(() => { |
| 245 | + requireBuiltCli(RUN_JS_RESOLVES_FROM_DIST); |
| 246 | + }); |
| 247 | + |
| 248 | + for (const signal of ['SIGTERM', 'SIGINT'] as const) { |
| 249 | + it( |
| 250 | + `os start: ${signal} to the parent leaves no child process and a free port`, |
| 251 | + () => signalParent('start', signal), |
| 252 | + BOOT_TIMEOUT_MS + EXIT_TIMEOUT_MS + 30_000, |
| 253 | + ); |
| 254 | + |
| 255 | + it( |
| 256 | + `os dev (control): ${signal} to the parent leaves no child process and a free port`, |
| 257 | + () => signalParent('dev', signal), |
| 258 | + BOOT_TIMEOUT_MS + EXIT_TIMEOUT_MS + 30_000, |
| 259 | + ); |
| 260 | + } |
| 261 | +}); |
0 commit comments