Add ctrl-c propagation e2e test with graceful SIGINT handling#299
Draft
branchseer wants to merge 7 commits intomainfrom
Draft
Add ctrl-c propagation e2e test with graceful SIGINT handling#299branchseer wants to merge 7 commits intomainfrom
branchseer wants to merge 7 commits intomainfrom
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Add a `vtt exit-on-ctrlc` subcommand and e2e fixture that verifies SIGINT propagates to concurrent tasks when the user presses Ctrl+C. The test runs two packages with `vt run -r dev`, synchronizes them via a filesystem barrier, then sends ctrl-c and verifies both tasks receive and handle it. Also adds `ctrl-c` as a new `write-key` interaction type for e2e snapshot tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
711e66e to
b49617e
Compare
0aba31b to
ad17ff1
Compare
Add a `vtt exit-on-ctrlc` subcommand and e2e fixture that verifies SIGINT propagates to concurrent tasks when the user presses Ctrl+C. The test runs two packages with `vt run -r dev`, synchronizes via milestone protocol, then sends ctrl-c and verifies both tasks receive and handle it. Also adds `ctrl-c` as a new `write-key` interaction type, fixes `expect_milestone` to preserve unmatched milestones, strips `^C` terminal echo in redaction, and normalizes Windows CTRL_C exit code (512) to match Unix (1) for cross-platform snapshot consistency. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ad17ff1 to
7646aec
Compare
Use parent().parent() instead of canonicalize() to find the repo root from CARGO_MANIFEST_DIR. Avoids resolving symlinks which can cause issues on some CI environments. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use staged temp dir for insta snapshot paths so writes work on read-only filesystems (e.g. WebDAV mount during cargo-xtest). Skip INSTA_REQUIRE_FULL_MATCH when INSTA_UPDATE is set, allowing cross-platform runs to accept metadata-only snapshot changes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Register a ctrlc no-op handler in vt before the tokio runtime starts,
so Ctrl+C doesn't kill the runner via the default signal handler. Child
tasks receive SIGINT directly from the terminal driver.
- Use std::process::exit to avoid non-zero exit codes from Rust runtime
cleanup on Windows when background ctrlc threads are active.
- Add argv spawn mode for e2e steps: `{ argv = ["vt", "run", ...] }`
spawns directly without a shell wrapper, avoiding bash's CTRL_C exit
code interference on Windows.
- Clear Windows CTRL_C ignore flag in vtt exit-on-ctrlc before registering
the handler (Rust runtime sets this flag and it persists to children).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Investigation confirmed Rust std, ctrlc crate, and portable-pty have zero calls to SetConsoleCtrlHandler(NULL, TRUE). The inheritable ignore flag is set by the Windows ConPTY subsystem for spawned processes. The SetConsoleCtrlHandler(None, 0) workaround is still needed — without it the test times out on Windows — but the comment was wrong about the source. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
bb29dc4 to
3a54fbc
Compare
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.
Summary
vtt exit-on-ctrlcsubcommand that sets up a ctrl-c handler, emits a "ready" milestone, then prints "ctrl-c received" and exits when interruptedctrl-cas a newwrite-keyinteraction type in the e2e test runnerargvstep spawn mode ({ argv = ["vt", "run", ...] }) that spawns directly without a shell wrapper, avoiding bash's CTRL_C exit code interference on Windowsvt run -r devto verify SIGINT propagates to concurrent tasks and they exit cleanlyvt: register a no-op ctrlc handler before the tokio runtime so the runner survives Ctrl+C and reports actual task exit statusvtt exit-on-ctrlc(Rust runtime sets this and it persists to children)expect_milestoneto preserve unmatched milestones instead of silently dropping them viatake_unhandled_osc_sequences^Cterminal echo and normalizectrl-c receivedcount in e2e redaction for cross-platform snapshot consistency🤖 Generated with Claude Code