Skip to content

Conversation

@Brendonovich
Copy link
Contributor

@Brendonovich Brendonovich commented Sep 18, 2025

Fixes delete from editor on Windows by doing the delete after closing the window, preventing the 'File is in use' error

Summary by CodeRabbit

  • New Features

    • Added a unified delete action that closes an editor and deletes its project from the app.
  • Refactor

    • Editor header now delegates project deletion to the centralized backend command after confirmation.
  • Bug Fixes

    • More reliable deletion flow and window cleanup to reduce partial deletions or lingering windows.
  • Chores

    • Made the new delete command available to the frontend command set.
  • Style

    • Adjusted root container width/overflow in several settings pages for cleaner layout.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 18, 2025

Walkthrough

Adds a new Tauri command editor_delete_project that closes an editor window, deletes its project directory, and emits a RecordingDeleted event; frontend delete flows are updated to call commands.editorDeleteProject() instead of performing removal and window-close logic client-side.

Changes

Cohort / File(s) Summary
Tauri backend: delete command & event
apps/desktop/src-tauri/src/lib.rs
Adds #[tauri::command] #[specta::specta] async fn editor_delete_project(app: tauri::AppHandle, editor_instance: WindowEditorInstance, window: tauri::Window) which closes the window (errors ignored), sleeps 20ms, drops the instance, calls tokio::fs::remove_dir_all on editor_instance.0.project_path, and emits RecordingDeleted (now also Serialize) to the frontend; registers the command in the specta builder.
Frontend command API
apps/desktop/src/utils/tauri.ts
Adds async editorDeleteProject(): Promise<void> to exported commands, calling TAURI_INVOKE("editor_delete_project").
Editor UI delete flow
apps/desktop/src/routes/editor/Header.tsx
Replaces inline path-check, fs removal, event emit, and window close with await commands.editorDeleteProject() after confirmation; removes direct use of editorInstance.path.
Presentational tweaks: settings pages
apps/desktop/src/routes/(window-chrome)/settings/{experimental.tsx,general.tsx,hotkeys.tsx,license.tsx}
Removed w-full (and in one file overflow-y-auto) from root container class strings; purely presentational CSS class adjustments.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Header as Header.tsx
  participant Cmd as commands.editorDeleteProject()
  participant Tauri as Tauri Runtime
  participant Rust as editor_delete_project

  User->>Header: Click "Delete" + confirm
  Header->>Cmd: await editorDeleteProject()
  Cmd->>Tauri: invoke("editor_delete_project")
  Tauri->>Rust: call command (app, editor_instance, window)

  rect rgba(200,230,255,0.25)
    Rust->>Rust: close window (errors ignored)
    Rust->>Rust: sleep 20ms
  end

  rect rgba(230,255,230,0.25)
    Rust->>Rust: remove_dir_all(project_path) (async)
    Rust->>Tauri: emit RecordingDeleted(project_path)
  end

  Tauri-->>Cmd: resolved
  Cmd-->>Header: resolved
  Header-->>User: Deletion completed
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

codex

Poem

I hop and click, the window drifts away,
I chew the folder path, then tuck it away.
A tiny pause, a tidy, gentle sweep,
The project folds — I hum, then softly sleep. 🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Delete on backend after closing window" succinctly and accurately summarizes the primary change — moving the deletion to the backend to run after the editor window is closed to avoid Windows "file in use" errors — and it aligns with the PR description and the implemented changes (the new editor_delete_project command and frontend wiring).
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-windows-delete-via-editor

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ff8543a and 8df17be.

📒 Files selected for processing (1)
  • apps/desktop/src-tauri/src/lib.rs (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/desktop/src-tauri/src/lib.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
  • GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
  • GitHub Check: Analyze (rust)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
apps/desktop/src/routes/editor/Header.tsx (1)

78-79: Surface deletion failures to the user.

Wrap the call to handle backend errors (e.g., if Windows still holds file locks) and notify via the existing global dialog.

-            await commands.editorDeleteProject();
+            try {
+              await commands.editorDeleteProject();
+            } catch (e) {
+              await commands.globalMessageDialog("Failed to delete recording. Please try again.");
+            }

Also, getCurrentWindow and remove imports are now unused after this refactor; consider removing them.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 18c900e and 070d373.

📒 Files selected for processing (3)
  • apps/desktop/src-tauri/src/lib.rs (2 hunks)
  • apps/desktop/src/routes/editor/Header.tsx (1 hunks)
  • apps/desktop/src/utils/tauri.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/tauri.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Never edit auto-generated IPC bindings file: tauri.ts

Do not edit auto-generated file: tauri.ts

Files:

  • apps/desktop/src/utils/tauri.ts
apps/desktop/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

apps/desktop/**/*.{ts,tsx}: In the desktop app, rely on unplugin-icons auto-imports; do not manually import icon modules
Use generated tauri_specta commands/events (commands, events) in the desktop frontend; listen to generated events directly
Use @tanstack/solid-query for server state in the desktop app

Files:

  • apps/desktop/src/utils/tauri.ts
  • apps/desktop/src/routes/editor/Header.tsx
{apps/desktop,packages/ui-solid}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Component naming (Solid): components in PascalCase; hooks/utilities in camelCase starting with 'use' where applicable

Files:

  • apps/desktop/src/utils/tauri.ts
  • apps/desktop/src/routes/editor/Header.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use strict TypeScript and avoid any; prefer shared types from packages

**/*.{ts,tsx}: Use Biome to format/lint TypeScript with a 2-space indent
TypeScript file names should be kebab-case (e.g., user-menu.tsx)

Files:

  • apps/desktop/src/utils/tauri.ts
  • apps/desktop/src/routes/editor/Header.tsx
apps/desktop/src-tauri/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Use tauri_specta on the Rust side for IPC (typed commands/events) and emit events via generated types

Files:

  • apps/desktop/src-tauri/src/lib.rs
{crates,apps/cli,apps/desktop/src-tauri}/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

{crates,apps/cli,apps/desktop/src-tauri}/**/*.rs: Rust code must be formatted with rustfmt and adhere to workspace clippy lints
Rust module files should use snake_case names

Files:

  • apps/desktop/src-tauri/src/lib.rs
{crates,apps/cli,apps/desktop/src-tauri}/**/{src,tests}/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Rust tests should live in src (inline/unit) or tests (integration) directories

Files:

  • apps/desktop/src-tauri/src/lib.rs
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

React/Solid components should be named using PascalCase

Files:

  • apps/desktop/src/routes/editor/Header.tsx
🧠 Learnings (1)
📚 Learning: 2025-09-08T16:48:20.727Z
Learnt from: CR
PR: CapSoftware/Cap#0
File: AGENTS.md:0-0
Timestamp: 2025-09-08T16:48:20.727Z
Learning: Applies to **/tauri.ts : Do not edit auto-generated file: tauri.ts

Applied to files:

  • apps/desktop/src/utils/tauri.ts
🧬 Code graph analysis (1)
apps/desktop/src-tauri/src/lib.rs (1)
apps/desktop/src-tauri/src/editor_window.rs (3)
  • window (64-64)
  • window (76-76)
  • window (123-123)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
  • GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
  • GitHub Check: Analyze (rust)
🔇 Additional comments (2)
apps/desktop/src/utils/tauri.ts (1)

265-268: Generated bindings: confirm this came from specta export (do not hand‑edit).

This file is auto‑generated. The new editorDeleteProject() entry looks consistent, but please ensure it was produced by tauri_specta::Builder::export and not edited manually. If you change the Rust command signature later (e.g., to return Result), re‑export to regenerate this file.

apps/desktop/src-tauri/src/lib.rs (1)

1910-1911: Command registration looks good.

editor_delete_project is properly added to the specta builder. If you adopt the Result<(), String> return in the command, remember to re‑export TS bindings so the frontend receives rethrown errors as expected.

Comment on lines 1691 to 1697
#[tauri::command]
#[specta::specta]
async fn editor_delete_project(editor_instance: WindowEditorInstance, window: tauri::Window) {
let _ = window.close();

let _ = tokio::fs::remove_dir_all(&editor_instance.0.project_path).await;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Bug: incorrect field access; plus fragile delete timing on Windows.

  • Critical: editor_instance.0.project_path likely won’t compile; elsewhere WindowEditorInstance exposes project_path directly.
  • Reliability: closing the window and immediately deleting can still race with WebView teardown on Windows. Add a short wait/until‑destroyed check and retry deletion. Emit RecordingDeleted after success so the UI can update.
-#[tauri::command]
-#[specta::specta]
-async fn editor_delete_project(editor_instance: WindowEditorInstance, window: tauri::Window) {
-    let _ = window.close();
-
-    let _ = tokio::fs::remove_dir_all(&editor_instance.0.project_path).await;
-}
+#[tauri::command]
+#[specta::specta]
+async fn editor_delete_project(
+    editor_instance: WindowEditorInstance,
+    window: tauri::Window,
+) -> Result<(), String> {
+    let app = window.app_handle().clone();
+    let label = window.label().to_string();
+
+    // Best-effort close; proceed regardless.
+    let _ = window.close();
+
+    // Wait briefly for the window to actually disappear (helps on Windows).
+    for _ in 0..50 {
+        if !app.webview_windows().contains_key(&label) {
+            break;
+        }
+        tokio::time::sleep(std::time::Duration::from_millis(50)).await;
+    }
+
+    let path = editor_instance.project_path.clone();
+
+    // Retry deletion a few times to avoid "file in use" races.
+    let mut last_err: Option<std::io::Error> = None;
+    for _ in 0..10 {
+        match tokio::fs::remove_dir_all(&path).await {
+            Ok(_) => {
+                RecordingDeleted { path: path.clone() }.emit(&app).ok();
+                return Ok(());
+            }
+            Err(e) => {
+                last_err = Some(e);
+                tokio::time::sleep(std::time::Duration::from_millis(200)).await;
+            }
+        }
+    }
+    Err(format!(
+        "Failed to delete project at {:?}: {}",
+        path,
+        last_err.unwrap_or_else(|| std::io::Error::new(std::io::ErrorKind::Other, "unknown"))
+    ))
+}

Committable suggestion skipped: line range outside the PR's diff.

@Brendonovich Brendonovich merged commit 428bb0e into main Sep 18, 2025
15 checks passed
@Brendonovich Brendonovich deleted the fix-windows-delete-via-editor branch September 18, 2025 13:58
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.

3 participants