Skip to content

fix(message): don't mislabel signal 23 as SIGURG on macOS/BSD daemon hosts - #3110

Open
phil-opp wants to merge 1 commit into
mainfrom
claude/dreamy-bardeen-xphg86-signal-names-portable
Open

fix(message): don't mislabel signal 23 as SIGURG on macOS/BSD daemon hosts#3110
phil-opp wants to merge 1 commit into
mainfrom
claude/dreamy-bardeen-xphg86-signal-names-portable

Conversation

@phil-opp

Copy link
Copy Markdown
Collaborator

Issue

NodeError's Display impl (libraries/message/src/common.rs) maps NodeExitStatus::Signal(n) to a signal name from a hard-coded table:

22 => "SIGTTOU".into(),
23 => "SIGURG".into(),
other => other.to_string().into(),

Those are the Linux/glibc numbers. But NodeExitStatus::Signal is produced from ExitStatus::signal() under #[cfg(unix)] (same file), which also covers macOS and the BSDs — and signal numbers past the portable POSIX range diverge there:

number Linux macOS / BSD
23 SIGURG SIGIO

So a node killed by signal 23 on a macOS/BSD daemon host was reported as SIGURG when it was actually SIGIO. (Severity is low — it only affects the human-readable message, not control flow — but it is a genuine cross-platform mislabel, and the test was even named ..._uses_linux_signal_names, baking in the assumption silently.)

Fix

Signals 1..=22 in the table share the same number on every Unix dora targets, so they stay unconditional. Name signal 23 per target OS and fall back to the raw number on any other platform rather than printing a wrong name:

22 => "SIGTTOU".into(),
#[cfg(any(target_os = "linux", target_os = "android"))]
23 => "SIGURG".into(),
#[cfg(any(target_os = "macos", target_os = "ios", target_os = "freebsd",
          target_os = "openbsd", target_os = "netbsd", target_os = "dragonfly"))]
23 => "SIGIO".into(),
other => other.to_string().into(),

The existing test is renamed to node_error_signal_display_uses_platform_signal_names and made platform-aware, so the macOS nightly CI job actually exercises the SIGIO mapping.

Validation

cargo test -p dora-message      # 140 passed (incl. the updated signal test)
cargo clippy -p dora-message -- -D warnings
cargo fmt --all -- --check

Verified against 1.97.1 (the CI-pinned toolchain).


⚠️ This is a machine-generated PR authored by Claude (Claude Code) as part of an automated codebase review. The finding was verified by hand against origin/main before opening. Please review carefully before merging.

🤖 Generated with Claude Code


Generated by Claude Code

`NodeError`'s `Display` mapped `NodeExitStatus::Signal` numbers to names
from a hard-coded Linux/glibc table. `NodeExitStatus::Signal` is produced
from `ExitStatus::signal()` under `#[cfg(unix)]`, which includes macOS and
the BSDs, where signal numbers past the portable POSIX range diverge:
signal 23 is `SIGURG` on Linux but `SIGIO` on macOS/BSD. So a node killed
by signal 23 on a macOS/BSD daemon host was reported as `SIGURG` when it
was actually `SIGIO`.

Signals 1..=22 in the table share the same number on every Unix dora
targets, so they stay unconditional. Name signal 23 per target OS
(`SIGURG` on Linux/Android, `SIGIO` on macOS/BSD) and fall back to the raw
number on any other platform rather than printing a wrong name. The
existing test is renamed and made platform-aware so the macOS nightly CI
job validates the `SIGIO` mapping.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PYiYgEo2WKBXfGooqP2WG5
@trunk-io

trunk-io Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Merging to main in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

Copy link
Copy Markdown
Collaborator Author

🤖 Automated review by Claude — this is a fully automated review with no human in the loop. Treat it as advisory.

Reviewed the diff — no issues found. The unconditional signal-name table only contains numbers whose name is identical on Linux and macOS/BSD (the divergent low numbers 7/10/12/16–21 are correctly left to the numeric fallback rather than mislabeled), and signal 23 is the only added divergent case, cfg-gated correctly to SIGURG (Linux/Android) vs SIGIO (macOS/BSD) with a raw-number fallback elsewhere. The renamed test asserts the real per-OS Display output, and the signal 40 → "signal 40" fallback test is a genuine assertion.


Generated by Claude Code

@phil-opp
phil-opp marked this pull request as ready for review August 11, 2026 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants