Skip to content

F-025: fix(app): log Ctrl+C handler install failures#26

Open
Sephyi wants to merge 1 commit intodevelopmentfrom
audit/f-025-ctrl-c-error-log
Open

F-025: fix(app): log Ctrl+C handler install failures#26
Sephyi wants to merge 1 commit intodevelopmentfrom
audit/f-025-ctrl-c-error-log

Conversation

@Sephyi
Copy link
Copy Markdown
Owner

@Sephyi Sephyi commented Apr 22, 2026

Summary

fix(app): log Ctrl+C handler install failures.

Audit context

Closes audit entry F-025 from #3.

Verification

  • cargo fmt --check
  • cargo clippy --all-targets --all-features -- -D warnings
  • cargo test --all-targets

Note: one pre-existing test porcelain_exits_within_timeout_with_no_staged_changes is a known macOS cold-start flake that reproduces on unmodified development — unrelated to this change.

…carding

The Ctrl+C handler task used `signal::ctrl_c().await.ok();` which
silently swallowed platform-specific signal-registration errors. On
Unix, `signal::ctrl_c()` can fail when installing the SIGINT handler
(for example, if another library already captured SIGINT non-reentrantly,
or if the runtime lacks signal capabilities). Discarding that error
meant we would proceed to cancel the token on an unrelated future-ready
state, or worse, never react to Ctrl+C at all without any diagnostic.

Replace `.ok()` with an explicit match that logs the install failure at
`warn!` level (including the error via `%e`) and returns from the task
without firing `cancel.cancel()`. This makes the failure mode observable
in debug logs and avoids a spurious cancellation when signal registration
fails outright. Normal test runs never exercise this branch — signal
registration succeeds on all supported platforms during testing.

Closes audit entry F-025 from #3.
Copilot AI review requested due to automatic review settings April 22, 2026 19:51
@Sephyi Sephyi added the audit Codebase audit cleanup (issue #3) label Apr 22, 2026
@Sephyi Sephyi self-assigned this Apr 22, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the CLI app’s Ctrl+C handling to avoid silently discarding failures from tokio::signal::ctrl_c(), addressing audit finding F-025.

Changes:

  • Replace .ok() on signal::ctrl_c().await with explicit error handling and a warn! log on failure.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/app.rs
signal::ctrl_c().await.ok();
if let Err(e) = signal::ctrl_c().await {
warn!(error = %e, "failed to install Ctrl+C handler");
return;
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Ctrl+C handler setup failure, the spawned task returns without cancelling. This changes behavior vs the previous .ok() (which would still call cancel.cancel() after an immediate Err), and can leave the app running without any CancellationToken-based shutdown path (streaming tasks may keep going, and Ctrl+C may no longer trigger graceful cancellation). Consider cancelling after logging (to preserve the prior fail-fast behavior) or surfacing this as a fatal startup error instead of silently continuing.

Suggested change
return;

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

audit Codebase audit cleanup (issue #3)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants