feat: add unified app settings persistence by vlordier · Pull Request #57 · Liquid4All/cookbook

vlordier · 2026-03-06T19:46:07Z

Summary

Add AppSettings struct that consolidates:
- active_model_key
- allowed_paths
- theme preference
- show_tool_traces
- sampling config (integrated)
Add atomic save/load with JSON
Add export/import settings as JSON
Add poll_settings_changed() for external change detection
Settings stored in com.localcowork.app/settings.json

This enables settings to persist across app restarts.

New Tauri Commands

get_app_settings
update_app_settings
add_allowed_path
remove_allowed_path
export_settings
import_settings
poll_settings_changed

Testing

All 365 tests pass
Clippy clean

- Add AppSettings struct that consolidates: - active_model_key - allowed_paths - theme preference - show_tool_traces - sampling config (integrated) - Add atomic save/load with JSON - Add export/import settings as JSON - Add poll_settings_changed() for external change detection - Settings stored in com.localcowork.app/settings.json This enables settings to persist across app restarts.

Copilot

Pull request overview

Adds a unified, persisted AppSettings model to the LocalCowork Tauri backend, exposing new IPC commands to load/update/export/import settings and to poll for settings changes across restarts.

Changes:

Introduces AppSettings (active model key, allowed paths, theme, tool traces, sampling config) with JSON load/save and import/export helpers.
Adds new Tauri commands for reading/updating settings and managing allowed paths, plus a poll_settings_changed() flag.
Adds a cache_dir() helper and wires the new settings commands into invoke_handler.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.

File	Description
examples/localcowork/src-tauri/src/lib.rs	Adds `cache_dir()` and registers new settings-related Tauri commands.
examples/localcowork/src-tauri/src/commands/settings.rs	Implements `AppSettings` persistence + new Tauri commands for unified settings and change polling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-06T19:50:27Z

+/// Check if settings have changed since last check (for file watching).
+#[tauri::command]
+pub fn poll_settings_changed() -> bool {
+    has_settings_changed()
+}


poll_settings_changed() only reflects calls to settings_changed() within this process (currently only AppSettings::save()), so it won’t detect external edits to settings.json and also won’t flip when SamplingConfig is changed via update_sampling_config. If the intent is “external change detection” as described, consider implementing an mtime/hash check against the settings file (or clarify/rename this to reflect it’s an in-process dirty flag).

Copilot · 2026-03-06T19:50:28Z

+            show_tool_traces: true,
+            sampling: SamplingConfig::default(),
+        }
+        // Default allowed paths


The comment // Default allowed paths in AppSettings::default() is currently misleading because no defaults are actually added (the function returns immediately). Either remove the comment or implement the intended default path seeding so the code and comment stay consistent.

Suggested change

// Default allowed paths

Copilot · 2026-03-06T19:50:28Z

+/// Update app settings and persist to disk.
+#[tauri::command]
+pub fn update_app_settings(settings: AppSettings) -> AppSettings {
+    settings.save();
+    tracing::info!(
+        active_model = ?settings.active_model_key,
+        theme = %settings.theme,
+        allowed_paths = settings.allowed_paths.len(),
+        "app settings updated"
+    );
+    settings


update_app_settings persists only settings.json via settings.save(). If the frontend updates settings.sampling, the new values won’t be applied to the SamplingConfig held in Tauri state (used by send_message), and won’t be written to sampling_config.json unless another command is called. Consider making this command update the TokioMutex<SamplingConfig> state and call settings.sampling.save() as part of the same update (or remove sampling from AppSettings if it’s intentionally separate).

Copilot · 2026-03-06T19:50:28Z

+    pub fn save(&self) {
+        let path = Self::persist_path();
+        let content = match serde_json::to_string_pretty(self) {
+            Ok(c) => c,
+            Err(e) => {
+                tracing::error!(error = %e, "failed to serialize settings");
+                return;
+            }
+        };
+        if let Some(parent) = path.parent() {
+            let _ = std::fs::create_dir_all(parent);
+        }
+        let tmp_path = path.with_extension("json.tmp");
+        if let Err(e) = std::fs::write(&tmp_path, &content) {
+            tracing::error!(error = %e, "failed to write settings temp file");
+            return;
+        }
+        if let Err(e) = std::fs::rename(&tmp_path, &path) {
+            tracing::error!(error = %e, "failed to rename settings file");
+            return;
+        }
+        settings_changed();
+        tracing::debug!("saved app settings");
+    }


save() returns () and the Tauri commands return AppSettings even if persisting fails (errors are only logged). This makes it hard for the frontend to surface failures (e.g., permissions/IO errors) and can lead to UI showing settings as “saved” when they weren’t. Consider changing save()/update commands to return Result<_, String> and propagating a useful error message to the caller.

Copilot · 2026-03-06T19:50:28Z

+    let path_clone = path.clone();
+    settings.allowed_paths.retain(|p| p != &path);
+    settings.save();
+    tracing::info!(path = %path_clone, "allowed path removed");


remove_allowed_path creates path_clone solely for logging, but path is still available after retain() (it’s only borrowed there). Removing the extra clone avoids an unnecessary allocation on every call.

Suggested change

let path_clone = path.clone();

settings.allowed_paths.retain(|p| p != &path);

settings.save();

tracing::info!(path = %path_clone, "allowed path removed");

settings.allowed_paths.retain(|p| p != &path);

settings.save();

tracing::info!(path = %path, "allowed path removed");

Copilot · 2026-03-06T19:50:29Z

+/// Add an allowed path to settings.
+#[tauri::command]
+pub fn add_allowed_path(path: String) -> AppSettings {
+    let mut settings = AppSettings::load_or_default();
+    if !settings.allowed_paths.contains(&path) {
+        settings.allowed_paths.push(path.clone());
+        settings.save();
+        tracing::info!(path = %path, "allowed path added");


All AppSettings mutations currently follow a load-modify-save pattern without any shared in-memory lock/state. Since Tauri commands can run concurrently, simultaneous updates (e.g., add_allowed_path racing update_app_settings) can easily overwrite each other’s changes on disk (“last writer wins”). Consider managing a single TokioMutex<AppSettings> in Tauri state (loaded once at startup) so updates are serialized and persisted from the locked value.

Copilot · 2026-03-06T19:50:29Z

        .join(".localcowork")
 }

+/// Returns the cache directory for the app (embedding indexes, etc.).


cache_dir() is currently unused anywhere in the crate (no other references found). In this binary/lib setup it will trigger dead_code warnings unless it’s used soon; consider either wiring it up where embedding indexes are stored, or removing it (or adding a scoped #[allow(dead_code)] with a short rationale).

Suggested change

/// Returns the cache directory for the app (embedding indexes, etc.).

/// Returns the cache directory for the app (embedding indexes, etc.).

#[allow(dead_code)] // Currently unused; kept for future embedding/cache storage integration.

Copilot · 2026-03-06T19:50:29Z

+            return Self::default();
+        }
+        match std::fs::read_to_string(&path) {
+            Ok(content) => match serde_json::from_str::<Self>(&content) {
+                Ok(settings) => {
+                    tracing::info!(path = %path.display(), "loaded app settings");
+                    settings
+                }
+                Err(e) => {
+                    tracing::warn!(error = %e, "failed to parse settings, using defaults");
+                    Self::default()
+                }
+            },
+            Err(e) => {
+                tracing::warn!(error = %e, "failed to read settings, using defaults");
+                Self::default()


AppSettings includes a sampling: SamplingConfig field, but load_or_default() loads it only from settings.json (or defaults) and does not incorporate the already-persisted sampling_config.json / in-memory SamplingConfig state used by send_message. This will make get_app_settings and export_settings return stale/default sampling values after sampling is changed via existing commands. Consider making AppSettings::load_or_default() hydrate sampling from SamplingConfig::load_or_default() (or from Tauri state) to avoid split sources of truth.

Suggested change

return Self::default();

}

match std::fs::read_to_string(&path) {

Ok(content) => match serde_json::from_str::<Self>(&content) {

Ok(settings) => {

tracing::info!(path = %path.display(), "loaded app settings");

settings

}

Err(e) => {

tracing::warn!(error = %e, "failed to parse settings, using defaults");

Self::default()

}

},

Err(e) => {

tracing::warn!(error = %e, "failed to read settings, using defaults");

Self::default()

// No settings.json yet; still hydrate sampling from its own persisted state.

let mut settings = Self::default();

settings.sampling = SamplingConfig::load_or_default();

return settings;

}

match std::fs::read_to_string(&path) {

Ok(content) => match serde_json::from_str::<Self>(&content) {

Ok(mut settings) => {

// Always hydrate sampling from the canonical SamplingConfig storage

// to avoid stale or divergent sampling settings.

settings.sampling = SamplingConfig::load_or_default();

tracing::info!(path = %path.display(), "loaded app settings");

settings

}

Err(e) => {

tracing::warn!(error = %e, "failed to parse settings, using defaults");

let mut settings = Self::default();

settings.sampling = SamplingConfig::load_or_default();

settings

}

},

Err(e) => {

tracing::warn!(error = %e, "failed to read settings, using defaults");

let mut settings = Self::default();

settings.sampling = SamplingConfig::load_or_default();

settings

Copilot · 2026-03-06T19:50:29Z

+    pub fn import_from_json(json: &str) -> Result<Self, String> {
+        let settings: Self =
+            serde_json::from_str(json).map_err(|e| format!("invalid settings JSON: {}", e))?;
+        settings.sampling.save();


import_from_json writes the imported sampling config to disk, but it doesn’t update the in-memory SamplingConfig stored in Tauri state. As a result, send_message will continue using the old sampling values until restart or until update_sampling_config is called. Consider making import_settings accept the sampling state and update it (and/or consolidate sampling persistence so there’s a single authoritative location).

Suggested change

settings.sampling.save();

- Validate path exists (or parent exists) when adding allowed path - Remove non-existent paths on settings load - Return Result from add_allowed_path for error handling This improves on PR Liquid4All#57 by preventing invalid paths in settings.

Copilot AI review requested due to automatic review settings March 6, 2026 19:46

Copilot started reviewing on behalf of vlordier March 6, 2026 19:46 View session

Copilot AI reviewed Mar 6, 2026

View reviewed changes

vlordier mentioned this pull request Mar 6, 2026

fix: add path validation to allowed_paths #70

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add unified app settings persistence#57

feat: add unified app settings persistence#57
vlordier wants to merge 1 commit intoLiquid4All:mainfrom
vlordier:feat/settings-persistence

vlordier commented Mar 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	/// Returns the cache directory for the app (embedding indexes, etc.).
	/// Returns the cache directory for the app (embedding indexes, etc.).
	#[allow(dead_code)] // Currently unused; kept for future embedding/cache storage integration.

Conversation

vlordier commented Mar 6, 2026

Summary

New Tauri Commands

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants