diff --git a/Cargo.lock b/Cargo.lock index 0aa28f386..c27275a6d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -592,6 +592,7 @@ name = "dirsql-napi" version = "0.1.0" dependencies = [ "dirsql", + "libc", "napi", "napi-build", "napi-derive", diff --git a/packages/ts/changelog.d/2026-09-20-sigint-ends-a-run-in-progress.md b/packages/ts/changelog.d/2026-09-20-sigint-ends-a-run-in-progress.md new file mode 100644 index 000000000..fa56a5ccb --- /dev/null +++ b/packages/ts/changelog.d/2026-09-20-sigint-ends-a-run-in-progress.md @@ -0,0 +1 @@ +**Fixed** Ctrl-C now ends a `dirsql` run that is still in progress through the npm launcher. The launcher's `process.on("SIGINT")` listener could never fire during a scan: the addon's `runCli` export is synchronous, so it blocks the event loop for the whole of the core's run and the listener is only reached once the work has already finished. The addon now drops SIGINT and SIGTERM to their default disposition for that window, which the kernel acts on unaided, matching the standalone binary. `dirsql server`'s graceful shutdown still exits 0 — the core registers its own handler over the default once it starts. diff --git a/packages/ts/e2e-attestations/claude-1166-ts-sigint.json b/packages/ts/e2e-attestations/claude-1166-ts-sigint.json new file mode 100644 index 000000000..16f7e187c --- /dev/null +++ b/packages/ts/e2e-attestations/claude-1166-ts-sigint.json @@ -0,0 +1,7 @@ +{ + "command": "pnpm test:e2e", + "ran_at": 1789916885, + "exit_code": 0, + "commit": "618f340fed1f91cb42689de96f7bc5b0d9d1fd5b", + "branch": "claude/1166-ts-sigint" +} diff --git a/packages/ts/migrations.d/2026-09-20-sigint-ends-a-run-in-progress.md b/packages/ts/migrations.d/2026-09-20-sigint-ends-a-run-in-progress.md new file mode 100644 index 000000000..26ef9a439 --- /dev/null +++ b/packages/ts/migrations.d/2026-09-20-sigint-ends-a-run-in-progress.md @@ -0,0 +1,35 @@ +### SIGINT ends a run in progress through the npm launcher + +**Summary** + +The npm launcher registered JS listeners for SIGINT and SIGTERM, but the napi `runCli` export is synchronous and blocks the event loop for the whole of the core's run, so neither listener could run while a scan was in progress. A signal arriving then was absorbed and the process could only be ended with SIGKILL. The addon now sets both signals to their default disposition for the duration of the core's run and restores the listeners' disposition after. No API changes; the observable difference is that a signal that used to do nothing now ends the process with the usual 128+signal status. + +**Required changes** + +| Before | After | +| --- | --- | +| `kill -INT ` during a query: absorbed; the run continues to completion | `kill -INT ` during a query: the process dies of SIGINT (shell reports `130`) | +| `kill -TERM ` during a query: absorbed; SIGKILL was the only way out | `kill -TERM ` during a query: the process dies of SIGTERM (shell reports `143`) | + +A script that sent SIGINT to a `dirsql` process and relied on it being ignored must stop sending it. A script that escalated to SIGKILL after an absorbed signal can drop the escalation. + +`dirsql server` is unchanged: its graceful shutdown still exits `0` on SIGINT and SIGTERM. + +**Deprecations removed** + +_None._ + +**Behavior changes without code changes** + +The exit status of an interrupted run changes from "no exit" to death by the signal. This aligns the npm launcher with the standalone `dirsql` binary and with the Python launcher. + +**Verification** + +Against a directory large enough to take a few seconds to scan: + +```bash +npx dirsql "SELECT count(*) FROM './'" # press Ctrl-C while it runs +echo $? +``` + +Expected: the process ends promptly and the shell reports `130`. Before this change it kept running and the signal was absorbed. Run it in the foreground: a non-interactive shell sets SIGINT to `SIG_IGN` for `&` background jobs, which hides the difference. diff --git a/packages/ts/napi/Cargo.toml b/packages/ts/napi/Cargo.toml index 4448ff12c..7047e651d 100644 --- a/packages/ts/napi/Cargo.toml +++ b/packages/ts/napi/Cargo.toml @@ -12,6 +12,7 @@ crate-type = ["cdylib"] # calls it in-process instead of spawning a bundled binary (#739), which is # what lets the `@dirsql/cli-*` package family go away. dirsql = { workspace = true, features = ["cli"] } +libc = "0.2" napi = { version = "3", features = ["napi6"] } napi-derive = "3" diff --git a/packages/ts/napi/src/lib.rs b/packages/ts/napi/src/lib.rs index 58e9f9e4a..b9fd7e6f8 100644 --- a/packages/ts/napi/src/lib.rs +++ b/packages/ts/napi/src/lib.rs @@ -45,7 +45,41 @@ pub fn run_cli(argv: Vec) -> i32 { let mut full = Vec::with_capacity(argv.len() + 1); full.push("dirsql".to_string()); full.extend(argv); - dirsql::cli::run_cli(full) + with_default_signal_disposition(|| dirsql::cli::run_cli(full)) +} + +const FATAL_SIGNALS: [i32; 2] = [libc::SIGINT, libc::SIGTERM]; + +/// Run `f` with SIGINT/SIGTERM at `SIG_DFL`, restoring the prior disposition +/// after. +/// +/// The launcher's JS listeners cannot fire while this call blocks the event +/// loop, so without this the signal is absorbed and a scan becomes +/// SIGKILL-only. `server` still exits gracefully: the core registers its own +/// handler over `SIG_DFL` once it starts. +#[cfg(unix)] +fn with_default_signal_disposition(f: impl FnOnce() -> T) -> T { + #[expect( + unsafe_code, + reason = "no safe API reaches a signal disposition; the raw call is the whole point" + )] + let saved: Vec<_> = FATAL_SIGNALS + .iter() + .map(|&sig| (sig, unsafe { libc::signal(sig, libc::SIG_DFL) })) + .collect(); + let out = f(); + for (sig, prev) in saved { + #[expect(unsafe_code, reason = "restores what the call above saved")] + unsafe { + libc::signal(sig, prev); + } + } + out +} + +#[cfg(not(unix))] +fn with_default_signal_disposition(f: impl FnOnce() -> T) -> T { + f() } /// The config paths the npm launcher must inspect for package-name diff --git a/packages/ts/src/cli/keep-signals-fatal.ts b/packages/ts/src/cli/keep-signals-fatal.ts index 0fdf8d677..35f1ce277 100644 --- a/packages/ts/src/cli/keep-signals-fatal.ts +++ b/packages/ts/src/cli/keep-signals-fatal.ts @@ -16,9 +16,10 @@ * is the whole remedy — no `unsafe`, no `sigaction`. */ export function keepSignalsFatal(): void { - // While the core is running, its own handler drives shutdown and this - // listener is the tail of the chain; it matters for signals arriving - // outside that window, where exiting is exactly right. + // This listener cannot fire while the core runs — the napi call blocks the + // event loop — so the addon drops to `SIG_DFL` for that window. + // Registering here is still what gives signal-hook a disposition to chain + // to, and it handles signals arriving outside that window. process.on("SIGINT", () => process.exit(130)); process.on("SIGTERM", () => process.exit(143)); } diff --git a/packages/ts/tests/e2e/sigint-interrupt.test.ts b/packages/ts/tests/e2e/sigint-interrupt.test.ts new file mode 100644 index 000000000..c068b6f4b --- /dev/null +++ b/packages/ts/tests/e2e/sigint-interrupt.test.ts @@ -0,0 +1,90 @@ +// CLI e2e: SIGINT ends a run that is still in progress. +// +// Spawns the real launcher over a real config whose `on-file` hook blocks, so +// the core is provably mid-scan when the signal lands, then asserts the +// process dies of it. Nothing is mocked: real launcher, real process, real +// filesystem, real core. + +import { spawn } from "node:child_process"; +import { existsSync } from "node:fs"; +import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { setTimeout as delay } from "node:timers/promises"; +import { fileURLToPath } from "node:url"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const PKG_ROOT = join(__dirname, "..", ".."); + +const PKG: { bin: { dirsql: string } } = JSON.parse( + await readFile(join(PKG_ROOT, "package.json"), "utf8"), +); +const CLI_ENTRY = join(PKG_ROOT, PKG.bin.dirsql); + +async function waitUntilScanning(ready: string, timeoutMs = 30_000) { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (existsSync(ready)) { + return; + } + await delay(50); + } + throw new Error("the scan never reached the on-file hook"); +} + +describe("SIGINT during a scan (CLI)", () => { + let dir: string; + let configPath: string; + let ready: string; + + beforeEach(async () => { + dir = await mkdtemp(join(tmpdir(), "dirsql-sigint-e2e-")); + configPath = join(dir, ".dirsql.toml"); + ready = join(dir, "scanning"); + await mkdir(join(dir, "data"), { recursive: true }); + await writeFile(join(dir, "data", "a.txt"), "hello"); + await writeFile( + configPath, + ` +[[table]] +name = "files" +ddl = "CREATE TABLE files (path TEXT)" +glob = "data/*.txt" +on-file = "sh -c 'touch ${ready}; sleep 120; printf \\"[]\\"'" +`, + ); + }); + + afterEach(async () => { + await rm(dir, { recursive: true, force: true }); + }); + + it("kills the process", async () => { + const proc = spawn( + process.execPath, + [CLI_ENTRY, "query", "SELECT * FROM files", "--config", configPath], + { cwd: dir, detached: true, stdio: ["ignore", "ignore", "ignore"] }, + ); + const died = new Promise<[number | null, NodeJS.Signals | null]>( + (resolve) => proc.on("close", (code, signal) => resolve([code, signal])), + ); + + try { + await waitUntilScanning(ready); + proc.kill("SIGINT"); + const settled = await Promise.race([ + died, + delay(15_000).then(() => "still running" as const), + ]); + expect(settled).toEqual([null, "SIGINT"]); + } finally { + const pid = proc.pid; + if (pid && proc.exitCode === null && proc.signalCode === null) { + // The blocking hook is a child in the same group; take the group. + process.kill(-pid, "SIGKILL"); + await died; + } + } + }, 60_000); +});