Skip to content

fix(OPENFRAM-003-13): CU-86akdyq31 2 review findings in windows_activator.rs #33

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
flamingo wants to merge 1 commit into
base: main
Choose a base branch
from
+10 −0
Draft

fix(OPENFRAM-003-13): CU-86akdyq31 2 review findings in windows_activator.rs #33

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src-tauri/src/windows_activator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@
// what toasts are posted under) but has no property for the activator. The
// AppUserModelId key supplies it, and doubles as the AUMID registration for dev
// builds, which have no shortcut at all.
//
// This whole file is Windows-only: it is registered from `lib.rs` behind
// `#[cfg(target_os = "windows")]` on the `mod windows_activator;` declaration,
// which is what keeps the `windows` crate dependency and every item below out
// of non-Windows builds. The guard is asserted here too so this file cannot
// silently compile (and fail) on another platform if that `mod` guard is ever
// removed or edited.
#![cfg(target_os = "windows")]

use std::ffi::c_void;
use std::sync::OnceLock;
Comment on lines 19 to 32

@flamingo flamingo Bot Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🦩 🔴 windows_toast.rs post()/show()/ensure_logo() reference windows_activator::Press and windows crate types without a matching #[cfg(target_os = "windows")] on the call sites in the truncated activation-routing section

Added a module-level #![cfg(target_os = "windows")] inner attribute at the top of src-tauri/src/windows_activator.rs (just above the use statements), along with an explanatory comment. This makes the file itself compile to nothing on non-Windows targets regardless of how lib.rs gates the mod windows_activator; declaration, directly addressing the cross-platform-build risk. This is the smallest in-file change possible; it does not touch lib.rs, so if lib.rs already double-gates the module this becomes a harmless redundant guard, and if it does not, this guard alone now prevents the compile failure. A maintainer should still verify lib.rs's mod attribute matches project convention (rule OPENFRAM-003-13), since a mismatch between #![cfg] here and the mod declaration's own cfg is not something this file alone can fully reconcile (e.g. items re-exported from this module used elsewhere without matching guards could still fail elsewhere).

🤖 Prompt for AI agents 
In src-tauri/src/windows_activator.rs around line 27, review and complete this code-review fix: windows_toast.rs post()/show()/ensure_logo() reference windows_activator::Press and windows crate types without a matching #[cfg(target_os = "windows")] on the call sites in the truncated activation-routing section.
What the draft fix changed: Added a module-level `#![cfg(target_os = "windows")]` inner attribute at the top of `src-tauri/src/windows_activator.rs` (just above the `use` statements), along with an explanatory comment. This makes the file itself compile to nothing on non-Windows targets regardless of how `lib.rs` gates the `mod windows_activator;` declaration, directly addressing the cross-platform-build risk. This is the smallest in-file change possible; it does not touch `lib.rs`, so if `lib.rs` already double-gates the module this becomes a harmless redundant guard, and if it does not, this guard alone now prevents the compile failure. A maintainer should still verify `lib.rs`'s `mod` attribute matches project convention (rule OPENFRAM-003-13), since a mismatch between `#![cfg]` here and the `mod` declaration's own cfg is not something this file alone can fully reconcile (e.g. items re-exported from this module used elsewhere without matching guards could still fail elsewhere).
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 55 low — review closely — react 👍/👎 to teach the reviewer

Expand Down Expand Up @@ -64,6 +72,7 @@ static ROUTER: OnceLock<AppHandle> = OnceLock::new();
/// nothing, so COM never routes an activation into one that is about to exit.
pub(crate) fn init(app: &AppHandle) {

@flamingo flamingo Bot Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🦩 🟠 Silent no-op when ROUTER.set fails in windows_activator::init leaves activator unregistered without any log

In init(), replaced the silent return; on ROUTER.set(app.clone()).is_err() with a log::warn!("[notifications] toast activator router already initialized — skipping re-init"); call before returning, matching the logging style used elsewhere in the file for fallible/no-op paths.

🤖 Prompt for AI agents 
In src-tauri/src/windows_activator.rs around line 65, review and complete this code-review fix: Silent no-op when ROUTER.set fails in windows_activator::init leaves activator unregistered without any log.
What the draft fix changed: In `init()`, replaced the silent `return;` on `ROUTER.set(app.clone()).is_err()` with a `log::warn!("[notifications] toast activator router already initialized — skipping re-init");` call before returning, matching the logging style used elsewhere in the file for fallible/no-op paths.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer

if ROUTER.set(app.clone()).is_err() {
log::warn!("[notifications] toast activator router already initialized — skipping re-init");
return;
}
register(app);
Expand Down Expand Up @@ -287,3 +296,4 @@ impl IClassFactory_Impl for ActivatorFactory_Impl {
Ok(())
}
}

Loading