Resume file watching automatically after restart

[cohogan]

Summary

With autostart enabled (added in #22), the app now launches at login — but it forgot what it was doing before the restart. This PR makes sync continue across restarts with no user interaction:

• Persist the watched folder in the Tauri Store (watched_folder key) whenever watching starts; cleared only when the user explicitly hits Stop.
• Resume on launch: setup() restores the saved folder and restarts the watcher on a background thread. The existing initial-scan path re-checks every file against the server (CRC32C dedup), so files that changed while the app was closed get synced too — unchanged files are skipped server-side.
• Persist upload config: previously ignored patterns / delays / toggles silently reset to defaults on every app restart, which would make a headless resume run with the wrong settings. Now saved under upload_config and restored at startup. The server URL is deliberately always taken from the build env rather than the store, so a config written by a dev build can never point a production build at localhost.
• UI sync: new get_watched_folder command + watch_resumed event, so the frontend shows the watched folder whether the webview loads before or after the resume finishes.
• Single instance (release builds only): with auto-resume, a second app instance would immediately start a duplicate watcher and double-upload every change, so tauri-plugin-single-instance now focuses the existing window instead. Gated out of debug builds so pnpm tauri dev still works alongside the installed app.

Testing

Verified end-to-end on Windows with a debug build against the production backend:

• Seeded watched_folder in the store, launched the app with no UI interaction → log shows Resumed watching <folder> after restart, the initial scan queued and uploaded the folder's file, and heartbeat continued.
• Created a new file after resume → watcher picked it up and uploaded it within seconds.
• Folder-missing-at-startup path skips resume with a warning but keeps the stored path so a temporarily unmounted drive is retried next boot.
• cargo check, cargo clippy, and tsc all clean.

🤖 Generated with Claude Code

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[github-actions]

Claude finished @cohogan's task in 2m 56s —— View job

---

Code Review: Resume file watching after restart

• Read and analyze all changed files
• Review Rust backend changes (lib.rs, upload.rs)
• Review frontend changes (simple.tsx)
• Review dependency changes (Cargo.toml, Cargo.lock)
• Post detailed review findings

---

Overall this is a well-structured PR. The refactor of start_watching into a shared start_watching_impl is clean, the store persistence is straightforward, and the single-instance gating behind #[cfg(not(debug_assertions))] is a nice touch. A few things worth addressing:

---

Bug: Frontend shows folder as watched when resume fails

src-tauri/src/lib.rs:582-611

During resume, the code eagerly sets watched_folder_state to Some(folder) at line 586 before spawning the blocking task. If start_watching_impl subsequently fails (line 606-609), watched_folder_state is cleared back to None — but the frontend has already called get_watched_folder, received Some(folder), and set selectedFolder. There is no error event emitted to the frontend, so the UI continues to show the folder as being watched when it is not.

The watch_resumed event only fires on success, so the frontend has no way to learn about the failure.

Suggested fix: Either (a) don't set watched_folder_state eagerly — let start_watching_impl handle it on success, and emit watch_resumed before the frontend polls; or (b) emit a watch_resume_failed event on failure so the frontend can clear selectedFolder.

Option (b) is probably better UX since it lets you show a transient "Resuming…" state:

// On failure inside the spawn_blocking:
Err(e) => {
    log::error!("Failed to resume watching {folder}: {e}");
    *watched_folder_state.lock() = None;
    let _ = app_handle.emit("watch_resume_failed", &folder); // <-- new
}

// In simple.tsx
const unlistenWatchResumeFailed = listen("watch_resume_failed", () => {
  setSelectedFolder("");
});

Fix this →

---

Nit: Redundant store write on resume path

src-tauri/src/lib.rs:196-198

start_watching_impl always persists watched_folder to the store. During resume, this re-writes the exact same value that was just read from the store moments earlier. Harmless but unnecessary. You could pass a flag or skip the write when the value already matches, though this is genuinely minor.

---

Observation: unwrap_or_default() silently stores null

src-tauri/src/upload.rs:874

serde_json::to_value(&config).unwrap_or_default(),

If serialization ever fails (very unlikely since UploadConfig derives Serialize), this stores serde_json::Value::Null, which would cause restore_upload_config to silently return None on next startup — the config resets to defaults without any log. Consider using unwrap() or logging the error. Very low risk given the derive, but worth noting.

---

Good decisions in this PR

• server_url override in restore_upload_config (upload.rs:890): Always taking the server URL from the build environment is the right call. A dev-build config can never pollute a production build. Clean and well-documented.

• spawn_blocking for resume (lib.rs:593): capture_initial_contents does synchronous file I/O — running it on the blocking pool avoids stalling the async runtime. Correct use of the API.

• Race-safe frontend sync (simple.tsx:191-198): The belt-and-suspenders approach of both listening for watch_resumed and polling get_watched_folder on mount handles the timing race between backend resume and webview load. Well done.

• Single-instance gating (lib.rs:487): #[cfg(not(debug_assertions))] is clean — dev can run alongside the installed app, production prevents duplicate watchers.

• Folder-missing-at-startup resilience (lib.rs:612-616): Keeping the stored path when the folder doesn't exist allows for retrying on next boot (e.g., unmounted drive). Sensible default.

---

Summary

The primary concern is the UI desync when resume fails — the frontend can show a folder as watched when the watcher actually failed to start. The rest is solid. The architecture of persisting state + restoring on setup + syncing to the frontend is well thought out.

[cohogan]

Addressed the review in 37f4e1e:

• UI desync on resume failure: now emits watch_resume_failed; the frontend clears the watched folder and shows an error toast (option b from the review).
• unwrap_or_default() on config serialization: replaced with an explicit match that logs the error instead of silently persisting null.
• Redundant store write on resume: leaving as-is — it rewrites an identical value once per launch, and skipping it would add a flag/comparison for no observable benefit.

🤖 Generated with Claude Code

[github-actions]

Claude finished @cohogan's task in 2m 30s —— View job

---

• Read and analyze all changed files
• Review Rust backend changes (lib.rs, upload.rs)
• Review frontend changes (simple.tsx)
• Review dependency changes (Cargo.toml)
• Post detailed review findings

---

Code Review: Resume file watching after restart (post-fix)

The fixes from the first review (commit 37f4e1e) look correct — watch_resume_failed is emitted, the frontend handles it with an error toast, and unwrap_or_default was replaced with an explicit match + log::error!. The overall PR is well-structured. Two items worth addressing:

---

Issue: UploadConfig deserialization breaks on version upgrades

src-tauri/src/upload.rs:46-54

UploadConfig derives Deserialize without #[serde(default)]. When a future version of the app adds a new field to the struct (e.g. max_file_size), restore_upload_config will fail to deserialize the old stored config — serde_json::from_value returns Err because the new field is missing, .ok()? maps that to None, and the entire config silently falls back to defaults. That loses the user's customized ignored patterns, upload delay, and enabled state.

Since the config is now persisted across app restarts, this will happen on every upgrade that touches UploadConfig.

Suggested fix: Add #[serde(default)] to the struct so missing fields get their Default values while preserving existing fields:

#[derive(Clone, Serialize, Deserialize)]
#[serde(default)]
pub struct UploadConfig {
    // ...
}

This is a one-line change that prevents future breakage.

Fix this →

---

Observation: Race between auto-resume and user-initiated start

src-tauri/src/lib.rs:582-612

The resume runs in spawn_blocking while the webview loads. If a user opens the app and clicks "Start Watching" on a different folder before the resume's capture_initial_contents finishes, both calls enter start_watching_impl concurrently. The watcher_state mutex serializes watcher creation, so the last writer wins and the final state is consistent — but the losing scan's file events still fire against the old folder path, producing spurious file_change / upload-queue entries.

In practice this window is very narrow (the user would need to interact within the first few seconds), so this is more of a design note than an actionable bug. If it ever surfaces, the fix would be a generation counter or cancellation token on the initial scan.

---

Good decisions in this PR

• server_url override (upload.rs:890): Always reading from the build env prevents cross-environment contamination. Well-documented.
• spawn_blocking for resume (lib.rs:593): Keeps synchronous file I/O off the async runtime. Correct API choice.
• Belt-and-suspenders frontend sync (simple.tsx:191-202): Polling get_watched_folder + listening for watch_resumed covers the timing race between backend resume and webview load.
• Single-instance gating (lib.rs:487): #[cfg(not(debug_assertions))] is the right granularity — dev can run alongside the installed app, production prevents duplicate watchers.
• Folder-missing-at-startup resilience (lib.rs:613-617): Keeping the stored path for unmounted drives is a sensible default.
• watch_resume_failed event (37f4e1e fix): Frontend correctly clears state and shows an error toast.

---

Summary

One actionable item: add #[serde(default)] to UploadConfig so future field additions don't silently discard persisted settings. The race note is informational. Otherwise this is solid — the resume architecture, store persistence, and frontend sync are all well thought out. Ship it.

[cohogan]

Applied the #[serde(default)] fix in the latest commit — stored configs now survive future field additions.

On the resume-vs-user-start race: agreed it's a design note rather than a bug. The window is a few seconds at launch, the mutex keeps final state consistent, and the worst case is a few spurious queue entries from the losing scan (which the server-side CRC dedup absorbs). A generation counter can be added if it ever shows up in practice.

🤖 Generated with Claude Code

[github-actions]

Claude finished @cohogan's task in 2m 23s —— View job

---

• Read and analyze all changed files against origin/main
• Review Rust backend changes (lib.rs, upload.rs)
• Review frontend changes (simple.tsx)
• Review dependency changes (Cargo.toml)
• Post detailed review findings

---

Code Review: Resume file watching after restart (round 3)

Previous review rounds caught and fixed the main issues (UI desync on resume failure, silent unwrap_or_default, forward-compatible deserialization). The codebase looks solid after those fixes. One remaining item and a few observations:

---

Issue: restore_upload_config silently discards deserialization errors

src-tauri/src/upload.rs:893

let mut config: UploadConfig = serde_json::from_value(value.clone()).ok()?;

The previous review fixed set_upload_config to log serialization errors explicitly (line 878), but restore_upload_config still uses .ok()? which silently maps any deserialization failure to None. If the stored config is somehow corrupted (e.g. a value has the wrong type — upload_delay_ms stored as a string instead of a number), this silently falls back to defaults with no log trace. This is inconsistent with the explicit error handling in set_upload_config.

#[serde(default)] protects against missing fields but not wrong-typed fields. A type mismatch from a buggy write or manual edit of settings.json would still fail here silently.

Suggested fix: Log the error before falling back:

let mut config: UploadConfig = match serde_json::from_value(value.clone()) {
    Ok(c) => c,
    Err(e) => {
        log::warn!("Failed to restore upload config, using defaults: {e}");
        return None;
    }
};

Minor — but would save debugging time if it ever hits.

Fix this →

---

Observation: Resume-then-stop race can undo user intent

src-tauri/src/lib.rs:582-612

Already discussed in the prior review round, noting for completeness: if the user clicks "Stop" while capture_initial_contents is still running in spawn_blocking, stop_watching clears the watcher and deletes the store key — but when start_watching_impl completes it re-creates the watcher, re-sets watched_folder_state, and re-persists the store key, effectively undoing the stop. The failure path (line 606-609) clears the state, but the success path (line 602-604) doesn't check whether a stop was requested in the interim.

As the author noted, this window is narrow (a few seconds at startup) and the worst case is absorbed by server-side dedup. A generation counter or cancellation token would fix it cleanly if it surfaces in practice.

---

Good decisions confirmed in this round

• #[serde(default)] on UploadConfig (upload.rs:50): Future field additions won't discard existing persisted settings.
• watch_resume_failed event (lib.rs:609): Frontend correctly clears state and shows an error toast — the UI desync bug from round 1 is fixed.
• Explicit match in set_upload_config (upload.rs:876-879): Serialization errors are now logged instead of silently storing null.
• server_url override (upload.rs:894): Build-env URL always wins over the stored value.
• Single-instance gating (lib.rs:487): #[cfg(not(debug_assertions))] is correct — no capabilities entry needed since this plugin is Rust-only with no frontend API.
• Cleanup in stop_watching (lib.rs:248-264): Both in-memory state and store are cleared atomically.
• Belt-and-suspenders frontend sync (simple.tsx:191-202): Event listener + initial poll covers the timing race between backend resume and webview load. Cleanup functions are properly returned from useEffect.

---

Summary

One minor actionable item: log the error in restore_upload_config for consistency with the rest of the error handling in this PR. Everything else is solid. The three-commit progression (initial implementation → UI desync fix → serde(default)) is clean and well-documented. Ship it.

[cohogan]

Applied the restore-side logging fix in the latest commit — a corrupted stored config now logs a warning before falling back to defaults, matching the write side.

The stop-during-resume race remains a known narrow window as discussed; a generation counter is the right fix if it ever surfaces in practice.

🤖 Generated with Claude Code

[github-actions]

Claude finished @cohogan's task in 2m 6s —— View job

---

• Read and analyze all changed files against origin/main
• Review Rust backend changes (lib.rs, upload.rs)
• Review frontend changes (simple.tsx)
• Review dependency changes (Cargo.toml)
• Post detailed review findings

---

Code Review: Resume file watching after restart (round 4)

The three prior review rounds caught the main issues (UI desync on resume failure, silent unwrap_or_default, forward-compatible #[serde(default)], and restore-side logging). All fixes look correctly applied. The PR is in good shape. One remaining issue and a couple of observations:

---

Issue: start_watching_impl re-persists the store key during resume — silently undoes a "stop" that races with it

src-tauri/src/lib.rs:193-198

start_watching_impl unconditionally writes watched_folder to the store at lines 195-198. During resume, this re-persists a value that was just read from the store — harmless in isolation (noted as a nit in round 1). But it creates a subtle correctness issue when combined with the stop-during-resume race that round 2 flagged as informational:

1. Resume starts, spawn_blocking enters start_watching_impl
2. User clicks Stop → stop_watching clears watcher, sets watched_folder_state to None, deletes the store key
3. start_watching_impl finishes → re-writes the store key at line 197, re-sets watched_folder_state to Some(folder) at line 195

The user explicitly stopped, but the store now has the folder persisted again. On next launch, the app will resume watching a folder the user asked to stop watching. The in-memory state and the watcher itself are also restored, so the app is actively watching something the user told it to stop.

This isn't purely theoretical — the spawn_blocking runs capture_initial_contents which does synchronous directory traversal. On a large folder or a network drive, this can take tens of seconds, making the race window practically reachable.

Suggested fix: The lightest fix is to skip the store write when called from the resume path (since the value is already in the store). A boolean parameter persist: bool on start_watching_impl would do it:

fn start_watching_impl(
    folder_path: String,
    app_handle: &AppHandle,
    watcher_state: &WatcherState,
    upload_queue: &UploadQueue,
    upload_config: &UploadConfigState,
    watched_folder: &WatchedFolderState,
    persist: bool,  // false when resuming — value is already in the store
) -> Result<String, String> {
    // ...existing code...

    *watched_folder.lock() = Some(folder_path.clone());
    if persist {
        if let Ok(store) = app_handle.store(SETTINGS_STORE_FILENAME) {
            store.set(WATCHED_FOLDER_STORE_KEY, serde_json::json!(folder_path));
        }
    }

    Ok(format!("Started watching: {folder_path}"))
}

Then pass true from the start_watching command and false from the resume path. This eliminates the race entirely: if the user stops during resume, stop_watching deletes the key and it stays deleted. This also addresses the round-1 nit about the redundant store write.

Fix this →

---

Observation: watched_folder_state lock set eagerly in setup still races with start_watching_impl

src-tauri/src/lib.rs:586 and lib.rs:195

The resume path sets *watched_folder_state.lock() = Some(folder.clone()) at line 586 (before spawn_blocking), and then start_watching_impl sets it again at line 195 (after the scan completes). If stop_watching runs between these two points, the second write at line 195 silently overwrites the None that stop_watching set. The persist: bool fix above addresses the store-persistence half but not the in-memory half.

If you want full race safety, you could also gate the watched_folder memory write on persist — or use a generation counter as mentioned in prior rounds. Given the narrow window and server-side dedup, this remains low priority, but worth noting for completeness.

---

Observation: enabled field is persisted and restored — headless resume with enabled: false silently does nothing

src-tauri/src/upload.rs:51-52

UploadConfig.enabled is persisted alongside ignored patterns and delays. If a user toggles uploads off and then reboots, the resumed watcher starts, the initial scan queues files, but the upload processor loop skips them because enabled is false. From the user's perspective the app is "watching" but nothing uploads — no log, no UI indication. This is arguably correct (the user's last intent was "uploads off"), but it might be surprising in the headless autostart case where the whole point is unattended syncing.

Not actionable now — just a UX consideration for a follow-up if users report "sync stopped working after reboot."

---

Good decisions confirmed

• server_url override (upload.rs:900): Build-env URL always wins. Clean defense against cross-environment contamination.
• #[serde(default)] (upload.rs:50): Future field additions won't discard persisted settings.
• spawn_blocking for resume (lib.rs:593): capture_initial_contents does synchronous I/O — correct API choice.
• Belt-and-suspenders frontend sync (simple.tsx:191-202): Event listener + initial poll covers the timing race between backend resume and webview load. Cleanup functions properly returned.
• watch_resume_failed event (lib.rs:609): Frontend correctly clears state and shows an error toast.
• Explicit error logging in both set_upload_config and restore_upload_config: Consistent error handling on both sides.
• Single-instance gating (lib.rs:487): #[cfg(not(debug_assertions))] is the right granularity.
• Folder-missing-at-startup resilience (lib.rs:613-617): Keeping the stored path for unmounted drives is a sensible default.

---

Summary

One actionable item: the resume path re-persisting the store key creates a real (if narrow) race where a user's "Stop" can be undone on slow folders. A persist: bool parameter on start_watching_impl fixes both this and the round-1 redundant-write nit. Everything else is solid — the four-commit progression is clean and all prior review feedback has been addressed well. Ship it after the persist fix, or accept the race as-is given server-side dedup.

[cohogan]

Took the round-4 race seriously and went one step further than the suggested persist: bool flag in the latest commit. The flag would keep the store clean after a raced Stop, but the in-flight scan would still re-install the watcher and re-set the in-memory state, so the app would keep watching (and the UI would show it) until the next restart.

Instead, a WatchGeneration counter (AtomicU64) now fully closes the race:

• every user start/stop bumps the generation; the resume path captures its own generation at spawn
• start_watching_impl re-checks the generation under the watcher lock at install time and discards its work (WatchStartOutcome::Superseded) if anything newer happened — watcher, in-memory state, and store key are only committed atomically with that check
• the resume failure path also checks the generation before rolling back state, so it can't clobber a watch the user started during the scan

Re-verified the happy path end-to-end on Windows (seeded store → headless launch → Resumed watching ... after restart in the log).

🤖 Generated with Claude Code

[github-actions]

Claude finished @cohogan's task in 4m 57s —— View job

---

• Read and analyze all changed files against origin/main
• Review Rust backend changes (lib.rs, upload.rs)
• Review frontend changes (simple.tsx)
• Review dependency changes (Cargo.toml)
• Post detailed review findings

---

Code Review: Resume file watching after restart (round 5)

All four prior review rounds' fixes are correctly applied and verified. The WatchGeneration counter (commit 4f38ff8) is a solid solution to the stop-during-resume race — cleaner than the persist: bool flag that was suggested, and it covers both the store and in-memory state. One remaining observation:

---

Observation: Superseded scan still queues files for upload

src-tauri/src/lib.rs:138 and lib.rs:211-216

The generation check happens after capture_initial_contents completes. On a large folder, the scan can run for tens of seconds, queuing thousands of files via add_to_upload_queue_with_event_type. If the start is then superseded (user clicked Stop or changed folders during the scan), those queued files remain in the upload queue and will be uploaded.

// Scan runs fully — no generation check here
capture_initial_contents(&folder_path, app_handle, upload_queue, upload_config)?;

// ... create watcher ...

// Generation check only happens here, after the scan
{
    let mut watcher_guard = watcher_state.lock();
    if generation.load(Ordering::SeqCst) != expected_generation {
        return Ok(WatchStartOutcome::Superseded);  // scan's queue entries persist
    }
}

This is consistent with pre-existing behavior (stop_watching never clears the upload queue), and server-side CRC dedup absorbs any duplicates. But it means a user who stops watching during a slow resume scan will still see those files uploaded. Not a blocker — just worth being aware of if users report unexpected uploads after canceling.

A future improvement could periodically check the generation during the scan (e.g., every N files) to bail out early, but that adds complexity for marginal benefit.

---

Pre-existing: start_watching command blocks the async runtime

src-tauri/src/lib.rs:95-118

The user-initiated start_watching is an async fn that synchronously calls start_watching_impl, which does blocking directory traversal in capture_initial_contents. This blocks the tokio runtime thread for the duration of the scan. The resume path correctly uses spawn_blocking — the user-initiated path should ideally do the same for consistency. Not introduced by this PR, just noting since the resume path got it right.

---

Verified fixes from prior rounds

• watch_resume_failed event (lib.rs:652): Correctly emitted on failure, gated by generation check so it doesn't clobber a newer user action. Frontend handles it with setSelectedFolder("") + error toast.
• #[serde(default)] on UploadConfig (upload.rs:50): Stored configs survive future field additions.
• Restore-side logging (upload.rs:893-898): Deserialization errors are now logged before falling back to defaults, matching the write-side error handling.
• Generation counter (lib.rs:83-92): AtomicU64 with SeqCst ordering. Bumped on every start/stop. Checked under the watcher lock before committing state. Both stop_watching and start_watching bump it. The resume failure path also checks before rolling back. Correct and complete.

---

Good decisions in the final state

• Atomic state commit (lib.rs:211-222): Watcher installation, in-memory state, and store persistence all happen under the watcher lock after the generation check. No partial commits.
• server_url override (upload.rs:900): Build-env URL always wins over the stored value. Clean defense against cross-environment contamination.
• Belt-and-suspenders frontend sync (simple.tsx:191-202): Polling get_watched_folder + listening for watch_resumed covers the timing race between backend resume and webview load.
• Single-instance gating (lib.rs:516-521): #[cfg(not(debug_assertions))] allows dev to run alongside the installed app.
• Folder-missing resilience (lib.rs:657-661): Keeping the stored path for unmounted drives allows retry on next boot.
• Superseded log in resume (lib.rs:641-645): Clean handling when the user takes action during resume — logs without touching state.

---

Summary

No actionable items remain. The generation counter fully closes the stop-during-resume race, the scan-queues-orphaned-entries observation is consistent with existing behavior and absorbed by server-side dedup, and all prior fixes are correctly applied. Ship it.

[cohogan]

Acknowledging the two round-5 observations without further changes:

• Superseded scan leaves queue entries: pre-existing behavior (Stop has never cleared the upload queue) and server-side CRC dedup absorbs duplicates; periodic generation checks mid-scan aren't worth the complexity today.
• Blocking scan in the user-initiated start_watching: pre-existing and orthogonal to this PR — worth a small follow-up that wraps the command body in spawn_blocking like the resume path.

All actionable findings from rounds 1–4 are applied and verified. Ready for merge.

🤖 Generated with Claude Code