fix(ts): keep SIGINT fatal for the core's run in the npm launcher - #1167
Merged
Merged
Conversation
A new CLI e2e case blocks the scan in an `on-file` hook and asserts the launcher dies of the signal. skip-changelog: tests only
The launcher's JS listeners cannot fire while `runCli` runs: the napi export is synchronous and blocks the event loop for the whole of the core's run, so a signal arriving mid-scan was absorbed and the process was SIGKILL-only. The addon now sets SIGINT and SIGTERM to SIG_DFL for that window and restores the prior disposition after. `dirsql server` keeps its exit code: the core registers its own handler over SIG_DFL once it starts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1166
Ctrl-C did not interrupt a run in progress through the npm launcher. The
launcher's
process.on("SIGINT")listener cannot fire during a scan: theaddon's
runCliexport is synchronous and blocks the event loop for the wholeof the core's run, so the listener is only reached once the work has already
finished.
The addon now sets SIGINT and SIGTERM to
SIG_DFLfor the duration of thecore's run and restores the prior disposition after — the Node analogue of the
Python launcher's fix in #1165, one layer lower because JS cannot reach a
signal disposition.
Measured
Probed against the built addon with the launcher's two listeners registered,
over a config whose
on-filehook blocks (sleep 120). The spawner resetsSIGINT to
SIG_DFLbefore exec — a non-interactive shell setsSIG_IGNfor&jobs, which hides the difference.runClidirsql servershutdownSIG_DFLbelow JS, listeners kept (this PR)The middle row is why
keepSignalsFatalstays: registering the listeners iswhat gives signal-hook a disposition to chain to, and without it
servershutdown never runs. SIGTERM mid-scan dies at 143 with this change.
Hand-run against the built
dist/cli/dirsql.js: query mid-scan dies of SIGINT,serverexits 0.Red/green
The behavior lives at the e2e tier (a real launcher process and a real signal),
and no CI lane observes it: e2e is local-only by policy, and the
rust-napi-bindinglane runscolocated-test+unit-lintonly. CI was greenon the pushed test-only commit (115c459) for that reason, so the AGENTS.md
"CI red before implementation" gate could not be met here. Local evidence
instead:
115c459(test only) —vitest run --dir tests/e2e sigint-interrupt:AssertionError: expected 'still running' to deeply equal [ null, 'SIGINT' ]618f340(implementation) — same command passes; full TS e2e suite 18/18.A colocated napi unit test for the new guard was written and then removed: the
unit-lintgate'sno-out-of-module-callrule rejects a unit test that callslibc::signal, and injecting a trait double around two FFI calls is not worththe seam.
Changelog / Migrations
packages/ts/changelog.d/packages/ts/migrations.d/(runtimebehavior change: exit status of an interrupted run)
E2E Verification
packages/python/e2e-attestations/<branch>.jsonreceipt written ifpackages/pythonchanged: N/Apackages/ts/e2e-attestations/<branch>.jsonreceipt written (just e2e-attest-ts)pnpm test:e2e(packages/ts),pnpm test(packages/ts),cargo test --manifest-path packages/ts/napi/Cargo.toml,just preflightpreflight 0 failing pairs.