From 62b751b2b551d866e2aebfdecf89f7a003d64ebc Mon Sep 17 00:00:00 2001 From: Ogulcan Celik Date: Mon, 6 Apr 2026 21:11:42 +0300 Subject: [PATCH] refactor: centralize structural session snapshot compatibility --- src/persist.rs | 219 ++++++++++-------- .../session/current-herdr-dev-session.json | 78 +++++++ .../session/current-herdr-session.json | 65 ++++++ .../fixtures/session/legacy-pre-tabs-v2.json | 33 +++ 4 files changed, 298 insertions(+), 97 deletions(-) create mode 100644 tests/fixtures/session/current-herdr-dev-session.json create mode 100644 tests/fixtures/session/current-herdr-session.json create mode 100644 tests/fixtures/session/legacy-pre-tabs-v2.json diff --git a/src/persist.rs b/src/persist.rs index 292af92d..9c87cf4d 100644 --- a/src/persist.rs +++ b/src/persist.rs @@ -8,7 +8,7 @@ use std::sync::atomic::AtomicBool; use std::sync::Arc; use ratatui::layout::Direction; -use serde::{Deserialize, Deserializer, Serialize}; +use serde::{Deserialize, Serialize}; use tracing::{error, info, warn}; use tokio::sync::{mpsc, Notify}; @@ -36,7 +36,7 @@ pub struct SessionSnapshot { pub sidebar_width: Option, } -#[derive(Serialize)] +#[derive(Serialize, Deserialize)] pub struct WorkspaceSnapshot { #[serde(default)] pub id: Option, @@ -48,30 +48,6 @@ pub struct WorkspaceSnapshot { pub active_tab: usize, } -#[derive(Deserialize)] -struct WorkspaceSnapshotWire { - #[serde(default)] - id: Option, - #[serde(default)] - custom_name: Option, - #[serde(default)] - identity_cwd: Option, - #[serde(default)] - tabs: Vec, - #[serde(default)] - active_tab: usize, - #[serde(default)] - layout: Option, - #[serde(default)] - panes: HashMap, - #[serde(default)] - zoomed: Option, - #[serde(default)] - focused: Option, - #[serde(default)] - root_pane: Option, -} - #[derive(Deserialize)] struct LegacyWorkspaceSnapshot { #[serde(default)] @@ -143,41 +119,49 @@ impl From for WorkspaceSnapshot { } } -impl<'de> Deserialize<'de> for WorkspaceSnapshot { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - let snap = WorkspaceSnapshotWire::deserialize(deserializer)?; +#[derive(Deserialize)] +struct RawSessionSnapshot { + #[serde(default)] + version: u32, + #[serde(default)] + workspaces: Vec, + #[serde(default)] + active: Option, + #[serde(default)] + selected: usize, + #[serde(default)] + agent_panel_scope: crate::app::state::AgentPanelScope, + #[serde(default)] + sidebar_width: Option, +} - if let Some(identity_cwd) = snap.identity_cwd { - return Ok(Self { - id: snap.id, - custom_name: snap.custom_name, - identity_cwd, - tabs: snap.tabs, - active_tab: snap.active_tab, - }); - } +fn migrate_snapshot(raw: RawSessionSnapshot) -> Result { + Ok(SessionSnapshot { + version: raw.version, + workspaces: raw + .workspaces + .into_iter() + .map(migrate_workspace) + .collect::, _>>()?, + active: raw.active, + selected: raw.selected, + agent_panel_scope: raw.agent_panel_scope, + sidebar_width: raw.sidebar_width, + }) +} - if let Some(layout) = snap.layout { - // Backward-compat path for pre-tab session snapshots (<= v0.2.4). - // Remove this migration once we intentionally drop support for v2 sessions. - return Ok(LegacyWorkspaceSnapshot { - custom_name: snap.custom_name, - layout, - panes: snap.panes, - zoomed: snap.zoomed.unwrap_or(false), - focused: snap.focused, - root_pane: snap.root_pane, - } - .into()); - } - - Err(::custom( - "workspace snapshot is neither current nor legacy format", - )) +fn migrate_workspace(raw: serde_json::Value) -> Result { + if raw.get("identity_cwd").is_some() { + return serde_json::from_value(raw).map_err(|e| e.to_string()); } + + if raw.get("layout").is_some() { + let legacy = + serde_json::from_value::(raw).map_err(|e| e.to_string())?; + return Ok(legacy.into()); + } + + Err("workspace snapshot is neither current nor legacy format".to_string()) } fn legacy_identity_cwd(snap: &LegacyWorkspaceSnapshot) -> PathBuf { @@ -520,6 +504,17 @@ pub fn save(snapshot: &SessionSnapshot) { info!(workspaces = snapshot.workspaces.len(), "session saved"); } +fn parse_snapshot(content: &str) -> Result { + let raw = serde_json::from_str::(content).map_err(|e| e.to_string())?; + if raw.version > SNAPSHOT_VERSION { + return Err(format!( + "snapshot version {} is newer than supported {}", + raw.version, SNAPSHOT_VERSION + )); + } + migrate_snapshot(raw) +} + pub fn load() -> Option { let path = session_path(); if !path.exists() { @@ -532,19 +527,19 @@ pub fn load() -> Option { return None; } }; - match serde_json::from_str::(&content) { - Ok(snap) => { - if snap.version > SNAPSHOT_VERSION { - warn!( - file_version = snap.version, - supported = SNAPSHOT_VERSION, - "session file is from a newer herdr version, ignoring" - ); - return None; - } - Some(snap) - } + match parse_snapshot(&content) { + Ok(snap) => Some(snap), Err(e) => { + if let Ok(raw) = serde_json::from_str::(&content) { + if raw.version > SNAPSHOT_VERSION { + warn!( + file_version = raw.version, + supported = SNAPSHOT_VERSION, + "session file is from a newer herdr version, ignoring" + ); + return None; + } + } warn!(err = %e, "failed to parse session file, ignoring"); None } @@ -557,6 +552,19 @@ pub fn load() -> Option { mod tests { use super::*; + fn session_fixture(name: &str) -> &'static str { + match name { + "current-herdr" => include_str!("../tests/fixtures/session/current-herdr-session.json"), + "current-herdr-dev" => { + include_str!("../tests/fixtures/session/current-herdr-dev-session.json") + } + "legacy-pre-tabs-v2" => { + include_str!("../tests/fixtures/session/legacy-pre-tabs-v2.json") + } + other => panic!("unknown session fixture: {other}"), + } + } + #[test] fn round_trip_empty_session() { let snap = SessionSnapshot { @@ -568,7 +576,7 @@ mod tests { sidebar_width: Some(26), }; let json = serde_json::to_string(&snap).unwrap(); - let restored: SessionSnapshot = serde_json::from_str(&json).unwrap(); + let restored = parse_snapshot(&json).unwrap(); assert!(restored.workspaces.is_empty()); assert_eq!(restored.active, None); assert_eq!(restored.sidebar_width, Some(26)); @@ -641,7 +649,7 @@ mod tests { }; let json = serde_json::to_string_pretty(&snap).unwrap(); - let restored: SessionSnapshot = serde_json::from_str(&json).unwrap(); + let restored = parse_snapshot(&json).unwrap(); assert_eq!(restored.workspaces.len(), 1); assert_eq!(restored.workspaces[0].id.as_deref(), Some("wproj")); @@ -662,6 +670,40 @@ mod tests { assert_eq!(restored.sidebar_width, Some(26)); } + #[test] + fn current_session_fixture_parses() { + let snap = parse_snapshot(session_fixture("current-herdr")).unwrap(); + + assert_eq!(snap.version, 3); + assert_eq!(snap.workspaces.len(), 2); + assert_eq!(snap.active, Some(0)); + assert_eq!(snap.selected, 0); + assert_eq!( + snap.agent_panel_scope, + crate::app::state::AgentPanelScope::CurrentWorkspace + ); + assert_eq!(snap.sidebar_width, None); + assert_eq!(snap.workspaces[0].tabs.len(), 2); + assert_eq!( + snap.workspaces[1].identity_cwd, + PathBuf::from("/home/test/projects/project-b") + ); + } + + #[test] + fn current_dev_session_fixture_parses_additive_fields() { + let snap = parse_snapshot(session_fixture("current-herdr-dev")).unwrap(); + + assert_eq!(snap.version, 3); + assert_eq!(snap.workspaces.len(), 2); + assert_eq!( + snap.agent_panel_scope, + crate::app::state::AgentPanelScope::CurrentWorkspace + ); + assert_eq!(snap.workspaces[0].active_tab, 1); + assert_eq!(snap.workspaces[1].tabs[0].panes.len(), 2); + } + #[test] fn old_snapshot_defaults_agent_panel_scope() { let json = serde_json::json!({ @@ -672,7 +714,7 @@ mod tests { }) .to_string(); - let restored: SessionSnapshot = serde_json::from_str(&json).unwrap(); + let restored = parse_snapshot(&json).unwrap(); assert_eq!( restored.agent_panel_scope, @@ -683,34 +725,19 @@ mod tests { #[test] fn legacy_workspace_snapshot_migrates_to_single_tab() { - let json = serde_json::json!({ - "version": 2, - "workspaces": [{ - "custom_name": "legacy", - "layout": LayoutSnapshot::Pane(0), - "panes": { - "0": { "cwd": "/tmp/pion" } - }, - "zoomed": false, - "focused": 0, - "root_pane": 0 - }], - "active": 0, - "selected": 0 - }) - .to_string(); - - let snap: SessionSnapshot = serde_json::from_str(&json).unwrap(); + let snap = parse_snapshot(session_fixture("legacy-pre-tabs-v2")).unwrap(); let ws = &snap.workspaces[0]; + assert_eq!(snap.version, 2); assert_eq!(snap.workspaces.len(), 1); assert_eq!(ws.custom_name.as_deref(), Some("legacy")); assert_eq!(ws.identity_cwd, PathBuf::from("/tmp/pion")); assert_eq!(ws.active_tab, 0); assert_eq!(ws.tabs.len(), 1); - assert_eq!(ws.tabs[0].focused, Some(0)); + assert_eq!(ws.tabs[0].focused, Some(1)); assert_eq!(ws.tabs[0].root_pane, Some(0)); assert_eq!(ws.tabs[0].panes[&0].cwd, PathBuf::from("/tmp/pion")); + assert_eq!(ws.tabs[0].panes[&1].cwd, PathBuf::from("/tmp/herdr")); } #[test] @@ -742,16 +769,14 @@ mod tests { fn old_unversioned_snapshot_loads_as_version_0() { // Simulate a snapshot from before versioning was added let json = r#"{"workspaces":[],"active":null,"selected":0}"#; - let snap: SessionSnapshot = serde_json::from_str(json).unwrap(); - assert_eq!(snap.version, 0); // #[serde(default)] gives 0 + let snap = parse_snapshot(json).unwrap(); + assert_eq!(snap.version, 0); } #[test] fn future_version_is_rejected() { let json = r#"{"version":999,"workspaces":[],"active":null,"selected":0}"#; - let snap: SessionSnapshot = serde_json::from_str(json).unwrap(); - assert!(snap.version > SNAPSHOT_VERSION); - // load() would reject this — tested via the version check in load() + assert!(parse_snapshot(json).is_err()); } #[test] diff --git a/tests/fixtures/session/current-herdr-dev-session.json b/tests/fixtures/session/current-herdr-dev-session.json new file mode 100644 index 00000000..0721acb9 --- /dev/null +++ b/tests/fixtures/session/current-herdr-dev-session.json @@ -0,0 +1,78 @@ +{ + "version": 3, + "workspaces": [ + { + "id": "wdev1", + "custom_name": null, + "identity_cwd": "/home/test/projects/project-a", + "tabs": [ + { + "custom_name": null, + "layout": { + "Pane": 1 + }, + "panes": { + "1": { + "cwd": "/home/test/projects/project-a" + } + }, + "zoomed": false, + "focused": 1, + "root_pane": 1 + }, + { + "custom_name": "2", + "layout": { + "Pane": 2 + }, + "panes": { + "2": { + "cwd": "/home/test/projects/project-a" + } + }, + "zoomed": false, + "focused": 2, + "root_pane": 2 + } + ], + "active_tab": 1 + }, + { + "id": "wdev2", + "custom_name": null, + "identity_cwd": "/home/test/projects/project-c", + "tabs": [ + { + "custom_name": null, + "layout": { + "Split": { + "direction": "Vertical", + "ratio": 0.5, + "first": { + "Pane": 6 + }, + "second": { + "Pane": 7 + } + } + }, + "panes": { + "7": { + "cwd": "/home/test/projects/project-a" + }, + "6": { + "cwd": "/home/test/projects/project-c" + } + }, + "zoomed": false, + "focused": 6, + "root_pane": 6 + } + ], + "active_tab": 0 + } + ], + "active": 0, + "selected": 0, + "agent_panel_scope": "CurrentWorkspace" +} diff --git a/tests/fixtures/session/current-herdr-session.json b/tests/fixtures/session/current-herdr-session.json new file mode 100644 index 00000000..14178690 --- /dev/null +++ b/tests/fixtures/session/current-herdr-session.json @@ -0,0 +1,65 @@ +{ + "version": 3, + "workspaces": [ + { + "id": "wproj1", + "custom_name": null, + "identity_cwd": "/home/test/projects/project-a", + "tabs": [ + { + "custom_name": "separate-pane", + "layout": { + "Pane": 1 + }, + "panes": { + "1": { + "cwd": "/home/test/projects/project-a" + } + }, + "zoomed": false, + "focused": 1, + "root_pane": 1 + }, + { + "custom_name": "p", + "layout": { + "Pane": 2 + }, + "panes": { + "2": { + "cwd": "/home/test/projects/project-a" + } + }, + "zoomed": false, + "focused": 2, + "root_pane": 2 + } + ], + "active_tab": 0 + }, + { + "id": "wproj2", + "custom_name": null, + "identity_cwd": "/home/test/projects/project-b", + "tabs": [ + { + "custom_name": null, + "layout": { + "Pane": 3 + }, + "panes": { + "3": { + "cwd": "/home/test/projects/project-b" + } + }, + "zoomed": false, + "focused": 3, + "root_pane": 3 + } + ], + "active_tab": 0 + } + ], + "active": 0, + "selected": 0 +} diff --git a/tests/fixtures/session/legacy-pre-tabs-v2.json b/tests/fixtures/session/legacy-pre-tabs-v2.json new file mode 100644 index 00000000..5eb4e3ad --- /dev/null +++ b/tests/fixtures/session/legacy-pre-tabs-v2.json @@ -0,0 +1,33 @@ +{ + "version": 2, + "workspaces": [ + { + "custom_name": "legacy", + "layout": { + "Split": { + "direction": "Horizontal", + "ratio": 0.5, + "first": { + "Pane": 0 + }, + "second": { + "Pane": 1 + } + } + }, + "panes": { + "0": { + "cwd": "/tmp/pion" + }, + "1": { + "cwd": "/tmp/herdr" + } + }, + "zoomed": false, + "focused": 1, + "root_pane": 0 + } + ], + "active": 0, + "selected": 0 +}