fix(review): exit immediately on the second signal (#1185)
* fix(review): exit immediately on the second signal (#1141) review_cmd.go registered SIGINT/SIGTERM via signal.NotifyContext. Its watcher goroutine receives the first signal, cancels the context and exits -- without unregistering the channel: stop() only runs in the caller's defer, after the whole graceful shutdown has finished. For that entire window (report flush, retry-report freeze, session persist, MCP client close) every further signal lands in the stdlib's unread size-one buffer and is dropped while the default kill behavior stays suppressed, so a second Ctrl+C does nothing however long the cleanup takes. Replace NotifyContext with an owned signal channel: the first signal cancels the context exactly as before, and the watcher now stays alive across the whole shutdown window; a second signal force-exits the process at once with status 1, skipping the remaining cleanup -- the user has explicitly abandoned the graceful path. A signal queued while stop() was closing done must not turn an already-successful run into exit 1, so the watcher drops latecomers once shutdown completion won the race. The buffer holds two entries: os/signal sends non-blockingly, so a size-one buffer could drop the second of two back-to-back signals before the watcher consumes the first. stop() is idempotent, unregisters the channel, releases the watcher and cancels the context, mirroring NotifyContext's stop semantics. Exit status stays plain 1 to match the current effective behavior; switching to the 128+sig convention (130 SIGINT / 143 SIGTERM) deserves its own discussion and is raised in the PR description. scan_cmd.go is untouched: it registers no signal handling today, and keeping it that way until #996 lands preserves symmetry between the two commands. Tests: in-process unit tests cover first-signal cancellation (SIGINT and SIGTERM), the second-signal forced exit through a stubbed hook, and watcher release on stop-before-signal and parent cancellation. Two subprocess integration tests re-execute the test binary so the real os.Exit path is exercised: a second SIGINT during a simulated 30s graceful shutdown ends the process well under 100ms with status 1, and a single SIGINT still completes the graceful path with the partial report persisted on disk and status 0. * ci: retrigger checks
H
Hao Guo committed
d64a4c560a2e493a6bf89db74bbaf5eeb1a790b2
Parent: 1471cfa
Committed by GitHub <noreply@github.com>
on 9/9/2026, 7:39:24 AM