[rqd] Fix sigkill masking bug

[DiegoTavares]

Fixes #2311

Summary by CodeRabbit

• Bug Fixes

  • Corrected exit signal computation for Docker containers when exit codes exceed standard thresholds.
  • Fixed exit-status interpretation on Unix systems for proper signal detection.

• Tests

  • Added validation tests for exit-status handling across container and Unix environments.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f0348fa1-c631-4a91-9fbc-49598ae8ab5b

📥 Commits

Reviewing files that changed from the base of the PR and between 0ca6a10 and cd94142.

📒 Files selected for processing (2)

• rust/crates/rqd/src/frame/docker_running_frame.rs
• rust/crates/rqd/src/frame/running_frame.rs

---

📝 Walkthrough

Walkthrough

The PR fixes a statement-ordering bug in exit-signal interpretation where exit_code was overwritten before deriving exit_signal, always producing -127 for signal-killed frames. Both Unix and Docker paths are corrected, and Unix path validation tests are added.

Changes

Exit Signal Interpretation Fix

Layer / File(s)	Summary
Exit-signal interpretation fix rust/crates/rqd/src/frame/running_frame.rs, rust/crates/rqd/src/frame/docker_running_frame.rs	interprete_output (Unix) and interprete_output_docker now compute exit_signal from the original wrapped exit_code before setting exit_code to 1, correctly recovering the signal number from shell-wrapped codes (128 + signal_num).
Unit tests for exit-status decoding rust/crates/rqd/src/frame/running_frame.rs	Unix-only tests validate interprete_output for signal-wrapped exits (137/143/139 left-shifted), normal/non-signal exits (0, 1, 42), and direct-signal cases, asserting correct (exit_code, exit_signal) tuples.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

A signal once lost in the wrapping code,
Now shines through, bright and unbowed!
Statement reordering, a rabbit's delight,
Restored exit numbers, now perfectly right—
From chaos of -127, to truth in the light. 🐰✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically identifies the bug being fixed—a signal masking issue in rqd—which directly corresponds to the PR's main objective.
Linked Issues check	✅ Passed	All code changes in both files align with issue #2311 requirements: signal recovery logic is reordered before exit_code reassignment, variables are made mutable in docker path, and unit tests are added for exit-status decoding.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the signal-masking bug in running_frame.rs and docker_running_frame.rs, with no unrelated or out-of-scope modifications.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}