Repository navigation
fix(cli): os start forwards SIGTERM/SIGINT to its serve child and reaps it on exit - #21161
Conversation
…ps it on exit `os start` listened for its `serve` child's exit and nothing else, so a signal to the start pid alone orphaned the child with its port bound and /health answering. Supervise the child through ServeRestartCoordinator, the forwarding mechanism `os dev` already runs, used as is: beginShutdown on SIGINT/SIGTERM, killChildOnParentExit on exit, and a self-exiting child still ends the parent with `code ?? 0`. Pinned end to end (nightly e2e tier) with `os dev` as the control: a SIGTERM or SIGINT to the parent leaves no child process and a free port. Claude-Session: https://claude.ai/code/session_01VvcEokUG1tvVxkceYfR5XB Co-authored-by: Claude <noreply@anthropic.com>
…h os dev Claude-Session: https://claude.ai/code/session_01VvcEokUG1tvVxkceYfR5XB Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VvcEokUG1tvVxkceYfR5XB Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VvcEokUG1tvVxkceYfR5XB Co-authored-by: Claude <noreply@anthropic.com>
…arding Claude-Session: https://claude.ai/code/session_01VvcEokUG1tvVxkceYfR5XB Co-authored-by: Claude <noreply@anthropic.com>
The built entry (`bin/run.js`) puts a spawner into the population `check:cli-test-child-env` pins at six files. The pin's subject is the supervisor wiring in src/commands/start.ts, which both entries run, so it spawns `bin/run-dev.js` under the tsx loader as an `--import` flag: the spawned pid is the CLI parent itself and the `serve` process its direct child. The bound port is still read back from the child's banner. Claude-Session: https://claude.ai/code/session_01VvcEokUG1tvVxkceYfR5XB Co-authored-by: Claude <noreply@anthropic.com>
On a cold tsx transform cache the loader's `esbuild --service` process is a second child of the CLI parent and exits a moment after it. Counting every child read that as an orphaned server on the file's first boot. Claude-Session: https://claude.ai/code/session_01VvcEokUG1tvVxkceYfR5XB Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 16 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 6 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 25 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 0f744c6b8b60d12e01ebbc05f7a432e3db25de61 && git checkout 0f744c6b8b60d12e01ebbc05f7a432e3db25de61
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 2488b98b48f51e2a1bc5b5e50fc1c206c9565291 4d2d7f9bc68c75165f7e1f15af22c32ac3f1f8fd && git checkout -B drift-repro 2488b98b48f51e2a1bc5b5e50fc1c206c9565291 && git merge --no-ff 4d2d7f9bc68c75165f7e1f15af22c32ac3f1f8fd
node scripts/docs-audit/affected-docs.mjs --json 2488b98b48f51e2a1bc5b5e50fc1c206c9565291
|
Contract reviewServed-tier: Reviewed against: card #21114 (body; triage grade ① Derived judgments(a) One mechanism, no second copy — holds.
(b) Every other
(c) The pin — sound.
(d) The open question — nightly is acceptable; no per-PR guard is owed for PASS.
② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS |
Fixes #21114
Clause-②: no
What changed
os startnow supervises itsservechild throughServeRestartCoordinator, the forwarding mechanismos devalready runs, used as is (packages/cli/src/commands/start.ts):startprocess is forwarded to the child (beginShutdown). The child's exit then ends the parent with the child's exit code.killChildOnParentExitonprocess.on('exit')).code ?? 0, which is the one handlerstarthad before.Also in this PR: the pin
packages/cli/test/start-signal-forwarding.e2e.test.tsand.changeset/21114-start-signal-forwarding.md(@objectstack/clipatch).Why the coordinator is used as is, and nothing is extracted
Triage asked for one mechanism, "extracted to a shared helper if needed". It is not needed here:
starthas no restart semantics, so it never callsrequestRestart. The other paths are whatstartneeds:start(),beginShutdown,killChildOnParentExit, and the child-exit path (state === 'running'ends the parent withcode ?? 0).start. They are gated onspawnCount > 1or are insiderequestRestart. The spawn-failure message readsfailed to start server, becauserestartIndexis 0.dev-restart.tsanddev.tsare unchanged, and the coordinator's own unit tests (src/utils/dev-restart.test.ts) already coverbeginShutdownandkillChildOnParentExit.The only text the two commands now share is the three-line subscription (
process.onforSIGINT,SIGTERMandexit). It stays at each call site, as the dispatch directed for the no-extraction route. What a signal does lives in the coordinator alone.Measured, before and after
Run on
examples/app-showcasewith the built entry:node packages/cli/bin/run.js start -p PORT --no-ui &(control:dev -p PORT --no-watch &). The signal went to the parent's pid alone, on a random high port. The before tree isorigin/main7a606a9a34; the after tree is20eaab626f, wherestart.tsis byte-identical to this head.servechild after/api/v1/healthafteros startos startos startos startos dev(control)os dev(control)os dev(control)os dev(control)On the before tree the orphans had to be killed by their own pid. On the after tree every recorded pid was gone without help.
The Ctrl-C shape was measured on the after tree too (SIGINT to the whole process group). For both
startanddev, the parent and the child were gone and the port was free. Both logged oneShutdown already in progress, ignoring SIGINTline, because the child receives the signal from the group and again from the forward. The changeset states this.The pin
test/start-signal-forwarding.e2e.test.tsbootsos startandos devon the same minimal artifact. For SIGTERM and for SIGINT it signals the parent pid alone, never the group, and asserts that theservechild is gone and the port is free. Both readings have a positive control on the same boot before the signal: exactly oneservechild is seen alive, and the port reads bound.bin/run-dev.jsunder the tsx loader, passed as--import, so the spawned pid is the CLI parent itself. The built entry would have added a seventh spawner to the six-file population thatcheck:cli-test-child-envpins. The subject is the supervisor wiring insrc/commands/start.ts, which both entries run. The built entry is covered by the hand measurement above.esbuild --serviceprocess as a second child of the CLI parent. That process outlives the parent by a moment: it was alive at the parent's exit and gone 2 s later, in 2 of 2 probes withTSX_DISABLE_CACHE=1. One early run of the pin went red on exactly that. The probe now selects the child whose argv carries theservetoken, and the pin is green with the cache forced cold..e2ename puts the pin in the nightly tier (OS_TEST_TIERS=nightly), next tostart-port-banner-agreement.e2e.test.ts. It does not run in this PR's CI. Each run boots 4 kernels, about 16 s each on a shared box.Ablation (source mode: the pin reads
src/, so no build leg). The fix was committed first.scripts/ablation-replace.mjsreplaced the threeprocess.onlines with a planted marker statement and confirmed the change on disk (anchor 1 to 0, blobb6d246505be6to1055d17fdbeb). It ran the pin, then restored the file (blob back tob6d246505be6,git diff HEADempty, porcelain empty). Results at head4d2d7f9bc6:os startSIGTERM and SIGINT both red on both readings (left its serve child running,left port N bound);os devcontrol green on both signals; 2 failed, 2 passed;An earlier built-entry version of the pin was ablated the same way on
dist/, withscripts/ablation-dist-preflight.mjsconfirming the marker present and then absent. It gave the same direction onb299389161and on the mergeda096821511.Verification
All readings below were taken at head
4d2d7f9bc6unless another commit is named. This branch mergedorigin/mainc6954d6d09before the last commits, and the build state was refreshed after the merge.OS_TEST_TIERS=nightly pnpm --filter @objectstack/cli exec vitest run --project integration test/start-signal-forwarding.e2e.test.ts: 4 passed. With the transform cache forced cold (TSX_DISABLE_CACHE=1): 4 passed. Ablated: 2 failed, 2 passed. Restored: 4 passed.pnpm --filter @objectstack/cli typecheckexited 0 (check:test-typecheck: OK).unittier.pnpm --filter @objectstack/cli exec vitest run --project unit: 242 files and 3432 tests passed ata096821511. Since then only the e2e pin changed, and it is outside theunitproject. At4d2d7f9bc6,test/vitest-tiers-partition.test.ts,src/utils/port-contract-single-source.test.tsandsrc/utils/dev-restart.test.tswere re-run: 3 files and 51 tests passed. Theintegrationtier is left to CI, because this diff touches no spawn entry and no integration-layer file other than the new nightly pin.node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commandsderived 63 commands. Each was run, and every one exited 0.--ran, with an exit code on every line, reported63 derived, 63 run, 0 NOT-MEASURED, 0 UNRUN, a derived zero.check:dual-build-cjs-loadsfirst answered PREREQUISITE NOT MET: 8 packages outside the cli closure had nodist/. They were built, and it was re-run as part of the 63. Derivation residual: the tree was 9 commits behindorigin/main70dae533c5, and one derivation input,scripts/doc-authoring-prose-id.baseline.json, changed upstream.pnpm lint, which is left to CI:--print-configresolves 6 and 5 rules for them, and neither file is ignored.--format jsonread 2 files, 0 errors and 0 warnings.parserOptions.projectandprojectServiceare undefined for both files). Its only inputs on disk arescripts/slot-lookup-baseline.jsonandscripts/query-options-erasure-baseline.json, and neither is in this diff, so no verdict on an untouched file can change.Acceptance notes
AGENTS.mdwritten by turbo. The dev-dependency bump at840ec9dab3movedturbofrom 2.10.10 to 2.11.5. That version appends a managed "turborepo agent rules" block, wrapped in HTML comment markers, toAGENTS.mdon repository-scoped commands when it detects an AI agent.turbo.jsonsets noagentGuidanceopt-out. Measured in this worktree: the firstpnpm exec turbo run buildafter mergingmainleftM AGENTS.md(11 added lines).scripts/ablation-dist-preflight.mjs --absentthen answered exit 3 on its tree reading. The file was restored from HEAD and is not part of this diff. It is reported to the seat for filing, not fixed here.src/utils/port-contract-single-source.test.tsanchors on the textconst child = spawn(instart.tsto keep the port door ahead of the spawn. The spawn keeps that spelling inside the coordinator'sspawnChild, with a comment saying why, so that structural pin keeps measuring the same order.ServeRestartCoordinator's docblock indev-restart.tsstill describes onlyos dev. It was left alone becausedev-restart.tsis outside this card's surface on the no-extraction route.start.tsnames the coordinator and the pin instead.os startnow waits for the server's graceful shutdown and exits 0. Before, it died on the signal (shell status 143 or 130) while the server kept running. This matchesos dev.Generated by Claude Code