Skip to content

feat: add configurable close behavior (hide to tray / exit) with settings UI - #421

Open
LeonardW-sl wants to merge 6 commits into
xintaofei:mainfrom
LeonardW-sl:fix/linux-tray-hide-on-close
Open

feat: add configurable close behavior (hide to tray / exit) with settings UI#421
LeonardW-sl wants to merge 6 commits into
xintaofei:mainfrom
LeonardW-sl:fix/linux-tray-hide-on-close

Conversation

@LeonardW-sl

@LeonardW-sl LeonardW-sl commented Aug 7, 2026

Copy link
Copy Markdown

Problem

On Linux (and other platforms), clicking the main window's close button always exits the entire application. There is no way to minimize to the system tray and keep the app running in the background.

The existing can_hide_to_tray() check was already able to detect tray availability, but the close button simply checked can_hide_to_tray() without consulting any user preference — if the tray was available, it always hid; if not, it always exited. There was no UI for the user to choose their preferred behavior.

Solution

Add a configurable close-behavior setting with two options:

  1. Hide to tray (background) — default. When the close button is clicked and tray is available, the window hides to the system tray. The app keeps running, and the tray icon restores the window. When tray is not available (e.g., GNOME 45+ without AppIndicator), this falls back to exiting.
  2. Exit application — always exits the app on close button click, regardless of tray availability.

Changes

Backend (Rust):

  • models/system.rs: New CloseAction enum (HideToTray / Exit) and SystemCloseSettings struct, persisted via app_metadata_service.
  • commands/system_settings.rs: load_system_close_settings, get_system_close_settings, update_system_close_settings — all gated behind tauri-runtime to avoid dead_code warnings in sidecar builds.
  • lib.rs: Close button handler reads the stored setting and uses CloseAction::HideToTray && can_hide_to_tray() instead of can_hide_to_tray() alone.

Frontend (TypeScript/React):

  • lib/types.ts: CloseAction type and SystemCloseSettings interface.
  • lib/api.ts: getSystemCloseSettings() / updateSystemCloseSettings() transport wrappers.
  • components/settings/close-behavior-settings.tsx: Radio-button UI with loading/saving states and error toast.
  • components/settings/general-settings.tsx: Integrates the new section.
  • i18n/messages/*.json: All 10 locales updated with the 4 new strings.

Testing

  • 3,479 existing frontend tests pass (no regression).
  • Sidecar compiles cleanly (cargo build --no-default-features --bin codeg-mcp).
  • Main binary compiles cleanly (cargo build --release --bin codeg).
  • Setting persists across app restarts.
  • "Hide to tray" → close button hides window; tray icon restores it.
  • "Exit" → close button exits the app.
  • Default is "Hide to tray" (backward-compatible with existing behavior on tray-capable platforms).
  • Linux without tray: can_hide_to_tray() returns false, so both settings exit the app (no stranded process).

Copilot AI lite review requested due to automatic review settings August 7, 2026 00:52

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Tauri (desktop) tray-availability logic so Linux sessions with a real, usable system tray can safely “hide to tray” on window close, while preserving the existing fail-safe behavior on desktops where the tray icon would be invisible.

Changes:

  • Add a Linux-only D-Bus check (via gdbus call org.freedesktop.DBus.NameHasOwner) to detect whether org.kde.StatusNotifierWatcher is present.
  • Stop unconditionally disabling hide-to-tray on Linux; instead, set TRAY_AVAILABLE only when the watcher is detected.

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

@LeonardW-sl LeonardW-sl changed the title fix(linux): allow hide-to-tray when StatusNotifierWatcher is available feat: add configurable close behavior (hide to tray / exit) with settings UI Aug 7, 2026
@xintaofei

Copy link
Copy Markdown
Owner

Thanks for tackling this — hide-to-tray on Linux has been a real gap, and splitting it into "detect the tray properly" + "let the user choose" is the right shape. The code reads well, the comments explain the why, and CI is green on all seven cells. I did a fairly deep pass and re-ran the checks locally; a few things I'd like to see addressed before this lands.

Blockers

1. The settings section renders in web / server / remote mode, where the command doesn't exist

get_system_close_settings / update_system_close_settings are #[cfg(feature = "tauri-runtime")] Tauri commands, and no Axum route was added in web/router.rs. But <CloseBehaviorSettings /> is mounted unconditionally at general-settings.tsx:392, so both non-local transports break:

  • browser / Docker → WebTransport POSTs /api/get_system_close_settingsapi_not_found returns 501!res.ok → throws.
  • remote-workspace window on desktop → RemoteDesktopTransportinvoke("remote_http_call") → same 501 back through the Rust proxy.

The component catches, only console.errors, then finally { setLoading(false) } renders anyway — so the user sees a hardcoded "Hide to tray" presented as if it were the stored value. Switching to "Exit" then 501s into an error toast. It's also meaningless in a browser: no OS close button, no tray.

The precedent is 300 lines above in the same file — general-settings.tsx:69-75 documents this exact rule for the rendering section:

const closeSettingsLoadable = isDesktop() && getActiveRemoteConnectionId() === null

2. NameHasOwner("org.kde.StatusNotifierWatcher") isn't a reliable signal for "the tray is usable"

Three separate failure modes:

False positive → strands the user, which is the exact bug this PR exists to prevent. A watcher can be registered with no host displaying items. The spec puts that on a separate read-only boolean, IsStatusNotifierHostRegistered (with a matching StatusNotifierHostRegistered signal) — NameHasOwner can't see it. Probe says "available", the icon is invisible, window.hide() loses the workspace.

False negative → hide-to-tray stays off where it actually works. The chain here is Tauri 2.10 → tray-iconlibappindicator-rs, which loads libayatana-appindicator3.so.1 (what CI installs). That library watches the bus name and falls back to a legacy GtkStatusIcon when it vanishes — name_vanished_handlerstart_fallback_timerfallback_timer_expiregtk_status_icon_new(), reversed by unfallback in register_service_cb. On an XFCE/MATE/legacy-X11 tray with no watcher, the icon is visibly working and the probe still says no.

Stale snapshot. TRAY_AVAILABLE is written once at startup. The library tracks the watcher appearing and vanishing dynamically; this flag never does. Enable the AppIndicator extension, or restart the panel, and the app disagrees with reality until relaunch.

Credit where due — the motivating case is handled correctly. Plain GNOME 45 ships no StatusNotifierWatcher and doesn't instantiate its dormant legacy tray manager either, so neither path would render the icon and the probe's false is right.

On fixes: switching to IsStatusNotifierHostRegistered is a partial fix — it closes the false positive but not the false negative, since with no watcher at all you'd still report unavailable while Ayatana's GtkStatusIcon is happily visible. That case needs its own decision (detect the legacy tray too, or accept and document it). Likewise NameOwnerChanged won't catch host-registration changes while the watcher owner stays the same — probing lazily at close time is probably the simpler robust answer than any startup snapshot.

Two smaller things in the same function:

  • It shells out to gdbus, which isn't guaranteed installed — on Debian/Ubuntu it lives in libglib2.0-bin, a separate package from the glib runtime GTK pulls in. Missing binary → .unwrap_or(false) → hide-to-tray permanently off on a machine where the tray works fine.
  • .output() has no timeout. gdbus call carries GLib's ~25s default D-Bus timeout with no process-level bound on top, and install_tray_icon is on the startup path (lib.rs:349), so an unresponsive session bus delays launch.

zbus is already in the lockfile at 5.13.2 (transitively, via the Tauri plugins), so a native call would reuse a version already vetted here — it'd still need an explicit Linux-only Cargo.toml entry.

3. Choosing "Hide to tray" silently falls back to Exit when the tray isn't usable

should_hide = action == HideToTray && can_hide_to_tray(). When the probe says no, the radio still shows "Hide to tray" selected and the value still persists — but closing exits the app, and the only signal is a tracing::warn! in the log. For the feature's one control, that's precisely the confusion it was added to remove. Worth exposing can_hide_to_tray() to the frontend and either disabling the option or showing an inline hint ("System tray unavailable in this session — the app will exit on close").

Should fix

Blocking DB read on the main event-loop thread, on every close. tauri::async_runtime::block_on(load_system_close_settings(&db.conn)) inside CloseRequested. No nested-runtime panic risk — that callback runs on the GUI event-loop thread, not a tokio worker, and block_on is already used elsewhere in this file. And with WAL + max_connections(5), readers don't block on writers, so this is not a common stall. But a persistent SQLite busy condition can hold the main event loop for roughly five seconds (busy_timeout=5000 is a retry budget, not a fixed delay, and can overshoot slightly), and pool acquisition adds its own connect_timeout(10s) on top — for a value that never changes between reads. Two cheaper options already exist in-repo: cache it in an atomic like TRAY_AVAILABLE, loaded at setup right next to the existing block_on(load_system_language_settings(...)) at lib.rs:344 and refreshed by the update command; or store it in preferences.json the way SystemRenderingSettings does.

The default flips Linux behavior without opt-in. Before this PR, close on Linux always exited. Now, for Linux users whose tray builds and whose probe succeeds, the default becomes hide-to-tray. That's the PR's intent, but it's a silent change and "the app won't quit" is a classic bug report. Worth considering Exit as the Linux default — the setting makes it a one-click change now — or at least a release note.

No Rust tests. load_system_close_settings (missing key → default, malformed JSON → error) and the update round-trip aren't covered; only the React component is.

Please also drop PR_BODY.md — the PR description got committed as a tracked file at the repo root. Pure hygiene, not a correctness issue, but it shouldn't land.

Nits

  • All 10 src/i18n/messages/*.json lost their trailing newline (they have one on main) — looks like a json.load / json.dump round-trip.
  • The new closeActionSaveFailed drops the space / full-width colon before {message} in all 9 non-English locales, unlike every existing *SaveFailed sibling: zh-CN 保存关闭行为设置失败:{message} vs 保存终端设置失败:{message}; ja …に失敗:{message} vs …に失敗しました: {message}; fr also uses a straight ' where the file uses .
  • The tDynamic cast in close-behavior-settings.tsx isn't needed and defeats i18n type-checking — global.d.ts declares Messages: typeof enMessages, so t("closeActionSaveFailed", { message }) compiles directly (I checked: tsc --noEmit is clean without the cast). The cast in general-settings.tsx exists only because the backend-driven shell label keys are genuinely dynamic.
  • The close handler drops the load error entirely (.transpose().ok().flatten()…unwrap_or_default()), so someone who chose "Exit" would silently get hide-on-close for that attempt. The persisted value is untouched and the setup-time locale load does the same thing, so it's minor — but a tracing::warn! costs nothing.
  • models/mod.rs:54 is 105 chars, over rustfmt's 100 default. Cosmetic; CI doesn't run cargo fmt --check and the file already has unrelated drift.
  • Native <input type="radio"> rather than the repo's components/ui/radio-group.tsx — native radios ignore the theme tokens. Mitigated by the same file already using a raw checkbox for the rendering toggle.

What I verified

CI is green on all 7 cells for 867365a9. I also re-ran locally at the PR head: eslint, tsc --noEmit (worth noting pnpm build alone doesn't typecheck tests), the new close-behavior-settings tests 3/3, next build static export, cargo clippy --all-targets --features test-utils -- -D warnings (macOS desktop), and cargo clippy --no-default-features --bin codeg-server --lib -- -D warnings — all pass.

Checked and clean: the #[cfg]-on-if/else form is valid and the ubuntu desktop cell compiles it; #[serde(rename_all = "snake_case")] matches the TS "hide_to_tray" | "exit" mirror exactly; the ungated CloseAction / SystemCloseSettings re-exports from models/mod.rs are harmless (pub in a lib target, and server clippy passes); macOS and Windows keep their previous TRAY_AVAILABLE.store(true) behavior.


Overall: good direction and a genuinely useful feature — the main things standing between this and merge are (1) gating the section off the local-desktop transport and (2) making the tray-availability signal trustworthy, since everything else in the feature hangs off it. Happy to look again once those are in.

@LeonardW-sl
LeonardW-sl force-pushed the fix/linux-tray-hide-on-close branch from 867365a to 87aca41 Compare August 14, 2026 09:45
@LeonardW-sl

Copy link
Copy Markdown
Author

Update: All review fixes have been pushed to this PR (commit 87aca41). The branch now contains all 3 commits with the complete implementation. Ready for review. 🚀

Md-Nijam-62 and others added 3 commits August 14, 2026 17:50
On Linux, Tauri's tray build() succeeds even when the desktop session
does not provide a StatusNotifierWatcher (notably GNOME 45+ without an
AppIndicator extension). In that case the tray icon is silently invisible
and hiding the main window would leave the user with no way to recover it.

Previously codeg avoided this by unconditionally returning false from
can_hide_to_tray() on Linux, which prevented hide-to-tray even on KDE,
XFCE, Cinnamon, Budgie, and GNOME-with-AppIndicator — all of which
have a working tray.

Fix: detect the actual tray availability at install_tray_icon() time
by querying D-Bus for org.kde.StatusNotifierWatcher. Only set
TRAY_AVAILABLE when the service is present, so the close handler hides
the window on fully capable desktops and exits otherwise.
…ings UI

Add a new setting to let users choose what happens when the main window
close button is clicked:

- Hide to tray (background) — default. Window hides to system tray if
  available; falls back to exit if no tray is present.
- Exit application — always exits on close, regardless of tray.

Backend changes:
- New CloseAction enum + SystemCloseSettings model
- load_system_close_settings/get_system_close_settings/update_system_close_settings
- Close button handler reads stored setting instead of checking
  can_hide_to_tray() alone

Frontend changes:
- CloseBehaviorSettings component with radio-button UI
- Integrated into GeneralSettings page
- All 10 locales updated with 4 new strings
- 3 new unit tests covering load, save, and save-failure revert

All new items gated behind tauri-runtime feature to keep sidecar builds
clean. 3482 existing tests pass, no lint warnings.
… cache

Address three blocking issues from code review:

1. **Tray probe is advisory-only**: Split tray detection into hard fact
   (TRAY_AVAILABLE, set after install_tray_icon succeeds) vs best-effort
   guess (tray_probably_visible, used only for defaults and UI hints).
   An explicit HideToTray choice is always honored regardless of probe
   result. The probe prevents a misleading default but never overrides
   user intent.

2. **Linux defaults to Exit**: Platform-dependent CloseAction::default()
   returns Exit on Linux, HideToTray on Windows/macOS. Linux tray support
   is the least predictable of the three — defaulting to hide-to-tray
   would silently turn "I closed the app" into "the app won't quit" on
   upgrade, so Linux users opt in instead.

3. **Atomic cache instead of preferences.json**: Added CACHED_CLOSE_ACTION
   static and prime_close_settings_cache() to avoid block_on DB read on
   GUI event-loop thread. Rejected preferences.json approach (only exists
   for pre-runtime WebView2 flags; this setting has no such timing
   constraint).

Additional changes:
- Made zbus optional and feature-gated (Linux + tauri-runtime only)
- Linux tray probe uses zbus blocking API with 2s timeout on detached thread
- Rewrote close-behavior-settings.tsx on shared settings UI grammar
- Restored trailing newlines in all i18n message files
- Fixed punctuation and terminology consistency across 10 locales
@LeonardW-sl
LeonardW-sl force-pushed the fix/linux-tray-hide-on-close branch from 87aca41 to 0e067a3 Compare August 14, 2026 09:51
@LeonardW-sl

Copy link
Copy Markdown
Author

@xintaofei The refreshed CI is 6/7 green, but Rust desktop on ubuntu-22.04 has been stuck in Install Linux desktop dependencies (apt-get update/install) since 2026-08-19 07:55 UTC, before reaching any Rust build or test step. Recent runs complete the entire Ubuntu desktop job in roughly 8–20 minutes, so this appears to be a hung GitHub-hosted runner rather than a code failure.

I attempted to cancel the workflow, but fork contributors do not have permission to manage Actions runs in the upstream repository (403 Must have admin rights to Repository). Could you please cancel run 32228873876 and rerun the workflow/job?

@xintaofei

Copy link
Copy Markdown
Owner

Re-reviewed at 78e503a1. This is a really solid revision — every item from my last pass is addressed, and in one case with a better answer than the one I proposed.

What's fixed

Previous item Fix
Section renders in web/server/remote Gated on isLocalDesktop() — the canonical helper already on main (lib/platform.ts:22), with a comment on why
NameHasOwner is the wrong signal Now reads IsStatusNotifierHostRegistered (closes the false positive), and the probe is advisory-only — can_hide_to_tray() gates on tray installation alone, so the Ayatana GtkStatusIcon false negative can't block hiding any more. That's a better fix than what I suggested; I'd only closed one of the two directions.
Silent no-op SystemCloseSettingsInfo { action, tray_available } + closeActionTrayUnavailableHint, shown only when the probe says no and hide-to-tray is selected. The hint text even admits the false-negative case, which is the honest thing to tell a user.
gdbus may be missing / no timeout zbus with method_timeout(2s) and an outer wait bound. Declared optional under [target.'cfg(target_os = "linux")'.dependencies] and pulled in by tauri-runtime; the Cargo.lock diff is one line, so no new crates enter the graph. Nice.
Blocking DB read on the GUI thread CACHED_CLOSE_ACTION, primed in setup next to the other loads and refreshed after a successful write; CloseRequested reads it synchronously.
Linux default flip CloseAction::default() is now Exit on Linux, HideToTray elsewhere. I checked it against the AtomicBool pre-prime default (!cfg!(target_os = "linux")) — they agree.
No Rust tests 5 tests: missing key → platform default, malformed JSON → error, both actions round-trip, snake_case wire form, cache tracks writes.
PR_BODY.md, i18n newlines, i18n punctuation, tDynamic cast, raw radio, error swallowing All fixed. The punctuation now matches the sibling keys in all 10 locales, including full-width for zh and : with the typographic for fr.

Also unprompted and appreciated: the models are tauri-runtime-gated now, save() takes an explicit previous, and CLOSE_SETTINGS_WRITE_LOCK serializes concurrent writes so the cache reflects the last committed value.

Remaining — all in linux_status_notifier_host_registered()

1. The probe result is cached for the process lifetime, and four comments say otherwise

ProbeState::Complete(result) => return result is terminal. ProbeState::Idle appears exactly twice — the OnceLock initializer (windows.rs:1992) and the Idle => { *state = Running } arm (:2017) — so nothing ever resets it. The probe runs once per process and every later call replays the first answer.

Four comments describe something else:

  • windows.rs:1938 — "Probed lazily rather than cached at startup: a user can install the GNOME AppIndicator extension or restart their panel while the app is running."
  • windows.rs:1926 — "Only ever used to pick a default and to warn in the settings UI". It isn't used to pick a default: CloseAction::default() decides on cfg!(target_os = "linux"), and the probe's only two call sites populate the advisory response.
  • system_settings.rs:388 — "Probed per read rather than cached: the user may have installed a tray extension or restarted their panel since launch, and opening this page is exactly when a stale answer would mislead them."
  • close-behavior-settings.tsx, in save — "the probe runs per call, so the session may have gained a tray since the load."

Nothing breaks — the hint is advisory and a first-probe-on-settings-open is still fresher than a startup snapshot. But the journey those comments promise (install the AppIndicator extension → reopen settings → warning clears) doesn't happen, and a comment that contradicts its code is a trap for whoever touches this next. Either give the cache a TTL / reset on read, or correct the comments.

Trivial, same block: "an outer recv_timeout covers connection establishment" — there's no recv_timeout; the outer bound is Condvar::wait_timeout.

2. A failed thread spawn wedges the probe at Running permanently

std::thread::Builder::new().name("tray-probe".into()).spawn(move || {}).ok();

.ok() discards the io::Result. If the spawn fails, the state stays Running and nothing ever notifies. The current caller waits the full PROBE_TIMEOUT + 100ms, warns and returns false — fine. But every subsequent call then takes the ProbeState::Running branch and blocks another 2.1 s before returning false, for the rest of the process's life. Both get_system_close_settings and update_system_close_settings call this, so opening the settings page or changing the setting would each hang ~2.1 s from then on.

Likewise, if the probe worker panics before publishing Complete, the state stays Running and later calls keep timing out the same way. (Only a panic while holding the state mutex would poison it and make the later unwrap()s panic — the D-Bus work happens before the lock is taken, so that specific path is safe.)

Fix: on the spawn-error path, set Idle (or Complete(false)) and notify_all().

3. Both waits use wait_timeout with no predicate loop

Two things at once:

  • Spurious wakeups are permitted by Condvar. The code ignores the returned WaitTimeoutResult and falls into _ => false, which also logs the misleading "Probe thread did not complete".
  • Lost notification: the runner does drop(state) → spawn → state_lock.lock()wait_timeout(...) without re-checking the state first. If the worker finished in that window, notify_all() has already fired and isn't buffered, so the runner blocks the full 2.1 s before reading the (correct) result. Unlikely — the worker has to complete a D-Bus connect and property read in the time it takes to spawn a thread — but it's the same latent bug.

wait_timeout_while(guard, |s| !matches!(s, ProbeState::Complete(_))) handles both, and folds naturally into the #2 repair.

4. Blocking inside async Tauri commands

get_system_close_settings and update_system_close_settings are async fn commands; Tauri's generated wrapper submits them through tauri::async_runtime::spawn, i.e. Tokio. Both call tray_probably_visible() (system_settings.rs:391, :430), so the synchronous condvar wait parks a Tokio worker for up to ~2.1 s while a probe is initial or in flight (cached calls return immediately, and macOS/Windows never probe at all). You clearly weighed this — the write mutex is dropped before probing, with a comment about the 2 s — so it's a deliberate trade-off; tokio::task::spawn_blocking would just keep it off the worker pool.

Verification

CI on 78e503a1 is 6/7. Rust desktop on ubuntu-22.04 is cancelled — and that's the only cell that compiles the new #[cfg(target_os = "linux")] zbus code, on exactly the platform this PR targets. Worth getting a green run there before merge.

To narrow that gap I lifted linux_status_notifier_host_registered verbatim into a scratch crate and cargo checked it against the exact locked versions (zbus =5.13.2, zbus_macros =5.13.2, zvariant =5.9.2, zvariant_derive =5.9.2, zbus_names =4.3.1) — it compiles clean, so blocking::connection::Builder::session(), .method_timeout(), .build(), blocking::Proxy::new() and .get_property::<bool>() are all correct for the pinned version. zbus is cross-platform so this type-checks off-Linux, but it only validates the API surface — it doesn't replace compiling and linking the full Linux desktop target.

Re-ran locally at this head: eslint, tsc --noEmit, vitest on both settings suites (8/8), cargo clippy --all-targets --features test-utils -- -D warnings (macOS desktop), and the 5 new Rust tests — all pass. Also confirmed SystemCloseSettingsInfo serializes to exactly action / tray_available / "hide_to_tray" / "exit", matching the TS mirror; that the target-specific optional zbus arrangement is valid Cargo (Linux desktop gets a direct codeg → zbus edge, --no-default-features gets none); and that db::test_helpers being #[cfg(any(test, feature = "test-utils"))] means the new test module builds under a plain cargo test too.


Summary: the three blockers are genuinely resolved and the design is better for it. I'd fix #2 (its failure mode is permanent) and reconcile #1's comments with the code — folding #3 into the same state-machine repair — and then merge once the Ubuntu desktop cell has actually run green. Nice work on this one.

@xintaofei

Copy link
Copy Markdown
Owner

Thanks for the PR contribution; I am currently optimizing the issue mentioned above.

…probe

`linux_status_notifier_host_registered` completed into a terminal
`ProbeState::Complete`, so the first answer was reused for the rest of the
process — while its own doc comment, the one on `get_system_close_settings`
and the one in `close-behavior-settings.tsx` all described a per-read probe.
Installing the GNOME AppIndicator extension or restarting a panel therefore
left the "no system tray was detected" hint stuck until the app relaunched.

A worker that never published was worse. `.spawn(…).ok()` discarded the spawn
error and the timeout path left the slot `Running`, so every later call waited
out the deadline and answered `false` for the rest of the session. Both waits
used `wait_timeout` with no predicate, so a spurious wakeup read as a timeout,
and an answer published between unlocking and waiting was missed entirely.

Replace it with `SingleFlightProbe`: an answer is reused for `TRAY_PROBE_TTL`
and then re-probed, concurrent callers share one worker, and every give-up path
— waiter timeout, runner timeout, failed spawn — hands the slot back so the
next call starts fresh. A `generation` counter tracks slot ownership: a worker
publishes only while it still owns the slot, so an abandoned worker's late
answer cannot overwrite the one that replaced it, and a caller whose own worker
was already replaced cannot evict that replacement.

The two clocks are kept apart. A caller waiting on someone else's worker is
bounded by its own budget and releases nothing when that budget expires, since
the worker may still be healthy; only a worker past its *own* deadline is
released. A caller running the probe waits on that worker's deadline rather
than its entry-relative budget, which expires a few microseconds earlier and
would otherwise leave a worker already known to be dead for the next caller to
discover, costing that caller a wasted `false`.

The type compiles on Linux and in test builds on every target, so the state
machine is covered by unit tests on all CI cells rather than only the one that
compiles the D-Bus call.
`get_system_close_settings` and `update_system_close_settings` are async
commands, which Tauri drives on the Tokio runtime, and both called the
synchronous `tray_probably_visible` directly — parking a worker thread for as
long as the session-bus probe takes. Route it through `spawn_blocking`. A probe
that panics reports "no tray", the same as one that fails.
@xintaofei

Copy link
Copy Markdown
Owner

Pushed two commits to this branch (3a52dc4, f2ebe22) rather than leaving you another round of review comments — the remaining items were all inside one function, so it was easier to just do them. Please look them over; happy to drop or rework anything you disagree with.

What changed

linux_status_notifier_host_registered is now built on a small SingleFlightProbe:

  • Re-probes instead of caching one answer for the process. ProbeState::Complete was terminal, so the first answer was reused forever — while three comments (its own, get_system_close_settings, and the save handler in close-behavior-settings.tsx) described a per-read probe. An answer is now reused for TRAY_PROBE_TTL (5s) and then re-probed, so installing the AppIndicator extension and reopening the page actually clears the hint. The TTL keeps a settings load plus an immediate save down to one bus round trip.
  • Recovers from a stuck worker. .spawn(…).ok() was discarding the spawn error, and the timeout path left the slot Running, so a worker that hung or panicked pinned the answer to false — with every later call first waiting out the full deadline. Every give-up path now hands the slot back.
  • Generation counter. A worker publishes only while it still owns the slot, so an abandoned worker's late answer can't overwrite the one that replaced it, and a caller whose own worker was already replaced can't evict the replacement.
  • wait_timeout_while with a generation-aware predicate — the previous bare wait_timeout read spurious wakeups as timeouts and missed an answer published between unlocking and waiting.
  • spawn_blocking for the two async commands, so the probe doesn't park a Tokio worker.

Also corrected the doc lines that no longer matched: IsStatusNotifierHostRegistered isn't used "to pick a default" (CloseAction::default() decides from the target OS), and there's no recv_timeout — the outer bound is the condvar deadline.

A subtlety worth flagging, since it's the least obvious part of the diff: a caller's own budget and its worker's deadline are different clocks. A caller waiting on someone else's worker must not release it when its own budget runs out — the worker may be perfectly healthy, and evicting it throws away a good probe. But a caller that runs the probe has to wait on its own worker's deadline, not its entry-relative budget: the budget starts a few microseconds earlier, so it would give up just before its worker was formally overdue, decline to release it, and leave a worker already known to be dead for the next caller to trip over. That cost the next call a wasted false — it showed up as a 1-in-30 flaky test before I tracked it down.

Tests. SingleFlightProbe is compiled on Linux and in test builds on every target, so 11 unit tests now cover the state machine on all CI cells rather than only the one that compiles the D-Bus call: TTL reuse and expiry, single flight, hung worker, panicking worker, failed spawn recovery, late-answer rejection, and the two generation rules. Assertions are on invocation counts and returned values, never elapsed time.

I checked each one is load-bearing by regressing the fix and confirming failures — removing both recovery paths fails 3, removing the generation guard fails 1, removing the worker-deadline check fails 2.

Verified: full cargo test --features test-utils and the server-mode suite, both clippy matrices at -D warnings, eslint/tsc, full vitest (318 files / 4348 tests). Probe tests 0 failures in 60 runs at --test-threads 8 and 40 at 16.

The Linux-gated code still isn't compiled on macOS, so I lifted it verbatim into a scratch crate pinned to the exact locked versions (zbus =5.13.2, zbus_macros =5.13.2, zvariant =5.9.2, zbus_names =4.3.1) — clippy clean, and it returns false without hanging on a machine with no session bus. That covers the API surface but isn't a substitute for the real cell, so the Rust desktop on ubuntu-22.04 job still needs a green run before this merges — it's the only one that compiles the D-Bus path, and it's the platform this PR is for. I'll get that job re-run.

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.

4 participants