feat(browser): per-tab persistence + legacy state.json retirement

feat(browser): per-tab persistence + legacy state.json retirement

Author: BunsDev

app/src/app_state.rs

@@ -130,6 +130,9 @@ pub enum LeafContents {
     ExecutionProfileEditor,
     CodeReview(CodeReviewPaneSnapshot),
     AmbientAgent(AmbientAgentPaneSnapshot),
+    /// An embedded browser pane. Carries the stable per-pane session id
+    /// keying the WebKit data dir plus the open tabs / active tab state.
+    Browser(BrowserPaneSnapshot),
     /// The in-app network log pane. Not persisted across restarts because the
     /// backing log is an in-memory ring buffer that starts empty on launch.
     NetworkLog,
@@ -173,12 +176,26 @@ impl LeafContents {
             | LeafContents::ExecutionProfileEditor
             | LeafContents::CodeReview(_)
             | LeafContents::AmbientAgent(_)
+            | LeafContents::Browser(_)
             | LeafContents::Welcome { .. }
             | LeafContents::GetStarted => true,
         }
     }
 }
 
+/// Snapshot of an embedded browser pane.
+#[derive(Clone, Debug, PartialEq)]
+pub struct BrowserPaneSnapshot {
+    /// Stable per-pane UUID that keys the WebKit data dir at
+    /// `<warp_home>/browser/data/<session_id>/`. Generated when the pane
+    /// is first created (see `Workspace::open_browser_pane`); preserved
+    /// across snapshot/restore so cookies/localStorage survive.
+    pub session_id: String,
+    /// Open intra-pane browser tabs, active index, etc. Serialized as
+    /// JSON in the SQLite row.
+    pub state: crate::pane_group::pane::browser::browser_model::BrowserState,
+}
+
 /// Snapshot of an ambient agent pane.
 #[derive(Clone, Debug, PartialEq)]
 pub struct AmbientAgentPaneSnapshot {

app/src/browser/browser_view.rs

@@ -1,6 +1,5 @@
-// BrowserView is a non-wasm-only render surface; many of its helpers
-// (persistence import, internal model accessor) compile on wasm but
-// the WKWebView-driven code paths that consume them don't.
+// BrowserView is a non-wasm-only render surface; some helpers compile on
+// wasm even though the WKWebView-driven code paths that consume them don't.
 #![cfg_attr(target_family = "wasm", allow(dead_code, unused_imports))]
 
 use std::collections::HashMap;
@@ -47,7 +46,6 @@ use super::browser_model::{BrowserModel, TabId, DEFAULT_BROWSER_URL};
 #[cfg(not(target_family = "wasm"))]
 use super::data_dir;
 use super::find::FindState;
-use super::persistence;
 use super::url_input::{resolve_with_engine, Resolved};
 #[cfg(not(target_family = "wasm"))]
 use super::webview_host::SharedWebContext;
@@ -284,6 +282,11 @@ pub struct BrowserView {
     /// per wry's docs). `None` on wasm.
     #[cfg(not(target_family = "wasm"))]
     web_context: Option<SharedWebContext>,
+    /// Stable per-pane UUID that keys this pane's WebKit data dir. Captured
+    /// at construction (fresh `Uuid::v4` for new panes; restored from
+    /// `BrowserPaneSnapshot` for rehydrated panes) so the data dir at
+    /// `<warp_home>/browser/data/<session_id>/` survives app restarts.
+    session_id: String,
     /// Per-tab UI mouse states keyed by stable [`TabId`] so they survive tab
     /// closures (which shift indices).
     tab_ui_states: HashMap<TabId, TabUiState>,
@@ -314,6 +317,11 @@ impl BrowserView {
     pub(crate) fn model(&self) -> &BrowserModel {
         &self.model
     }
+
+    /// Stable per-pane session id. Empty on wasm (no WebKit data store).
+    pub(crate) fn session_id(&self) -> &str {
+        &self.session_id
+    }
 }
 
 impl BrowserView {
@@ -322,6 +330,8 @@ impl BrowserView {
         #[cfg(not(target_family = "wasm"))] session_id: &str,
         ctx: &mut ViewContext<Self>,
     ) -> Self {
+        #[cfg(not(target_family = "wasm"))]
+        let session_id = data_dir::normalize_session_id(session_id);
         let model = BrowserModel::new(initial_url.unwrap_or_default());
         let pane_configuration =
             ctx.add_model(|_ctx| PaneConfiguration::new(model.display_title()));
@@ -334,7 +344,7 @@ impl BrowserView {
             // WKWebsiteDataStore default store regardless (wry 0.38
             // limitation, see `data_dir`), but creating the directory keeps
             // the layout consistent for future macOS plumbing.
-            let dir = data_dir::browser_data_dir(session_id);
+            let dir = data_dir::browser_data_dir(&session_id);
             // Construct the WebContext even when dir is None — wry handles
             // the missing-dir case internally with its platform default.
             Some(Rc::new(RefCell::new(wry::WebContext::new(dir))))
@@ -414,6 +424,10 @@ impl BrowserView {
             event_tx,
             #[cfg(not(target_family = "wasm"))]
             web_context,
+            #[cfg(not(target_family = "wasm"))]
+            session_id,
+            #[cfg(target_family = "wasm")]
+            session_id: String::new(),
             tab_ui_states,
             workspace_tab_visible: true,
             back_button_mouse_state: MouseStateHandle::default(),
@@ -443,14 +457,15 @@ impl BrowserView {
         session_id: &str,
         ctx: &mut ViewContext<Self>,
     ) -> Self {
+        let session_id = data_dir::normalize_session_id(session_id);
         let model = BrowserModel::restore(state);
         let pane_configuration =
             ctx.add_model(|_ctx| PaneConfiguration::new(model.display_title()));
         let (event_tx, event_rx) = async_channel::unbounded::<NativeWebViewEvent>();
 
         let web_context: Option<SharedWebContext> = {
             // Per-session data dir; see `Self::new` for the platform notes.
-            let dir = data_dir::browser_data_dir(session_id);
+            let dir = data_dir::browser_data_dir(&session_id);
             Some(Rc::new(RefCell::new(wry::WebContext::new(dir))))
         };
 
@@ -531,6 +546,7 @@ impl BrowserView {
             webviews,
             event_tx,
             web_context,
+            session_id,
             tab_ui_states,
             workspace_tab_visible: true,
             back_button_mouse_state: MouseStateHandle::default(),
@@ -935,14 +951,6 @@ impl BrowserView {
         });
     }
 
-    #[cfg(not(target_family = "wasm"))]
-    fn persist_open_state(&self, open: bool) {
-        let state = self.model.snapshot(open);
-        if let Err(err) = persistence::save_to_default_dir(&state) {
-            log::warn!("failed to persist browser state: {err}");
-        }
-    }
-
     #[allow(clippy::too_many_arguments)]
     fn render_toolbar_button(
         &self,
@@ -1557,7 +1565,6 @@ impl BackingView for BrowserView {
             for webview in &self.webviews {
                 webview.borrow_mut().detach_native();
             }
-            self.persist_open_state(false);
         }
         ctx.emit(BrowserViewEvent::Pane(PaneEvent::Close));
     }

app/src/browser/data_dir.rs

@@ -1,17 +1,10 @@
 //! Per-session WebKit data directory resolution.
 //!
-//! Returns `<warp_home_config_dir>/browser/data/<session_id>/`. Each
-//! workspace tab owns its own browser pane and its own session id (the
-//! pane group's [`EntityId`]), so cookies, localStorage, IndexedDB, and
-//! cache are isolated per workspace tab. Sessions remain scoped to the
-//! CastCodes app (i.e., separate from any system browser).
-//!
-//! Session ids are runtime [`EntityId`]s — stable within a single app
-//! launch but not across restarts. That is acceptable for Layer B: a
-//! fresh app launch yields fresh tabs with fresh contexts. Persistence
-//! across restarts (Layer C) will need a stable per-tab UUID stored on
-//! `TabData`; when that lands, the same directory layout still applies
-//! and only the id source changes.
+//! Returns `<warp_home_config_dir>/browser/data/<session_id>/`, where
+//! `session_id` is a UUID v4 string persisted with the browser pane
+//! snapshot. Each browser pane owns its own session id, so cookies,
+//! localStorage, IndexedDB, and cache are isolated per pane while staying
+//! scoped to the CastCodes app (i.e., separate from any system browser).
 //!
 //! ## Platform reality check (wry 0.38)
 //!
@@ -29,28 +22,108 @@
 //!   isolation works as expected.
 //! - **Windows / WebView2**: Honored via the same path.
 //!
-//! [`EntityId`]: warpui_core::EntityId
-
 use std::path::PathBuf;
 
+use uuid::Uuid;
+
+fn parse_session_uuid(session_id: &str) -> Option<Uuid> {
+    let uuid = Uuid::parse_str(session_id).ok()?;
+    (uuid.get_version_num() == 4).then_some(uuid)
+}
+
+/// Returns a canonical UUID session id, replacing invalid persisted values
+/// with a fresh UUID v4 before they can become path components.
+pub fn normalize_session_id(session_id: &str) -> String {
+    match parse_session_uuid(session_id) {
+        Some(uuid) => uuid.to_string(),
+        None => {
+            log::warn!("invalid browser session id; generated a replacement");
+            Uuid::new_v4().to_string()
+        }
+    }
+}
+
 /// Resolves the per-session WebKit data directory, creating it on disk
-/// if missing. `session_id` is typically the owning pane group's
-/// [`EntityId`] rendered via `Display` — any short, filesystem-safe
-/// identifier works.
+/// if missing. `session_id` must be a UUID string; callers restoring
+/// persisted state should call [`normalize_session_id`] before storing
+/// the id on live browser views.
 ///
 /// Returns `None` if the CastCodes home dir is unavailable or the
-/// directory could not be created (filesystem error logged at WARN
-/// level).
-///
-/// [`EntityId`]: warpui_core::EntityId
+/// session id is invalid, or if the directory could not be created
+/// (filesystem error logged at WARN level).
 pub fn browser_data_dir(session_id: &str) -> Option<PathBuf> {
+    let session_uuid = match parse_session_uuid(session_id) {
+        Some(uuid) => uuid,
+        None => {
+            log::warn!("refusing browser data dir for invalid session id");
+            return None;
+        }
+    };
+
     let mut dir = warp_core::paths::warp_home_config_dir()?;
     dir.push("browser");
     dir.push("data");
-    dir.push(session_id);
+    dir.push(session_uuid.to_string());
     if let Err(err) = std::fs::create_dir_all(&dir) {
         log::warn!("failed to create browser data dir {dir:?}: {err}");
         return None;
     }
     Some(dir)
 }
+
+/// Best-effort one-shot cleanup of the pre-Layer-C
+/// `<warp_home_config_dir>/browser/state.json` file. That file was the
+/// single-pane JSON store; the SQLite app-state path now persists every
+/// browser pane, so the legacy file is orphaned. Safe to call repeatedly
+/// — missing file is silent, other errors log at WARN.
+pub fn delete_legacy_state_file() {
+    let Some(mut path) = warp_core::paths::warp_home_config_dir() else {
+        return;
+    };
+    path.push("browser");
+    path.push("state.json");
+    match std::fs::remove_file(&path) {
+        Ok(()) => log::info!("removed legacy browser state file {path:?}"),
+        Err(err) if err.kind() == std::io::ErrorKind::NotFound => {}
+        Err(err) => log::warn!("failed to remove legacy browser state file {path:?}: {err}"),
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn normalize_session_id_preserves_valid_uuid() {
+        let id = "5f2b3ce0-94f3-49ac-bab5-4f229a159f50";
+        assert_eq!(normalize_session_id(id), id);
+    }
+
+    #[test]
+    fn normalize_session_id_canonicalizes_uuid() {
+        assert_eq!(
+            normalize_session_id("5F2B3CE0-94F3-49AC-BAB5-4F229A159F50"),
+            "5f2b3ce0-94f3-49ac-bab5-4f229a159f50"
+        );
+    }
+
+    #[test]
+    fn normalize_session_id_replaces_invalid_values() {
+        let id = normalize_session_id("../state.json");
+        assert_ne!(id, "../state.json");
+        assert!(Uuid::parse_str(&id).is_ok());
+    }
+
+    #[test]
+    fn normalize_session_id_replaces_non_v4_uuid() {
+        let id = normalize_session_id("550e8400-e29b-11d4-a716-446655440000");
+        assert_ne!(id, "550e8400-e29b-11d4-a716-446655440000");
+        assert_eq!(Uuid::parse_str(&id).unwrap().get_version_num(), 4);
+    }
+
+    #[test]
+    fn browser_data_dir_rejects_invalid_session_ids() {
+        assert!(browser_data_dir("../state.json").is_none());
+        assert!(browser_data_dir("550e8400-e29b-11d4-a716-446655440000").is_none());
+    }
+}

app/src/browser/mod.rs

@@ -6,7 +6,6 @@ pub(crate) mod data_dir;
 #[cfg(not(target_family = "wasm"))]
 pub(crate) mod downloads;
 pub(crate) mod find;
-pub(crate) mod persistence;
 pub(crate) mod popup_policy;
 pub(crate) mod url_input;
 pub(crate) mod webview_host;