Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions apps/desktop/src-tauri/src/general_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ pub struct GeneralSettingsStore {
pub camera_window_position: Option<WindowPosition>,
#[serde(default)]
pub camera_window_positions_by_monitor_name: BTreeMap<String, WindowPosition>,
#[serde(default = "default_true")]
pub has_completed_onboarding: bool,
Comment on lines +152 to +153
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 serde default and Default impl disagree on has_completed_onboarding

The field uses #[serde(default = "default_true")] (deserialises missing field as true) but Default::default() returns false. The intent is clear: existing users whose settings files pre-date this field should be treated as having completed onboarding, while a truly fresh install (no file → Default::default()) must go through the flow. However, the mismatch is a maintenance footgun — any code that constructs a GeneralSettingsStore via Default::default() (e.g., in tests or fallback paths) silently gets false, which would force unexpected onboarding. A comment on the field explaining the deliberate discrepancy would help future contributors avoid breakage.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src-tauri/src/general_settings.rs
Line: 152-153

Comment:
**`serde` default and `Default` impl disagree on `has_completed_onboarding`**

The field uses `#[serde(default = "default_true")]` (deserialises missing field as `true`) but `Default::default()` returns `false`. The intent is clear: existing users whose settings files pre-date this field should be treated as having completed onboarding, while a truly fresh install (no file → `Default::default()`) must go through the flow. However, the mismatch is a maintenance footgun — any code that constructs a `GeneralSettingsStore` via `Default::default()` (e.g., in tests or fallback paths) silently gets `false`, which would force unexpected onboarding. A comment on the field explaining the deliberate discrepancy would help future contributors avoid breakage.

How can I resolve this? If you propose a fix, please make it concise.

}

fn default_enable_native_camera_preview() -> bool {
Expand Down Expand Up @@ -229,6 +231,7 @@ impl Default for GeneralSettingsStore {
main_window_position: None,
camera_window_position: None,
camera_window_positions_by_monitor_name: BTreeMap::new(),
has_completed_onboarding: false,
}
}
}
Expand Down
68 changes: 46 additions & 22 deletions apps/desktop/src-tauri/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,15 @@ async fn set_camera_input(
drop(app);

if id == current_id && camera_in_use {
if id.is_some() && !skip_camera_window.unwrap_or(false) {
let show_result = ShowCapWindow::Camera { centered: false }
.show(&app_handle)
.await;
show_result
.map_err(|err| error!("Failed to show camera preview window: {err}"))
.ok();
}

return Ok(());
}

Expand Down Expand Up @@ -2693,16 +2702,19 @@ async fn reset_camera_permissions(_app: AppHandle) -> Result<(), String> {

#[tauri::command]
#[specta::specta]
#[instrument(skip(app))]
async fn reset_microphone_permissions(app: AppHandle) -> Result<(), ()> {
let bundle_id = app.config().identifier.clone();
#[instrument(skip(_app))]
async fn reset_microphone_permissions(_app: AppHandle) -> Result<(), String> {
#[cfg(target_os = "macos")]
{
let bundle_id = _app.config().identifier.clone();

Command::new("tccutil")
.arg("reset")
.arg("Microphone")
.arg(bundle_id)
.output()
.expect("Failed to reset microphone permissions");
Command::new("tccutil")
Copy link
Copy Markdown

@tembo tembo bot Mar 25, 2026

Choose a reason for hiding this comment

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

This only errors on spawn failure; if tccutil exits non-zero, it’ll still return Ok(()). Capturing the output and checking status.success() makes failures actionable.

Suggested change
Command::new("tccutil")
let output = Command::new("tccutil")
.arg("reset")
.arg("Microphone")
.arg(bundle_id)
.output()
.map_err(|_| "Failed to reset microphone permissions".to_string())?;
if !output.status.success() {
let stderr = String::from_utf8_lossy(&output.stderr).to_string();
return Err(if stderr.is_empty() {
"Failed to reset microphone permissions".to_string()
} else {
stderr
});
}

.arg("reset")
.arg("Microphone")
.arg(bundle_id)
.output()
.map_err(|_| "Failed to reset microphone permissions".to_string())?;
}

Ok(())
}
Expand Down Expand Up @@ -3336,7 +3348,7 @@ pub async fn run(recording_logging_handle: LoggingHandle, logs_dir: PathBuf) {
flags
})
.with_denylist(&[
CapWindowId::Setup.label().as_str(),
CapWindowId::Onboarding.label().as_str(),
CapWindowId::Main.label().as_str(),
"window-capture-occluder",
"target-select-overlay",
Expand Down Expand Up @@ -3501,18 +3513,24 @@ pub async fn run(recording_logging_handle: LoggingHandle, logs_dir: PathBuf) {
tokio::spawn({
let app = app.clone();
async move {
if !permissions.screen_recording.permitted()
|| !permissions.accessibility.permitted()
|| GeneralSettingsStore::get(&app)
.ok()
.flatten()
.map(|s| !s.has_completed_startup)
.unwrap_or(false)
let settings = GeneralSettingsStore::get(&app).ok().flatten();
let startup_completed = settings
.as_ref()
.map(|s| s.has_completed_startup)
.unwrap_or(false);
let onboarding_completed = settings
.as_ref()
.map(|s| s.has_completed_onboarding)
.unwrap_or(false);

if !startup_completed
|| !onboarding_completed
|| !permissions.necessary_granted()
{
let _ = ShowCapWindow::Setup.show(&app).await;
println!("Showing onboarding");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: println

println! used instead of tracing macros for startup logs. Structured logging is bypassed. Use info! or debug! macros instead.

View Details

Location: apps/desktop/src-tauri/src/lib.rs (lines 3530)

Analysis

println! used instead of tracing macros for startup logs. Structured logging is bypassed

What fails Startup decision logging uses println! instead of the tracing crate's structured logging macros, so these messages bypass log levels, filtering, and structured output.
Result Messages go to raw stdout, not captured by the tracing subscriber or log files.
Expected Messages should use info!() or debug!() so they appear in structured logs with timestamps and levels.
Impact Debugging startup flow is harder in production since these messages aren't in the app's log files.
How to reproduce
1. Build and run the desktop app. 2. Observe stdout for 'Showing onboarding' or 'Showing main window' messages. 3. Note these don't appear in structured logs.
Patch Details
-                        println!("Showing onboarding");
+                        info!("Showing onboarding");
AI Fix Prompt
Fix this issue: println! used instead of tracing macros for startup logs. Structured logging is bypassed. Use info! or debug! macros instead.

Location: apps/desktop/src-tauri/src/lib.rs (lines 3530)
Problem: Startup decision logging uses println! instead of the tracing crate's structured logging macros, so these messages bypass log levels, filtering, and structured output.
Current behavior: Messages go to raw stdout, not captured by the tracing subscriber or log files.
Expected: Messages should use info!() or debug!() so they appear in structured logs with timestamps and levels.
Steps to reproduce: 1. Build and run the desktop app. 2. Observe stdout for 'Showing onboarding' or 'Showing main window' messages. 3. Note these don't appear in structured logs.

Provide a code fix.


Tip: Reply with @paragon-run to automatically fix this issue

let _ = ShowCapWindow::Onboarding.show(&app).await;
} else {
println!("Permissions granted, showing main window");

println!("Showing main window");
let _ = ShowCapWindow::Main {
init_target_mode: None,
}
Expand Down Expand Up @@ -3864,12 +3882,18 @@ pub async fn run(recording_logging_handle: LoggingHandle, logs_dir: PathBuf) {
.run(move |_handle, event| match event {
#[cfg(target_os = "macos")]
tauri::RunEvent::Reopen { .. } => {
if let Some(onboarding) = CapWindowId::Onboarding.get(_handle) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: Dock Reopen handler always focuses onboarding over all windows

Dock Reopen handler always focuses onboarding over all windows. Users can't access other windows via dock click. Move onboarding check after other window checks.

View Details

Location: apps/desktop/src-tauri/src/lib.rs (lines 3885)

Analysis

Dock Reopen handler always focuses onboarding over all windows

What fails When onboarding is open (e.g., via Help & Tour), clicking the dock icon always focuses the onboarding window and returns early, preventing access to editor, settings, or other windows.
Result The onboarding window is focused. The editor or other windows cannot be reached via the dock icon.
Expected Dock click should show/focus the most relevant window, not always trap on onboarding during revisits.
Impact Users who open Help & Tour are effectively locked out of using the dock icon to navigate to other open windows until they close onboarding.
How to reproduce
1. Open the main window. 2. Click 'Help & Tour' to open onboarding. 3. Open an editor window. 4. Click the dock icon (macOS).
AI Fix Prompt
Fix this issue: Dock Reopen handler always focuses onboarding over all windows. Users can't access other windows via dock click. Move onboarding check after other window checks.

Location: apps/desktop/src-tauri/src/lib.rs (lines 3885)
Problem: When onboarding is open (e.g., via Help & Tour), clicking the dock icon always focuses the onboarding window and returns early, preventing access to editor, settings, or other windows.
Current behavior: The onboarding window is focused. The editor or other windows cannot be reached via the dock icon.
Expected: Dock click should show/focus the most relevant window, not always trap on onboarding during revisits.
Steps to reproduce: 1. Open the main window. 2. Click 'Help & Tour' to open onboarding. 3. Open an editor window. 4. Click the dock icon (macOS).

Provide a code fix.


Tip: Reply with @paragon-run to automatically fix this issue

onboarding.show().ok();
onboarding.set_focus().ok();
return;
}

let has_window = _handle.webview_windows().iter().any(|(label, _)| {
label.starts_with("editor-")
|| label.starts_with("screenshot-editor-")
|| label.as_str() == "settings"
|| label.as_str() == "signin"
|| label.as_str() == "setup"
|| label.as_str() == "onboarding"
});

if has_window {
Expand All @@ -3881,7 +3905,7 @@ pub async fn run(recording_logging_handle: LoggingHandle, logs_dir: PathBuf) {
|| label.starts_with("screenshot-editor-")
|| label.as_str() == "settings"
|| label.as_str() == "signin"
|| label.as_str() == "setup"
|| label.as_str() == "onboarding"
})
.map(|(_, window)| window.clone())
{
Expand Down
8 changes: 4 additions & 4 deletions apps/desktop/src-tauri/src/tray.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,8 +330,8 @@ fn get_current_mode(app: &AppHandle) -> RecordingMode {
.unwrap_or_default()
}

fn is_setup_window_open(app: &AppHandle) -> bool {
app.webview_windows().contains_key("setup")
fn is_onboarding_window_open(app: &AppHandle) -> bool {
app.webview_windows().contains_key("onboarding")
}

fn create_mode_submenu(app: &AppHandle) -> tauri::Result<Submenu<tauri::Wry>> {
Expand Down Expand Up @@ -365,7 +365,7 @@ fn create_mode_submenu(app: &AppHandle) -> tauri::Result<Submenu<tauri::Wry>> {
}

fn build_tray_menu(app: &AppHandle, cache: &PreviousItemsCache) -> tauri::Result<Menu<tauri::Wry>> {
if is_setup_window_open(app) {
if is_onboarding_window_open(app) {
return Menu::with_items(
app,
&[
Expand Down Expand Up @@ -809,7 +809,7 @@ pub fn create_tray(app: &AppHandle) -> tauri::Result<()> {
Ok(TrayItem::RequestPermissions) => {
let app = app.clone();
tokio::spawn(async move {
let _ = ShowCapWindow::Setup.show(&app).await;
let _ = ShowCapWindow::Onboarding.show(&app).await;
});
}
_ => {}
Expand Down
Loading
Loading