various: system notifications

[r0chd]

Implemented system notifications with dbus notification spec, notification is sent right before password prompt, also implemented --ask flag with notification actions

its currently implemented only for nixos, looking for some input before implementing more for interfaces

• [x] I have updated the changelog as per my changes
• [x] I have tested, and self-reviewed my code
• Style and consistency
  • [x] I ran nix fmt to format my Nix code
  • [x] I ran cargo fmt to format my Rust code
  • [x] I have added appropriate documentation to new code
  • [x] My changes are consistent with the rest of the codebase
• Correctness
  • [x] I ran cargo clippy and fixed any new linter warnings.
• If new changes are particularly complex:
  • [x] My code includes comments in particularly complex areas to explain the logic
  • [x] I have documented the motive for those changes in the PR body or commit description.
• Tested on platform(s)
  • [x] x86_64-linux
  • [ ] aarch64-linux
  • [ ] x86_64-darwin
  • [ ] aarch64-darwin

---

Add a :+1: reaction to pull requests you find important.

Summary by CodeRabbit

• New Features

  • --ask becomes tri-state (prompt, notify, both); flag without value defaults to prompt.
  • Commands can send desktop notifications to request or drive confirmations.
  • Informational and warning messages may also appear as desktop notifications alongside terminal output.

• Chores

  • Added a desktop notification backend and integrated it into logging and confirmation flows.

[NotAShelf]

I'd rather if this was done with the notify crate and not zbus. Zbus has a history of holding critical fixes for months, and I'd rather avoid Linux-only features since it turns out as future maintenance work one way or another.

Also on a more personal note, this seems to be a bit too much. NixOS icons dependency? zbus? Having to switch between notifications or terminal prompt? Rather drastic imo. We should be able to send a notification using the aforemetioned crate with much less code, including with actions using libnotify.

[r0chd]

notify crate makes sense to me, but it'd still depend on zbus, unless you prefer dbus-rs backend (but that would require dependency on libdbus and pkg-config).

The terminal prompt was just a fallback in case daemon wasnt available or notification was dismissed/expired, I'm not sure how to handle it in a better way.

Also do you think the notification should still use nixos-snowflake icon but just get rid of the dependency or ditch icons all together?

[NotAShelf]

We could inline a SVG for the icon, probably preferable.

[coderabbitai[bot]]

[!NOTE]

Reviews paused

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Walkthrough

Adds desktop notification support (notify-rust), a tri-state confirmation flag (NotifyAskMode), replaces several boolean --ask fields with Option<NotifyAskMode>, introduces NotificationSender and nh_info!/nh_warn! macros, and wires notification-based prompts and logging through multiple command flows.

Changes

Cohort / File(s)	Summary
New Dependency \Cargo.toml``	Adds notify-rust = "4.11.7".
Notification Module \src/notify.rs``	New NotificationSender wrapper around notify_rust::Notification with new, urgency, send, and ask; respects NH_NOTIFY; Unix (non-macos) implements interactive actions, macOS largely no-op for ask.
Logging Macros \src/logging.rs``	Adds exported macros nh_info! and nh_warn! that log via tracing and send desktop notifications (uses NotificationSender + Urgency); minor formatting error-handling tweak.
Module Declarations \src/lib.rs``, \src/main.rs``	Declares new notify module (pub mod notify; in lib, mod notify; in main).
CLI Tri-State Ask \src/interface.rs``	Adds public NotifyAskMode enum (variants: Prompt, and Unix-only Notify); changes multiple ask fields from bool to Option<NotifyAskMode> with clap attributes (default_missing_value = "prompt", num_args = 0..=1).
Command Confirmation Flows \src/clean.rs``, \src/home.rs``, \src/darwin.rs``, \src/nixos.rs``	Replace boolean ask logic with Option<NotifyAskMode> matching: Prompt = terminal prompt, Notify = NotificationSender::ask() (Unix), Both scaffolding; various info! → nh_info! swaps.
Commands & Utilities Logging \src/commands.rs``, \src/util.rs``, \src/util/platform.rs``	Replace several user-facing info! logs with nh_info!; adjust imports to bring nh_info into scope.
Public Surface Changes \src/lib.rs``, \src/interface.rs``, \src/nixos.rs``	Public API expanded: new notify module and NotificationSender; public NotifyAskMode added; several public ask fields change type to Option<NotifyAskMode>.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant CLI
    participant Command
    participant AskMode as NotifyAskMode
    participant Notifier as NotificationSender
    participant Tracing

    User->>CLI: run command (optional --ask)
    CLI->>Command: parse args -> Option<NotifyAskMode>

    alt ask is None
        Command->>Command: proceed without confirmation
    else ask is Some
        Command->>AskMode: match variant
        alt Prompt
            Command->>User: terminal prompt (confirm)
            User-->>Command: accept/reject
        else Notify
            Command->>Notifier: new(summary, body).urgency(Critical)
            Notifier->>Notifier: show notification (actions on unix)
            alt unix && not macos
                User-->>Notifier: accept/reject action
                Notifier-->>Command: returns bool
            else macos or unsupported
                Notifier-->>Command: returns false (no interactive ask)
            end
        else Both
            Note right of Command: both path scaffolded / partial
        end
    end

    Command->>Tracing: nh_info!/nh_warn! (log + optional notification)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

• Pay special attention to:
  • src/notify.rs ask/send platform-specific behavior and NH_NOTIFY gating.
  • src/logging.rs macros for correct import scopes and error handling.
  • Clap attribute changes in src/interface.rs to ensure CLI parsing behaves as intended.
  • All places where info! → nh_info! replaced to confirm logging/notification side effects.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'various: system notifications' accurately describes the core change—adding system notification support across multiple command paths in the nh project.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• [ ] 📝 Generate docstrings

🧪 Generate unit tests (beta)

• [ ] Create PR with unit tests
• [ ] Post copyable unit tests in a comment

---

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.}

[NotAShelf]

Actual yapmachine holy hell

[r0chd]

https://github.com/nix-community/nh/pull/387/files#diff-fb0614459ea1fee2430f5df8e36530289fa54b04e088be756d2db2054c8d7081R91-R136

@NotAShelf Is this too much voodoo for our purposes, for our mission statement?

[leiserfg]

Couldn't this be done with a simple OSC-99 so nh does not have to contain a dbus client just for a notification?

[NotAShelf]

Thanks for taking the time to work on this, @r0chd. I appreciate the level of effort that clearly went into the PR. The scope has grown quite a bit compared to the original intention, and that makes it harder to evaluate confidently. We've had situations in the past where increased complexity didn't pay off in terms of maintainability, and I want to avoid repeating that pattern.

In addition, several other PRs have recently been merged that touch large parts of the codebase, so this branch will need a rebase before it can move forward.

Before reviewing in depth, I'd like to ask for another pass over the implementation with an eye toward simplification. As @leiserfg pointed out, an OSC99-based approach might give us the same functionality while keeping both the crate size and the conceptual load under control. If there are parts of the current design that can be streamlined or reduced, that would make the review process significantly easier and improve the long-term maintainability of the feature.

I am, of course, open to discussing alternatives if needed, but a simplified iteration would be a solid next step.

[JonnieCache]

FWIW claude code uses in-terminal control code notifications (presumably OSC99,) that being a popular product it's likely to be well-supported by terminal clients. It uses them for precisely this purpose, when it's waiting for input after a long task, and clicking on them switches my desktop to reveal the window as well.

I believe there are even control codes to detect if the terminal window has focus/is visible?

[r0chd]

I wasn't aware of OSC-99 before, at first glance it does look like a way better way, but it has a major problem, I wasn't able to find a terminal besides kitty that supports the escape sequence (ghostty seems to plan on implementing it tho)

generally I agree that the escape sequence should be used instead of depending on dbus, just let me know if you'd prefer to stay with the current impl, use escape sequence or just ditch the feature all together

P.S. if you have something to add, please tag me, I assume that all notifications from this pr are spam from the bot and just ignore them

[NotAShelf]

@coderabbitai shut up

[coderabbitai[bot]]

@NotAShelf, understood. I'll pause automated reviews on this PR.

✅ Actions performed

Reviews paused.

[NotAShelf]

That suppresses the roomba.

I wasn't aware of OSC-99 before, at first glance it does look like a way better way, but it has a major problem, I wasn't able to find a terminal besides kitty that supports the escape sequence (ghostty seems to plan on implementing it tho)

From a quick glance, Kitty and Foot seem to be the ones that support it. I don't know if Alacritty does, but I generally dislike Alacritty and how it is maintained so I'm happy to tell people to use a better terminal instead of supporting it. I would also expect most DE-specific terminals such as gnome-terminal and konsole to support them but you're right, it is not too wide-spread.

generally I agree that the escape sequence should be used instead of depending on dbus, just let me know if you'd prefer to stay with the current impl, use escape sequence or just ditch the feature all together

I'm guessing the current impl will be fine, but I'd like for it to be simplified. While I haven't looked at a better solution myself, I would hope that adding notifications (dbus or not) is a trivial task.

RE: @r0chd

[leiserfg]

There is also OSC 9, which has wider adoption but is less feature-full (one can only set the message of the notification). Alacrity does not support it either, but if some one uses that term, they does not care about features.

[r0chd]

From a quick glance, Kitty and Foot seem to be the ones that support it. I don't know if Alacritty does, but I generally dislike Alacritty and how it is maintained so I'm happy to tell people to use a better terminal instead of supporting it. I would also expect most DE-specific terminals such as gnome-terminal and konsole to support them but you're right, it is not too wide-spread.

tried running it in foot, kitty, ghostty, gnome-terminal, konsole and alacritty and only kitty worked for me, ghostty planning to implement it imo is good enough for it not to feel like a kitty specific feature. Could someone else test OSC-99 support in those terminals? maybe I'm just stupid