feat(cli): graceful termination signals + broken-pipe guard#185
Conversation
Install handlers for SIGINT/SIGTERM/SIGHUP that print a one-line explanation (any started run keeps executing server-side; resume with `testsprite test list` or `testsprite test wait <runId>`) and exit with the conventional 128+signum code (SIGINT -> 130). Also guard EPIPE on stdout/stderr so piping to a reader that closes early (`... | head`) exits cleanly instead of dumping a raw `write EPIPE` stack. process and streams are injectable, so both are unit-tested without spawning a subprocess or sending a real signal. Fixes TestSprite#75
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds a new ChangesGraceful Termination and Broken-Pipe Handling
Estimated code review effort: 2 (Simple) | ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/interrupt.ts`:
- Around line 57-75: installSignalHandlers() exits immediately after writing the
interrupt hint, so the stderr output can be lost when stderr is piped or
redirected. Update the signal handler in installSignalHandlers to wait for the
stderr writes to complete or flush synchronously before calling exit, and keep
the logic centered around the existing stderr and exit deps plus
formatInterruptMessage/TERMINATION_EXIT_CODES so the hint is reliably emitted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0ba898e4-8a5d-48fd-8b21-d31ecca050d8
📒 Files selected for processing (3)
src/index.tssrc/lib/interrupt.test.tssrc/lib/interrupt.ts
A signal handler calls process.exit() immediately after writing the interrupt hint. When stderr is a pipe, an async process.stderr.write() may not flush before the process terminates, so the hint could be lost. The default stderr writer now uses fs.writeSync (best-effort, guarded against EPIPE) so the hint is reliably emitted. Added a test mocking fs.writeSync to assert the synchronous write on the default path.
|
Addressed the review: the signal handler now writes the interrupt hint synchronously via |
zeshi-du
left a comment
There was a problem hiding this comment.
Reviewed: scoped, tests included, CI green across the matrix.
Adds process-lifecycle hardening (fixes #75, the SIGINT ask, plus its natural siblings).
Termination signals: without a handler, Node kills the process abruptly on Ctrl+C during a long
test run --wait, leaving the user unsure whether the run was cancelled or is still executing server-side (it is: the CLI only polls; the run lives on the backend). Handlers for SIGINT/SIGTERM/SIGHUP now print a one-line explanation plus how to resume (testsprite test list/testsprite test wait <runId>) and exit with the conventional 128+signum code (SIGINT -> 130, SIGTERM -> 143, SIGHUP -> 129).Broken pipe: piping to a reader that closes early (
testsprite ... | head) raised an unhandledwrite EPIPEand dumped a raw stack (exit 1). The guard swallows EPIPE and exits 0 (the SIGPIPE-equivalent), while any other stream error still surfaces.SIGTERM/SIGHUP reuse the same mechanism as the SIGINT ask; the EPIPE guard is bundled as related process-teardown hardening. Happy to split if you prefer #75 scoped to SIGINT only.
Fixes #75
Summary by CodeRabbit