fix(stream): report a signal-terminated child instead of exiting 128 + signal silently - #3709
Open
Niko-Wu wants to merge 1 commit into
Open
fix(stream): report a signal-terminated child instead of exiting 128 + signal silently#3709Niko-Wu wants to merge 1 commit into
Niko-Wu wants to merge 1 commit into
Conversation
…+ signal silently A child killed by a signal has no exit code of its own, so `128 + signal` is all that reaches the caller — a number indistinguishable from a tool that genuinely exited with it. The streaming path emitted nothing else, so a child killed by SIGKILL surfaced as exit 137 with empty stdout and empty stderr, while the proxy path already reported the same situation on stderr. status_to_exit_code now delegates to utils::exit_code_from_status, which already maps the code and reports the signal, so every path that spawns a child shares one implementation and one wording. run_streaming passes the program basename as the label, so the line reads `[rtk] git: process terminated by signal 9`. Exit codes are unchanged; the only new output is one stderr line on the signal path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Niko-Wu
marked this pull request as draft
August 25, 2026 08:06
Niko-Wu
marked this pull request as ready for review
August 25, 2026 08:08
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 #3708
Problem
A child killed by a signal has no exit code of its own, so
128 + signalis all that reaches the caller — and that number alone is indistinguishable from a tool which genuinely exited with it. On the streaming path nothing else is emitted, sortk git pushwith a child killed by SIGKILL exits 137 with empty stdout and empty stderr. The proxy path already reports the kill, so the two paths disagreed.Reproduction from the issue (fake
gitthat kills itself, no repo or network needed):Fix
stream::status_to_exit_codenow delegates to the existingutils::exit_code_from_status, which already both maps the code and reports the signal. That removes a near-duplicate of the mapping logic and guarantees one wording for every path that spawns a child, per the Extensibility rule in CONTRIBUTING ("use components already in place to avoid duplication").run_streamingcomputes the label once and passes it to both call sites, so the message names the tool:[rtk] git: process terminated by signal 9.Exit codes are unchanged — the only new behaviour is one stderr line on the signal path, which cannot happen on a normal exit.
Trade-offs
labelparameter.status_to_exit_codegainslabel: &str. The smaller alternative was printing without a label (no signature change), but every other rtk diagnostic names the command, and the name is what makes the line actionable when several tools are involved. The blast radius is small and crate-internal: 2 call sites plus 3 test assertions.PATHreports asgitrather than the absolute path it happened to be found at — shorter, stable across machines, and consistent with how the other diagnostics read.eprintln!. Copying the message intostream.rswould have been a one-line diff but would leave two copies of the same wording free to drift apart.Tests
tests/signal_termination_test.rs(new,#![cfg(unix)]): puts a fakegitthat SIGKILLs itself onPATH, runsrtk git push, and asserts both the137exit code and a stderr diagnostic that names the command. This covers the streaming path end to end through the real binary.test_command_label_is_program_basename(new unit test): the label is the basename for an absolute path and for a bare name.test_exit_code_signal_kill(existing, still asserting137) and the two neighbouring exit-code tests are kept, updated only for the new argument.Gates run locally:
cargo fmt --all -- --check,cargo clippy --all-targets,cargo test --all— all pass.