fix: confirm desktop close behavior before hiding to tray#118
Open
zouyonghe wants to merge 11 commits intoAstrBotDevs:mainfrom
Open
fix: confirm desktop close behavior before hiding to tray#118zouyonghe wants to merge 11 commits intoAstrBotDevs:mainfrom
zouyonghe wants to merge 11 commits intoAstrBotDevs:mainfrom
Conversation
* fix: add backend startup heartbeat liveness probe * fix: tighten startup heartbeat validation * refactor: centralize startup heartbeat metadata * fix: surface heartbeat invalidation sooner * fix: harden startup heartbeat parsing * fix: warn on stop-time heartbeat failures * refactor: simplify startup heartbeat control flow * refactor: flatten readiness heartbeat helpers * refactor: clarify heartbeat helper responsibilities * docs: clarify startup heartbeat path coupling * fix: harden startup heartbeat coordination * fix: make startup heartbeat checks monotonic * fix: clean up heartbeat test and exit handling
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
desktop_bridge_submit_close_prompt, consider treating failures to close theclose-confirmwindow as non-fatal (log and still returnok: true) since the primary action (tray/exit) has already been executed and surfacing this as an error unnecessarily complicates the frontend flow. - The
CloseActionstring mapping is duplicated via theserdeannotations,CloseAction::parse,parse_close_action, andas_state_value; you could simplify by relying onserdefor serialization/deserialization and removing the manual string conversions to reduce drift when new actions are added. - The localization logic in
ui/close-confirm.htmloverridestrayButton.textContentandexitButton.textContentconditionally even thoughmessagesalready holds localized strings; simplifying this to always usecopy.tray/copy.exit(and optionally avoiding leaking rawinvokeErrormessages to users) would make the UI code more consistent and robust.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `desktop_bridge_submit_close_prompt`, consider treating failures to close the `close-confirm` window as non-fatal (log and still return `ok: true`) since the primary action (tray/exit) has already been executed and surfacing this as an error unnecessarily complicates the frontend flow.
- The `CloseAction` string mapping is duplicated via the `serde` annotations, `CloseAction::parse`, `parse_close_action`, and `as_state_value`; you could simplify by relying on `serde` for serialization/deserialization and removing the manual string conversions to reduce drift when new actions are added.
- The localization logic in `ui/close-confirm.html` overrides `trayButton.textContent` and `exitButton.textContent` conditionally even though `messages` already holds localized strings; simplifying this to always use `copy.tray`/`copy.exit` (and optionally avoiding leaking raw `invokeError` messages to users) would make the UI code more consistent and robust.
## Individual Comments
### Comment 1
<location path="src-tauri/src/bridge/commands.rs" line_range="330-341" />
<code_context>
+ }
+
+ match action {
+ CloseAction::Tray => {
+ window::actions::hide_main_window(
+ &app_handle,
+ DEFAULT_SHELL_LOCALE,
+ append_desktop_log,
+ );
+ if let Some(prompt_window) = app_handle.get_webview_window("close-confirm") {
+ if let Err(error) = prompt_window.close() {
+ let reason = format!("Failed to close close confirm prompt window: {error}");
+ append_desktop_log(&reason);
+ return BackendBridgeResult {
+ ok: false,
+ reason: Some(reason),
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider treating prompt-window close failures as non-fatal for the selected tray action.
In the tray path the main window is already hidden before `prompt_window.close()` is called, so the chosen action (minimize to tray) has succeeded even if closing the `close-confirm` window fails. Returning `{ ok: false }` here will surface this as a failed action in the UI, which is misleading because only a cosmetic cleanup failed. Consider logging the error but still returning `{ ok: true }` (or otherwise distinguishing cosmetic failures) so the UI doesn’t show an error for a successful tray action.
```suggestion
if let Some(prompt_window) = app_handle.get_webview_window("close-confirm") {
if let Err(error) = prompt_window.close() {
append_desktop_log(&format!(
"Failed to close close confirm prompt window: {error}"
));
}
}
BackendBridgeResult {
```
</issue_to_address>
### Comment 2
<location path="ui/close-confirm.html" line_range="204-205" />
<code_context>
+ document.getElementById("title").textContent = copy.title;
+ document.getElementById("description").textContent = copy.description;
+ document.getElementById("remember-label").textContent = copy.remember;
+ trayButton.textContent = locale === "zh-CN" ? copy.tray : "Minimize to tray";
+ exitButton.textContent = locale === "zh-CN" ? copy.exit : "Exit now";
+
+ function setPending(pending) {
</code_context>
<issue_to_address>
**nitpick:** Avoid duplicating the English labels already present in the locale map.
For `en-US`, `copy.tray` and `copy.exit` already provide these strings, so the ternaries only special-case `zh-CN` while duplicating the English literals. This risks the literals and the locale map drifting out of sync. Prefer always using `copy.tray` / `copy.exit` so the `messages` map remains the single source of truth.
</issue_to_address>
### Comment 3
<location path="src-tauri/src/close_behavior.rs" line_range="15" />
<code_context>
+ Value::Object(Map::new())
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
+#[serde(rename_all = "lowercase")]
+pub(crate) enum CloseAction {
</code_context>
<issue_to_address>
**issue (complexity):** Consider leveraging serde derives, a small typed DesktopState struct, and simpler path handling to reduce the amount of custom JSON and enum plumbing in this module while keeping the same behavior.
You can keep the same behavior while trimming quite a bit of surface area. Concretely:
### 1. Let serde handle enum parsing/serialization
`CloseAction::parse`, `as_state_value`, and `parse_close_action` duplicate what serde already does for you given the `#[serde(rename_all = "lowercase")]` derive.
You can drop those helpers and use serde directly:
```rust
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")]
pub(crate) enum CloseAction {
Tray,
Exit,
}
// Example usage in read/write:
fn parse_close_action_value(v: &Value) -> Option<CloseAction> {
serde_json::from_value::<CloseAction>(v.clone()).ok()
}
fn close_action_to_value(action: CloseAction) -> Value {
serde_json::to_value(action).expect("enum to serialize")
}
```
Then your read/write code can work directly with `CloseAction` via serde instead of manual `"tray"` / `"exit"` mapping.
### 2. Replace generic `Value` machinery with a small state struct
You currently have `empty_state_object`, `load_state_value`, and `ensure_object` to normalize JSON into an object and preserve unknown fields. You can keep all of that behavior but collapse it into a single typed struct that uses `#[serde(flatten)]`:
```rust
use serde::{Deserialize, Serialize};
use serde_json::{Map, Value};
const CLOSE_ACTION_FIELD: &str = "closeActionOnWindowClose";
#[derive(Serialize, Deserialize, Default)]
struct DesktopState {
#[serde(rename = "closeActionOnWindowClose")]
#[serde(skip_serializing_if = "Option::is_none")]
close_action: Option<CloseAction>,
// Preserve all other fields verbatim
#[serde(flatten)]
rest: Map<String, Value>,
}
fn load_desktop_state(raw: &str, log_subject: &str) -> DesktopState {
match serde_json::from_str::<DesktopState>(raw) {
Ok(state) => state,
Err(error) => {
crate::append_desktop_log(&format!(
"failed to parse {log_subject}: {error}. resetting state semantics"
));
DesktopState::default()
}
}
}
```
Then your read/write become much simpler:
```rust
fn read_cached_close_action_from_state_path(state_path: Option<PathBuf>) -> Option<CloseAction> {
let raw = fs::read_to_string(state_path?).ok()?;
let state = load_desktop_state(&raw, "desktop close behavior state");
state.close_action
}
fn write_cached_close_action_to_state_path(
action: Option<CloseAction>,
state_path: Option<PathBuf>,
) -> Result<(), String> {
let Some(state_path) = state_path else {
crate::append_desktop_log(
"close behavior state path is unavailable; skipping close action persistence",
);
return Ok(());
};
let mut state = match fs::read_to_string(&state_path) {
Ok(raw) => load_desktop_state(
&raw,
&format!("close behavior state {}", state_path.display()),
),
Err(e) if e.kind() == std::io::ErrorKind::NotFound => DesktopState::default(),
Err(e) => {
return Err(format!(
"Failed to read close behavior state {}: {}",
state_path.display(),
e
));
}
};
state.close_action = action;
save_state(&state_path, &serde_json::to_value(&state).unwrap())
}
```
This removes `empty_state_object`, `ensure_object`, manual map insert/remove, and the `CLOSE_ACTION_FIELD` string manipulation, while still:
- Treating malformed/incorrect-root JSON as “reset to empty object”.
- Preserving unrelated fields via `rest`.
- Allowing the `None` case to remove the close-action field.
### 3. Collapse the `Option<Path>` plumbing
The `*_from_state_path` / `*_to_state_path` pairs add an extra level of indirection for the `Option<PathBuf>` that doesn’t buy much.
You can push the `Option` handling to the public API and make the internal helpers work with `&Path`, which simplifies signatures and branching:
```rust
pub(crate) fn read_cached_close_action(
packaged_root_dir: Option<&Path>,
) -> Option<CloseAction> {
let state_path =
crate::desktop_state::resolve_desktop_state_path(packaged_root_dir)?;
read_cached_close_action_at_path(&state_path)
}
fn read_cached_close_action_at_path(state_path: &Path) -> Option<CloseAction> {
let raw = fs::read_to_string(state_path).ok()?;
let state = load_desktop_state(&raw, "desktop close behavior state");
state.close_action
}
pub(crate) fn write_cached_close_action(
action: Option<CloseAction>,
packaged_root_dir: Option<&Path>,
) -> Result<(), String> {
let Some(state_path) =
crate::desktop_state::resolve_desktop_state_path(packaged_root_dir)
else {
crate::append_desktop_log(
"close behavior state path is unavailable; skipping close action persistence",
);
return Ok(());
};
write_cached_close_action_at_path(action, &state_path)
}
fn write_cached_close_action_at_path(
action: Option<CloseAction>,
state_path: &Path,
) -> Result<(), String> {
// same as above, but with &Path
}
```
This keeps the behavior (no-op + log when path is unavailable) but reduces the number of functions and avoids threading `Option<PathBuf>` through the whole module.
---
If there is already a shared “atomic JSON save” helper elsewhere in the crate, wiring `save_state` through that helper instead of hand-rolling the temp-file pattern here would further cut down conceptual surface area while preserving the durability guarantees.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request implements a close confirmation dialog that allows users to choose between minimizing the application to the system tray or exiting completely. It introduces a new UI component, a persistence layer for saving user preferences, and logic within the window event loop to handle close requests based on these preferences. Key feedback includes addressing a potential file-locking issue on Windows during state persistence, migrating from internal to public Tauri APIs, and optimizing performance by avoiding synchronous I/O on the main thread. Other suggestions focus on improving code maintainability through the use of constants and simplifying localization logic.
Comment on lines
+112
to
+128
| let mut file = fs::File::create(&tmp_path).map_err(|error| { | ||
| format!( | ||
| "Failed to create temporary close behavior state file {}: {}", | ||
| tmp_path.display(), | ||
| error | ||
| ) | ||
| })?; | ||
| file.write_all(serialized.as_bytes()) | ||
| .and_then(|_| file.sync_all()) | ||
| .map_err(|error| { | ||
| format!( | ||
| "Failed to write temporary close behavior state file {}: {}", | ||
| tmp_path.display(), | ||
| error | ||
| ) | ||
| })?; | ||
| fs::rename(&tmp_path, path).map_err(|error| { |
Contributor
There was a problem hiding this comment.
On Windows, fs::rename will likely fail if the source file is still open. The file handle created at line 112 remains in scope and holds an open handle to the temporary file until the end of the function. You should explicitly drop the file handle or wrap the write operations in a block before attempting to rename the file.
Suggested change
| let mut file = fs::File::create(&tmp_path).map_err(|error| { | |
| format!( | |
| "Failed to create temporary close behavior state file {}: {}", | |
| tmp_path.display(), | |
| error | |
| ) | |
| })?; | |
| file.write_all(serialized.as_bytes()) | |
| .and_then(|_| file.sync_all()) | |
| .map_err(|error| { | |
| format!( | |
| "Failed to write temporary close behavior state file {}: {}", | |
| tmp_path.display(), | |
| error | |
| ) | |
| })?; | |
| fs::rename(&tmp_path, path).map_err(|error| { | |
| let mut file = fs::File::create(&tmp_path).map_err(|error| { | |
| format!( | |
| "Failed to create temporary close behavior state file {}: {}", | |
| tmp_path.display(), | |
| error | |
| ) | |
| })?; | |
| file.write_all(serialized.as_bytes()) | |
| .and_then(|_| file.sync_all()) | |
| .map_err(|error| { | |
| format!( | |
| "Failed to write temporary close behavior state file {}: {}", | |
| tmp_path.display(), | |
| error | |
| ) | |
| })?; | |
| drop(file); | |
| fs::rename(&tmp_path, path).map_err(|error| { |
ui/close-confirm.html
Outdated
| setPending(true); | ||
|
|
||
| try { | ||
| const result = await window.__TAURI_INTERNALS__.invoke( |
Contributor
There was a problem hiding this comment.
Using window.__TAURI_INTERNALS__ is discouraged as it targets internal APIs that are subject to change without notice. For Tauri v2, you should use the standard window.__TAURI__.core.invoke if the global Tauri object is enabled, or import invoke from the @tauri-apps/api package if using a bundler.
Suggested change
| const result = await window.__TAURI_INTERNALS__.invoke( | |
| const result = await window.__TAURI__.core.invoke( |
src-tauri/src/app_runtime.rs
Outdated
Comment on lines
+27
to
+29
| let packaged_root_dir = crate::runtime_paths::default_packaged_root_dir(); | ||
| let saved_close_action = | ||
| close_behavior::read_cached_close_action(packaged_root_dir.as_deref()); |
Contributor
There was a problem hiding this comment.
Performing synchronous file I/O and environment/home directory resolution within the window event loop can lead to UI unresponsiveness or stutters, as this closure runs on the main thread. Consider caching the packaged_root_dir and the saved_close_action in the BackendState during application startup to avoid blocking the event loop on every close request.
src-tauri/src/bridge/commands.rs
Outdated
| DEFAULT_SHELL_LOCALE, | ||
| append_desktop_log, | ||
| ); | ||
| if let Some(prompt_window) = app_handle.get_webview_window("close-confirm") { |
Contributor
There was a problem hiding this comment.
Avoid using hardcoded strings for window labels. It is better to use the constant defined in the window::close_confirm module to ensure consistency across the codebase.
Suggested change
| if let Some(prompt_window) = app_handle.get_webview_window("close-confirm") { | |
| if let Some(prompt_window) = app_handle.get_webview_window(window::close_confirm::CLOSE_CONFIRM_WINDOW_LABEL) { |
| @@ -0,0 +1,78 @@ | |||
| use tauri::{AppHandle, Manager, WebviewUrl, WebviewWindowBuilder}; | |||
|
|
|||
| const CLOSE_CONFIRM_WINDOW_LABEL: &str = "close-confirm"; | |||
Contributor
There was a problem hiding this comment.
This constant should be made pub(crate) so it can be referenced by the bridge commands instead of using hardcoded strings.
Suggested change
| const CLOSE_CONFIRM_WINDOW_LABEL: &str = "close-confirm"; | |
| pub(crate) const CLOSE_CONFIRM_WINDOW_LABEL: &str = "close-confirm"; |
ui/close-confirm.html
Outdated
Comment on lines
+204
to
+205
| trayButton.textContent = locale === "zh-CN" ? copy.tray : "Minimize to tray"; | ||
| exitButton.textContent = locale === "zh-CN" ? copy.exit : "Exit now"; |
Contributor
There was a problem hiding this comment.
The ternary logic here is redundant because the copy object is already localized based on the detected locale. You can directly use the properties from the copy object.
Suggested change
| trayButton.textContent = locale === "zh-CN" ? copy.tray : "Minimize to tray"; | |
| exitButton.textContent = locale === "zh-CN" ? copy.exit : "Exit now"; | |
| trayButton.textContent = copy.tray; | |
| exitButton.textContent = copy.exit; |
Member
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src-tauri/src/bridge/commands.rs" line_range="177-178" />
<code_context>
+ Log: Fn(&str),
+{
+ if let Err(error) = cleanup_result {
+ log(&format!(
+ "Failed to close close confirm prompt window: {error}"
+ ));
+ }
</code_context>
<issue_to_address>
**nitpick (typo):** The log message for close-confirm cleanup reads slightly awkwardly with a repeated "close".
The string "Failed to close close confirm prompt window" has a duplicated "close" that makes the log harder to scan. Consider rephrasing to something like "Failed to close confirm prompt window" or "Failed to close the close-confirm prompt window" for readability.
Suggested implementation:
```rust
fn finish_tray_close_prompt_cleanup<Log>(
cleanup_result: Result<(), String>,
log: Log,
) -> BackendBridgeResult
where
Log: Fn(&str),
{
if let Err(error) = cleanup_result {
log(&format!(
"Failed to close confirm prompt window: {error}"
));
}
Ok(BackendBridgeResponse::Success)
}
```
1. If `BackendBridgeResult` / `BackendBridgeResponse::Success` are not the correct result types for this function in your codebase, adjust the return to match the existing convention (e.g. `Ok(())` or another success variant).
2. Ensure `BackendBridgeResponse` is imported or in scope if that's the expected return type; otherwise, replace it with the appropriate type already used elsewhere in `commands.rs`.
</issue_to_address>
### Comment 2
<location path="src-tauri/src/bridge/commands.rs" line_range="164" />
<code_context>
}
}
+fn parse_close_prompt_action(raw_action: &str) -> Result<CloseAction, String> {
+ close_behavior::parse_close_action(raw_action)
+ .ok_or_else(|| "Invalid close action. Expected 'tray' or 'exit'.".to_string())
</code_context>
<issue_to_address>
**issue (complexity):** Consider inlining the close-prompt cleanup and parsing logic into the command to avoid extra helper layers that add indirection without clear benefit.
You can reduce indirection here without losing any functionality.
### 1. Remove `finish_tray_close_prompt_cleanup` indirection
The generic helper is only used once and always with `append_desktop_log`. You can inline the logic and drop the generic function and its tests:
```rust
match action {
CloseAction::Tray => {
window::actions::hide_main_window(
&app_handle,
DEFAULT_SHELL_LOCALE,
append_desktop_log,
);
if let Some(prompt_window) = app_handle.get_webview_window("close-confirm") {
if let Err(error) = prompt_window.close() {
append_desktop_log(&format!(
"Failed to close close confirm prompt window: {error}"
));
}
}
BackendBridgeResult {
ok: true,
reason: None,
}
}
CloseAction::Exit => {
let state = app_handle.state::<BackendState>();
state.mark_quitting();
app_handle.exit(0);
BackendBridgeResult {
ok: true,
reason: None,
}
}
}
```
Then you can delete `finish_tray_close_prompt_cleanup` and its tests.
### 2. Avoid wrapping `parse_close_action` with another parser
Instead of `parse_close_prompt_action`, you can map the `Option` from `close_behavior::parse_close_action` directly at the call site, keeping only one parsing API:
```rust
#[tauri::command]
pub(crate) fn desktop_bridge_submit_close_prompt(
app_handle: AppHandle,
action: String,
remember: bool,
) -> BackendBridgeResult {
let action = match close_behavior::parse_close_action(&action) {
Some(action) => action,
None => {
return BackendBridgeResult {
ok: false,
reason: Some("Invalid close action. Expected 'tray' or 'exit'.".to_string()),
};
}
};
// ... rest unchanged ...
}
```
If you still want a testable parsing function, you can move an ergonomic `Result`-returning parser into `close_behavior` itself:
```rust
// in close_behavior
pub fn parse_close_action_result(raw: &str) -> Result<CloseAction, String> {
parse_close_action(raw)
.ok_or_else(|| "Invalid close action. Expected 'tray' or 'exit'.".to_string())
}
```
And in the command:
```rust
let action = match close_behavior::parse_close_action_result(&action) {
Ok(action) => action,
Err(error) => {
return BackendBridgeResult {
ok: false,
reason: Some(error),
};
}
};
```
This removes the extra abstraction layer in the bridge module and keeps the flow in `desktop_bridge_submit_close_prompt` more direct.
</issue_to_address>
### Comment 3
<location path="src-tauri/src/close_behavior.rs" line_range="34" />
<code_context>
+ rest: Map<String, Value>,
+}
+
+pub(crate) fn parse_close_action(raw: &str) -> Option<CloseAction> {
+ serde_json::from_value(Value::String(raw.to_string())).ok()
+}
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying CloseAction parsing and deserialization by matching directly on strings instead of routing through serde_json::Value and from_value, which will keep behavior while making the code easier to follow.
You can reduce indirection around `CloseAction` parsing/deserialization without changing behavior.
### 1. Avoid going through `serde_json::Value` for `CloseAction`
Both `parse_close_action` and `deserialize_close_action_option` currently route through `serde_json::Value` and `serde_json::from_value`, which makes a simple string→enum mapping harder to follow.
You can keep the “invalid → None (no error)” semantics and the `#[serde(rename_all = "lowercase")]` behavior while simplifying to direct string matching:
```rust
pub(crate) fn parse_close_action(raw: &str) -> Option<CloseAction> {
match raw {
"tray" => Some(CloseAction::Tray),
"exit" => Some(CloseAction::Exit),
_ => None,
}
}
```
Then reuse this in the custom deserializer, instead of touching `serde_json::Value` at all:
```rust
fn deserialize_close_action_option<'de, D>(
deserializer: D,
) -> Result<Option<CloseAction>, D::Error>
where
D: Deserializer<'de>,
{
let raw = Option::<String>::deserialize(deserializer)?;
Ok(raw.as_deref().and_then(parse_close_action))
}
```
This keeps:
- `tray` / `exit` accepted,
- anything else treated as `None` (as your tests expect),
- unknown fields preserved via `rest: Map<String, Value>`,
but makes the control flow much easier to read and debug by removing JSON-specific machinery from a simple enum mapping.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| } | ||
| } | ||
|
|
||
| fn parse_close_prompt_action(raw_action: &str) -> Result<CloseAction, String> { |
There was a problem hiding this comment.
issue (complexity): Consider inlining the close-prompt cleanup and parsing logic into the command to avoid extra helper layers that add indirection without clear benefit.
You can reduce indirection here without losing any functionality.
1. Remove finish_tray_close_prompt_cleanup indirection
The generic helper is only used once and always with append_desktop_log. You can inline the logic and drop the generic function and its tests:
match action {
CloseAction::Tray => {
window::actions::hide_main_window(
&app_handle,
DEFAULT_SHELL_LOCALE,
append_desktop_log,
);
if let Some(prompt_window) = app_handle.get_webview_window("close-confirm") {
if let Err(error) = prompt_window.close() {
append_desktop_log(&format!(
"Failed to close close confirm prompt window: {error}"
));
}
}
BackendBridgeResult {
ok: true,
reason: None,
}
}
CloseAction::Exit => {
let state = app_handle.state::<BackendState>();
state.mark_quitting();
app_handle.exit(0);
BackendBridgeResult {
ok: true,
reason: None,
}
}
}Then you can delete finish_tray_close_prompt_cleanup and its tests.
2. Avoid wrapping parse_close_action with another parser
Instead of parse_close_prompt_action, you can map the Option from close_behavior::parse_close_action directly at the call site, keeping only one parsing API:
#[tauri::command]
pub(crate) fn desktop_bridge_submit_close_prompt(
app_handle: AppHandle,
action: String,
remember: bool,
) -> BackendBridgeResult {
let action = match close_behavior::parse_close_action(&action) {
Some(action) => action,
None => {
return BackendBridgeResult {
ok: false,
reason: Some("Invalid close action. Expected 'tray' or 'exit'.".to_string()),
};
}
};
// ... rest unchanged ...
}If you still want a testable parsing function, you can move an ergonomic Result-returning parser into close_behavior itself:
// in close_behavior
pub fn parse_close_action_result(raw: &str) -> Result<CloseAction, String> {
parse_close_action(raw)
.ok_or_else(|| "Invalid close action. Expected 'tray' or 'exit'.".to_string())
}And in the command:
let action = match close_behavior::parse_close_action_result(&action) {
Ok(action) => action,
Err(error) => {
return BackendBridgeResult {
ok: false,
reason: Some(error),
};
}
};This removes the extra abstraction layer in the bridge module and keeps the flow in desktop_bridge_submit_close_prompt more direct.
Member
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The exit flow is implemented both in
MainWindowAction::ExitApplicationand indesktop_bridge_submit_close_prompt(markingquitting, logging, and callingapp_handle.exit(0)), which could be refactored into a shared helper to keep shutdown behavior and logging consistent across all exit paths. - The close-confirm dialog script calls
window.__TAURI_INTERNALS__.invokedirectly; if you already have a wrapper or public Tauri API usage elsewhere, consider routing this through that instead to avoid depending on Tauri internals and keep the shell layer consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The exit flow is implemented both in `MainWindowAction::ExitApplication` and in `desktop_bridge_submit_close_prompt` (marking `quitting`, logging, and calling `app_handle.exit(0)`), which could be refactored into a shared helper to keep shutdown behavior and logging consistent across all exit paths.
- The close-confirm dialog script calls `window.__TAURI_INTERNALS__.invoke` directly; if you already have a wrapper or public Tauri API usage elsewhere, consider routing this through that instead to avoid depending on Tauri internals and keep the shell layer consistent.
## Individual Comments
### Comment 1
<location path="src-tauri/src/close_behavior.rs" line_range="12-16" />
<code_context>
+ Exit,
+}
+
+fn deserialize_close_action_option<'de, D>(deserializer: D) -> Result<Option<CloseAction>, D::Error>
+where
+ D: Deserializer<'de>,
+{
+ let raw = Option::<String>::deserialize(deserializer)?;
+ Ok(raw.as_deref().and_then(parse_close_action))
+}
</code_context>
<issue_to_address>
**issue (bug_risk):** The custom deserializer for close_action will fail hard on non-string values, dropping the entire state.
Because this helper deserializes via `Option::<String>::deserialize`, any non-string `closeActionOnWindowClose` (e.g., `true`, `0`, or an object) will cause `serde_json::from_str::<DesktopState>` to fail and `load_desktop_state` to fall back to `DesktopState::default()`, discarding the saved state. To avoid losing the whole state on malformed values while keeping the field optional, you could deserialize as `Option<serde_json::Value>` and only call `parse_close_action` for `Value::String(s)`, treating other types like invalid strings (i.e., `None`) instead of an error.
</issue_to_address>
### Comment 2
<location path="src-tauri/src/close_behavior.rs" line_range="65" />
<code_context>
+ state.close_action
+}
+
+fn ensure_parent_dir(path: &Path) -> Result<(), String> {
+ if let Some(parent_dir) = path.parent() {
+ fs::create_dir_all(parent_dir).map_err(|error| {
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the I/O helpers by specializing `save_state` to `DesktopState` and inlining `ensure_parent_dir` to reduce unnecessary abstraction and surface area without changing behavior.
You can shave off a layer of abstraction in the I/O helpers without changing behavior by specializing `save_state` to `DesktopState` and inlining `ensure_parent_dir`. This simplifies the signatures and error surface while keeping the same semantics and tests.
For example:
```rust
fn save_desktop_state(path: &Path, state: &DesktopState) -> Result<(), String> {
if let Some(parent_dir) = path.parent() {
fs::create_dir_all(parent_dir).map_err(|error| {
format!(
"Failed to create close behavior directory {}: {}",
parent_dir.display(),
error
)
})?;
}
let serialized = serde_json::to_string_pretty(state)
.map_err(|error| format!("Failed to serialize close behavior state: {error}"))?;
let tmp_name = format!(
"{}.tmp",
path.file_name()
.map(|value| value.to_string_lossy())
.unwrap_or_default()
);
let tmp_path = path.with_file_name(tmp_name);
let mut file = fs::File::create(&tmp_path).map_err(|error| {
format!(
"Failed to create temporary close behavior state file {}: {}",
tmp_path.display(),
error
)
})?;
file.write_all(serialized.as_bytes())
.and_then(|_| file.sync_all())
.map_err(|error| {
format!(
"Failed to write temporary close behavior state file {}: {}",
tmp_path.display(),
error
)
})?;
fs::rename(&tmp_path, path).map_err(|error| {
format!(
"Failed to atomically replace close behavior state file {}: {}",
path.display(),
error
)
})
}
```
Then update the call site:
```rust
fn write_cached_close_action_at_path(
action: Option<CloseAction>,
state_path: &Path,
) -> Result<(), String> {
let mut state = match fs::read_to_string(state_path) {
Ok(raw) => load_desktop_state(
&raw,
&format!("close behavior state {}", state_path.display()),
),
Err(error) if error.kind() == std::io::ErrorKind::NotFound => DesktopState::default(),
Err(error) => {
return Err(format!(
"Failed to read close behavior state {}: {}",
state_path.display(),
error
));
}
};
state.close_action = action;
save_desktop_state(state_path, &state)
}
```
This keeps all existing behavior (including directory creation, temp file usage, and error messages) while removing a generic and a helper that currently only serve this one type.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src-tauri/src/bridge/commands.rs" line_range="177-179" />
<code_context>
+ .ok_or_else(|| "Invalid close action. Expected 'tray' or 'exit'.".to_string())
+}
+
+fn finish_tray_close_prompt_cleanup<Log>(
+ cleanup_result: Result<(), String>,
+ log: Log,
+) -> BackendBridgeResult
+where
+ Log: Fn(&str),
+{
+ if let Err(error) = cleanup_result {
+ log(&format!(
+ "Failed to close close confirm prompt window: {error}"
+ ));
+ }
</code_context>
<issue_to_address>
**nitpick (typo):** Adjust wording in the log message to avoid the duplicated "close" phrasing.
The log message currently says `"Failed to close close confirm prompt window"`, which is awkward. Please rephrase to remove the duplicated "close", for example: `"Failed to close confirm prompt window"` or `"Failed to close close-confirm prompt window"`.
```suggestion
log(&format!(
"Failed to close confirm prompt window: {error}"
));
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The window label "close-confirm" is hard-coded in
desktop_bridge_submit_close_promptwhen looking up the prompt window; consider reusing theCLOSE_CONFIRM_WINDOW_LABELconstant fromwindow::close_confirmto avoid label drift between modules. - The close action values ("tray"/"exit") are duplicated as string literals in both the Rust layer (
parse_close_action) and the HTML dialog (submit("tray"/"exit")); wiring these through a shared enum/constant or a generated config would reduce the risk of the two drifting out of sync.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The window label "close-confirm" is hard-coded in `desktop_bridge_submit_close_prompt` when looking up the prompt window; consider reusing the `CLOSE_CONFIRM_WINDOW_LABEL` constant from `window::close_confirm` to avoid label drift between modules.
- The close action values ("tray"/"exit") are duplicated as string literals in both the Rust layer (`parse_close_action`) and the HTML dialog (`submit("tray"/"exit")`); wiring these through a shared enum/constant or a generated config would reduce the risk of the two drifting out of sync.
## Individual Comments
### Comment 1
<location path="ui/close-confirm.html" line_range="196-199" />
<code_context>
+ const trayButton = document.getElementById("tray-button");
+ const exitButton = document.getElementById("exit-button");
+ const error = document.getElementById("error");
+ const invokeTauri = window.__TAURI__?.core?.invoke || window.__TAURI__?.invoke || window.__TAURI_INTERNALS__?.invoke;
+ let recoveryTimer = null;
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Relying on `window.__TAURI_INTERNALS__` as a fallback is brittle; consider failing gracefully instead of depending on private APIs.
This fallback couples the dialog to a private Tauri API that may change without notice. Since there is already a user‑friendly path when `invokeTauri` is not a function, consider removing the `__TAURI_INTERNALS__` branch and relying only on `window.__TAURI__?.core?.invoke || window.__TAURI__?.invoke` to stay within supported APIs.
```suggestion
const exitButton = document.getElementById("exit-button");
const error = document.getElementById("error");
const invokeTauri = window.__TAURI__?.core?.invoke || window.__TAURI__?.invoke;
let recoveryTimer = null;
```
</issue_to_address>
### Comment 2
<location path="src-tauri/src/close_behavior.rs" line_range="12" />
<code_context>
+ Exit,
+}
+
+fn deserialize_close_action_option<'de, D>(deserializer: D) -> Result<Option<CloseAction>, D::Error>
+where
+ D: Deserializer<'de>,
</code_context>
<issue_to_address>
**issue (complexity):** Consider deserializing `close_action` via `Option<String>` instead of `Option<Value>` to express the optional string-to-`CloseAction` mapping more directly.
You can simplify the custom deserialization for `close_action` and avoid going through `serde_json::Value`, which reduces indirection without changing behavior.
Currently:
```rust
fn deserialize_close_action_option<'de, D>(deserializer: D) -> Result<Option<CloseAction>, D::Error>
where
D: Deserializer<'de>,
{
let raw = Option::<Value>::deserialize(deserializer)?;
Ok(match raw {
Some(Value::String(raw)) => parse_close_action(&raw),
_ => None,
})
}
```
This can be expressed more directly by deserializing to `Option<String>` instead of `Option<Value>` (you still treat non-strings and invalid values as `None`):
```rust
fn deserialize_close_action_option<'de, D>(deserializer: D) -> Result<Option<CloseAction>, D::Error>
where
D: Deserializer<'de>,
{
let raw = Option::<String>::deserialize(deserializer)?;
Ok(raw.as_deref().and_then(parse_close_action))
}
```
This keeps all current semantics:
- Non-string `closeActionOnWindowClose` → `None`
- Invalid string value → `None`
- Other fields preserved via `rest` unchanged
…but removes the need to pattern-match on `Value` and makes the intent of “optional string, mapped through `parse_close_action`” clearer.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
close_behavior::load_desktop_stateandwrite_cached_close_action, consider accepting a logging callback instead of callingcrate::append_desktop_logdirectly so the module remains easier to reuse and test in isolation without depending on global logging. - In
parse_close_prompt_action, build the error message usingCLOSE_ACTION_TRAYandCLOSE_ACTION_EXITrather than hard-coded'tray'/'exit'strings so the validation message stays in sync if the allowed values ever change.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `close_behavior::load_desktop_state` and `write_cached_close_action`, consider accepting a logging callback instead of calling `crate::append_desktop_log` directly so the module remains easier to reuse and test in isolation without depending on global logging.
- In `parse_close_prompt_action`, build the error message using `CLOSE_ACTION_TRAY` and `CLOSE_ACTION_EXIT` rather than hard-coded `'tray'`/`'exit'` strings so the validation message stays in sync if the allowed values ever change.
## Individual Comments
### Comment 1
<location path="src-tauri/src/bridge/commands.rs" line_range="164-167" />
<code_context>
}
}
+fn parse_close_prompt_action(raw_action: &str) -> Result<CloseAction, String> {
+ close_behavior::parse_close_action(raw_action)
+ .ok_or_else(|| "Invalid close action. Expected 'tray' or 'exit'.".to_string())
+}
</code_context>
<issue_to_address>
**suggestion:** Avoid hardcoding expected action strings in the error message to stay consistent with `close_behavior` constants.
The error text hardcodes `'tray'` and `'exit'` while the accepted values are defined in `close_behavior::CLOSE_ACTION_TRAY` / `CLOSE_ACTION_EXIT`. If those constants change, the message will be wrong. Please build the error text from those constants (or the `CloseAction` enum) so it stays in sync.
```suggestion
fn parse_close_prompt_action(raw_action: &str) -> Result<CloseAction, String> {
close_behavior::parse_close_action(raw_action).ok_or_else(|| {
format!(
"Invalid close action. Expected '{}' or '{}'.",
close_behavior::CLOSE_ACTION_TRAY,
close_behavior::CLOSE_ACTION_EXIT,
)
})
}
```
</issue_to_address>
### Comment 2
<location path="src-tauri/src/window/close_confirm.rs" line_range="26-30" />
<code_context>
+where
+ F: Fn(&str),
+{
+ if let Some(window) = app_handle.get_webview_window(CLOSE_CONFIRM_WINDOW_LABEL) {
+ if let Err(error) = window.unminimize() {
+ log(&format!(
+ "failed to unminimize close confirm window: {error}"
+ ));
+ }
+ if let Err(error) = window.show() {
+ log(&format!("failed to show close confirm window: {error}"));
+ }
+ if let Err(error) = window.set_focus() {
+ log(&format!("failed to focus close confirm window: {error}"));
+ }
+ return Ok(());
+ }
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The reopen path silently ignores failures to unminimize/show/focus the existing window, which might mask issues.
When the window already exists, errors from `unminimize`, `show`, and `set_focus` are only logged and the function still returns `Ok(())`. This can leave callers believing the dialog is visible and focused when it isn’t. Consider returning an `Err` on the first failure so callers can apply a fallback (e.g., hide the main window directly or show an alternative UX) instead of treating this as success.
```suggestion
log: F,
) -> Result<(), String>
where
F: Fn(&str),
{
if let Some(window) = app_handle.get_webview_window(CLOSE_CONFIRM_WINDOW_LABEL) {
if let Err(error) = window.unminimize() {
let message = format!("failed to unminimize close confirm window: {error}");
log(&message);
return Err(message);
}
if let Err(error) = window.show() {
let message = format!("failed to show close confirm window: {error}");
log(&message);
return Err(message);
}
if let Err(error) = window.set_focus() {
let message = format!("failed to focus close confirm window: {error}");
log(&message);
return Err(message);
}
return Ok(());
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src-tauri/src/close_behavior.rs" line_range="55" />
<code_context>
+ }
+}
+
+fn load_desktop_state<F>(raw: &str, log_subject: &str, log: &F) -> DesktopState
+where
+ F: Fn(&str),
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the logging and file-writing utilities by centralizing logger usage and extracting a reusable atomic JSON writer helper to reduce boilerplate and clarify responsibilities.
You can reduce complexity most effectively by simplifying the logging plumbing and centralizing the IO helpers, without changing behavior.
### 1. Narrow the logging surface (`Fn` generics → shared logger type)
Right now almost every function is generic over `F: Fn(&str)`, including internal helpers like `load_desktop_state` and `*_at_path`. That adds noise and duplicates the “logging capability” concept at every layer.
You can define a shared logger type and use `&Logger` internally, while keeping the public API generic for flexibility:
```rust
type Logger = dyn Fn(&str);
fn load_desktop_state(
raw: &str,
log_subject: &str,
log: &Logger,
) -> DesktopState {
match serde_json::from_str::<DesktopState>(raw) {
Ok(state) => state,
Err(error) => {
log(&format!(
"failed to parse {log_subject}: {error}. resetting state semantics"
));
DesktopState::default()
}
}
}
fn read_cached_close_action_at_path(
state_path: &Path,
log: &Logger,
) -> Option<CloseAction> {
let raw = fs::read_to_string(state_path).ok()?;
let state = load_desktop_state(&raw, "desktop close behavior state", log);
state.close_action
}
fn write_cached_close_action_at_path(
action: Option<CloseAction>,
state_path: &Path,
log: &Logger,
) -> Result<(), String> {
// unchanged body, just `log` is now `&Logger`
// ...
Ok(())
}
// Public API stays generic and just passes a `&Logger` through:
pub(crate) fn read_cached_close_action<F>(
packaged_root_dir: Option<&Path>,
log: F,
) -> Option<CloseAction>
where
F: Fn(&str),
{
let state_path = crate::desktop_state::resolve_desktop_state_path(packaged_root_dir)?;
read_cached_close_action_at_path(&state_path, &log)
}
pub(crate) fn write_cached_close_action<F>(
action: Option<CloseAction>,
packaged_root_dir: Option<&Path>,
log: F,
) -> Result<(), String>
where
F: Fn(&str),
{
let Some(state_path) = crate::desktop_state::resolve_desktop_state_path(packaged_root_dir)
else {
log("close behavior state path is unavailable; skipping close action persistence");
return Ok(());
};
write_cached_close_action_at_path(action, &state_path, &log)
}
```
Tests can keep using `noop_log` as `&Logger` with no behavior change. This removes generic parameters from internal helpers and makes signatures more readable.
### 2. Extract a tiny reusable atomic JSON writer
`save_desktop_state` mixes state semantics with low-level atomic write details. You can keep all behavior but move the IO pattern into a small helper, which also makes future reuse easier:
```rust
fn atomic_write_json(path: &Path, value: &impl Serialize, what: &str) -> Result<(), String> {
if let Some(parent_dir) = path.parent() {
fs::create_dir_all(parent_dir).map_err(|error| {
format!("Failed to create {what} directory {}: {}", parent_dir.display(), error)
})?;
}
let serialized = serde_json::to_string_pretty(value)
.map_err(|error| format!("Failed to serialize {what}: {error}"))?;
let tmp_name = format!(
"{}.tmp",
path.file_name()
.map(|value| value.to_string_lossy())
.unwrap_or_default()
);
let tmp_path = path.with_file_name(tmp_name);
let mut file = fs::File::create(&tmp_path).map_err(|error| {
format!(
"Failed to create temporary {what} file {}: {}",
tmp_path.display(),
error
)
})?;
file.write_all(serialized.as_bytes())
.and_then(|_| file.sync_all())
.map_err(|error| {
format!(
"Failed to write temporary {what} file {}: {}",
tmp_path.display(),
error
)
})?;
fs::rename(&tmp_path, path).map_err(|error| {
format!(
"Failed to atomically replace {what} file {}: {}",
path.display(),
error
)
})
}
fn save_desktop_state(path: &Path, state: &DesktopState) -> Result<(), String> {
atomic_write_json(path, state, "close behavior state")
}
```
This keeps all existing error messages structurally the same (only `what` is parameterized), preserves atomic behavior, and makes the state-related code (`state.close_action = action`) clearly separate from IO mechanics, which should address part of the “responsibility mix” concern without changing functionality.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| } | ||
| } | ||
|
|
||
| fn load_desktop_state<F>(raw: &str, log_subject: &str, log: &F) -> DesktopState |
There was a problem hiding this comment.
issue (complexity): Consider simplifying the logging and file-writing utilities by centralizing logger usage and extracting a reusable atomic JSON writer helper to reduce boilerplate and clarify responsibilities.
You can reduce complexity most effectively by simplifying the logging plumbing and centralizing the IO helpers, without changing behavior.
1. Narrow the logging surface (Fn generics → shared logger type)
Right now almost every function is generic over F: Fn(&str), including internal helpers like load_desktop_state and *_at_path. That adds noise and duplicates the “logging capability” concept at every layer.
You can define a shared logger type and use &Logger internally, while keeping the public API generic for flexibility:
type Logger = dyn Fn(&str);
fn load_desktop_state(
raw: &str,
log_subject: &str,
log: &Logger,
) -> DesktopState {
match serde_json::from_str::<DesktopState>(raw) {
Ok(state) => state,
Err(error) => {
log(&format!(
"failed to parse {log_subject}: {error}. resetting state semantics"
));
DesktopState::default()
}
}
}
fn read_cached_close_action_at_path(
state_path: &Path,
log: &Logger,
) -> Option<CloseAction> {
let raw = fs::read_to_string(state_path).ok()?;
let state = load_desktop_state(&raw, "desktop close behavior state", log);
state.close_action
}
fn write_cached_close_action_at_path(
action: Option<CloseAction>,
state_path: &Path,
log: &Logger,
) -> Result<(), String> {
// unchanged body, just `log` is now `&Logger`
// ...
Ok(())
}
// Public API stays generic and just passes a `&Logger` through:
pub(crate) fn read_cached_close_action<F>(
packaged_root_dir: Option<&Path>,
log: F,
) -> Option<CloseAction>
where
F: Fn(&str),
{
let state_path = crate::desktop_state::resolve_desktop_state_path(packaged_root_dir)?;
read_cached_close_action_at_path(&state_path, &log)
}
pub(crate) fn write_cached_close_action<F>(
action: Option<CloseAction>,
packaged_root_dir: Option<&Path>,
log: F,
) -> Result<(), String>
where
F: Fn(&str),
{
let Some(state_path) = crate::desktop_state::resolve_desktop_state_path(packaged_root_dir)
else {
log("close behavior state path is unavailable; skipping close action persistence");
return Ok(());
};
write_cached_close_action_at_path(action, &state_path, &log)
}Tests can keep using noop_log as &Logger with no behavior change. This removes generic parameters from internal helpers and makes signatures more readable.
2. Extract a tiny reusable atomic JSON writer
save_desktop_state mixes state semantics with low-level atomic write details. You can keep all behavior but move the IO pattern into a small helper, which also makes future reuse easier:
fn atomic_write_json(path: &Path, value: &impl Serialize, what: &str) -> Result<(), String> {
if let Some(parent_dir) = path.parent() {
fs::create_dir_all(parent_dir).map_err(|error| {
format!("Failed to create {what} directory {}: {}", parent_dir.display(), error)
})?;
}
let serialized = serde_json::to_string_pretty(value)
.map_err(|error| format!("Failed to serialize {what}: {error}"))?;
let tmp_name = format!(
"{}.tmp",
path.file_name()
.map(|value| value.to_string_lossy())
.unwrap_or_default()
);
let tmp_path = path.with_file_name(tmp_name);
let mut file = fs::File::create(&tmp_path).map_err(|error| {
format!(
"Failed to create temporary {what} file {}: {}",
tmp_path.display(),
error
)
})?;
file.write_all(serialized.as_bytes())
.and_then(|_| file.sync_all())
.map_err(|error| {
format!(
"Failed to write temporary {what} file {}: {}",
tmp_path.display(),
error
)
})?;
fs::rename(&tmp_path, path).map_err(|error| {
format!(
"Failed to atomically replace {what} file {}: {}",
path.display(),
error
)
})
}
fn save_desktop_state(path: &Path, state: &DesktopState) -> Result<(), String> {
atomic_write_json(path, state, "close behavior state")
}This keeps all existing error messages structurally the same (only what is parameterized), preserves atomic behavior, and makes the state-related code (state.close_action = action) clearly separate from IO mechanics, which should address part of the “responsibility mix” concern without changing functionality.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test Plan
Summary by Sourcery
Add a dedicated close confirmation flow for the desktop app that can remember the user’s preference and integrate it into the existing window, tray, and lifecycle handling.
New Features:
Enhancements:
Tests: