From b7dae874120735ee68e208a8d393dabb111c8d88 Mon Sep 17 00:00:00 2001 From: Ariestar <1941137088@qq.com> Date: Sun, 9 Aug 2026 12:52:58 +0800 Subject: [PATCH 01/10] feat(origin): unified source origins with a single registry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every memory source — local workspaces, remote device mounts, cloud accounts (reserved) — is now one Origin with the same four fields (name, kind, current, detail), so upper layers render and resolve sources without ever branching on kind. Kind-specific details never enter Origin: display strings are composed by each source at construction, and whether a remote or cloud source is ingested locally is a resolution-layer concern. - core origin.rs: Origin + OriginKind (non_exhaustive, adding a category breaks nothing) + OriginRegistry (enumerate all, resolve by name, case-insensitive) - src/origins.rs: the single composition point — local workspaces (from workspace metadata) + the current workspace's remote mounts (via daemon) + cloud (reserved) - ws list now renders every origin through the registry (no local-only branch) - workspace_display_name moved from the CLI into core (one definition, used by origin construction and local-workspace-by-name resolution) --- crates/sivtr-core/src/lib.rs | 1 + crates/sivtr-core/src/origin.rs | 147 +++++++++++++++++++++++++++++ crates/sivtr-core/src/workspace.rs | 51 +++++++++- src/commands/remote/share.rs | 2 +- src/commands/remote/workspace.rs | 88 +++-------------- src/lib.rs | 1 + src/mcp/server.rs | 2 +- src/origins.rs | 57 +++++++++++ 8 files changed, 271 insertions(+), 78 deletions(-) create mode 100644 crates/sivtr-core/src/origin.rs create mode 100644 src/origins.rs diff --git a/crates/sivtr-core/src/lib.rs b/crates/sivtr-core/src/lib.rs index 82462759..17f03939 100644 --- a/crates/sivtr-core/src/lib.rs +++ b/crates/sivtr-core/src/lib.rs @@ -6,6 +6,7 @@ pub mod capture; pub mod config; pub mod export; pub mod history; +pub mod origin; pub mod parse; pub mod query; pub mod record; diff --git a/crates/sivtr-core/src/origin.rs b/crates/sivtr-core/src/origin.rs new file mode 100644 index 00000000..fe641558 --- /dev/null +++ b/crates/sivtr-core/src/origin.rs @@ -0,0 +1,147 @@ +//! Unified source origins. +//! +//! Every memory source — a local workspace, a remote device mount, a cloud +//! account — is described by one [`Origin`] with the same four fields, so +//! upper layers (listing, rendering, future scope resolution) never branch on +//! which kind of source they are looking at. Kind-specific details (root +//! paths, peer/share ids, cloud account) never enter [`Origin`]: the display +//! [`Origin::detail`] is composed by the source at construction time, and +//! whether a remote or cloud source happens to be ingested into local files +//! is a resolution-layer concern, not an origin concern. +//! +//! [`OriginRegistry`] is the single lookup surface: enumerate every +//! addressable origin, or resolve one by its logical name. + +/// Major source category. +/// +/// `#[non_exhaustive]`: adding a category (WSL, container, archive, …) only +/// requires a new variant plus a `label()` arm here; code outside this crate +/// is forced to handle the wildcard, so nothing downstream breaks. +#[non_exhaustive] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum OriginKind { + /// Local files on this machine (workspaces). + Local, + /// Another device, forwarded through the daemon. + Remote, + /// A cloud account (synced and/or fetched). + Cloud, +} + +impl OriginKind { + /// Stable lowercase label for display and serialization. + pub fn label(self) -> &'static str { + match self { + Self::Local => "local", + Self::Remote => "remote", + Self::Cloud => "cloud", + } + } +} + +/// A single addressable memory source. +/// +/// All fields exist for every kind; `detail` is the display projection the +/// source composed when it was constructed. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Origin { + /// Logical name (scope name / alias), used by [`OriginRegistry::resolve`]. + pub name: String, + /// Major source category. + pub kind: OriginKind, + /// Whether this origin is the current context (e.g. the current workspace). + pub current: bool, + /// Display projection composed by the source (e.g. `root (key)`). + pub detail: String, +} + +/// Every origin addressable from the current context. +/// +/// A pure resolution view: sources construct their own [`Origin`]s and hand +/// them in; this type owns no I/O and never interprets kind-specific fields. +#[derive(Debug, Clone, Default)] +pub struct OriginRegistry { + origins: Vec, +} + +impl OriginRegistry { + pub fn new(origins: Vec) -> Self { + Self { origins } + } + + /// All origins in construction order. + pub fn all(&self) -> &[Origin] { + &self.origins + } + + /// Resolve an origin by logical name, case-insensitively. + pub fn resolve(&self, name: &str) -> Option<&Origin> { + self.origins + .iter() + .find(|origin| origin.name.eq_ignore_ascii_case(name)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn sample() -> OriginRegistry { + OriginRegistry::new(vec![ + Origin { + name: "sivtr".to_string(), + kind: OriginKind::Local, + current: true, + detail: "D:\\Coding\\sivtr (key1)".to_string(), + }, + Origin { + name: "desk".to_string(), + kind: OriginKind::Remote, + current: false, + detail: "alice/sivtr".to_string(), + }, + ]) + } + + #[test] + fn labels_are_stable() { + assert_eq!(OriginKind::Local.label(), "local"); + assert_eq!(OriginKind::Remote.label(), "remote"); + assert_eq!(OriginKind::Cloud.label(), "cloud"); + } + + #[test] + fn resolve_matches_name_case_insensitively() { + let registry = sample(); + assert_eq!( + registry.resolve("desk").map(|o| o.name.as_str()), + Some("desk") + ); + assert_eq!( + registry.resolve("DESK").map(|o| o.name.as_str()), + Some("desk") + ); + assert_eq!( + registry.resolve("Sivtr").map(|o| o.name.as_str()), + Some("sivtr") + ); + assert_eq!(registry.resolve("missing"), None); + } + + #[test] + fn all_preserves_construction_order() { + let registry = sample(); + let names: Vec<_> = registry.all().iter().map(|o| o.name.as_str()).collect(); + assert_eq!(names, vec!["sivtr", "desk"]); + } + + #[test] + fn origin_fields_are_uniform_across_kinds() { + // Upper layers read the same four fields no matter the kind. + for origin in sample().all() { + assert!(!origin.name.is_empty()); + assert!(!origin.detail.is_empty()); + assert!(matches!(origin.kind.label(), "local" | "remote" | "cloud")); + } + } +} diff --git a/crates/sivtr-core/src/workspace.rs b/crates/sivtr-core/src/workspace.rs index 5156ce5d..39a2c089 100644 --- a/crates/sivtr-core/src/workspace.rs +++ b/crates/sivtr-core/src/workspace.rs @@ -14,6 +14,25 @@ pub struct WorkspaceMetadata { pub last_seen_at: String, } +/// Human origin label for a workspace: directory basename, lowercased. +/// Accepts both `/` and `\` so Windows roots still yield a useful label on Unix. +pub fn workspace_display_name(meta: &WorkspaceMetadata) -> String { + path_basename(&meta.root) + .unwrap_or(meta.key.as_str()) + .to_ascii_lowercase() +} + +fn path_basename(path: &str) -> Option<&str> { + let trimmed = path.trim_end_matches(['/', '\\']); + if trimmed.is_empty() { + return None; + } + trimmed + .rsplit(['/', '\\']) + .next() + .filter(|segment| !segment.is_empty()) +} + #[derive(Debug, Clone)] pub struct WorkspacePaths { pub key: String, @@ -399,8 +418,8 @@ fn modified_time(path: &Path) -> std::time::SystemTime { #[cfg(test)] mod tests { use super::{ - git_root, inspect_workspace_keys, migrate_workspace_keys, terminal_session_id_from_path, - workspace_key, WorkspaceMetadata, + git_root, inspect_workspace_keys, migrate_workspace_keys, path_basename, + terminal_session_id_from_path, workspace_display_name, workspace_key, WorkspaceMetadata, }; use std::path::{Path, PathBuf}; use std::time::{SystemTime, UNIX_EPOCH}; @@ -415,6 +434,34 @@ mod tests { dir } + fn meta(key: &str, root: &str) -> WorkspaceMetadata { + WorkspaceMetadata { + key: key.to_string(), + root: root.to_string(), + created_at: "t".to_string(), + last_seen_at: "t".to_string(), + } + } + + #[test] + fn display_name_uses_basename() { + assert_eq!( + workspace_display_name(&meta("abc", "/home/user/Coding/sivtr")), + "sivtr" + ); + assert_eq!( + workspace_display_name(&meta("abc", r"D:\Coding\sivtr")), + "sivtr" + ); + } + + #[test] + fn basename_trims_trailing_separators() { + assert_eq!(path_basename(r"D:\Coding\sivtr\"), Some("sivtr")); + assert_eq!(path_basename("/home/user/sivtr/"), Some("sivtr")); + assert_eq!(path_basename("/"), None); + } + #[test] fn workspace_key_normalizes_case_and_separators() { assert_eq!(workspace_key("D:\\sivtr"), workspace_key("d:/sivtr")); diff --git a/src/commands/remote/share.rs b/src/commands/remote/share.rs index 48f367ee..f869bcc4 100644 --- a/src/commands/remote/share.rs +++ b/src/commands/remote/share.rs @@ -6,10 +6,10 @@ use sivtr_core::workspace::{self, WorkspaceMetadata}; use crate::cli::{ShareAction, ShareCommand}; use crate::commands::interactive; -use crate::commands::remote::workspace::workspace_display_name; use crate::output; use crate::remote::ipc; use crate::remote::protocol::{LocalRequest, LocalResponse, ShareInfo}; +use sivtr_core::workspace::workspace_display_name; use super::serve; diff --git a/src/commands/remote/workspace.rs b/src/commands/remote/workspace.rs index dc88b1da..48288ccf 100644 --- a/src/commands/remote/workspace.rs +++ b/src/commands/remote/workspace.rs @@ -1,7 +1,7 @@ use std::path::PathBuf; -use anyhow::{bail, Result}; -use sivtr_core::workspace::{self, WorkspaceMetadata}; +use anyhow::{bail, Context, Result}; +use sivtr_core::workspace; use crate::cli::{WorkspaceAction, WorkspaceCommand}; use crate::output; @@ -12,64 +12,35 @@ pub fn execute(command: WorkspaceCommand) -> Result<()> { } } +/// List every addressable origin (local workspaces + remote mounts + cloud) +/// through the unified [`OriginRegistry`] — rendering never branches on kind. fn list() -> Result<()> { - let current = workspace::resolve_current_workspace()?.map(|paths| paths.key); - let mut workspaces = workspace::list_workspaces()?; - if workspaces.is_empty() { - output::plain("no workspaces recorded yet"); + let cwd = std::env::current_dir().context("Failed to resolve current directory")?; + let registry = crate::origins::collect(&cwd)?; + if registry.all().is_empty() { + output::plain("no origins recorded yet"); output::hint("run a command inside a git repo after `sivtr init`"); return Ok(()); } - // Prefer current workspace first, then keep most-recently-seen order. - if let Some(current_key) = current.as_deref() { - workspaces.sort_by(|a, b| { - let a_cur = a.key == current_key; - let b_cur = b.key == current_key; - b_cur - .cmp(&a_cur) - .then_with(|| b.last_seen_at.cmp(&a.last_seen_at)) - }); - } - - for meta in workspaces { - let name = workspace_display_name(&meta); - let marker = if current.as_deref() == Some(meta.key.as_str()) { - "current" + for origin in registry.all() { + let label = if origin.current { + format!("{}:current", origin.kind.label()) } else { - "local" + origin.kind.label().to_string() }; - output::detail(name, format!("[{marker}] {} ({})", meta.root, meta.key)); + output::detail(origin.name.clone(), format!("[{label}] {}", origin.detail)); } Ok(()) } -/// Human origin label for a local workspace: directory basename, lowercased. -/// Accepts both `/` and `\` so Windows roots still yield a useful label on Unix. -pub fn workspace_display_name(meta: &WorkspaceMetadata) -> String { - path_basename(&meta.root) - .unwrap_or(meta.key.as_str()) - .to_ascii_lowercase() -} - -fn path_basename(path: &str) -> Option<&str> { - let trimmed = path.trim_end_matches(['/', '\\']); - if trimmed.is_empty() { - return None; - } - trimmed - .rsplit(['/', '\\']) - .next() - .filter(|segment| !segment.is_empty()) -} - /// Resolve a local workspace by origin label (`docs`, `sivtr`, …). /// Prefers exact basename match; ambiguous names error. pub fn resolve_local_workspace_by_name(name: &str) -> Result> { let needle = name.to_ascii_lowercase(); let matches: Vec<_> = workspace::list_workspaces()? .into_iter() - .filter(|meta| workspace_display_name(meta) == needle) + .filter(|meta| workspace::workspace_display_name(meta) == needle) .collect(); match matches.as_slice() { [] => Ok(None), @@ -84,34 +55,3 @@ pub fn resolve_local_workspace_by_name(name: &str) -> Result> { } } } - -#[cfg(test)] -mod tests { - use super::{path_basename, workspace_display_name}; - use sivtr_core::workspace::WorkspaceMetadata; - - #[test] - fn display_name_uses_basename() { - let unix = WorkspaceMetadata { - key: "abc".into(), - root: "/home/user/Coding/sivtr".into(), - created_at: "t".into(), - last_seen_at: "t".into(), - }; - assert_eq!(workspace_display_name(&unix), "sivtr"); - - let windows = WorkspaceMetadata { - key: "abc".into(), - root: r"D:\Coding\sivtr".into(), - created_at: "t".into(), - last_seen_at: "t".into(), - }; - assert_eq!(workspace_display_name(&windows), "sivtr"); - } - - #[test] - fn basename_trims_trailing_separators() { - assert_eq!(path_basename(r"D:\Coding\sivtr\"), Some("sivtr")); - assert_eq!(path_basename("/home/user/sivtr/"), Some("sivtr")); - } -} diff --git a/src/lib.rs b/src/lib.rs index e393d826..5442ad56 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -6,6 +6,7 @@ pub mod cli; pub mod commands; pub mod mcp; +pub mod origins; pub mod output; pub mod pane; pub mod remote; diff --git a/src/mcp/server.rs b/src/mcp/server.rs index 088dc283..b634ae8d 100644 --- a/src/mcp/server.rs +++ b/src/mcp/server.rs @@ -15,9 +15,9 @@ use sivtr_core::ai::AgentProvider; use sivtr_core::workspace; use crate::commands::memory::{filter, search, show, workset, zoom}; -use crate::commands::remote::workspace::workspace_display_name; use crate::remote::ipc; use crate::remote::protocol::{LocalRequest, LocalResponse}; +use sivtr_core::workspace::workspace_display_name; use super::types::{ memory_result, show_result, to_filter_args, to_search_args, to_show_args, to_zoom_args, diff --git a/src/origins.rs b/src/origins.rs new file mode 100644 index 00000000..58728b1c --- /dev/null +++ b/src/origins.rs @@ -0,0 +1,57 @@ +//! Unified origin composition. +//! +//! The single place that assembles every addressable memory source (local +//! workspaces, remote device mounts, cloud accounts) into one +//! [`OriginRegistry`]. Upper layers consume the registry — new sources add a +//! constructor block here, and [`Origin`] itself never changes. + +use anyhow::Result; +use std::path::Path; + +use sivtr_core::origin::{Origin, OriginKind, OriginRegistry}; +use sivtr_core::workspace; + +use crate::commands::remote::serve; +use crate::remote::ipc; +use crate::remote::protocol::{LocalRequest, LocalResponse}; + +/// All origins addressable from `cwd`: every local workspace (the current one +/// flagged), the current workspace's remote mounts, and cloud sources +/// (reserved — none yet). +pub fn collect(cwd: &Path) -> Result { + let mut origins = Vec::new(); + + let current_key = workspace::resolve_workspace_for_dir(cwd)?.map(|paths| paths.key); + for meta in workspace::list_workspaces()? { + let current = current_key.as_deref() == Some(meta.key.as_str()); + origins.push(Origin { + name: workspace::workspace_display_name(&meta), + kind: OriginKind::Local, + current, + detail: format!("{} ({})", meta.root, meta.key), + }); + } + + if let Some(workspace_key) = current_key.as_deref() { + serve::ensure_running()?; + match ipc::call(LocalRequest::RemoteList { + workspace_key: workspace_key.to_string(), + })? { + LocalResponse::Mounts(mounts) => { + for mount in mounts { + origins.push(Origin { + name: mount.alias, + kind: OriginKind::Remote, + current: false, + detail: format!("{}/{}", mount.peer_name, mount.share_name), + }); + } + } + response => anyhow::bail!("Unexpected daemon response: {response:?}"), + } + } + + // Cloud origins: reserved — cloud sources will construct here. + + Ok(OriginRegistry::new(origins)) +} From 1d066986d2002187fee26cba5d52f86d44d47bff Mon Sep 17 00:00:00 2001 From: Ariestar <1941137088@qq.com> Date: Sun, 9 Aug 2026 13:13:37 +0800 Subject: [PATCH 02/10] refactor(origin): route all source handling through the unified registry Replace the remaining split source-handling code with the single Origin shape from b323707; no parallel types, no fallback paths. - MCP status: drop WorkspaceOrigin + MountStatus, expose one origins list built by OriginRegistry (Origin/OriginKind now serialize with lowercase kinds and carry a JSON schema) - TUI: WorkspaceSource carries its OriginKind (local/remote/cloud); the origin glyph and style are kind-based instead of a binary remote bool, and the renderer no longer second-guesses the source from record refs - scope resolution: query() and query_remote_bounded() resolve the scope once through OriginRegistry and dispatch per kind, replacing the mounts -> groups -> local fallback chain; groups stay a registry-miss fan-out, and named locals keep the ambiguity guard - origins::collect() registers the cwd workspace so status lists it even before its first capture --- Cargo.lock | 1 + crates/sivtr-core/Cargo.toml | 1 + crates/sivtr-core/src/origin.rs | 21 +++++- src/commands/browse/load.rs | 4 +- src/commands/memory/workset/source.rs | 100 ++++++++++++++------------ src/mcp/server.rs | 76 ++------------------ src/mcp/types.rs | 20 +----- src/origins.rs | 4 ++ src/tui/theme.rs | 30 +++++--- src/tui/workspace/model.rs | 21 +++++- src/tui/workspace/render.rs | 10 +-- 11 files changed, 131 insertions(+), 157 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 55fcdb8c..4d0601f8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4550,6 +4550,7 @@ dependencies = [ "regex", "rmp-serde", "rusqlite", + "schemars", "serde", "serde_json", "shell-words", diff --git a/crates/sivtr-core/Cargo.toml b/crates/sivtr-core/Cargo.toml index c638e806..82c78175 100644 --- a/crates/sivtr-core/Cargo.toml +++ b/crates/sivtr-core/Cargo.toml @@ -16,6 +16,7 @@ regex = "1" chrono = { version = "0.4", features = ["serde"] } serde = { version = "1", features = ["derive"] } serde_json = "1" +schemars = "1" rusqlite = { version = "0.40", features = ["bundled", "vtab"] } arboard = "3" dirs = "6" diff --git a/crates/sivtr-core/src/origin.rs b/crates/sivtr-core/src/origin.rs index fe641558..ed732046 100644 --- a/crates/sivtr-core/src/origin.rs +++ b/crates/sivtr-core/src/origin.rs @@ -12,13 +12,17 @@ //! [`OriginRegistry`] is the single lookup surface: enumerate every //! addressable origin, or resolve one by its logical name. +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + /// Major source category. /// /// `#[non_exhaustive]`: adding a category (WSL, container, archive, …) only /// requires a new variant plus a `label()` arm here; code outside this crate /// is forced to handle the wildcard, so nothing downstream breaks. #[non_exhaustive] -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "lowercase")] pub enum OriginKind { /// Local files on this machine (workspaces). Local, @@ -43,7 +47,7 @@ impl OriginKind { /// /// All fields exist for every kind; `detail` is the display projection the /// source composed when it was constructed. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] pub struct Origin { /// Logical name (scope name / alias), used by [`OriginRegistry::resolve`]. pub name: String, @@ -144,4 +148,17 @@ mod tests { assert!(matches!(origin.kind.label(), "local" | "remote" | "cloud")); } } + + #[test] + fn kind_serializes_as_lowercase_label() { + assert_eq!( + serde_json::to_string(&OriginKind::Remote).expect("serialize kind"), + "\"remote\"" + ); + let origin = sample().all()[1].clone(); + let json = serde_json::to_string(&origin).expect("serialize origin"); + assert!(json.contains("\"kind\":\"remote\"")); + let round: Origin = serde_json::from_str(&json).expect("deserialize origin"); + assert_eq!(round, origin); + } } diff --git a/src/commands/browse/load.rs b/src/commands/browse/load.rs index 02ade956..a16922bb 100644 --- a/src/commands/browse/load.rs +++ b/src/commands/browse/load.rs @@ -688,12 +688,12 @@ pub fn workspace_source_catalog( sources.push(WorkspaceSource::agent(*provider)); } for alias in list_remote_aliases(cwd)? { - sources.push(WorkspaceSource::scoped( + sources.push(WorkspaceSource::remote( &alias, WorkspaceSourceKind::Terminal, )); for provider in providers { - sources.push(WorkspaceSource::scoped( + sources.push(WorkspaceSource::remote( &alias, WorkspaceSourceKind::Agent(*provider), )); diff --git a/src/commands/memory/workset/source.rs b/src/commands/memory/workset/source.rs index 0d74be70..0e4640e1 100644 --- a/src/commands/memory/workset/source.rs +++ b/src/commands/memory/workset/source.rs @@ -6,6 +6,7 @@ use std::thread; use std::time::Duration; use anyhow::{Context, Result}; +use sivtr_core::origin::OriginKind; use sivtr_core::query::load_workspace_source; use sivtr_core::record::{expand_source, WorkPath, WorkRecord, WorkRef}; use sivtr_core::workspace; @@ -101,30 +102,42 @@ pub fn query(source: &str, filter: Filter, cwd: Option<&Path>) -> Result match origin.kind { + OriginKind::Local => { + let root = + crate::commands::remote::workspace::resolve_local_workspace_by_name( + &origin.name, + )? + .with_context(|| { + format!("local workspace `{}` disappeared", origin.name) + })?; + run_local(path, &root, filter) + } + OriginKind::Remote => { + let ws = workspace::resolve_workspace_for_dir(&cwd)?.with_context(|| { + format!("remote `{}` needs a current workspace", origin.name) + })?; + try_remote(&ws.key, &origin.name, path, filter, &cwd)? + .with_context(|| format!("remote mount `{}` unavailable", origin.name)) + } + OriginKind::Cloud => { + anyhow::bail!("cloud source `{}` is not available yet", origin.name) + } + _ => anyhow::bail!("unknown origin kind for `{}`", origin.name), + }, + None => match try_group(&scope, path, filter, &cwd)? { + Some(set) => Ok(set), + None => anyhow::bail!( + "unknown scope `{scope}`; use `sivtr ws list` for local workspaces or `sivtr remote list` for remotes" + ), + }, + }; } run_local(&source, &cwd, filter) @@ -256,27 +269,26 @@ fn query_remote_bounded( cwd: &Path, read_timeout: Duration, ) -> Result { - // Prefer the timed IPC path for `scope:path` remotes so the daemon socket - // itself respects the interactive deadline. - if let Some((scope, path)) = selector.split_once(':') { - if !path.is_empty() && !path.starts_with('/') && !scope.eq_ignore_ascii_case("local") { - if !scope.contains('/') { - if let Some(ws) = workspace::resolve_workspace_for_dir(cwd)? { - if let Some(set) = - try_remote_timed(&ws.key, scope, path, filter.clone(), cwd, read_timeout)? - { - return Ok(set); - } - } - } - // Groups (`team` or `team/alice`) are device-global; mount aliases - // are workspace-scoped and were checked above. - if let Some(set) = try_group_timed(scope, path, filter.clone(), cwd, read_timeout)? { - return Ok(set); - } - } + // Only confirmed mounts need the timed IPC path, so the daemon socket + // itself respects the interactive deadline. Everything else — groups, + // named local workspaces, plain selectors — goes through the unified + // [`query`], which resolves it in one registry lookup. + let Some((scope, path)) = selector.split_once(':') else { + return query(selector, filter, Some(cwd)); + }; + if path.is_empty() || path.starts_with('/') || scope.eq_ignore_ascii_case("local") { + return query(selector, filter, Some(cwd)); + } + let registry = crate::origins::collect(cwd)?; + if let Some(origin) = registry + .resolve(scope) + .filter(|origin| origin.kind == OriginKind::Remote) + { + let ws = workspace::resolve_workspace_for_dir(cwd)? + .with_context(|| format!("remote `{}` needs a current workspace", origin.name))?; + return try_remote_timed(&ws.key, &origin.name, path, filter, cwd, read_timeout)? + .with_context(|| format!("remote mount `{}` unavailable", origin.name)); } - // Fall back to the normal query (named local workspace, etc.). query(selector, filter, Some(cwd)) } diff --git a/src/mcp/server.rs b/src/mcp/server.rs index b634ae8d..ed3399d1 100644 --- a/src/mcp/server.rs +++ b/src/mcp/server.rs @@ -12,17 +12,14 @@ use rmcp::{ }; use serde::Serialize; use sivtr_core::ai::AgentProvider; -use sivtr_core::workspace; use crate::commands::memory::{filter, search, show, workset, zoom}; use crate::remote::ipc; -use crate::remote::protocol::{LocalRequest, LocalResponse}; -use sivtr_core::workspace::workspace_display_name; use super::types::{ memory_result, show_result, to_filter_args, to_search_args, to_show_args, to_zoom_args, - FilterParams, MountStatus, ProviderStatus, SearchParams, ShowParams, StatusParams, - StatusResult, VarStatus, WorkspaceOrigin, ZoomParams, + FilterParams, ProviderStatus, SearchParams, ShowParams, StatusParams, StatusResult, VarStatus, + ZoomParams, }; #[derive(Clone)] @@ -100,7 +97,7 @@ impl SivtrMcp { } #[tool( - description = "Environment and origin status: version, hooks, providers, daemon, local workspace origin labels (ws), remote mounts, and saved WorkSet vars." + description = "Environment and origin status: version, hooks, providers, daemon, unified origins (local workspaces + remote mounts), and saved WorkSet vars." )] fn sivtr_status( &self, @@ -252,8 +249,7 @@ fn collect_status(cwd: Option<&str>) -> anyhow::Result { let shell_hooks_installed = shell_hooks_installed(); let providers = provider_status(); let (daemon_running, daemon_node_id) = daemon_status(); - let local_workspaces = local_workspace_origins(&cwd)?; - let mounts = mount_status(&cwd); + let origins = crate::origins::collect(&cwd)?.all().to_vec(); let vars = workset::list_saved().ok().map(|list| { list.into_iter() .map(|var| VarStatus { @@ -274,8 +270,7 @@ fn collect_status(cwd: Option<&str>) -> anyhow::Result { providers, daemon_running, daemon_node_id, - local_workspaces, - mounts, + origins, vars, }) } @@ -356,64 +351,3 @@ fn daemon_status() -> (bool, Option) { Err(_) => (false, None), } } - -fn local_workspace_origins(cwd: &Path) -> anyhow::Result> { - let current = workspace::resolve_current_workspace()?.map(|paths| paths.key); - // Ensure cwd is registered when possible. - let _ = workspace::ensure_workspace_for_dir(cwd); - let mut metas = workspace::list_workspaces()?; - if let Some(current_key) = current.as_deref() { - metas.sort_by(|a, b| { - let a_cur = a.key == current_key; - let b_cur = b.key == current_key; - b_cur - .cmp(&a_cur) - .then_with(|| b.last_seen_at.cmp(&a.last_seen_at)) - }); - } - Ok(metas - .into_iter() - .map(|meta| { - let current = current.as_deref() == Some(meta.key.as_str()); - WorkspaceOrigin { - name: workspace_display_name(&meta), - root: meta.root, - key: meta.key, - current, - } - }) - .collect()) -} - -fn mount_status(cwd: &Path) -> Vec { - let key = workspace::resolve_workspace_for_dir(cwd) - .ok() - .flatten() - .map(|paths| paths.key) - .or_else(|| { - workspace::resolve_current_workspace() - .ok() - .flatten() - .map(|paths| paths.key) - }); - mount_status_for_key(key.as_deref()) -} - -fn mount_status_for_key(workspace_key: Option<&str>) -> Vec { - let Some(workspace_key) = workspace_key else { - return Vec::new(); - }; - match ipc::call(LocalRequest::RemoteList { - workspace_key: workspace_key.to_string(), - }) { - Ok(LocalResponse::Mounts(mounts)) => mounts - .into_iter() - .map(|mount| MountStatus { - alias: mount.alias, - peer_name: mount.peer_name, - share_name: mount.share_name, - }) - .collect(), - _ => Vec::new(), - } -} diff --git a/src/mcp/types.rs b/src/mcp/types.rs index b43d9f84..ecfcd9e3 100644 --- a/src/mcp/types.rs +++ b/src/mcp/types.rs @@ -4,6 +4,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use crate::cli::{FilterArgs, SearchArgs, ShowArgs, ZoomArgs}; +use sivtr_core::origin::Origin; use sivtr_core::record::WorkOutcome; use sivtr_core::search::{Field, PartKind}; @@ -195,8 +196,8 @@ pub struct StatusResult { pub providers: Vec, pub daemon_running: bool, pub daemon_node_id: Option, - pub local_workspaces: Vec, - pub mounts: Vec, + /// Every addressable memory source through the unified [`Origin`] shape. + pub origins: Vec, #[serde(skip_serializing_if = "Option::is_none")] pub vars: Option>, } @@ -209,21 +210,6 @@ pub struct ProviderStatus { pub error: Option, } -#[derive(Debug, Clone, Serialize, JsonSchema)] -pub struct WorkspaceOrigin { - pub name: String, - pub root: String, - pub key: String, - pub current: bool, -} - -#[derive(Debug, Clone, Serialize, JsonSchema)] -pub struct MountStatus { - pub alias: String, - pub peer_name: String, - pub share_name: String, -} - #[derive(Debug, Clone, Serialize, JsonSchema)] pub struct VarStatus { pub name: String, diff --git a/src/origins.rs b/src/origins.rs index 58728b1c..90d61fd3 100644 --- a/src/origins.rs +++ b/src/origins.rs @@ -21,6 +21,10 @@ use crate::remote::protocol::{LocalRequest, LocalResponse}; pub fn collect(cwd: &Path) -> Result { let mut origins = Vec::new(); + // Register `cwd` when it is a git repo, so the current workspace is part + // of the registry even before its first capture. + let _ = workspace::ensure_workspace_for_dir(cwd); + let current_key = workspace::resolve_workspace_for_dir(cwd)?.map(|paths| paths.key); for meta in workspace::list_workspaces()? { let current = current_key.as_deref() == Some(meta.key.as_str()); diff --git a/src/tui/theme.rs b/src/tui/theme.rs index 254d32d3..af5fc68e 100644 --- a/src/tui/theme.rs +++ b/src/tui/theme.rs @@ -2,6 +2,7 @@ use ratatui::prelude::{Color, Modifier, Style}; use sivtr_core::ai::AgentProvider; +use sivtr_core::origin::OriginKind; /// Active panel chrome (focused border / scrollbar). pub(crate) fn accent() -> Color { @@ -28,6 +29,11 @@ pub(crate) fn remote_origin() -> Color { Color::Rgb(244, 114, 182) // pink-400 } +/// Cloud origin marker. +pub(crate) fn cloud_origin() -> Color { + Color::Rgb(125, 211, 252) // sky-300 +} + /// Cursor / focus highlight on a list row. pub(crate) fn focus_row() -> Style { Style::default() @@ -72,20 +78,22 @@ pub(crate) fn terminal_color() -> Color { Color::Rgb(148, 163, 184) // slate-400 } -/// Local `·` / remote `↗` glyph. -pub(crate) fn origin_glyph(remote: bool) -> &'static str { - if remote { - "↗" - } else { - "·" +/// Origin glyph by kind: local `·`, remote `↗`, cloud `☁`. +pub(crate) fn origin_glyph(kind: OriginKind) -> &'static str { + match kind { + OriginKind::Local => "·", + OriginKind::Remote => "↗", + OriginKind::Cloud => "☁", + _ => "?", } } -pub(crate) fn origin_style(remote: bool) -> Style { - Style::default().fg(if remote { - remote_origin() - } else { - local_origin() +pub(crate) fn origin_style(kind: OriginKind) -> Style { + Style::default().fg(match kind { + OriginKind::Local => local_origin(), + OriginKind::Remote => remote_origin(), + OriginKind::Cloud => cloud_origin(), + _ => dim(), }) } diff --git a/src/tui/workspace/model.rs b/src/tui/workspace/model.rs index b1c73ebe..932f17bc 100644 --- a/src/tui/workspace/model.rs +++ b/src/tui/workspace/model.rs @@ -3,6 +3,7 @@ use ratatui::prelude::Color; use ratatui::widgets::ListState; use sivtr_core::ai::AgentProvider; +use sivtr_core::origin::OriginKind; use sivtr_core::record::{WorkAt, WorkRecord, WorkRef}; use std::collections::HashSet; use std::time::SystemTime; @@ -71,11 +72,17 @@ pub(crate) struct WorkspaceSource { /// Named scope (`desk`, `docs`); `None` = current local workspace. pub(crate) scope: Option, pub(crate) kind: WorkspaceSourceKind, + /// Origin category this source belongs to, set by its constructor. + pub(crate) origin_kind: OriginKind, } impl WorkspaceSource { pub(crate) fn local(kind: WorkspaceSourceKind) -> Self { - Self { scope: None, kind } + Self { + scope: None, + kind, + origin_kind: OriginKind::Local, + } } pub(crate) fn terminal() -> Self { @@ -86,10 +93,12 @@ impl WorkspaceSource { Self::local(WorkspaceSourceKind::Agent(provider)) } - pub(crate) fn scoped(scope: impl Into, kind: WorkspaceSourceKind) -> Self { + /// A source on another device, addressed by its mount alias. + pub(crate) fn remote(scope: impl Into, kind: WorkspaceSourceKind) -> Self { Self { scope: Some(scope.into()), kind, + origin_kind: OriginKind::Remote, } } @@ -117,8 +126,14 @@ impl WorkspaceSource { self.kind.color() } + /// Origin category of this source. + pub(crate) fn origin_kind(&self) -> OriginKind { + self.origin_kind + } + + /// Whether this source needs remote transport (mount on another device). pub(crate) fn is_remote(&self) -> bool { - self.scope.is_some() + self.origin_kind == OriginKind::Remote } pub(crate) fn is_agent(&self) -> bool { diff --git a/src/tui/workspace/render.rs b/src/tui/workspace/render.rs index 8e4cb0ab..2d12be01 100644 --- a/src/tui/workspace/render.rs +++ b/src/tui/workspace/render.rs @@ -317,11 +317,6 @@ fn session_row_line( highlight: Option<&Regex>, body_failed: bool, ) -> Line<'static> { - let remote = choice.source.is_remote() - || choice - .records - .first() - .is_some_and(|record| !record.work_ref.is_local()); let check = if active_panel { if selected { "● " @@ -331,7 +326,8 @@ fn session_row_line( } else { "" }; - let origin = theme::origin_glyph(remote); + let origin_kind = choice.source.origin_kind(); + let origin = theme::origin_glyph(origin_kind); let badge = choice.source.badge(); let title = compact_session_title(choice); // Keep search highlighting over the full visible text, but paint origin/badge @@ -353,7 +349,7 @@ fn session_row_line( } spans.push(Span::styled( format!("{origin} "), - theme::origin_style(remote), + theme::origin_style(origin_kind), )); spans.push(Span::styled( format!("{badge} "), From 9a035c32061ff91f08214bd8410b8d5fc182375d Mon Sep 17 00:00:00 2001 From: Ariestar <1941137088@qq.com> Date: Sun, 9 Aug 2026 13:51:11 +0800 Subject: [PATCH 03/10] refactor(origin): carry reach payloads in the registry, resolve once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Registry entries now pair the display Origin with its Reach payload (local root / remote workspace_key+alias / cloud reserved), composed in a single pass by origins::collect — resolution no longer re-looks-up what composition already knew. - query() and query_remote_bounded() resolve the scope once and dispatch on Reach: locals get their root directly, remotes query the daemon with the mount already confirmed (no second RemoteList IPC) - delete resolve_local_workspace_by_name; ambiguity detection moves into OriginRegistry::resolve, covering every name collision (local-local, local-mount, mount-mount) instead of only local workspaces - Origin stays the four-field display type; ws list / MCP status / TUI consumers are unchanged in shape --- crates/sivtr-core/src/origin.rs | 176 +++++++++++++++++++++----- src/commands/memory/workset/source.rs | 107 ++++++---------- src/commands/remote/workspace.rs | 30 +---- src/mcp/server.rs | 2 +- src/origins.rs | 39 ++++-- 5 files changed, 206 insertions(+), 148 deletions(-) diff --git a/crates/sivtr-core/src/origin.rs b/crates/sivtr-core/src/origin.rs index ed732046..a0e72367 100644 --- a/crates/sivtr-core/src/origin.rs +++ b/crates/sivtr-core/src/origin.rs @@ -2,16 +2,20 @@ //! //! Every memory source — a local workspace, a remote device mount, a cloud //! account — is described by one [`Origin`] with the same four fields, so -//! upper layers (listing, rendering, future scope resolution) never branch on -//! which kind of source they are looking at. Kind-specific details (root -//! paths, peer/share ids, cloud account) never enter [`Origin`]: the display -//! [`Origin::detail`] is composed by the source at construction time, and -//! whether a remote or cloud source happens to be ingested into local files -//! is a resolution-layer concern, not an origin concern. +//! upper layers (listing, rendering) never branch on which kind of source +//! they are looking at. Kind-specific details (root paths, peer/share ids, +//! cloud account) never enter [`Origin`]: the display [`Origin::detail`] is +//! composed by the source at construction time, and whether a remote or +//! cloud source happens to be ingested into local files is a +//! resolution-layer concern, not an origin concern. //! //! [`OriginRegistry`] is the single lookup surface: enumerate every -//! addressable origin, or resolve one by its logical name. +//! addressable origin, or resolve one by its logical name. Each entry pairs +//! the display [`Origin`] with its [`Reach`] — the kind-specific payload the +//! resolution layer needs to actually load data. Display layers only ever +//! see [`Origin`]; the resolution layer dispatches on [`Reach`]. +use anyhow::{bail, Result}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -59,30 +63,69 @@ pub struct Origin { pub detail: String, } +/// How to reach an origin's data. Resolution-layer only: carries the +/// kind-specific payload display layers never see. Exhaustive, so adding a +/// kind forces every resolution dispatch to gain an arm. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Reach { + /// A workspace on this machine: its root directory. + Local { root: String }, + /// A mount on another device: which workspace's mount list and which alias. + Remote { + workspace_key: String, + alias: String, + }, + /// A cloud account (reserved — none yet). + Cloud, +} + +/// One registry entry: the display [`Origin`] plus its [`Reach`]. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Entry { + pub origin: Origin, + pub reach: Reach, +} + /// Every origin addressable from the current context. /// -/// A pure resolution view: sources construct their own [`Origin`]s and hand +/// A pure resolution view: sources construct their own [`Entry`]s and hand /// them in; this type owns no I/O and never interprets kind-specific fields. #[derive(Debug, Clone, Default)] pub struct OriginRegistry { - origins: Vec, + entries: Vec, } impl OriginRegistry { - pub fn new(origins: Vec) -> Self { - Self { origins } + pub fn new(entries: Vec) -> Self { + Self { entries } } - /// All origins in construction order. - pub fn all(&self) -> &[Origin] { - &self.origins + /// Display origins in construction order. + pub fn all(&self) -> impl Iterator + '_ { + self.entries.iter().map(|entry| &entry.origin) } - /// Resolve an origin by logical name, case-insensitively. - pub fn resolve(&self, name: &str) -> Option<&Origin> { - self.origins + pub fn is_empty(&self) -> bool { + self.entries.is_empty() + } + + /// Resolve an origin by logical name, case-insensitively, returning its + /// entry (display [`Origin`] + [`Reach`]). Errors when more than one + /// origin shares the name. + pub fn resolve(&self, name: &str) -> Result> { + let mut matched = self + .entries .iter() - .find(|origin| origin.name.eq_ignore_ascii_case(name)) + .filter(|entry| entry.origin.name.eq_ignore_ascii_case(name)); + let Some(first) = matched.next() else { + return Ok(None); + }; + if let Some(second) = matched.next() { + let mut details = vec![first.origin.detail.as_str(), second.origin.detail.as_str()]; + details.extend(matched.map(|entry| entry.origin.detail.as_str())); + bail!("ambiguous origin `{name}`; matches: {}", details.join(", ")); + } + Ok(Some(first)) } } @@ -92,17 +135,28 @@ mod tests { fn sample() -> OriginRegistry { OriginRegistry::new(vec![ - Origin { - name: "sivtr".to_string(), - kind: OriginKind::Local, - current: true, - detail: "D:\\Coding\\sivtr (key1)".to_string(), + Entry { + origin: Origin { + name: "sivtr".to_string(), + kind: OriginKind::Local, + current: true, + detail: "D:\\Coding\\sivtr (key1)".to_string(), + }, + reach: Reach::Local { + root: "D:\\Coding\\sivtr".to_string(), + }, }, - Origin { - name: "desk".to_string(), - kind: OriginKind::Remote, - current: false, - detail: "alice/sivtr".to_string(), + Entry { + origin: Origin { + name: "desk".to_string(), + kind: OriginKind::Remote, + current: false, + detail: "alice/sivtr".to_string(), + }, + reach: Reach::Remote { + workspace_key: "key1".to_string(), + alias: "desk".to_string(), + }, }, ]) } @@ -118,24 +172,78 @@ mod tests { fn resolve_matches_name_case_insensitively() { let registry = sample(); assert_eq!( - registry.resolve("desk").map(|o| o.name.as_str()), + registry + .resolve("desk") + .expect("resolve") + .map(|entry| entry.origin.name.as_str()), Some("desk") ); assert_eq!( - registry.resolve("DESK").map(|o| o.name.as_str()), + registry + .resolve("DESK") + .expect("resolve") + .map(|entry| entry.origin.name.as_str()), Some("desk") ); assert_eq!( - registry.resolve("Sivtr").map(|o| o.name.as_str()), + registry + .resolve("Sivtr") + .expect("resolve") + .map(|entry| entry.origin.name.as_str()), Some("sivtr") ); - assert_eq!(registry.resolve("missing"), None); + assert_eq!(registry.resolve("missing").expect("resolve"), None); + } + + #[test] + fn resolve_errors_on_ambiguous_names() { + let registry = OriginRegistry::new(vec![ + Entry { + origin: Origin { + name: "return".to_string(), + kind: OriginKind::Local, + current: false, + detail: "D:\\a (k1)".to_string(), + }, + reach: Reach::Local { + root: "D:\\a".to_string(), + }, + }, + Entry { + origin: Origin { + name: "return".to_string(), + kind: OriginKind::Local, + current: false, + detail: "D:\\b (k2)".to_string(), + }, + reach: Reach::Local { + root: "D:\\b".to_string(), + }, + }, + ]); + let error = registry.resolve("return").expect_err("ambiguous"); + assert!(error.to_string().contains("ambiguous origin `return`")); + } + + #[test] + fn resolve_returns_reach_payload_per_kind() { + let registry = sample(); + let entry = registry.resolve("desk").expect("resolve").expect("found"); + assert!(matches!( + &entry.reach, + Reach::Remote { alias, .. } if alias == "desk" + )); + let entry = registry.resolve("sivtr").expect("resolve").expect("found"); + assert!(matches!( + &entry.reach, + Reach::Local { root } if root == "D:\\Coding\\sivtr" + )); } #[test] fn all_preserves_construction_order() { let registry = sample(); - let names: Vec<_> = registry.all().iter().map(|o| o.name.as_str()).collect(); + let names: Vec<_> = registry.all().map(|origin| origin.name.as_str()).collect(); assert_eq!(names, vec!["sivtr", "desk"]); } @@ -155,7 +263,7 @@ mod tests { serde_json::to_string(&OriginKind::Remote).expect("serialize kind"), "\"remote\"" ); - let origin = sample().all()[1].clone(); + let origin = sample().all().nth(1).expect("second origin").clone(); let json = serde_json::to_string(&origin).expect("serialize origin"); assert!(json.contains("\"kind\":\"remote\"")); let round: Origin = serde_json::from_str(&json).expect("deserialize origin"); diff --git a/src/commands/memory/workset/source.rs b/src/commands/memory/workset/source.rs index 0e4640e1..d570c6b1 100644 --- a/src/commands/memory/workset/source.rs +++ b/src/commands/memory/workset/source.rs @@ -6,10 +6,9 @@ use std::thread; use std::time::Duration; use anyhow::{Context, Result}; -use sivtr_core::origin::OriginKind; +use sivtr_core::origin::Reach; use sivtr_core::query::load_workspace_source; use sivtr_core::record::{expand_source, WorkPath, WorkRecord, WorkRef}; -use sivtr_core::workspace; use crate::commands::memory::filter::{self, Filter}; use crate::commands::memory::records::warn_skipped; @@ -103,33 +102,26 @@ pub fn query(source: &str, filter: Filter, cwd: Option<&Path>) -> Result match origin.kind { - OriginKind::Local => { - let root = - crate::commands::remote::workspace::resolve_local_workspace_by_name( - &origin.name, - )? - .with_context(|| { - format!("local workspace `{}` disappeared", origin.name) - })?; - run_local(path, &root, filter) + return match registry.resolve(&scope)? { + Some(entry) => match &entry.reach { + Reach::Local { root } => run_local(path, Path::new(root), filter), + Reach::Remote { workspace_key, alias } => try_remote_timed( + workspace_key, + alias, + path, + filter, + &cwd, + Duration::from_secs(30), + ) + .with_context(|| format!("remote mount `{alias}` unavailable")), + Reach::Cloud => { + anyhow::bail!("cloud source `{}` is not available yet", entry.origin.name) } - OriginKind::Remote => { - let ws = workspace::resolve_workspace_for_dir(&cwd)?.with_context(|| { - format!("remote `{}` needs a current workspace", origin.name) - })?; - try_remote(&ws.key, &origin.name, path, filter, &cwd)? - .with_context(|| format!("remote mount `{}` unavailable", origin.name)) - } - OriginKind::Cloud => { - anyhow::bail!("cloud source `{}` is not available yet", origin.name) - } - _ => anyhow::bail!("unknown origin kind for `{}`", origin.name), }, None => match try_group(&scope, path, filter, &cwd)? { Some(set) => Ok(set), @@ -280,16 +272,17 @@ fn query_remote_bounded( return query(selector, filter, Some(cwd)); } let registry = crate::origins::collect(cwd)?; - if let Some(origin) = registry - .resolve(scope) - .filter(|origin| origin.kind == OriginKind::Remote) - { - let ws = workspace::resolve_workspace_for_dir(cwd)? - .with_context(|| format!("remote `{}` needs a current workspace", origin.name))?; - return try_remote_timed(&ws.key, &origin.name, path, filter, cwd, read_timeout)? - .with_context(|| format!("remote mount `{}` unavailable", origin.name)); + let Some(entry) = registry.resolve(scope)? else { + return query(selector, filter, Some(cwd)); + }; + match &entry.reach { + Reach::Remote { + workspace_key, + alias, + } => try_remote_timed(workspace_key, alias, path, filter, cwd, read_timeout) + .with_context(|| format!("remote mount `{alias}` unavailable")), + _ => query(selector, filter, Some(cwd)), } - query(selector, filter, Some(cwd)) } fn is_timeout_error(message: &str) -> bool { @@ -344,62 +337,34 @@ fn apply_loaded(set: WorkSet, filter: Filter) -> Result { filter::apply(PathBuf::from(&set.cwd), set.records, set.anchors, filter) } -fn try_remote( - workspace_key: &str, - remote_name: &str, - path: &str, - filter: Filter, - cwd: &Path, -) -> Result> { - try_remote_timed( - workspace_key, - remote_name, - path, - filter, - cwd, - Duration::from_secs(30), - ) -} - fn try_remote_timed( workspace_key: &str, - remote_name: &str, + alias: &str, path: &str, filter: Filter, cwd: &Path, read_timeout: Duration, -) -> Result> { +) -> Result { use crate::remote::ipc; use crate::remote::protocol::{LocalRequest, LocalResponse}; + // The registry already confirmed the mount; ensure the daemon is up so a + // stale socket yields a clear error instead of a confusing one. crate::commands::remote::serve::ensure_running()?; - let mounts = match ipc::call(LocalRequest::RemoteList { - workspace_key: workspace_key.to_string(), - })? { - LocalResponse::Mounts(mounts) => mounts, - _ => return Ok(None), - }; - if !mounts - .iter() - .any(|mount| mount.alias.eq_ignore_ascii_case(remote_name)) - { - return Ok(None); - } - match ipc::call_with_read_timeout( LocalRequest::RemoteQuery { workspace_key: workspace_key.to_string(), - alias: remote_name.to_ascii_lowercase(), + alias: alias.to_ascii_lowercase(), source: path.to_string(), filter, }, read_timeout, )? { - LocalResponse::Query(response) => Ok(Some(WorkSet::with_anchors( + LocalResponse::Query(response) => Ok(WorkSet::with_anchors( cwd.display().to_string(), response.records, response.anchors, - ))), + )), response => anyhow::bail!("Unexpected daemon response: {response:?}"), } } diff --git a/src/commands/remote/workspace.rs b/src/commands/remote/workspace.rs index 48288ccf..ba4aa3a5 100644 --- a/src/commands/remote/workspace.rs +++ b/src/commands/remote/workspace.rs @@ -1,10 +1,6 @@ -use std::path::PathBuf; - -use anyhow::{bail, Context, Result}; -use sivtr_core::workspace; - use crate::cli::{WorkspaceAction, WorkspaceCommand}; use crate::output; +use anyhow::{Context, Result}; pub fn execute(command: WorkspaceCommand) -> Result<()> { match command.action.unwrap_or(WorkspaceAction::List) { @@ -17,7 +13,7 @@ pub fn execute(command: WorkspaceCommand) -> Result<()> { fn list() -> Result<()> { let cwd = std::env::current_dir().context("Failed to resolve current directory")?; let registry = crate::origins::collect(&cwd)?; - if registry.all().is_empty() { + if registry.is_empty() { output::plain("no origins recorded yet"); output::hint("run a command inside a git repo after `sivtr init`"); return Ok(()); @@ -33,25 +29,3 @@ fn list() -> Result<()> { } Ok(()) } - -/// Resolve a local workspace by origin label (`docs`, `sivtr`, …). -/// Prefers exact basename match; ambiguous names error. -pub fn resolve_local_workspace_by_name(name: &str) -> Result> { - let needle = name.to_ascii_lowercase(); - let matches: Vec<_> = workspace::list_workspaces()? - .into_iter() - .filter(|meta| workspace::workspace_display_name(meta) == needle) - .collect(); - match matches.as_slice() { - [] => Ok(None), - [only] => Ok(Some(PathBuf::from(&only.root))), - many => { - let roots = many - .iter() - .map(|meta| meta.root.as_str()) - .collect::>() - .join(", "); - bail!("ambiguous local workspace `{name}`; matches: {roots}") - } - } -} diff --git a/src/mcp/server.rs b/src/mcp/server.rs index ed3399d1..b2d67586 100644 --- a/src/mcp/server.rs +++ b/src/mcp/server.rs @@ -249,7 +249,7 @@ fn collect_status(cwd: Option<&str>) -> anyhow::Result { let shell_hooks_installed = shell_hooks_installed(); let providers = provider_status(); let (daemon_running, daemon_node_id) = daemon_status(); - let origins = crate::origins::collect(&cwd)?.all().to_vec(); + let origins = crate::origins::collect(&cwd)?.all().cloned().collect(); let vars = workset::list_saved().ok().map(|list| { list.into_iter() .map(|var| VarStatus { diff --git a/src/origins.rs b/src/origins.rs index 90d61fd3..1fa4440f 100644 --- a/src/origins.rs +++ b/src/origins.rs @@ -2,13 +2,15 @@ //! //! The single place that assembles every addressable memory source (local //! workspaces, remote device mounts, cloud accounts) into one -//! [`OriginRegistry`]. Upper layers consume the registry — new sources add a +//! [`OriginRegistry`]. Each entry pairs the display [`Origin`] with its +//! [`Reach`] payload, so resolution never re-looks-up what composition +//! already knew. Upper layers consume the registry — new sources add a //! constructor block here, and [`Origin`] itself never changes. use anyhow::Result; use std::path::Path; -use sivtr_core::origin::{Origin, OriginKind, OriginRegistry}; +use sivtr_core::origin::{Entry, Origin, OriginKind, OriginRegistry, Reach}; use sivtr_core::workspace; use crate::commands::remote::serve; @@ -19,7 +21,7 @@ use crate::remote::protocol::{LocalRequest, LocalResponse}; /// flagged), the current workspace's remote mounts, and cloud sources /// (reserved — none yet). pub fn collect(cwd: &Path) -> Result { - let mut origins = Vec::new(); + let mut entries = Vec::new(); // Register `cwd` when it is a git repo, so the current workspace is part // of the registry even before its first capture. @@ -28,11 +30,14 @@ pub fn collect(cwd: &Path) -> Result { let current_key = workspace::resolve_workspace_for_dir(cwd)?.map(|paths| paths.key); for meta in workspace::list_workspaces()? { let current = current_key.as_deref() == Some(meta.key.as_str()); - origins.push(Origin { - name: workspace::workspace_display_name(&meta), - kind: OriginKind::Local, - current, - detail: format!("{} ({})", meta.root, meta.key), + entries.push(Entry { + origin: Origin { + name: workspace::workspace_display_name(&meta), + kind: OriginKind::Local, + current, + detail: format!("{} ({})", meta.root, meta.key), + }, + reach: Reach::Local { root: meta.root }, }); } @@ -43,11 +48,17 @@ pub fn collect(cwd: &Path) -> Result { })? { LocalResponse::Mounts(mounts) => { for mount in mounts { - origins.push(Origin { - name: mount.alias, - kind: OriginKind::Remote, - current: false, - detail: format!("{}/{}", mount.peer_name, mount.share_name), + entries.push(Entry { + origin: Origin { + name: mount.alias.clone(), + kind: OriginKind::Remote, + current: false, + detail: format!("{}/{}", mount.peer_name, mount.share_name), + }, + reach: Reach::Remote { + workspace_key: workspace_key.to_string(), + alias: mount.alias, + }, }); } } @@ -57,5 +68,5 @@ pub fn collect(cwd: &Path) -> Result { // Cloud origins: reserved — cloud sources will construct here. - Ok(OriginRegistry::new(origins)) + Ok(OriginRegistry::new(entries)) } From 0c3372b6acec7ade3db1758f421fe666f3ef5648 Mon Sep 17 00:00:00 2001 From: Ariestar <1941137088@qq.com> Date: Sun, 9 Aug 2026 16:10:02 +0800 Subject: [PATCH 04/10] refactor(origin): one-shot group queries and typed empty-selector contract - Drop the GroupResolve probe: GroupQuery answers None for unknown groups, so the scope cascade needs a single IPC round trip. Mirrors the identical protocol/daemon change on the group refactor branch (dedupes on merge). - Collapse try_group_timed into try_group: the 10s budget was its only caller, so the read_timeout parameter and the max() clamp were dead. - Export NO_RECORD_FOR_SELECTOR from core and use it at all three match sites; the error text is no longer an implicit API contract. - Mention groups in the unknown-scope error message. --- crates/sivtr-core/src/query/mod.rs | 8 ++++- src/commands/memory/workset/source.rs | 43 ++++++++++----------------- 2 files changed, 23 insertions(+), 28 deletions(-) diff --git a/crates/sivtr-core/src/query/mod.rs b/crates/sivtr-core/src/query/mod.rs index 7f833c38..5e1b2924 100644 --- a/crates/sivtr-core/src/query/mod.rs +++ b/crates/sivtr-core/src/query/mod.rs @@ -18,6 +18,12 @@ use crate::ai::AgentSessionProvider; use crate::record::{WorkPath, WorkRecord, WorkRecordIndex, WorkRef, WorkRefSelector}; use crate::{session, workspace}; +/// Prefix of the error [`load_workspace_source`] raises when a selector +/// matches no records. An empty source is a normal browse outcome (a +/// workspace with no sessions yet), so callers treat this exact error as an +/// empty result; keep it a named constant so that contract cannot drift. +pub const NO_RECORD_FOR_SELECTOR: &str = "No record found for ref selector"; + /// A session file that could not be parsed, retained so callers can warn. #[derive(Debug, Clone)] pub struct SkippedSession { @@ -117,7 +123,7 @@ pub fn load_workspace_source(cwd: &Path, source: &str) -> Result= the daemon's group sync pull budget plus the per-share -/// fan-out budget (`remote::groups` constants). +/// Socket-read headroom for one group fan-out inside [`query`]: the daemon +/// dials every member in parallel under its own per-share budget, so this +/// must stay above that budget plus local query time. const GROUP_QUERY_TIMEOUT: Duration = Duration::from_secs(10); /// How one source is scheduled inside [`query_many`]. @@ -126,7 +125,7 @@ pub fn query(source: &str, filter: Filter, cwd: Option<&Path>) -> Result match try_group(&scope, path, filter, &cwd)? { Some(set) => Ok(set), None => anyhow::bail!( - "unknown scope `{scope}`; use `sivtr ws list` for local workspaces or `sivtr remote list` for remotes" + "unknown scope `{scope}`; use `sivtr ws list` for local workspaces, `sivtr remote list` for remotes, or `sivtr group list` for groups" ), }, }; @@ -168,7 +167,7 @@ pub fn query_many( Err(error) => { let message = error.to_string(); // Empty selector is normal for browse; keep parity with single-source callers. - if message.starts_with("No record found for ref selector") { + if message.starts_with(NO_RECORD_FOR_SELECTOR) { results[idx] = Some(QuerySourceResult::Ok(WorkSet::with_anchors( cwd.display().to_string(), Vec::new(), @@ -200,7 +199,7 @@ pub fn query_many( Ok(set) => Ok(set), Err(error) => { let message = error.to_string(); - if message.starts_with("No record found for ref selector") { + if message.starts_with(NO_RECORD_FOR_SELECTOR) { Ok(WorkSet::with_anchors( cwd.display().to_string(), Vec::new(), @@ -313,11 +312,7 @@ pub fn run_on_share( } Ok((set.records, set.anchors)) } - Err(error) - if error - .to_string() - .starts_with("No record found for ref selector") => - { + Err(error) if error.to_string().starts_with(NO_RECORD_FOR_SELECTOR) => { Ok((Vec::new(), Vec::new())) } Err(error) => Err(error), @@ -369,19 +364,11 @@ fn try_remote_timed( } } +/// Group fan-out: `team:...` (all members), `team/alice:...` (one member), or +/// `team/alice/proj-b:...` (one member, one contributed share). Returns +/// `Ok(None)` when `scope` is not a group on this device so the caller can +/// continue the scope cascade. fn try_group(scope: &str, path: &str, filter: Filter, cwd: &Path) -> Result> { - try_group_timed(scope, path, filter, cwd, GROUP_QUERY_TIMEOUT) -} - -/// Group fan-out with a deadline. Returns `Ok(None)` when `scope` is not a -/// group on this device so the caller can continue the scope cascade. -fn try_group_timed( - scope: &str, - path: &str, - filter: Filter, - cwd: &Path, - read_timeout: Duration, -) -> Result> { use crate::remote::ipc; use crate::remote::protocol::{LocalRequest, LocalResponse}; @@ -390,6 +377,9 @@ fn try_group_timed( }; crate::commands::remote::serve::ensure_running() .context("failed to start the sivtr daemon for a group query")?; + // The daemon answers `None` for an unknown group; the fan-out happens + // inside it (parallel per-member dials), so the socket read gets enough + // headroom beyond the daemon's per-peer budget. match ipc::call_with_read_timeout( LocalRequest::GroupQuery { group, @@ -398,11 +388,10 @@ fn try_group_timed( source: path.to_string(), filter, }, - read_timeout, + GROUP_QUERY_TIMEOUT, ) .context("group query failed")? { - // Unknown group: fall through to the rest of the scope cascade. LocalResponse::GroupQuery(None) => Ok(None), LocalResponse::GroupQuery(Some(response)) => { if !response.skipped.is_empty() { From ca5c7304960be88d933f0a414781ac62a5b5a8f0 Mon Sep 17 00:00:00 2001 From: Ariestar <1941137088@qq.com> Date: Mon, 10 Aug 2026 23:06:31 +0800 Subject: [PATCH 05/10] feat(origin): commondir workspace identity and unified origin rename Workspace identity is now the repo's shared git dir (commondir), so a main checkout, its worktrees, and nested subdirs all resolve to one workspace with unified terminal logs and agent sessions; session matching reuses the same identity instead of comparing remote URLs. Adds persisted workspace aliases with `sivtr origin rename` covering both local workspaces and remote mounts, and drops the legacy workspace-key migration now that keys are identity-derived. --- crates/sivtr-core/src/agents/jsonl.rs | 27 +- crates/sivtr-core/src/agents/model.rs | 298 ++++-------- crates/sivtr-core/src/lib.rs | 3 + crates/sivtr-core/src/origin.rs | 79 ++- crates/sivtr-core/src/test_fixtures.rs | 20 + crates/sivtr-core/src/workspace.rs | 639 +++++++++++-------------- src/cli/mod.rs | 4 + src/cli/remote.rs | 18 + src/commands/memory/copy/mod.rs | 17 +- src/commands/memory/workset/source.rs | 16 +- src/commands/remote/mod.rs | 1 + src/commands/remote/origin.rs | 19 + src/commands/remote/share.rs | 4 +- src/commands/system/doctor.rs | 134 ------ src/commands/system/setup.rs | 9 - src/lib.rs | 3 + src/origins.rs | 93 +++- 17 files changed, 622 insertions(+), 762 deletions(-) create mode 100644 crates/sivtr-core/src/test_fixtures.rs create mode 100644 src/commands/remote/origin.rs diff --git a/crates/sivtr-core/src/agents/jsonl.rs b/crates/sivtr-core/src/agents/jsonl.rs index 2f50c439..9f1d1480 100644 --- a/crates/sivtr-core/src/agents/jsonl.rs +++ b/crates/sivtr-core/src/agents/jsonl.rs @@ -354,15 +354,6 @@ mod tests { format!("{}\n", json!({ "sessionId": id, "cwd": cwd })) } - fn write_git_remote(repo: &Path, name: &str, url: &str) { - fs::create_dir_all(repo.join(".git")).unwrap(); - fs::write( - repo.join(".git").join("config"), - format!("[remote \"{name}\"]\n\turl = {url}\n"), - ) - .unwrap(); - } - #[test] fn includes_sessions_with_later_matching_cwd_metadata() { let _guard = env_lock(); @@ -370,21 +361,11 @@ mod tests { let previous = std::env::var_os("SIVTR_DATA_DIR"); std::env::set_var("SIVTR_DATA_DIR", dir.path().join("data")); let sessions = dir.path().join("sessions"); - let target = dir.path().join("oh-my-ppt-fork"); - let candidate = dir.path().join("oh-my-ppt"); + let target = dir.path().join("sivtr"); + let candidate = dir.path().join("sivtr-worktree"); fs::create_dir_all(&sessions).unwrap(); - fs::create_dir_all(&target).unwrap(); - fs::create_dir_all(&candidate).unwrap(); - write_git_remote( - &target, - "upstream", - "https://github.com/arcsin1/oh-my-ppt.git", - ); - write_git_remote( - &candidate, - "origin", - "https://github.com/arcsin1/oh-my-ppt.git", - ); + crate::test_fixtures::make_repo(&target); + crate::test_fixtures::make_worktree(&target, &candidate, "sivtr-worktree"); let transcript = sessions.join("session.jsonl"); let first_event = serde_json::json!({ "sessionId": "abc", diff --git a/crates/sivtr-core/src/agents/model.rs b/crates/sivtr-core/src/agents/model.rs index 03056253..e8ecd3c4 100644 --- a/crates/sivtr-core/src/agents/model.rs +++ b/crates/sivtr-core/src/agents/model.rs @@ -2,8 +2,7 @@ use anyhow::Result; use serde::{Deserialize, Serialize}; use serde_json::Value; use std::cell::RefCell; -use std::collections::{HashMap, HashSet}; -use std::fs; +use std::collections::HashMap; use std::path::{Path, PathBuf}; use std::time::SystemTime; @@ -383,7 +382,9 @@ pub fn normalize_path_for_match(path: &Path) -> String { /// Policy (all providers): /// - `cwd == None` → keep every session /// - session has no cwd metadata → **keep** (unbound / weixin / cron / missing) -/// - session has cwd → keep only when it matches the target path **or** shares a git remote +/// - session has cwd → keep only when it resolves to the same repository +/// (commondir identity) as the target, or exactly matches the target path +/// when neither side is inside a git checkout pub fn filter_sessions_by_workspace( sessions: Vec, cwd: Option<&Path>, @@ -419,16 +420,16 @@ pub(crate) fn workspace_matches_candidates( pub(crate) struct WorkspaceMatchTarget { normalized_path: String, - remote_keys: HashSet, - candidate_remote_keys: RefCell>>, + identity: Option, + candidate_identities: RefCell>>, } impl WorkspaceMatchTarget { pub(crate) fn new(path: &Path) -> Self { Self { normalized_path: normalize_path_for_match(path), - remote_keys: git_remote_keys(path), - candidate_remote_keys: RefCell::new(HashMap::new()), + identity: crate::workspace::repo_identity(path), + candidate_identities: RefCell::new(HashMap::new()), } } @@ -437,138 +438,23 @@ impl WorkspaceMatchTarget { if normalized_candidate == self.normalized_path { return true; } - - if self.remote_keys.is_empty() { - return false; - } - - { - let cache = self.candidate_remote_keys.borrow(); - if let Some(candidate_keys) = cache.get(&normalized_candidate) { - return candidate_keys - .iter() - .any(|candidate_key| self.remote_keys.contains(candidate_key)); - } + let candidate_identity = self.candidate_identity(&normalized_candidate, candidate); + match (&self.identity, candidate_identity) { + (Some(target), Some(candidate)) => *target == candidate, + _ => false, } - - let candidate_keys = git_remote_keys(candidate); - let matches = candidate_keys - .iter() - .any(|candidate_key| self.remote_keys.contains(candidate_key)); - self.candidate_remote_keys - .borrow_mut() - .insert(normalized_candidate, candidate_keys); - matches } -} - -fn git_remote_keys(path: &Path) -> HashSet { - let Some(root) = git_root(path) else { - return HashSet::new(); - }; - let Some(config_path) = git_config_path(&root) else { - return HashSet::new(); - }; - parse_git_remote_keys(&config_path) -} - -fn git_root(path: &Path) -> Option { - let mut dir = if path.is_dir() { - path.to_path_buf() - } else { - path.parent() - .map(Path::to_path_buf) - .unwrap_or_else(|| path.to_path_buf()) - }; - loop { - if dir.join(".git").exists() { - return Some(dir); + fn candidate_identity(&self, key: &str, path: &Path) -> Option { + if let Some(existing) = self.candidate_identities.borrow().get(key) { + return existing.clone(); } - if !dir.pop() { - return None; - } - } -} - -fn git_config_path(root: &Path) -> Option { - let dot_git = root.join(".git"); - if dot_git.is_dir() { - return Some(dot_git.join("config")); - } - - let gitdir = fs::read_to_string(&dot_git).ok()?; - let relative = gitdir.trim().strip_prefix("gitdir:")?.trim(); - let git_dir = resolve_gitdir(root, relative); - Some(git_dir.join("config")) -} - -fn resolve_gitdir(root: &Path, gitdir: &str) -> PathBuf { - let path = PathBuf::from(gitdir); - if path.is_absolute() { - path - } else { - root.join(path) - } -} - -fn parse_git_remote_keys(config_path: &Path) -> HashSet { - fs::read_to_string(config_path) - .ok() - .map(|config| { - config - .lines() - .filter_map(remote_key_from_config_line) - .collect() - }) - .unwrap_or_default() -} - -fn remote_key_from_config_line(line: &str) -> Option { - let trimmed = line.trim(); - let url = trimmed.strip_prefix("url")?.trim_start(); - let url = url.strip_prefix('=')?.trim(); - normalize_remote_url(url) -} - -fn normalize_remote_url(url: &str) -> Option { - let trimmed = url.trim().trim_end_matches('/'); - if trimmed.is_empty() { - return None; - } - - let without_suffix = trimmed.strip_suffix(".git").unwrap_or(trimmed); - let normalized = if let Some((_, rest)) = without_suffix.split_once("://") { - normalize_remote_authority_path(rest).unwrap_or_else(|| without_suffix.to_string()) - } else if let Some((authority, path)) = split_scp_like_remote(without_suffix) { - format!( - "{}/{}", - authority.rsplit('@').next().unwrap_or(authority), - path.trim_start_matches('/') - ) - } else { - without_suffix.to_string() - }; - - Some(normalized.replace('\\', "/").to_lowercase()) -} - -fn normalize_remote_authority_path(rest: &str) -> Option { - let (authority, path) = rest.split_once('/')?; - let host = authority.rsplit('@').next()?.trim(); - let path = path.trim_start_matches('/').trim(); - if host.is_empty() || path.is_empty() { - return None; - } - Some(format!("{host}/{path}")) -} - -fn split_scp_like_remote(remote: &str) -> Option<(&str, &str)> { - let (authority, path) = remote.split_once(':')?; - if !authority.contains('@') || path.trim().is_empty() { - return None; + let identity = crate::workspace::repo_identity(path); + self.candidate_identities + .borrow_mut() + .insert(key.to_string(), identity.clone()); + identity } - Some((authority, path)) } pub fn select_blocks(session: &AgentSession, selection: AgentSelection) -> Vec { @@ -672,49 +558,9 @@ pub fn is_structure_block(kind: AgentBlockKind) -> bool { #[cfg(test)] mod tests { use super::*; + use crate::test_fixtures::{make_repo, make_worktree}; use std::fs; - fn write_git_remote(repo: &Path, name: &str, url: &str) { - fs::create_dir_all(repo.join(".git")).unwrap(); - fs::write( - repo.join(".git").join("config"), - format!("[remote \"{name}\"]\n\turl = {url}\n"), - ) - .unwrap(); - } - - #[test] - fn normalizes_common_github_remote_url_forms() { - assert_eq!( - normalize_remote_url("https://github.com/Ariestar/sivtr.git").as_deref(), - Some("github.com/ariestar/sivtr") - ); - assert_eq!( - normalize_remote_url("git@github.com:Ariestar/sivtr.git").as_deref(), - Some("github.com/ariestar/sivtr") - ); - assert_eq!( - normalize_remote_url("ssh://git@github.com/Ariestar/sivtr.git/").as_deref(), - Some("github.com/ariestar/sivtr") - ); - } - - #[test] - fn normalizes_generic_git_remote_url_forms() { - assert_eq!( - normalize_remote_url("https://gitlab.example.com/team/sivtr.git").as_deref(), - Some("gitlab.example.com/team/sivtr") - ); - assert_eq!( - normalize_remote_url("git@gitlab.example.com:team/sivtr.git").as_deref(), - Some("gitlab.example.com/team/sivtr") - ); - assert_eq!( - normalize_remote_url("ssh://git@gitlab.example.com:2222/team/sivtr.git").as_deref(), - Some("gitlab.example.com:2222/team/sivtr") - ); - } - #[test] fn cwd_candidates_do_not_duplicate_the_primary_cwd() { let mut tracked = AgentSessionMeta::default(); @@ -733,49 +579,56 @@ mod tests { } #[test] - fn matches_repositories_with_shared_remote() { + fn matches_sessions_across_worktrees_of_same_repo() { let dir = tempfile::tempdir().unwrap(); - let target = dir.path().join("oh-my-ppt-fork"); - let candidate = dir.path().join("oh-my-ppt"); - fs::create_dir_all(&target).unwrap(); - fs::create_dir_all(&candidate).unwrap(); - write_git_remote( - &target, - "upstream", - "https://github.com/arcsin1/oh-my-ppt.git", - ); - write_git_remote(&candidate, "origin", "git@github.com:arcsin1/oh-my-ppt.git"); + let main = dir.path().join("sivtr"); + let worktree = dir.path().join("sivtr-tui-stack"); + make_repo(&main); + make_worktree(&main, &worktree, "sivtr-tui-stack"); - assert!(WorkspaceMatchTarget::new(&target).matches(&candidate)); + // A session recorded in the main checkout is visible from the worktree + // and vice versa: they share one repository identity. + assert!(WorkspaceMatchTarget::new(&main).matches(&worktree)); + assert!(WorkspaceMatchTarget::new(&worktree).matches(&main)); } #[test] - fn does_not_match_unrelated_repositories() { + fn matches_subdirectories_of_same_repo() { let dir = tempfile::tempdir().unwrap(); - let target = dir.path().join("oh-my-ppt-fork"); - let candidate = dir.path().join("sivtr"); - fs::create_dir_all(&target).unwrap(); - fs::create_dir_all(&candidate).unwrap(); - write_git_remote( - &target, - "upstream", - "https://github.com/arcsin1/oh-my-ppt.git", - ); - write_git_remote( - &candidate, - "origin", - "https://github.com/Ariestar/sivtr.git", - ); + let repo = dir.path().join("sivtr"); + make_repo(&repo); + let subdir = repo.join("crates").join("core"); + fs::create_dir_all(&subdir).unwrap(); + assert!(WorkspaceMatchTarget::new(&repo).matches(&subdir)); + } - assert!(!WorkspaceMatchTarget::new(&target).matches(&candidate)); + #[test] + fn does_not_match_different_repos() { + let dir = tempfile::tempdir().unwrap(); + let first = dir.path().join("sivtr"); + let second = dir.path().join("md-dragger"); + make_repo(&first); + make_repo(&second); + assert!(!WorkspaceMatchTarget::new(&first).matches(&second)); + } + + #[test] + fn does_not_match_sessions_outside_any_repo() { + let dir = tempfile::tempdir().unwrap(); + let repo = dir.path().join("repo"); + make_repo(&repo); + // A session recorded in a non-repo parent (e.g. `D:\Coding`) must not + // leak into a repo workspace. + assert!(!WorkspaceMatchTarget::new(&repo).matches(dir.path())); + // ...but it still matches when browsing that parent dir itself. + assert!(WorkspaceMatchTarget::new(dir.path()).matches(dir.path())); } #[test] fn filter_sessions_keeps_unbound_and_matching_cwd() { let dir = tempfile::tempdir().unwrap(); let repo = dir.path().join("repo"); - fs::create_dir_all(&repo).unwrap(); - write_git_remote(&repo, "origin", "https://github.com/Ariestar/sivtr.git"); + make_repo(&repo); let sessions = vec![ AgentSessionInfo { @@ -808,4 +661,39 @@ mod tests { .collect(); assert_eq!(ids, vec!["u", "m"]); } + + #[test] + fn filter_keeps_sessions_recorded_in_linked_worktree() { + let dir = tempfile::tempdir().unwrap(); + let main = dir.path().join("sivtr"); + let worktree = dir.path().join("sivtr-tui-stack"); + make_repo(&main); + make_worktree(&main, &worktree, "sivtr-tui-stack"); + + // A session recorded in the main checkout shows up when browsing the + // worktree, and one recorded in the worktree shows up from the main. + let main_session = vec![AgentSessionInfo { + path: PathBuf::from("main-session"), + id: Some("m".into()), + cwd: Some(main.to_string_lossy().into_owned()), + title: None, + modified: SystemTime::UNIX_EPOCH, + }]; + assert_eq!( + filter_sessions_by_workspace(main_session, Some(&worktree)).len(), + 1 + ); + + let worktree_session = vec![AgentSessionInfo { + path: PathBuf::from("wt-session"), + id: Some("w".into()), + cwd: Some(worktree.to_string_lossy().into_owned()), + title: None, + modified: SystemTime::UNIX_EPOCH, + }]; + assert_eq!( + filter_sessions_by_workspace(worktree_session, Some(&main)).len(), + 1 + ); + } } diff --git a/crates/sivtr-core/src/lib.rs b/crates/sivtr-core/src/lib.rs index 17f03939..e7abc7f4 100644 --- a/crates/sivtr-core/src/lib.rs +++ b/crates/sivtr-core/src/lib.rs @@ -25,6 +25,9 @@ pub use agents::openclaw; pub use agents::opencode; pub use agents::pi; +#[cfg(test)] +pub(crate) mod test_fixtures; + /// Serialize tests that mutate process-global env vars. /// /// `std::env` is process-global, so any two tests that point e.g. diff --git a/crates/sivtr-core/src/origin.rs b/crates/sivtr-core/src/origin.rs index a0e72367..f4306b45 100644 --- a/crates/sivtr-core/src/origin.rs +++ b/crates/sivtr-core/src/origin.rs @@ -25,7 +25,7 @@ use serde::{Deserialize, Serialize}; /// requires a new variant plus a `label()` arm here; code outside this crate /// is forced to handle the wildcard, so nothing downstream breaks. #[non_exhaustive] -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "lowercase")] pub enum OriginKind { /// Local files on this machine (workspaces). @@ -110,22 +110,44 @@ impl OriginRegistry { } /// Resolve an origin by logical name, case-insensitively, returning its - /// entry (display [`Origin`] + [`Reach`]). Errors when more than one - /// origin shares the name. + /// entry (display [`Origin`] + [`Reach`]). When the name collides across + /// kinds — a mount alias may equal a local workspace's basename — the + /// higher-priority kind wins, matching the lookup order that predated the + /// registry. Collisions within one kind are an error. pub fn resolve(&self, name: &str) -> Result> { let mut matched = self .entries .iter() - .filter(|entry| entry.origin.name.eq_ignore_ascii_case(name)); - let Some(first) = matched.next() else { + .filter(|entry| entry.origin.name.eq_ignore_ascii_case(name)) + .collect::>(); + if matched.is_empty() { return Ok(None); - }; - if let Some(second) = matched.next() { - let mut details = vec![first.origin.detail.as_str(), second.origin.detail.as_str()]; - details.extend(matched.map(|entry| entry.origin.detail.as_str())); + } + // Remote before local before cloud: the higher-priority kind wins a + // cross-kind collision; within one kind the name is still ambiguous. + matched.sort_by_key(|entry| kind_priority(entry.origin.kind)); + if matched.len() > 1 + && kind_priority(matched[0].origin.kind) == kind_priority(matched[1].origin.kind) + { + let details: Vec<&str> = matched + .iter() + .map(|entry| entry.origin.detail.as_str()) + .collect(); bail!("ambiguous origin `{name}`; matches: {}", details.join(", ")); } - Ok(Some(first)) + Ok(Some(matched[0])) + } +} + +/// Resolution priority when a name collides across kinds. A remote mount +/// wins over a local workspace of the same name — remote lookup ran before +/// local workspace resolution before the registry existed — and cloud is +/// reserved, resolving last. +fn kind_priority(kind: OriginKind) -> u8 { + match kind { + OriginKind::Remote => 0, + OriginKind::Local => 1, + OriginKind::Cloud => 2, } } @@ -225,6 +247,43 @@ mod tests { assert!(error.to_string().contains("ambiguous origin `return`")); } + #[test] + fn remote_mount_wins_over_colliding_local_workspace() { + // A mount alias may equal a local workspace's basename; the mount + // resolves (remote lookup ran before local workspaces pre-registry). + let registry = OriginRegistry::new(vec![ + Entry { + origin: Origin { + name: "proj".to_string(), + kind: OriginKind::Local, + current: true, + detail: "D:\\a\\proj (k1)".to_string(), + }, + reach: Reach::Local { + root: "D:\\a\\proj".to_string(), + }, + }, + Entry { + origin: Origin { + name: "proj".to_string(), + kind: OriginKind::Remote, + current: false, + detail: "alice/proj".to_string(), + }, + reach: Reach::Remote { + workspace_key: "k1".to_string(), + alias: "proj".to_string(), + }, + }, + ]); + let entry = registry.resolve("proj").expect("resolve").expect("found"); + assert_eq!(entry.origin.kind, OriginKind::Remote); + assert!(matches!( + &entry.reach, + Reach::Remote { alias, .. } if alias == "proj" + )); + } + #[test] fn resolve_returns_reach_payload_per_kind() { let registry = sample(); diff --git a/crates/sivtr-core/src/test_fixtures.rs b/crates/sivtr-core/src/test_fixtures.rs new file mode 100644 index 00000000..c84a6558 --- /dev/null +++ b/crates/sivtr-core/src/test_fixtures.rs @@ -0,0 +1,20 @@ +//! Shared on-disk git fixtures for tests that need repo/worktree layouts. + +use std::fs; +use std::path::Path; + +/// Create a normal repo (`root/.git` dir). +pub(crate) fn make_repo(root: &Path) { + fs::create_dir_all(root.join(".git")).unwrap(); +} + +/// Create a linked worktree of `main` (mirrors `git worktree add`): the +/// worktree's `.git` is a `gitdir:` pointer to `
/.git/worktrees/`, +/// whose `commondir` points back at the main `.git` dir. +pub(crate) fn make_worktree(main: &Path, wt: &Path, name: &str) { + let gitdir = main.join(".git").join("worktrees").join(name); + fs::create_dir_all(&gitdir).unwrap(); + fs::write(gitdir.join("commondir"), "../..").unwrap(); + fs::create_dir_all(wt).unwrap(); + fs::write(wt.join(".git"), format!("gitdir: {}\n", gitdir.display())).unwrap(); +} diff --git a/crates/sivtr-core/src/workspace.rs b/crates/sivtr-core/src/workspace.rs index 39a2c089..cbd32b2a 100644 --- a/crates/sivtr-core/src/workspace.rs +++ b/crates/sivtr-core/src/workspace.rs @@ -1,6 +1,7 @@ use anyhow::{Context, Result}; use chrono::{SecondsFormat, Utc}; use serde::{Deserialize, Serialize}; +use std::collections::HashSet; use std::fs; use std::path::{Path, PathBuf}; @@ -10,18 +11,14 @@ const WORKSPACES_DIR: &str = "workspaces"; pub struct WorkspaceMetadata { pub key: String, pub root: String, + /// Persisted origin alias (`sivtr origin rename`); `None` = derive from + /// the root basename. Auto-assigned unique on first sight. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub alias: Option, pub created_at: String, pub last_seen_at: String, } -/// Human origin label for a workspace: directory basename, lowercased. -/// Accepts both `/` and `\` so Windows roots still yield a useful label on Unix. -pub fn workspace_display_name(meta: &WorkspaceMetadata) -> String { - path_basename(&meta.root) - .unwrap_or(meta.key.as_str()) - .to_ascii_lowercase() -} - fn path_basename(path: &str) -> Option<&str> { let trimmed = path.trim_end_matches(['/', '\\']); if trimmed.is_empty() { @@ -160,179 +157,6 @@ pub fn list_workspaces() -> Result> { Ok(out) } -/// Outcome of [`inspect_workspace_keys`] / [`migrate_workspace_keys`]. -#[derive(Debug, Default)] -pub struct WorkspaceMigration { - /// `(old_key, new_key)` for each workspace that needs (or received) a rename. - pub migrated: Vec<(String, String)>, - /// Workspaces already on the current key scheme (no rename needed). - pub current: usize, - /// Legacy dirs whose target key already exists (duplicate of current scheme). - /// On migrate, these are merged into the current dir then removed. - pub duplicates: Vec<(String, String)>, - /// `(old_key, kept_key)` for duplicates removed during migrate. - pub removed_duplicates: Vec<(String, String)>, - /// Dirs that could not be migrated, with the reason. - pub skipped: Vec<(PathBuf, String)>, -} - -impl WorkspaceMigration { - pub fn changed(&self) -> bool { - !self.migrated.is_empty() || !self.removed_duplicates.is_empty() - } - - pub fn needs_attention(&self) -> bool { - !self.migrated.is_empty() || !self.duplicates.is_empty() || !self.skipped.is_empty() - } -} - -/// Dry-run: report which workspace dirs need re-keying without renaming them. -pub fn inspect_workspace_keys() -> Result { - scan_workspace_keys(false) -} - -/// Re-key workspace dirs whose stored root predates the absolute-based key -/// scheme. -/// -/// Legacy `workspace.json` roots were stored via `std::fs::canonicalize`, which -/// prepends a `\\?\` verbatim prefix on Windows. The current scheme derives the -/// key from `std::path::absolute` (no prefix), so legacy dirs no longer match -/// the key a fresh access computes — their captured sessions become unreachable. -/// This strips the legacy prefix, recomputes the key, and renames the dir + -/// rewrites `workspace.json` when they differ. -/// -/// If the target key already exists, unique terminal logs are copied into the -/// current dir and the legacy dir is removed. Idempotent: a second run is a -/// no-op. -pub fn migrate_workspace_keys() -> Result { - scan_workspace_keys(true) -} - -fn scan_workspace_keys(apply: bool) -> Result { - let base = data_dir().join(WORKSPACES_DIR); - let mut report = WorkspaceMigration::default(); - if !base.exists() { - return Ok(report); - } - - for entry in fs::read_dir(&base)? { - let Ok(entry) = entry else { - continue; - }; - let dir = entry.path(); - let Some(old_key) = dir.file_name().and_then(|n| n.to_str()).map(str::to_string) else { - continue; - }; - let meta_path = dir.join("workspace.json"); - let Some(mut meta) = load_workspace_metadata(&meta_path) else { - continue; - }; - - // Legacy canonicalize roots carry a `\\?\` (or `\\?\UNC\`) prefix that - // the current absolute-based scheme does not produce. Strip it to - // recompute the key the way a fresh access would. - let cleaned = strip_legacy_verbatim(&meta.root); - let new_root = - std::path::absolute(Path::new(&cleaned)).unwrap_or_else(|_| PathBuf::from(&cleaned)); - let new_root_text = new_root.to_string_lossy().to_string(); - let new_key = workspace_key(&new_root_text); - - if new_key == old_key { - // Key matches; only tidy the root field when applying migration. - if apply && meta.root != new_root_text { - meta.root = new_root_text; - let _ = write_workspace_metadata(&meta_path, &meta); - } - report.current += 1; - continue; - } - - let target = base.join(&new_key); - if target.exists() { - // Current-scheme dir already owns this root. Keep it; drop the legacy - // duplicate after copying any terminal logs the current dir lacks. - if apply { - match merge_then_remove_duplicate(&dir, &target) { - Ok(()) => report - .removed_duplicates - .push((old_key.clone(), new_key.clone())), - Err(e) => report - .skipped - .push((dir, format!("failed to remove duplicate of {new_key}: {e}"))), - } - } else { - report.duplicates.push((old_key, new_key)); - } - continue; - } - if !apply { - report.migrated.push((old_key, new_key)); - continue; - } - match fs::rename(&dir, &target) { - Ok(()) => { - meta.key = new_key.clone(); - meta.root = new_root_text; - let _ = write_workspace_metadata(&target.join("workspace.json"), &meta); - report.migrated.push((old_key, new_key)); - } - Err(e) => report.skipped.push((dir, format!("rename failed: {e}"))), - } - } - Ok(report) -} - -/// Copy any terminal logs from `legacy` that `current` lacks, then delete `legacy`. -fn merge_then_remove_duplicate(legacy: &Path, current: &Path) -> Result<()> { - let legacy_terminals = legacy.join("terminals"); - let current_terminals = current.join("terminals"); - if legacy_terminals.is_dir() { - fs::create_dir_all(¤t_terminals)?; - for entry in fs::read_dir(&legacy_terminals)? { - let entry = entry?; - let src = entry.path(); - if !src.is_file() { - continue; - } - let dest = current_terminals.join(entry.file_name()); - if !dest.exists() { - fs::copy(&src, &dest).with_context(|| { - format!("failed to copy {} -> {}", src.display(), dest.display()) - })?; - } - } - } - fs::remove_dir_all(legacy) - .with_context(|| format!("failed to remove legacy workspace {}", legacy.display()))?; - Ok(()) -} - -fn load_workspace_metadata(path: &Path) -> Option { - let text = fs::read_to_string(path).ok()?; - serde_json::from_str(&text).ok() -} - -fn write_workspace_metadata(path: &Path, meta: &WorkspaceMetadata) -> Result<()> { - if let Some(parent) = path.parent() { - fs::create_dir_all(parent)?; - } - fs::write(path, serde_json::to_string_pretty(meta)?)?; - Ok(()) -} - -/// Strip a legacy `\\?\` / `\\?\UNC\` verbatim prefix from a stored root, as -/// written by the old `canonicalize`-based scheme. Only used when reading -/// legacy `workspace.json` data during migration. -fn strip_legacy_verbatim(root: &str) -> String { - if let Some(rest) = root.strip_prefix(r"\\?\UNC\") { - format!(r"\\{rest}") - } else { - root.strip_prefix(r"\\?\") - .map(str::to_string) - .unwrap_or_else(|| root.to_string()) - } -} - pub fn terminal_session_id_from_path(path: &Path) -> String { path.file_stem() .and_then(|name| name.to_str()) @@ -346,16 +170,111 @@ fn paths_for_root(root: PathBuf) -> Result { // and resolves symlinks we don't need. `absolute` makes the path absolute // (so a relative `--cwd` still keys stably) without either side effect. let root = std::path::absolute(&root).unwrap_or(root); - let key = workspace_key(&root.to_string_lossy()); + // Workspace identity = the shared git dir (commondir), so every worktree of + // one repository resolves to the same key: main checkout + worktrees are a + // single workspace with unified terminal logs and agent sessions. + let common = repo_common_dir(&root).unwrap_or_else(|| root.clone()); + let key = workspace_key(&normalize_repo(&common)); + // Display/query root = the main checkout (commondir's parent when it is the + // `.git` dir), giving every worktree one stable name; fall back to the + // checkout itself for unusual layouts (bare dirs, submodules, …). + let display_root = if common.file_name().and_then(|name| name.to_str()) == Some(".git") { + common + .parent() + .map(Path::to_path_buf) + .unwrap_or_else(|| root.clone()) + } else { + root.clone() + }; let dir = data_dir().join(WORKSPACES_DIR).join(&key); Ok(WorkspacePaths { key, - root, + root: display_root, terminals_dir: dir.join("terminals"), dir, }) } +/// The shared git directory for a checkout: the `.git` dir itself for a normal +/// repository, or the commondir of a linked worktree. All worktrees of one repo +/// share one common dir, which is what makes repo identity stable across them. +fn repo_common_dir(root: &Path) -> Option { + let dot_git = root.join(".git"); + if dot_git.is_dir() { + return Some(real_path(&dot_git)); + } + // Linked worktree: `.git` is a file (`gitdir: `), and the shared + // config/objects/refs live in `/commondir` (main repo's `.git`). + let gitdir_line = fs::read_to_string(&dot_git).ok()?; + let gitdir = resolve_gitdir(root, gitdir_line.trim().strip_prefix("gitdir:")?.trim()); + let common = fs::read_to_string(gitdir.join("commondir")) + .ok() + .map(|text| resolve_gitdir(&gitdir, text.trim())) + .unwrap_or_else(|| gitdir.clone()); + Some(real_path(&common)) +} + +/// The real, filesystem-normalized directory. `fs::canonicalize` resolves the +/// `..` segments and symlinks in git's relative `gitdir:`/`commondir` +/// pointers, and the `\\?\` verbatim prefix it adds on Windows is stripped so +/// keys and displayed roots stay clean. Falls back to the lexical absolute +/// path when the directory no longer exists. +fn real_path(path: &Path) -> PathBuf { + fs::canonicalize(path) + .map(|canonical| { + let text = canonical.to_string_lossy(); + let cleaned = match text.strip_prefix(r"\\?\UNC\") { + Some(rest) => format!(r"\\{rest}"), + None => text.strip_prefix(r"\\?\").unwrap_or(&text).to_string(), + }; + PathBuf::from(cleaned) + }) + .unwrap_or_else(|_| absolutize(path)) +} + +/// Workspace identity of any path: the canonical commondir of the repository it +/// belongs to, lowercased with `/` separators. `None` when the path is not +/// inside any git checkout. Main repo, worktrees, and nested subdirectories of +/// one repository all yield the same identity. +pub(crate) fn repo_identity(path: &Path) -> Option { + let root = git_root(path).ok().flatten()?; + repo_common_dir(&root).map(|common| normalize_repo(&common)) +} + +fn normalize_repo(path: &Path) -> String { + path.to_string_lossy().replace('\\', "/").to_lowercase() +} + +fn absolutize(path: &Path) -> PathBuf { + std::path::absolute(path).unwrap_or_else(|_| path.to_path_buf()) +} + +fn resolve_gitdir(base: &Path, gitdir: &str) -> PathBuf { + let path = PathBuf::from(gitdir); + if path.is_absolute() { + path + } else { + base.join(path) + } +} + +/// Human origin label for a workspace: the persisted alias when set, +/// otherwise the root basename (lowercased), falling back to the key. Accepts +/// both `/` and `\` so Windows roots still yield a useful label on Unix. +pub fn workspace_alias(meta: &WorkspaceMetadata) -> String { + if let Some(alias) = meta + .alias + .as_deref() + .map(str::trim) + .filter(|alias| !alias.is_empty()) + { + return alias.to_string(); + } + path_basename(&meta.root) + .unwrap_or(meta.key.as_str()) + .to_ascii_lowercase() +} + fn ensure_workspace_metadata(paths: &WorkspacePaths) -> Result<()> { fs::create_dir_all(&paths.dir)?; let path = paths.dir.join("workspace.json"); @@ -367,6 +286,7 @@ fn ensure_workspace_metadata(paths: &WorkspacePaths) -> Result<()> { let metadata = WorkspaceMetadata { key: paths.key.clone(), root: paths.root.to_string_lossy().to_string(), + alias: assign_unique_alias(paths), created_at: now.clone(), last_seen_at: now, }; @@ -374,6 +294,52 @@ fn ensure_workspace_metadata(paths: &WorkspacePaths) -> Result<()> { Ok(()) } +/// First-seen alias for a new workspace: the root basename, or `basename-2`, +/// `basename-3`, … when that name is already taken by another workspace. +fn assign_unique_alias(paths: &WorkspacePaths) -> Option { + let base = paths + .root + .file_name() + .and_then(|name| name.to_str()) + .map(str::to_ascii_lowercase)?; + let taken: HashSet = list_workspaces() + .ok() + .into_iter() + .flatten() + .map(|meta| workspace_alias(&meta)) + .collect(); + if !taken.contains(&base) { + return Some(base); + } + (2..) + .map(|n| format!("{base}-{n}")) + .find(|candidate| !taken.contains(candidate)) +} + +/// Set a workspace's origin alias. Callers own name validation (uniqueness, +/// non-empty); this just persists the alias on the workspace owning `root`. +pub fn rename_workspace(root: &str, new_alias: &str) -> Result { + let mut updated = list_workspaces()? + .into_iter() + .find(|meta| meta.root == root) + .with_context(|| format!("no workspace with root `{root}`"))?; + updated.alias = Some(new_alias.to_string()); + let meta_path = data_dir() + .join(WORKSPACES_DIR) + .join(&updated.key) + .join("workspace.json"); + write_workspace_metadata(&meta_path, &updated)?; + Ok(updated) +} + +fn write_workspace_metadata(path: &Path, meta: &WorkspaceMetadata) -> Result<()> { + if let Some(parent) = path.parent() { + fs::create_dir_all(parent)?; + } + fs::write(path, serde_json::to_string_pretty(meta)?)?; + Ok(()) +} + fn git_root(cwd: &Path) -> Result> { let mut dir = if cwd.is_dir() { cwd.to_path_buf() @@ -418,9 +384,10 @@ fn modified_time(path: &Path) -> std::time::SystemTime { #[cfg(test)] mod tests { use super::{ - git_root, inspect_workspace_keys, migrate_workspace_keys, path_basename, - terminal_session_id_from_path, workspace_display_name, workspace_key, WorkspaceMetadata, + git_root, paths_for_root, real_path, rename_workspace, repo_identity, + terminal_session_id_from_path, workspace_alias, workspace_key, WorkspaceMetadata, }; + use crate::test_fixtures::{make_repo, make_worktree}; use std::path::{Path, PathBuf}; use std::time::{SystemTime, UNIX_EPOCH}; @@ -434,34 +401,6 @@ mod tests { dir } - fn meta(key: &str, root: &str) -> WorkspaceMetadata { - WorkspaceMetadata { - key: key.to_string(), - root: root.to_string(), - created_at: "t".to_string(), - last_seen_at: "t".to_string(), - } - } - - #[test] - fn display_name_uses_basename() { - assert_eq!( - workspace_display_name(&meta("abc", "/home/user/Coding/sivtr")), - "sivtr" - ); - assert_eq!( - workspace_display_name(&meta("abc", r"D:\Coding\sivtr")), - "sivtr" - ); - } - - #[test] - fn basename_trims_trailing_separators() { - assert_eq!(path_basename(r"D:\Coding\sivtr\"), Some("sivtr")); - assert_eq!(path_basename("/home/user/sivtr/"), Some("sivtr")); - assert_eq!(path_basename("/"), None); - } - #[test] fn workspace_key_normalizes_case_and_separators() { assert_eq!(workspace_key("D:\\sivtr"), workspace_key("d:/sivtr")); @@ -497,160 +436,164 @@ mod tests { } #[test] - fn terminal_session_id_uses_file_stem() { + fn real_path_normalizes_relative_commondir() { + // A worktree's `commondir` is relative to its gitdir and walks back + // to the main `.git`; `real_path` must collapse it to the same real + // directory on every platform (Windows `\\?\` prefix and short names + // included). + let dir = unique_test_dir("real-path"); + let repo = dir.join("repo"); + let gitdir = repo.join(".git").join("worktrees").join("stack"); + std::fs::create_dir_all(&gitdir).expect("gitdir should be created"); assert_eq!( - terminal_session_id_from_path(Path::new("session_123.jsonl")), - "session_123" + real_path(&gitdir.join("../..")), + real_path(&repo.join(".git")) ); + let _ = std::fs::remove_dir_all(dir); } #[test] - fn inspect_workspace_keys_is_dry_run_and_migrate_renames() { - let data = unique_test_dir("workspace-keys"); - let _guard = EnvGuard::set("SIVTR_DATA_DIR", &data); - - let root = unique_test_dir("legacy-root"); - let legacy_root = format!(r"\\?\{}", root.display()); - let old_key = workspace_key(&legacy_root); - let new_key = workspace_key(&root.to_string_lossy()); - assert_ne!(old_key, new_key); - - let old_dir = data.join("workspaces").join(&old_key); - std::fs::create_dir_all(&old_dir).expect("workspace dir"); - let meta = WorkspaceMetadata { - key: old_key.clone(), - root: legacy_root, - created_at: "t0".into(), - last_seen_at: "t0".into(), - }; - std::fs::write( - old_dir.join("workspace.json"), - serde_json::to_string_pretty(&meta).expect("serialize"), - ) - .expect("write meta"); - - let inspect = inspect_workspace_keys().expect("inspect"); - assert_eq!(inspect.migrated, vec![(old_key.clone(), new_key.clone())]); - assert!(old_dir.exists(), "inspect must not rename"); - - let migrate = migrate_workspace_keys().expect("migrate"); - assert_eq!(migrate.migrated, vec![(old_key, new_key.clone())]); - assert!(!old_dir.exists()); - assert!(data.join("workspaces").join(&new_key).exists()); - - let second = migrate_workspace_keys().expect("idempotent"); - assert!(second.migrated.is_empty()); - assert_eq!(second.current, 1); - - let _ = std::fs::remove_dir_all(root); - let _ = std::fs::remove_dir_all(data); + fn worktree_and_main_share_repo_identity() { + let dir = unique_test_dir("repo-identity"); + let main = dir.join("sivtr"); + let worktree = dir.join("sivtr-tui-stack"); + make_repo(&main); + make_worktree(&main, &worktree, "sivtr-tui-stack"); + + let main_identity = repo_identity(&main).expect("main repo identity"); + let worktree_identity = repo_identity(&worktree).expect("worktree identity"); + let subdir_identity = + repo_identity(&main.join("crates").join("core")).expect("subdir identity"); + assert_eq!(main_identity, worktree_identity); + assert_eq!(main_identity, subdir_identity); + + let _ = std::fs::remove_dir_all(dir); } #[test] - fn migrate_removes_legacy_duplicate_when_current_key_exists() { - let data = unique_test_dir("workspace-dup"); - let _guard = EnvGuard::set("SIVTR_DATA_DIR", &data); - - let root = unique_test_dir("dup-root"); - let legacy_root = format!(r"\\?\{}", root.display()); - let old_key = workspace_key(&legacy_root); - let new_key = workspace_key(&root.to_string_lossy()); - assert_ne!(old_key, new_key); - - let old_dir = data.join("workspaces").join(&old_key); - let new_dir = data.join("workspaces").join(&new_key); - std::fs::create_dir_all(old_dir.join("terminals")).expect("legacy terminals"); - std::fs::create_dir_all(new_dir.join("terminals")).expect("current terminals"); - - std::fs::write( - old_dir.join("workspace.json"), - serde_json::to_string_pretty(&WorkspaceMetadata { - key: old_key.clone(), - root: legacy_root, - created_at: "t0".into(), - last_seen_at: "t0".into(), - }) - .expect("serialize legacy"), - ) - .expect("write legacy meta"); - std::fs::write( - new_dir.join("workspace.json"), - serde_json::to_string_pretty(&WorkspaceMetadata { - key: new_key.clone(), - root: root.to_string_lossy().to_string(), - created_at: "t0".into(), - last_seen_at: "t0".into(), - }) - .expect("serialize current"), - ) - .expect("write current meta"); + fn different_repos_have_different_identity() { + let dir = unique_test_dir("repo-identity-diff"); + let first = dir.join("sivtr"); + let second = dir.join("md-dragger"); + make_repo(&first); + make_repo(&second); + + let first_identity = repo_identity(&first).expect("first identity"); + let second_identity = repo_identity(&second).expect("second identity"); + assert_ne!(first_identity, second_identity); + + let _ = std::fs::remove_dir_all(dir); + } - // Unique log only in legacy, shared name already in current. - std::fs::write( - old_dir.join("terminals").join("only-old.jsonl"), - b"old-only", - ) - .expect("legacy unique log"); - std::fs::write( - old_dir.join("terminals").join("shared.jsonl"), - b"legacy-shared", - ) - .expect("legacy shared log"); - std::fs::write( - new_dir.join("terminals").join("shared.jsonl"), - b"current-shared", - ) - .expect("current shared log"); + #[test] + fn worktree_and_main_share_workspace_key() { + let dir = unique_test_dir("workspace-key"); + let main = dir.join("sivtr"); + let worktree = dir.join("sivtr-tui-stack"); + make_repo(&main); + make_worktree(&main, &worktree, "sivtr-tui-stack"); + + let main_paths = paths_for_root(main.clone()).expect("main paths"); + let worktree_paths = paths_for_root(worktree.clone()).expect("worktree paths"); + assert_eq!(main_paths.key, worktree_paths.key); + // Display root collapses to the main checkout for both (compared + // through the real path: `canonicalize` resolves short names on + // Windows). + let main_root = real_path(&main); + assert_eq!(main_paths.root, main_root); + assert_eq!(worktree_paths.root, main_root); + + let _ = std::fs::remove_dir_all(dir); + } - let inspect = inspect_workspace_keys().expect("inspect"); - assert_eq!(inspect.duplicates, vec![(old_key.clone(), new_key.clone())]); - assert!(old_dir.exists(), "inspect must not delete"); + #[test] + fn non_repo_path_has_no_identity() { + let dir = unique_test_dir("repo-identity-none"); + assert_eq!(repo_identity(&dir), None); + let _ = std::fs::remove_dir_all(dir); + } - let migrate = migrate_workspace_keys().expect("migrate"); - assert_eq!(migrate.removed_duplicates, vec![(old_key, new_key.clone())]); - assert!(!old_dir.exists(), "legacy duplicate removed"); - assert_eq!( - std::fs::read_to_string(new_dir.join("terminals").join("only-old.jsonl")) - .expect("unique log copied"), - "old-only" - ); + #[test] + fn terminal_session_id_uses_file_stem() { assert_eq!( - std::fs::read_to_string(new_dir.join("terminals").join("shared.jsonl")) - .expect("shared log kept"), - "current-shared" + terminal_session_id_from_path(Path::new("session_123.jsonl")), + "session_123" ); - - let _ = std::fs::remove_dir_all(root); - let _ = std::fs::remove_dir_all(data); } - struct EnvGuard { - key: &'static str, - previous: Option, - _lock: std::sync::MutexGuard<'static, ()>, + #[test] + fn display_name_uses_basename_without_alias() { + let unix = WorkspaceMetadata { + key: "abc".into(), + root: "/home/user/Coding/sivtr".into(), + alias: None, + created_at: "t".into(), + last_seen_at: "t".into(), + }; + assert_eq!(workspace_alias(&unix), "sivtr"); + + let windows = WorkspaceMetadata { + key: "abc".into(), + root: r"D:\Coding\sivtr".into(), + alias: None, + created_at: "t".into(), + last_seen_at: "t".into(), + }; + assert_eq!(workspace_alias(&windows), "sivtr"); } - impl EnvGuard { - fn set(key: &'static str, value: &Path) -> Self { - // Env mutation is process-global; serialize every env-touching test. - let _lock = crate::test_env_lock(); - let previous = std::env::var_os(key); - // SAFETY: test-only temporary env mutation, restored in Drop, guarded by the lock. - unsafe { std::env::set_var(key, value) }; - Self { - key, - previous, - _lock, - } - } + #[test] + fn display_name_prefers_persisted_alias() { + let meta = WorkspaceMetadata { + key: "abc".into(), + root: "D:\\Coding\\sivtr-tui-stack".into(), + alias: Some("sivtr".into()), + created_at: "t".into(), + last_seen_at: "t".into(), + }; + assert_eq!(workspace_alias(&meta), "sivtr"); } - impl Drop for EnvGuard { - fn drop(&mut self) { - match &self.previous { - Some(value) => unsafe { std::env::set_var(self.key, value) }, - None => unsafe { std::env::remove_var(self.key) }, - } + #[test] + fn rename_workspace_persists_alias() { + let _guard = crate::test_env_lock(); + let data = unique_test_dir("rename-data"); + let previous = std::env::var_os("SIVTR_DATA_DIR"); + // SAFETY: test-only env mutation, guarded by the shared test lock. + unsafe { std::env::set_var("SIVTR_DATA_DIR", &data) }; + + let repo = unique_test_dir("rename-ws").join("sivtr"); + make_repo(&repo); + let paths = paths_for_root(repo).expect("paths"); + std::fs::create_dir_all(&paths.dir).expect("workspace dir"); + let now = "t"; + let meta = WorkspaceMetadata { + key: paths.key.clone(), + root: paths.root.to_string_lossy().to_string(), + alias: Some("sivtr".into()), + created_at: now.into(), + last_seen_at: now.into(), + }; + let meta_path = paths.dir.join("workspace.json"); + std::fs::write( + &meta_path, + serde_json::to_string_pretty(&meta).expect("json"), + ) + .expect("write meta"); + + let root = paths.root.to_string_lossy().to_string(); + let renamed = rename_workspace(&root, "core").expect("rename"); + assert_eq!(renamed.alias.as_deref(), Some("core")); + let persisted: WorkspaceMetadata = + serde_json::from_str(&std::fs::read_to_string(&meta_path).expect("read meta")) + .expect("parse meta"); + assert_eq!(persisted.alias.as_deref(), Some("core")); + + match previous { + Some(value) => unsafe { std::env::set_var("SIVTR_DATA_DIR", value) }, + None => unsafe { std::env::remove_var("SIVTR_DATA_DIR") }, } + let _ = std::fs::remove_dir_all(data); + let _ = std::fs::remove_dir_all(&paths.dir); } } diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 4409f2aa..e259b7b8 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -361,6 +361,10 @@ pub enum Commands { #[command(visible_alias = "g")] Group(GroupCommand), + /// Manage source origins: one name for every addressable memory source + /// (local workspace alias or remote mount), renamed through one command + Origin(OriginCommand), + /// Manage configuration Config(ConfigCommand), diff --git a/src/cli/remote.rs b/src/cli/remote.rs index 244b9eaa..dba91c3d 100644 --- a/src/cli/remote.rs +++ b/src/cli/remote.rs @@ -126,6 +126,24 @@ pub enum WorkspaceAction { List, } +#[derive(Parser, Debug)] +pub struct OriginCommand { + #[command(subcommand)] + pub action: OriginAction, +} + +#[derive(Subcommand, Debug)] +pub enum OriginAction { + /// Rename any origin by name: a local workspace alias or a remote mount + /// (unified — one command for both, resolved through the origin registry). + Rename { + /// Current origin name (from `sivtr ws list` / `sivtr remote list`) + name: String, + /// New name used in `name:body` refs + new_name: String, + }, +} + #[derive(Parser, Debug)] pub struct GroupCommand { #[command(subcommand)] diff --git a/src/commands/memory/copy/mod.rs b/src/commands/memory/copy/mod.rs index 40528e35..e6575a70 100644 --- a/src/commands/memory/copy/mod.rs +++ b/src/commands/memory/copy/mod.rs @@ -18,7 +18,9 @@ use sivtr_core::record::WorkRecord; use crate::commands::browse; use crate::output; -use crate::tui::workspace::{WorkspaceFocus, WorkspaceSession, WorkspaceSource}; +use crate::tui::workspace::{ + WorkspaceFocus, WorkspaceSession, WorkspaceSource, WorkspaceSourceKind, +}; use export::finish_text_pairs; use load::load_for_plan; @@ -113,10 +115,15 @@ fn execute_pick(plan: &CopyPlan) -> Result<()> { fn session_source_from_records(records: &[WorkRecord]) -> Option { let record = records.first()?; - if let Some(provider) = record.work_ref.provider() { - Some(WorkspaceSource::agent(provider)) - } else { - Some(WorkspaceSource::terminal()) + let kind = match record.work_ref.provider() { + Some(provider) => WorkspaceSourceKind::Agent(provider), + None => WorkspaceSourceKind::Terminal, + }; + // The records keep their named scope (`desk:`, `team/alice:`), so a + // remote pick renders with the remote origin, not a local glyph. + match record.work_ref.scope_name() { + Some(scope) => Some(WorkspaceSource::remote(scope, kind)), + None => Some(WorkspaceSource::local(kind)), } } diff --git a/src/commands/memory/workset/source.rs b/src/commands/memory/workset/source.rs index a5b9262f..dd7f698a 100644 --- a/src/commands/memory/workset/source.rs +++ b/src/commands/memory/workset/source.rs @@ -12,6 +12,7 @@ use sivtr_core::record::{expand_source, WorkPath, WorkRecord, WorkRef}; use crate::commands::memory::filter::{self, Filter}; use crate::commands::memory::records::warn_skipped; +use crate::commands::remote::serve; use crate::output; use super::WorkSet; @@ -100,11 +101,14 @@ pub fn query(source: &str, filter: Filter, cwd: Option<&Path>) -> Result match &entry.reach { @@ -270,6 +274,8 @@ fn query_remote_bounded( if path.is_empty() || path.starts_with('/') || scope.eq_ignore_ascii_case("local") { return query(selector, filter, Some(cwd)); } + // Remote mounts need the daemon; start it before the passive lookup. + serve::ensure_running()?; let registry = crate::origins::collect(cwd)?; let Some(entry) = registry.resolve(scope)? else { return query(selector, filter, Some(cwd)); diff --git a/src/commands/remote/mod.rs b/src/commands/remote/mod.rs index c6bd06b5..8a67f93b 100644 --- a/src/commands/remote/mod.rs +++ b/src/commands/remote/mod.rs @@ -1,5 +1,6 @@ pub mod group; pub mod mounts; +pub mod origin; pub mod peer; pub mod serve; pub mod share; diff --git a/src/commands/remote/origin.rs b/src/commands/remote/origin.rs new file mode 100644 index 00000000..a7f3a059 --- /dev/null +++ b/src/commands/remote/origin.rs @@ -0,0 +1,19 @@ +//! `sivtr origin rename` — one rename path for every addressable source. + +use anyhow::{Context, Result}; + +use crate::cli::{OriginAction, OriginCommand}; +use crate::output; + +pub fn execute(command: OriginCommand) -> Result<()> { + match command.action { + OriginAction::Rename { name, new_name } => rename(&name, &new_name), + } +} + +fn rename(name: &str, new_name: &str) -> Result<()> { + let cwd = std::env::current_dir().context("Failed to resolve current directory")?; + let updated = crate::origins::rename(&cwd, name, new_name)?; + output::success(format!("renamed origin `{name}` to `{updated}`")); + Ok(()) +} diff --git a/src/commands/remote/share.rs b/src/commands/remote/share.rs index f869bcc4..b4659890 100644 --- a/src/commands/remote/share.rs +++ b/src/commands/remote/share.rs @@ -9,7 +9,6 @@ use crate::commands::interactive; use crate::output; use crate::remote::ipc; use crate::remote::protocol::{LocalRequest, LocalResponse, ShareInfo}; -use sivtr_core::workspace::workspace_display_name; use super::serve; @@ -138,6 +137,7 @@ pub(crate) fn list_workspace_choices() -> Result> { WorkspaceMetadata { key: paths.key.clone(), root: paths.root.display().to_string(), + alias: None, created_at: String::new(), last_seen_at: String::new(), }, @@ -163,7 +163,7 @@ pub(crate) fn list_workspace_choices() -> Result> { .map(|meta| { let current = Some(meta.key.as_str()) == current_key; WorkspaceChoice { - name: workspace_display_name(&meta), + name: workspace::workspace_alias(&meta), key: meta.key, root: meta.root, current, diff --git a/src/commands/system/doctor.rs b/src/commands/system/doctor.rs index dab16eb7..14fe444e 100644 --- a/src/commands/system/doctor.rs +++ b/src/commands/system/doctor.rs @@ -92,7 +92,6 @@ impl Report { self.check_config(fix); self.check_session_dir(); self.check_shell_hooks(fix); - self.check_workspace_keys(fix); self.check_agent_hosts(); self.check_mcp_registration(fix); self.check_skill(fix); @@ -261,139 +260,6 @@ impl Report { } } - fn check_workspace_keys(&mut self, fix: bool) { - let result = if fix { - workspace::migrate_workspace_keys() - } else { - workspace::inspect_workspace_keys() - }; - match result { - Ok(report) => { - if !report.needs_attention() && report.removed_duplicates.is_empty() { - self.add(Check { - name: "workspace_keys", - label: "workspace keys", - status: Status::Pass, - detail: format!("{} workspace(s) on current scheme", report.current), - hint: None, - }); - return; - } - - if fix - && (!report.migrated.is_empty() || !report.removed_duplicates.is_empty()) - && report.skipped.is_empty() - { - let mut parts = Vec::new(); - if !report.migrated.is_empty() { - parts.push(format!("migrated {}", report.migrated.len())); - } - if !report.removed_duplicates.is_empty() { - parts.push(format!( - "removed {} duplicate(s)", - report.removed_duplicates.len() - )); - } - self.add(Check { - name: "workspace_keys", - label: "workspace keys", - status: Status::Fixed, - detail: parts.join(", "), - hint: None, - }); - return; - } - - if !report.migrated.is_empty() { - self.add(Check { - name: "workspace_keys", - label: "workspace keys", - status: Status::Fail, - detail: format!("{} workspace(s) need migration", report.migrated.len()), - hint: Some("run `sivtr doctor --fix`".to_string()), - }); - return; - } - - if !report.duplicates.is_empty() { - let samples: Vec = report - .duplicates - .iter() - .take(3) - .map(|(old, new)| format!("{old} -> {new}")) - .collect(); - let more = if report.duplicates.len() > 3 { - format!(", +{} more", report.duplicates.len() - 3) - } else { - String::new() - }; - self.add(Check { - name: "workspace_keys", - label: "workspace keys", - status: Status::Fail, - detail: format!( - "{} legacy duplicate(s) ({}{more})", - report.duplicates.len(), - samples.join("; ") - ), - hint: Some( - "run `sivtr doctor --fix` to merge unique logs and remove legacy dirs" - .to_string(), - ), - }); - return; - } - - if !report.skipped.is_empty() { - let reasons: Vec = report - .skipped - .iter() - .take(3) - .map(|(path, reason)| { - let name = path - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or("workspace"); - format!("{name}: {reason}") - }) - .collect(); - let more = if report.skipped.len() > 3 { - format!(", +{} more", report.skipped.len() - 3) - } else { - String::new() - }; - self.add(Check { - name: "workspace_keys", - label: "workspace keys", - status: Status::Manual, - detail: format!( - "{} workspace(s) could not be migrated ({reasons}{more})", - report.skipped.len(), - reasons = reasons.join("; ") - ), - hint: None, - }); - return; - } - - self.add(Check { - name: "workspace_keys", - label: "workspace keys", - status: Status::Pass, - detail: format!("{} workspace(s) on current scheme", report.current), - hint: None, - }); - } - Err(e) => self.add(Check { - name: "workspace_keys", - label: "workspace keys", - status: Status::Manual, - detail: format!("migration check failed: {e}"), - hint: None, - }), - } - } - fn check_skill(&mut self, fix: bool) { if skill::is_installed() { self.add(Check { diff --git a/src/commands/system/setup.rs b/src/commands/system/setup.rs index b254d68f..8f0042aa 100644 --- a/src/commands/system/setup.rs +++ b/src/commands/system/setup.rs @@ -31,15 +31,6 @@ pub fn execute() -> Result<()> { }, )?; - run_step("migrating legacy workspace keys", || { - let report = workspace::migrate_workspace_keys()?; - if report.migrated.is_empty() { - Ok(format!("{} workspace(s) on current scheme", report.current)) - } else { - Ok(format!("migrated {} workspace(s)", report.migrated.len())) - } - })?; - if !mcp_targets.is_empty() { run_step("installing MCP for selected agent hosts", || { for target in &mcp_targets { diff --git a/src/lib.rs b/src/lib.rs index 5442ad56..01608b17 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -102,6 +102,9 @@ fn run() -> Result<()> { Some(Commands::Group(cmd)) => { commands::remote::group::execute(cmd)?; } + Some(Commands::Origin(cmd)) => { + commands::remote::origin::execute(cmd)?; + } Some(Commands::Hotkey(cmd)) => { commands::system::hotkey::execute(cmd)?; } diff --git a/src/origins.rs b/src/origins.rs index 1fa4440f..04748bfb 100644 --- a/src/origins.rs +++ b/src/origins.rs @@ -7,7 +7,7 @@ //! already knew. Upper layers consume the registry — new sources add a //! constructor block here, and [`Origin`] itself never changes. -use anyhow::Result; +use anyhow::{bail, Context, Result}; use std::path::Path; use sivtr_core::origin::{Entry, Origin, OriginKind, OriginRegistry, Reach}; @@ -32,7 +32,7 @@ pub fn collect(cwd: &Path) -> Result { let current = current_key.as_deref() == Some(meta.key.as_str()); entries.push(Entry { origin: Origin { - name: workspace::workspace_display_name(&meta), + name: workspace::workspace_alias(&meta), kind: OriginKind::Local, current, detail: format!("{} ({})", meta.root, meta.key), @@ -42,27 +42,32 @@ pub fn collect(cwd: &Path) -> Result { } if let Some(workspace_key) = current_key.as_deref() { - serve::ensure_running()?; - match ipc::call(LocalRequest::RemoteList { - workspace_key: workspace_key.to_string(), - })? { - LocalResponse::Mounts(mounts) => { - for mount in mounts { - entries.push(Entry { - origin: Origin { - name: mount.alias.clone(), - kind: OriginKind::Remote, - current: false, - detail: format!("{}/{}", mount.peer_name, mount.share_name), - }, - reach: Reach::Remote { - workspace_key: workspace_key.to_string(), - alias: mount.alias, - }, - }); + // Passive enumeration: mounts are listed only while the daemon is + // already running. Read-only callers (`ws list`, `sivtr_status`) must + // not start the daemon; query paths start it explicitly before + // collecting, so a scoped query still sees its mounts. + if ipc::running() { + match ipc::call(LocalRequest::RemoteList { + workspace_key: workspace_key.to_string(), + })? { + LocalResponse::Mounts(mounts) => { + for mount in mounts { + entries.push(Entry { + origin: Origin { + name: mount.alias.clone(), + kind: OriginKind::Remote, + current: false, + detail: format!("{}/{}", mount.peer_name, mount.share_name), + }, + reach: Reach::Remote { + workspace_key: workspace_key.to_string(), + alias: mount.alias, + }, + }); + } } + response => anyhow::bail!("Unexpected daemon response: {response:?}"), } - response => anyhow::bail!("Unexpected daemon response: {response:?}"), } } @@ -70,3 +75,49 @@ pub fn collect(cwd: &Path) -> Result { Ok(OriginRegistry::new(entries)) } + +/// Rename any origin by its current name, resolving through the registry so +/// local workspace aliases and remote mount aliases share one path. Fails when +/// the new name is empty or already belongs to another origin. +pub fn rename(cwd: &Path, name: &str, new_name: &str) -> Result { + // A rename may address a remote mount, which only enters the registry + // while the daemon is running. + serve::ensure_running()?; + let registry = collect(cwd)?; + let entry = registry.resolve(name)?.with_context(|| { + format!("no origin named `{name}`; use `sivtr ws list` / `sivtr remote list`") + })?; + let new_name = new_name.trim().to_ascii_lowercase(); + if new_name.is_empty() { + bail!("new name must not be empty"); + } + if new_name == entry.origin.name.to_ascii_lowercase() { + return Ok(entry.origin.name.clone()); + } + // The new name must not belong to any other origin (local or remote). + if let Some(other) = registry.resolve(&new_name)? { + bail!( + "origin `{new_name}` already exists ({})", + other.origin.detail + ); + } + + match &entry.reach { + Reach::Local { root } => { + let updated = workspace::rename_workspace(root, &new_name)?; + Ok(workspace::workspace_alias(&updated)) + } + Reach::Remote { + workspace_key, + alias, + } => match ipc::call(LocalRequest::RemoteRename { + workspace_key: workspace_key.clone(), + alias: alias.clone(), + new_alias: new_name.clone(), + })? { + LocalResponse::Mount(mount) => Ok(mount.alias), + response => bail!("Unexpected daemon response: {response:?}"), + }, + Reach::Cloud => bail!("cloud origins are reserved"), + } +} From 244e6283dd492d5445f3bd37f5b8e9e48ab681a6 Mon Sep 17 00:00:00 2001 From: Ariestar <1941137088@qq.com> Date: Sat, 15 Aug 2026 20:07:02 +0800 Subject: [PATCH 06/10] fix(workspace): re-key legacy workspace dirs Keys were derived from the checkout root; since the commondir change every checkout of one repository resolves to the shared git dir, stored roots must be re-keyed or their sessions become unreachable. sivtr doctor now reports legacy dirs; --fix re-keys them and merges worktree terminals into the shared key, idempotently. --- crates/sivtr-core/src/workspace.rs | 233 +++++++++++++++++++++++++++-- src/commands/system/doctor.rs | 64 ++++++++ 2 files changed, 282 insertions(+), 15 deletions(-) diff --git a/crates/sivtr-core/src/workspace.rs b/crates/sivtr-core/src/workspace.rs index cbd32b2a..916a3cfe 100644 --- a/crates/sivtr-core/src/workspace.rs +++ b/crates/sivtr-core/src/workspace.rs @@ -170,10 +170,22 @@ fn paths_for_root(root: PathBuf) -> Result { // and resolves symlinks we don't need. `absolute` makes the path absolute // (so a relative `--cwd` still keys stably) without either side effect. let root = std::path::absolute(&root).unwrap_or(root); - // Workspace identity = the shared git dir (commondir), so every worktree of - // one repository resolves to the same key: main checkout + worktrees are a - // single workspace with unified terminal logs and agent sessions. - let common = repo_common_dir(&root).unwrap_or_else(|| root.clone()); + let (key, display_root) = workspace_identity(&root); + let dir = data_dir().join(WORKSPACES_DIR).join(&key); + Ok(WorkspacePaths { + key, + root: display_root, + terminals_dir: dir.join("terminals"), + dir, + }) +} + +/// The identity a checkout resolves to today: its workspace key and display +/// root, both derived from the shared git dir (commondir), so every worktree +/// of one repository resolves to the same key. Migration re-keys stored roots +/// with the same function so a migrated workspace matches a fresh lookup. +fn workspace_identity(root: &Path) -> (String, PathBuf) { + let common = repo_common_dir(root).unwrap_or_else(|| root.to_path_buf()); let key = workspace_key(&normalize_repo(&common)); // Display/query root = the main checkout (commondir's parent when it is the // `.git` dir), giving every worktree one stable name; fall back to the @@ -182,17 +194,11 @@ fn paths_for_root(root: PathBuf) -> Result { common .parent() .map(Path::to_path_buf) - .unwrap_or_else(|| root.clone()) + .unwrap_or_else(|| root.to_path_buf()) } else { - root.clone() + root.to_path_buf() }; - let dir = data_dir().join(WORKSPACES_DIR).join(&key); - Ok(WorkspacePaths { - key, - root: display_root, - terminals_dir: dir.join("terminals"), - dir, - }) + (key, display_root) } /// The shared git directory for a checkout: the `.git` dir itself for a normal @@ -340,6 +346,132 @@ fn write_workspace_metadata(path: &Path, meta: &WorkspaceMetadata) -> Result<()> Ok(()) } +/// Outcome of [`inspect_workspace_keys`] / [`migrate_workspace_keys`]. +#[derive(Debug, Default)] +pub struct WorkspaceMigration { + /// `(old_key, new_key)` dirs re-keyed (or needing one). + pub migrated: Vec<(String, String)>, + /// `(old_key, new_key)` legacy dirs merged into the current key's dir. + pub merged: Vec<(String, String)>, + /// Dirs that could not be migrated, with the reason. + pub skipped: Vec<(PathBuf, String)>, + /// Workspaces already on the current (commondir) key scheme. + pub current: usize, +} + +/// Dry-run: report which workspace dirs need re-keying without renaming. +pub fn inspect_workspace_keys() -> Result { + scan_workspace_keys(false) +} + +/// Re-key workspace dirs to the commondir scheme (run via `sivtr doctor --fix`). +/// +/// Keys were derived from the checkout root; since the commondir change every +/// checkout of one repository resolves to the shared git dir instead, so +/// stored roots must be re-keyed or their captured sessions become +/// unreachable. Worktrees and the main checkout of one repo converge on one +/// key, merging terminal logs. Idempotent: a second run is a no-op. +pub fn migrate_workspace_keys() -> Result { + scan_workspace_keys(true) +} + +fn scan_workspace_keys(apply: bool) -> Result { + let base = data_dir().join(WORKSPACES_DIR); + let mut report = WorkspaceMigration::default(); + if !base.exists() { + return Ok(report); + } + for entry in fs::read_dir(&base)? { + let Ok(entry) = entry else { + continue; + }; + let dir = entry.path(); + let Some(old_key) = dir.file_name().and_then(|n| n.to_str()).map(str::to_string) else { + continue; + }; + let meta_path = dir.join("workspace.json"); + let Some(mut meta) = load_workspace_metadata(&meta_path) else { + continue; + }; + let (new_key, display_root) = workspace_identity(Path::new(&meta.root)); + if new_key == old_key { + report.current += 1; + continue; + } + let target = base.join(&new_key); + if target.exists() { + if apply { + match merge_workspace_dir(&dir, &target) { + Ok(()) => report.merged.push((old_key, new_key)), + Err(e) => report + .skipped + .push((dir, format!("failed to merge into {new_key}: {e}"))), + } + } else { + report.merged.push((old_key, new_key)); + } + continue; + } + if !apply { + report.migrated.push((old_key, new_key)); + continue; + } + match fs::rename(&dir, &target) { + Ok(()) => { + meta.key = new_key.clone(); + meta.root = display_root.to_string_lossy().to_string(); + let _ = write_workspace_metadata(&target.join("workspace.json"), &meta); + report.migrated.push((old_key, new_key)); + } + Err(e) => report.skipped.push((dir, format!("rename failed: {e}"))), + } + } + Ok(report) +} + +/// Merge a legacy workspace dir into the current-key dir: copy terminal logs +/// the target lacks (and the source alias when the target has none), then +/// remove the legacy dir. +fn merge_workspace_dir(legacy: &Path, current: &Path) -> Result<()> { + let legacy_terminals = legacy.join("terminals"); + let current_terminals = current.join("terminals"); + if legacy_terminals.is_dir() { + fs::create_dir_all(¤t_terminals)?; + for entry in fs::read_dir(&legacy_terminals)? { + let entry = entry?; + let src = entry.path(); + if !src.is_file() { + continue; + } + let dest = current_terminals.join(entry.file_name()); + if !dest.exists() { + fs::copy(&src, &dest).with_context(|| { + format!("failed to copy {} -> {}", src.display(), dest.display()) + })?; + } + } + } + let current_meta_path = current.join("workspace.json"); + if let (Some(current_meta), Some(legacy_meta)) = ( + load_workspace_metadata(¤t_meta_path), + load_workspace_metadata(&legacy.join("workspace.json")), + ) { + if current_meta.alias.is_none() && legacy_meta.alias.is_some() { + let mut updated = current_meta; + updated.alias = legacy_meta.alias; + write_workspace_metadata(¤t_meta_path, &updated)?; + } + } + fs::remove_dir_all(legacy) + .with_context(|| format!("failed to remove legacy workspace {}", legacy.display()))?; + Ok(()) +} + +fn load_workspace_metadata(path: &Path) -> Option { + let text = fs::read_to_string(path).ok()?; + serde_json::from_str(&text).ok() +} + fn git_root(cwd: &Path) -> Result> { let mut dir = if cwd.is_dir() { cwd.to_path_buf() @@ -384,8 +516,9 @@ fn modified_time(path: &Path) -> std::time::SystemTime { #[cfg(test)] mod tests { use super::{ - git_root, paths_for_root, real_path, rename_workspace, repo_identity, - terminal_session_id_from_path, workspace_alias, workspace_key, WorkspaceMetadata, + git_root, inspect_workspace_keys, migrate_workspace_keys, paths_for_root, real_path, + rename_workspace, repo_identity, terminal_session_id_from_path, workspace_alias, + workspace_identity, workspace_key, WorkspaceMetadata, }; use crate::test_fixtures::{make_repo, make_worktree}; use std::path::{Path, PathBuf}; @@ -513,6 +646,76 @@ mod tests { let _ = std::fs::remove_dir_all(dir); } + #[test] + fn migrate_rekeys_root_based_keys_and_merges_worktree_logs() { + let _lock = crate::test_env_lock(); + let data = unique_test_dir("workspace-migrate"); + let old_data = std::env::var_os("SIVTR_DATA_DIR"); + std::env::set_var("SIVTR_DATA_DIR", &data); + + let main = unique_test_dir("repo-main"); + let worktree = unique_test_dir("repo-worktree"); + make_repo(&main); + make_worktree(&main, &worktree, "wt"); + + let new_key = workspace_identity(&main).0; + let main_old_key = workspace_key(&main.to_string_lossy()); + let wt_old_key = workspace_key(&worktree.to_string_lossy()); + assert_ne!(new_key, main_old_key); + assert_ne!(new_key, wt_old_key); + + let workspaces = data.join("workspaces"); + let main_dir = workspaces.join(&main_old_key); + let wt_dir = workspaces.join(&wt_old_key); + std::fs::create_dir_all(main_dir.join("terminals")).expect("main terminals"); + std::fs::create_dir_all(wt_dir.join("terminals")).expect("wt terminals"); + std::fs::write(main_dir.join("terminals/s1.jsonl"), "x").expect("s1"); + std::fs::write(wt_dir.join("terminals/s2.jsonl"), "y").expect("s2"); + for (dir, key, root) in [ + (&main_dir, &main_old_key, &main), + (&wt_dir, &wt_old_key, &worktree), + ] { + let meta = WorkspaceMetadata { + key: key.clone(), + root: root.to_string_lossy().to_string(), + alias: None, + created_at: "t0".into(), + last_seen_at: "t0".into(), + }; + std::fs::write( + dir.join("workspace.json"), + serde_json::to_string_pretty(&meta).expect("serialize"), + ) + .expect("write meta"); + } + + let inspect = inspect_workspace_keys().expect("inspect"); + assert_eq!(inspect.migrated.len(), 2); + assert!(main_dir.exists(), "inspect must not rename"); + + let migrated = migrate_workspace_keys().expect("migrate"); + assert_eq!(migrated.migrated.len() + migrated.merged.len(), 2); + + // Both checkouts converge on the commondir key with logs merged. + let merged = workspaces.join(&new_key); + assert!(merged.join("terminals/s1.jsonl").exists()); + assert!(merged.join("terminals/s2.jsonl").exists()); + assert!(!main_dir.exists()); + assert!(!wt_dir.exists()); + + // Idempotent: a second run is a no-op. + let second = migrate_workspace_keys().expect("idempotent"); + assert!(second.migrated.is_empty() && second.merged.is_empty()); + assert_eq!(second.current, 1); + + match old_data { + Some(value) => std::env::set_var("SIVTR_DATA_DIR", value), + None => std::env::remove_var("SIVTR_DATA_DIR"), + } + let _ = std::fs::remove_dir_all(main); + let _ = std::fs::remove_dir_all(data); + } + #[test] fn terminal_session_id_uses_file_stem() { assert_eq!( diff --git a/src/commands/system/doctor.rs b/src/commands/system/doctor.rs index 14fe444e..ac0c4d3a 100644 --- a/src/commands/system/doctor.rs +++ b/src/commands/system/doctor.rs @@ -92,6 +92,7 @@ impl Report { self.check_config(fix); self.check_session_dir(); self.check_shell_hooks(fix); + self.check_workspace_keys(fix); self.check_agent_hosts(); self.check_mcp_registration(fix); self.check_skill(fix); @@ -260,6 +261,69 @@ impl Report { } } + /// Workspaces whose stored roots predate the commondir key scheme become + /// unreachable (their captured sessions vanish from queries). `--fix` + /// re-keys and merges them; without it the check reports what needs it. + fn check_workspace_keys(&mut self, fix: bool) { + let result = if fix { + workspace::migrate_workspace_keys() + } else { + workspace::inspect_workspace_keys() + }; + let report = match result { + Ok(report) => report, + Err(e) => { + self.add(Check { + name: "workspace_keys", + label: "workspace keys", + status: Status::Manual, + detail: format!("migration check failed: {e}"), + hint: None, + }); + return; + } + }; + let mut pending = Vec::new(); + if !report.migrated.is_empty() { + pending.push(format!("{} to re-key", report.migrated.len())); + } + if !report.merged.is_empty() { + pending.push(format!("{} to merge", report.merged.len())); + } + if !pending.is_empty() { + self.add(Check { + name: "workspace_keys", + label: "workspace keys", + status: if fix { Status::Fixed } else { Status::Fail }, + detail: pending.join(", "), + hint: if fix { + None + } else { + Some("run `sivtr doctor --fix`".to_string()) + }, + }); + } else if !report.skipped.is_empty() { + self.add(Check { + name: "workspace_keys", + label: "workspace keys", + status: Status::Manual, + detail: format!( + "{} workspace(s) could not be migrated", + report.skipped.len() + ), + hint: None, + }); + } else { + self.add(Check { + name: "workspace_keys", + label: "workspace keys", + status: Status::Pass, + detail: format!("{} workspace(s) on current scheme", report.current), + hint: None, + }); + } + } + fn check_skill(&mut self, fix: bool) { if skill::is_installed() { self.add(Check { From 3d3572b54c889cfc7e5d82cfdeb59316c4dfd316 Mon Sep 17 00:00:00 2001 From: Ariestar <1941137088@qq.com> Date: Sat, 15 Aug 2026 20:07:14 +0800 Subject: [PATCH 07/10] fix(memory): gate remote glyph on registry mounts The copy plan rendered every named scope with the remote glyph; only a registry-confirmed remote mount is remote now - local aliases (docs:) and groups stay on the local style. Scoped queries no longer force-start the daemon; remote and group paths start it themselves. --- src/commands/memory/copy/mod.rs | 24 ++++++++++++++++++------ src/commands/memory/workset/source.rs | 5 ++--- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/commands/memory/copy/mod.rs b/src/commands/memory/copy/mod.rs index e6575a70..2ba6332f 100644 --- a/src/commands/memory/copy/mod.rs +++ b/src/commands/memory/copy/mod.rs @@ -14,6 +14,7 @@ pub use plan::{parse_address_dialogues, CopyFilters, CopyPlan, Projection}; use anyhow::{Context, Result}; use sivtr_core::ai::AgentProvider; +use sivtr_core::origin::OriginKind; use sivtr_core::record::WorkRecord; use crate::commands::browse; @@ -119,12 +120,23 @@ fn session_source_from_records(records: &[WorkRecord]) -> Option WorkspaceSourceKind::Agent(provider), None => WorkspaceSourceKind::Terminal, }; - // The records keep their named scope (`desk:`, `team/alice:`), so a - // remote pick renders with the remote origin, not a local glyph. - match record.work_ref.scope_name() { - Some(scope) => Some(WorkspaceSource::remote(scope, kind)), - None => Some(WorkspaceSource::local(kind)), - } + let Some(scope) = record.work_ref.scope_name() else { + return Some(WorkspaceSource::local(kind)); + }; + // Only a registry-confirmed remote mount renders with the remote glyph; + // named local aliases (`docs:`) and groups stay on the local style. + let cwd = std::env::current_dir().ok()?; + let registry = crate::origins::collect(&cwd).ok()?; + let remote = registry + .resolve(scope) + .ok() + .flatten() + .is_some_and(|entry| entry.origin.kind == OriginKind::Remote); + Some(if remote { + WorkspaceSource::remote(scope, kind) + } else { + WorkspaceSource::local(kind) + }) } /// Build plan from CLI pieces (projection sugar + free tokens + flags). diff --git a/src/commands/memory/workset/source.rs b/src/commands/memory/workset/source.rs index dd7f698a..034768b5 100644 --- a/src/commands/memory/workset/source.rs +++ b/src/commands/memory/workset/source.rs @@ -101,12 +101,11 @@ pub fn query(source: &str, filter: Filter, cwd: Option<&Path>) -> Result Date: Sun, 16 Aug 2026 12:12:32 +0800 Subject: [PATCH 08/10] fix(origin): resolve cold mounts and report partial migration honestly - query() starts the daemon and re-collects the registry when a scope misses passively, so a cold `desk:terminal` query resolves its mount instead of failing with "unknown scope" - doctor reports Fail (never Fixed) when any workspace migration was skipped, listing the skipped dirs and reasons alongside migrated counts - scan_workspace_keys records a failed post-rename metadata write as skipped and heals stale metadata on the next --fix run --- crates/sivtr-core/src/workspace.rs | 70 ++++++++++- src/commands/memory/workset/source.rs | 15 ++- src/commands/system/doctor.rs | 160 +++++++++++++++++++------- 3 files changed, 199 insertions(+), 46 deletions(-) diff --git a/crates/sivtr-core/src/workspace.rs b/crates/sivtr-core/src/workspace.rs index 916a3cfe..995f61e6 100644 --- a/crates/sivtr-core/src/workspace.rs +++ b/crates/sivtr-core/src/workspace.rs @@ -395,6 +395,18 @@ fn scan_workspace_keys(apply: bool) -> Result { }; let (new_key, display_root) = workspace_identity(Path::new(&meta.root)); if new_key == old_key { + // Heal metadata left stale by a failed post-rename write: the dir + // already carries the current key, so only the fields need repair. + if apply && (meta.key != new_key || meta.root != display_root.to_string_lossy()) { + meta.key = new_key; + meta.root = display_root.to_string_lossy().to_string(); + if let Err(e) = write_workspace_metadata(&meta_path, &meta) { + report + .skipped + .push((dir, format!("failed to rewrite workspace.json: {e}"))); + continue; + } + } report.current += 1; continue; } @@ -420,8 +432,13 @@ fn scan_workspace_keys(apply: bool) -> Result { Ok(()) => { meta.key = new_key.clone(); meta.root = display_root.to_string_lossy().to_string(); - let _ = write_workspace_metadata(&target.join("workspace.json"), &meta); - report.migrated.push((old_key, new_key)); + match write_workspace_metadata(&target.join("workspace.json"), &meta) { + Ok(()) => report.migrated.push((old_key, new_key)), + Err(e) => report.skipped.push(( + target, + format!("renamed to {new_key} but workspace.json update failed: {e}"), + )), + } } Err(e) => report.skipped.push((dir, format!("rename failed: {e}"))), } @@ -716,6 +733,55 @@ mod tests { let _ = std::fs::remove_dir_all(data); } + #[test] + fn migrate_heals_stale_metadata_after_failed_rename_write() { + let _lock = crate::test_env_lock(); + let data = unique_test_dir("workspace-heal"); + let old_data = std::env::var_os("SIVTR_DATA_DIR"); + std::env::set_var("SIVTR_DATA_DIR", &data); + + let main = unique_test_dir("repo-main"); + make_repo(&main); + + // The dir already carries the commondir key, but a previous + // post-rename metadata write failed: workspace.json still holds the + // stale key. A --fix run must repair the fields in place. + let new_key = workspace_identity(&main).0; + let dir = data.join("workspaces").join(&new_key); + std::fs::create_dir_all(&dir).expect("workspace dir"); + let meta = WorkspaceMetadata { + key: "stale-key".into(), + root: real_path(&main).to_string_lossy().to_string(), + alias: None, + created_at: "t0".into(), + last_seen_at: "t0".into(), + }; + let meta_path = dir.join("workspace.json"); + std::fs::write( + &meta_path, + serde_json::to_string_pretty(&meta).expect("serialize"), + ) + .expect("write meta"); + + let report = migrate_workspace_keys().expect("migrate"); + assert!(report.migrated.is_empty() && report.merged.is_empty()); + assert!(report.skipped.is_empty()); + assert_eq!(report.current, 1); + + let healed: WorkspaceMetadata = + serde_json::from_str(&std::fs::read_to_string(&meta_path).expect("read meta")) + .expect("parse meta"); + assert_eq!(healed.key, new_key); + assert_eq!(healed.root, real_path(&main).to_string_lossy().to_string()); + + match old_data { + Some(value) => std::env::set_var("SIVTR_DATA_DIR", value), + None => std::env::remove_var("SIVTR_DATA_DIR"), + } + let _ = std::fs::remove_dir_all(main); + let _ = std::fs::remove_dir_all(data); + } + #[test] fn terminal_session_id_uses_file_stem() { assert_eq!( diff --git a/src/commands/memory/workset/source.rs b/src/commands/memory/workset/source.rs index 034768b5..79a11968 100644 --- a/src/commands/memory/workset/source.rs +++ b/src/commands/memory/workset/source.rs @@ -104,11 +104,16 @@ pub fn query(source: &str, filter: Filter, cwd: Option<&Path>) -> Result match &entry.reach { Reach::Local { root } => run_local(path, Path::new(root), filter), diff --git a/src/commands/system/doctor.rs b/src/commands/system/doctor.rs index ac0c4d3a..bf61452f 100644 --- a/src/commands/system/doctor.rs +++ b/src/commands/system/doctor.rs @@ -283,45 +283,7 @@ impl Report { return; } }; - let mut pending = Vec::new(); - if !report.migrated.is_empty() { - pending.push(format!("{} to re-key", report.migrated.len())); - } - if !report.merged.is_empty() { - pending.push(format!("{} to merge", report.merged.len())); - } - if !pending.is_empty() { - self.add(Check { - name: "workspace_keys", - label: "workspace keys", - status: if fix { Status::Fixed } else { Status::Fail }, - detail: pending.join(", "), - hint: if fix { - None - } else { - Some("run `sivtr doctor --fix`".to_string()) - }, - }); - } else if !report.skipped.is_empty() { - self.add(Check { - name: "workspace_keys", - label: "workspace keys", - status: Status::Manual, - detail: format!( - "{} workspace(s) could not be migrated", - report.skipped.len() - ), - hint: None, - }); - } else { - self.add(Check { - name: "workspace_keys", - label: "workspace keys", - status: Status::Pass, - detail: format!("{} workspace(s) on current scheme", report.current), - hint: None, - }); - } + self.add(workspace_keys_check(report, fix)); } fn check_skill(&mut self, fix: bool) { @@ -542,6 +504,80 @@ impl Report { } } +/// Map a migration report onto the `workspace_keys` check. Partial success +/// keeps the migrated/merged counts but must never report `Fixed` while any +/// workspace was skipped — its sessions stay unreachable. +fn workspace_keys_check(report: workspace::WorkspaceMigration, fix: bool) -> Check { + let mut pending = Vec::new(); + if !report.migrated.is_empty() { + pending.push(format!("{} to re-key", report.migrated.len())); + } + if !report.merged.is_empty() { + pending.push(format!("{} to merge", report.merged.len())); + } + if !pending.is_empty() { + let mut detail = pending.join(", "); + if !report.skipped.is_empty() { + let reasons: Vec = report + .skipped + .iter() + .take(3) + .map(|(path, reason)| { + let name = path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("workspace"); + format!("{name}: {reason}") + }) + .collect(); + let more = if report.skipped.len() > 3 { + format!(", +{} more", report.skipped.len() - 3) + } else { + String::new() + }; + detail.push_str(&format!( + "; {} workspace(s) could not be migrated ({}{more})", + report.skipped.len(), + reasons.join("; ") + )); + } + return Check { + name: "workspace_keys", + label: "workspace keys", + status: if fix && report.skipped.is_empty() { + Status::Fixed + } else { + Status::Fail + }, + detail, + hint: if fix { + None + } else { + Some("run `sivtr doctor --fix`".to_string()) + }, + }; + } + if !report.skipped.is_empty() { + return Check { + name: "workspace_keys", + label: "workspace keys", + status: Status::Manual, + detail: format!( + "{} workspace(s) could not be migrated", + report.skipped.len() + ), + hint: None, + }; + } + Check { + name: "workspace_keys", + label: "workspace keys", + status: Status::Pass, + detail: format!("{} workspace(s) on current scheme", report.current), + hint: None, + } +} + fn print_human(report: &Report) { let total = report.checks.len(); let mut passed = 0; @@ -675,3 +711,49 @@ pub fn detect_current_shell() -> String { "bash".to_string() } } + +#[cfg(test)] +mod tests { + use super::*; + use sivtr_core::workspace::WorkspaceMigration; + + fn partial_report() -> WorkspaceMigration { + WorkspaceMigration { + migrated: vec![("old-a".into(), "new-a".into())], + merged: Vec::new(), + skipped: vec![(PathBuf::from("old-b"), "rename failed: boom".into())], + current: 1, + } + } + + #[test] + fn partial_fix_never_reports_fixed_and_lists_skipped() { + let check = workspace_keys_check(partial_report(), true); + assert_eq!(check.status, Status::Fail); + assert!(check.detail.contains("1 to re-key")); + assert!(check + .detail + .contains("1 workspace(s) could not be migrated")); + assert!(check.detail.contains("old-b: rename failed: boom")); + } + + #[test] + fn clean_fix_reports_fixed_without_skipped_noise() { + let report = WorkspaceMigration { + migrated: vec![("old-a".into(), "new-a".into())], + merged: Vec::new(), + skipped: Vec::new(), + current: 1, + }; + let check = workspace_keys_check(report, true); + assert_eq!(check.status, Status::Fixed); + assert_eq!(check.detail, "1 to re-key"); + } + + #[test] + fn inspect_mode_keeps_fail_with_fix_hint() { + let check = workspace_keys_check(partial_report(), false); + assert_eq!(check.status, Status::Fail); + assert_eq!(check.hint.as_deref(), Some("run `sivtr doctor --fix`")); + } +} From 9027fdf138a4f0f02d161c0f3e14e0acd6b9522c Mon Sep 17 00:00:00 2001 From: Ariestar <1941137088@qq.com> Date: Sun, 16 Aug 2026 12:13:38 +0800 Subject: [PATCH 09/10] refactor(origin): drop reserved cloud variants and duplicate lookup paths - remove the reserved OriginKind::Cloud / Reach::Cloud variants and their glyph, color, priority, and bail arms (the enum stays non_exhaustive) - browse consumes the origin registry for mount aliases instead of a second RemoteList IPC path - WorkspaceSource drops the origin_kind field derived from scope; is_remote() is scope-based again - inline one-caller color helpers and cwd_path; reuse absolutize in paths_for_root --- crates/sivtr-core/src/origin.rs | 37 ++++++++++--------------- crates/sivtr-core/src/workspace.rs | 2 +- src/commands/browse/load.rs | 32 +++++++-------------- src/commands/memory/workset/source.rs | 7 ++--- src/commands/remote/workspace.rs | 4 +-- src/mcp/types.rs | 12 +++----- src/origins.rs | 16 ++++------- src/tui/theme.rs | 40 ++++++++------------------- src/tui/workspace/model.rs | 17 ++---------- src/tui/workspace/render.rs | 6 ++-- 10 files changed, 55 insertions(+), 118 deletions(-) diff --git a/crates/sivtr-core/src/origin.rs b/crates/sivtr-core/src/origin.rs index f4306b45..6a395870 100644 --- a/crates/sivtr-core/src/origin.rs +++ b/crates/sivtr-core/src/origin.rs @@ -1,13 +1,12 @@ //! Unified source origins. //! -//! Every memory source — a local workspace, a remote device mount, a cloud -//! account — is described by one [`Origin`] with the same four fields, so -//! upper layers (listing, rendering) never branch on which kind of source -//! they are looking at. Kind-specific details (root paths, peer/share ids, -//! cloud account) never enter [`Origin`]: the display [`Origin::detail`] is -//! composed by the source at construction time, and whether a remote or -//! cloud source happens to be ingested into local files is a -//! resolution-layer concern, not an origin concern. +//! Every memory source — a local workspace or a remote device mount — is +//! described by one [`Origin`] with the same four fields, so upper layers +//! (listing, rendering) never branch on which kind of source they are +//! looking at. Kind-specific details (root paths, peer/share ids) never +//! enter [`Origin`]: the display [`Origin::detail`] is composed by the source +//! at construction time, and whether a remote source happens to be ingested +//! into local files is a resolution-layer concern, not an origin concern. //! //! [`OriginRegistry`] is the single lookup surface: enumerate every //! addressable origin, or resolve one by its logical name. Each entry pairs @@ -21,9 +20,9 @@ use serde::{Deserialize, Serialize}; /// Major source category. /// -/// `#[non_exhaustive]`: adding a category (WSL, container, archive, …) only -/// requires a new variant plus a `label()` arm here; code outside this crate -/// is forced to handle the wildcard, so nothing downstream breaks. +/// `#[non_exhaustive]`: adding a category (cloud account, WSL, container, …) +/// only requires a new variant plus a `label()` arm here; code outside this +/// crate is forced to handle the wildcard, so nothing downstream breaks. #[non_exhaustive] #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "lowercase")] @@ -32,8 +31,6 @@ pub enum OriginKind { Local, /// Another device, forwarded through the daemon. Remote, - /// A cloud account (synced and/or fetched). - Cloud, } impl OriginKind { @@ -42,7 +39,6 @@ impl OriginKind { match self { Self::Local => "local", Self::Remote => "remote", - Self::Cloud => "cloud", } } } @@ -75,8 +71,6 @@ pub enum Reach { workspace_key: String, alias: String, }, - /// A cloud account (reserved — none yet). - Cloud, } /// One registry entry: the display [`Origin`] plus its [`Reach`]. @@ -123,8 +117,8 @@ impl OriginRegistry { if matched.is_empty() { return Ok(None); } - // Remote before local before cloud: the higher-priority kind wins a - // cross-kind collision; within one kind the name is still ambiguous. + // Remote before local: the higher-priority kind wins a cross-kind + // collision; within one kind the name is still ambiguous. matched.sort_by_key(|entry| kind_priority(entry.origin.kind)); if matched.len() > 1 && kind_priority(matched[0].origin.kind) == kind_priority(matched[1].origin.kind) @@ -141,13 +135,11 @@ impl OriginRegistry { /// Resolution priority when a name collides across kinds. A remote mount /// wins over a local workspace of the same name — remote lookup ran before -/// local workspace resolution before the registry existed — and cloud is -/// reserved, resolving last. +/// local workspace resolution before the registry existed. fn kind_priority(kind: OriginKind) -> u8 { match kind { OriginKind::Remote => 0, OriginKind::Local => 1, - OriginKind::Cloud => 2, } } @@ -187,7 +179,6 @@ mod tests { fn labels_are_stable() { assert_eq!(OriginKind::Local.label(), "local"); assert_eq!(OriginKind::Remote.label(), "remote"); - assert_eq!(OriginKind::Cloud.label(), "cloud"); } #[test] @@ -312,7 +303,7 @@ mod tests { for origin in sample().all() { assert!(!origin.name.is_empty()); assert!(!origin.detail.is_empty()); - assert!(matches!(origin.kind.label(), "local" | "remote" | "cloud")); + assert!(matches!(origin.kind.label(), "local" | "remote")); } } diff --git a/crates/sivtr-core/src/workspace.rs b/crates/sivtr-core/src/workspace.rs index 995f61e6..cc31309f 100644 --- a/crates/sivtr-core/src/workspace.rs +++ b/crates/sivtr-core/src/workspace.rs @@ -169,7 +169,7 @@ fn paths_for_root(root: PathBuf) -> Result { // verbatim prefix on Windows (root cause of ugly displayed paths and keys), // and resolves symlinks we don't need. `absolute` makes the path absolute // (so a relative `--cwd` still keys stably) without either side effect. - let root = std::path::absolute(&root).unwrap_or(root); + let root = absolutize(&root); let (key, display_root) = workspace_identity(&root); let dir = data_dir().join(WORKSPACES_DIR).join(&key); Ok(WorkspacePaths { diff --git a/src/commands/browse/load.rs b/src/commands/browse/load.rs index a16922bb..b0fc4b44 100644 --- a/src/commands/browse/load.rs +++ b/src/commands/browse/load.rs @@ -20,14 +20,12 @@ use crate::pane::{ keep_keys, MetaNeed, Pane, PaneInput, SlidingPane, StorePhase, Viewport, WindowRow, FETCH_CEILING, FETCH_FLOOR, }; -use crate::remote::ipc; -use crate::remote::protocol::{LocalRequest, LocalResponse}; use crate::tui::workspace::{ SourceLoadMarker, WorkspaceSession, WorkspaceSource, WorkspaceSourceKind, }; use sivtr_core::ai::AgentProvider; +use sivtr_core::origin::OriginKind; use sivtr_core::record::WorkRecord; -use sivtr_core::workspace; /// Session meta without dialogue bodies. #[derive(Clone, Debug)] @@ -687,14 +685,20 @@ pub fn workspace_source_catalog( for provider in providers { sources.push(WorkspaceSource::agent(*provider)); } - for alias in list_remote_aliases(cwd)? { + // Mount aliases come from the same origin registry every query path uses; + // it lists mounts only while the daemon is already running. + let registry = crate::origins::collect(cwd)?; + for origin in registry.all() { + if origin.kind != OriginKind::Remote { + continue; + } sources.push(WorkspaceSource::remote( - &alias, + &origin.name, WorkspaceSourceKind::Terminal, )); for provider in providers { sources.push(WorkspaceSource::remote( - &alias, + &origin.name, WorkspaceSourceKind::Agent(*provider), )); } @@ -702,22 +706,6 @@ pub fn workspace_source_catalog( Ok(sources) } -fn list_remote_aliases(cwd: &Path) -> Result> { - let Some(ws) = workspace::resolve_workspace_for_dir(cwd)? else { - return Ok(Vec::new()); - }; - if !ipc::running() { - return Ok(Vec::new()); - } - match ipc::call(LocalRequest::RemoteList { - workspace_key: ws.key, - }) { - Ok(LocalResponse::Mounts(mounts)) => Ok(mounts.into_iter().map(|m| m.alias).collect()), - Ok(_) => Ok(Vec::new()), - Err(_) => Ok(Vec::new()), - } -} - /// Meta-only merge of ready sources (bodies remain in each source pane). pub fn collect_ready_sessions( sources: &[WorkspaceSource], diff --git a/src/commands/memory/workset/source.rs b/src/commands/memory/workset/source.rs index 79a11968..f472bb78 100644 --- a/src/commands/memory/workset/source.rs +++ b/src/commands/memory/workset/source.rs @@ -102,8 +102,8 @@ pub fn query(source: &str, filter: Filter, cwd: Option<&Path>) -> Result) -> Result { - anyhow::bail!("cloud source `{}` is not available yet", entry.origin.name) - } }, None => match try_group(&scope, path, filter, &cwd)? { Some(set) => Ok(set), diff --git a/src/commands/remote/workspace.rs b/src/commands/remote/workspace.rs index ba4aa3a5..1a28d187 100644 --- a/src/commands/remote/workspace.rs +++ b/src/commands/remote/workspace.rs @@ -8,8 +8,8 @@ pub fn execute(command: WorkspaceCommand) -> Result<()> { } } -/// List every addressable origin (local workspaces + remote mounts + cloud) -/// through the unified [`OriginRegistry`] — rendering never branches on kind. +/// List every addressable origin (local workspaces + remote mounts) through +/// the unified [`OriginRegistry`] — rendering never branches on kind. fn list() -> Result<()> { let cwd = std::env::current_dir().context("Failed to resolve current directory")?; let registry = crate::origins::collect(&cwd)?; diff --git a/src/mcp/types.rs b/src/mcp/types.rs index ecfcd9e3..566e6038 100644 --- a/src/mcp/types.rs +++ b/src/mcp/types.rs @@ -217,10 +217,6 @@ pub struct VarStatus { pub created_at: String, } -fn cwd_path(cwd: Option<&str>) -> Option { - cwd.map(PathBuf::from) -} - pub fn to_search_args(params: &SearchParams) -> Result { Ok(SearchArgs { source: params.source.clone(), @@ -234,7 +230,7 @@ pub fn to_search_args(params: &SearchParams) -> Result { min_duration: None, max_duration: None, sort: None, - cwd: cwd_path(params.cwd.as_deref()), + cwd: params.cwd.as_deref().map(PathBuf::from), since: params.since.clone(), until: params.until.clone(), last: params.last.clone(), @@ -261,7 +257,7 @@ pub fn to_filter_args(params: &FilterParams) -> Result { min_duration: None, max_duration: None, sort: None, - cwd: cwd_path(params.cwd.as_deref()), + cwd: params.cwd.as_deref().map(PathBuf::from), since: params.since.clone(), until: params.until.clone(), last: params.last.clone(), @@ -292,7 +288,7 @@ pub fn to_show_args(params: &ShowParams) -> Result { }; Ok(ShowArgs { source: params.source.clone(), - cwd: cwd_path(params.cwd.as_deref()), + cwd: params.cwd.as_deref().map(PathBuf::from), format, full: false, refs: false, @@ -306,7 +302,7 @@ pub fn to_zoom_args(params: &ZoomParams) -> ZoomArgs { context: params.context, before: params.before, after: params.after, - cwd: cwd_path(params.cwd.as_deref()), + cwd: params.cwd.as_deref().map(PathBuf::from), format: None, json: false, refs: false, diff --git a/src/origins.rs b/src/origins.rs index 04748bfb..7790e3f9 100644 --- a/src/origins.rs +++ b/src/origins.rs @@ -1,11 +1,11 @@ //! Unified origin composition. //! //! The single place that assembles every addressable memory source (local -//! workspaces, remote device mounts, cloud accounts) into one -//! [`OriginRegistry`]. Each entry pairs the display [`Origin`] with its -//! [`Reach`] payload, so resolution never re-looks-up what composition -//! already knew. Upper layers consume the registry — new sources add a -//! constructor block here, and [`Origin`] itself never changes. +//! workspaces, remote device mounts) into one [`OriginRegistry`]. Each entry +//! pairs the display [`Origin`] with its [`Reach`] payload, so resolution +//! never re-looks-up what composition already knew. Upper layers consume the +//! registry — new sources add a constructor block here, and [`Origin`] itself +//! never changes. use anyhow::{bail, Context, Result}; use std::path::Path; @@ -18,8 +18,7 @@ use crate::remote::ipc; use crate::remote::protocol::{LocalRequest, LocalResponse}; /// All origins addressable from `cwd`: every local workspace (the current one -/// flagged), the current workspace's remote mounts, and cloud sources -/// (reserved — none yet). +/// flagged) plus the current workspace's remote mounts. pub fn collect(cwd: &Path) -> Result { let mut entries = Vec::new(); @@ -71,8 +70,6 @@ pub fn collect(cwd: &Path) -> Result { } } - // Cloud origins: reserved — cloud sources will construct here. - Ok(OriginRegistry::new(entries)) } @@ -118,6 +115,5 @@ pub fn rename(cwd: &Path, name: &str, new_name: &str) -> Result { LocalResponse::Mount(mount) => Ok(mount.alias), response => bail!("Unexpected daemon response: {response:?}"), }, - Reach::Cloud => bail!("cloud origins are reserved"), } } diff --git a/src/tui/theme.rs b/src/tui/theme.rs index af5fc68e..69c53396 100644 --- a/src/tui/theme.rs +++ b/src/tui/theme.rs @@ -2,7 +2,6 @@ use ratatui::prelude::{Color, Modifier, Style}; use sivtr_core::ai::AgentProvider; -use sivtr_core::origin::OriginKind; /// Active panel chrome (focused border / scrollbar). pub(crate) fn accent() -> Color { @@ -19,21 +18,6 @@ pub(crate) fn dim() -> Color { Color::Rgb(71, 85, 105) // slate-600 } -/// Local origin marker. -pub(crate) fn local_origin() -> Color { - Color::Rgb(52, 211, 153) // emerald-400 -} - -/// Remote origin marker. -pub(crate) fn remote_origin() -> Color { - Color::Rgb(244, 114, 182) // pink-400 -} - -/// Cloud origin marker. -pub(crate) fn cloud_origin() -> Color { - Color::Rgb(125, 211, 252) // sky-300 -} - /// Cursor / focus highlight on a list row. pub(crate) fn focus_row() -> Style { Style::default() @@ -78,22 +62,20 @@ pub(crate) fn terminal_color() -> Color { Color::Rgb(148, 163, 184) // slate-400 } -/// Origin glyph by kind: local `·`, remote `↗`, cloud `☁`. -pub(crate) fn origin_glyph(kind: OriginKind) -> &'static str { - match kind { - OriginKind::Local => "·", - OriginKind::Remote => "↗", - OriginKind::Cloud => "☁", - _ => "?", +/// Origin glyph: local `·`, remote `↗`. +pub(crate) fn origin_glyph(remote: bool) -> &'static str { + if remote { + "↗" + } else { + "·" } } -pub(crate) fn origin_style(kind: OriginKind) -> Style { - Style::default().fg(match kind { - OriginKind::Local => local_origin(), - OriginKind::Remote => remote_origin(), - OriginKind::Cloud => cloud_origin(), - _ => dim(), +pub(crate) fn origin_style(remote: bool) -> Style { + Style::default().fg(if remote { + Color::Rgb(244, 114, 182) // pink-400 + } else { + Color::Rgb(52, 211, 153) // emerald-400 }) } diff --git a/src/tui/workspace/model.rs b/src/tui/workspace/model.rs index 932f17bc..e38e8e6d 100644 --- a/src/tui/workspace/model.rs +++ b/src/tui/workspace/model.rs @@ -3,7 +3,6 @@ use ratatui::prelude::Color; use ratatui::widgets::ListState; use sivtr_core::ai::AgentProvider; -use sivtr_core::origin::OriginKind; use sivtr_core::record::{WorkAt, WorkRecord, WorkRef}; use std::collections::HashSet; use std::time::SystemTime; @@ -72,17 +71,11 @@ pub(crate) struct WorkspaceSource { /// Named scope (`desk`, `docs`); `None` = current local workspace. pub(crate) scope: Option, pub(crate) kind: WorkspaceSourceKind, - /// Origin category this source belongs to, set by its constructor. - pub(crate) origin_kind: OriginKind, } impl WorkspaceSource { pub(crate) fn local(kind: WorkspaceSourceKind) -> Self { - Self { - scope: None, - kind, - origin_kind: OriginKind::Local, - } + Self { scope: None, kind } } pub(crate) fn terminal() -> Self { @@ -98,7 +91,6 @@ impl WorkspaceSource { Self { scope: Some(scope.into()), kind, - origin_kind: OriginKind::Remote, } } @@ -126,14 +118,9 @@ impl WorkspaceSource { self.kind.color() } - /// Origin category of this source. - pub(crate) fn origin_kind(&self) -> OriginKind { - self.origin_kind - } - /// Whether this source needs remote transport (mount on another device). pub(crate) fn is_remote(&self) -> bool { - self.origin_kind == OriginKind::Remote + self.scope.is_some() } pub(crate) fn is_agent(&self) -> bool { diff --git a/src/tui/workspace/render.rs b/src/tui/workspace/render.rs index 2d12be01..b5aa933a 100644 --- a/src/tui/workspace/render.rs +++ b/src/tui/workspace/render.rs @@ -326,8 +326,8 @@ fn session_row_line( } else { "" }; - let origin_kind = choice.source.origin_kind(); - let origin = theme::origin_glyph(origin_kind); + let remote = choice.source.is_remote(); + let origin = theme::origin_glyph(remote); let badge = choice.source.badge(); let title = compact_session_title(choice); // Keep search highlighting over the full visible text, but paint origin/badge @@ -349,7 +349,7 @@ fn session_row_line( } spans.push(Span::styled( format!("{origin} "), - theme::origin_style(origin_kind), + theme::origin_style(remote), )); spans.push(Span::styled( format!("{badge} "), From 29a814ceff707bf0a5cf22e6e63299f4dc3c432b Mon Sep 17 00:00:00 2001 From: Ariestar <1941137088@qq.com> Date: Sun, 16 Aug 2026 13:08:39 +0800 Subject: [PATCH 10/10] fix(origin): surface skipped migration reasons and registry context Address review feedback: - doctor reports the dirs and reasons for skipped-only migration failures instead of a bare count, reusing the sampled-detail format - browse annotates origin registry collection with error context - scan_workspace_keys snapshots the workspaces dir before mutating it during migration (platform-dependent iteration visibility) - migration test cleans up its worktree temp dir --- crates/sivtr-core/src/workspace.rs | 12 +++--- src/commands/browse/load.rs | 4 +- src/commands/system/doctor.rs | 68 ++++++++++++++++++------------ 3 files changed, 51 insertions(+), 33 deletions(-) diff --git a/crates/sivtr-core/src/workspace.rs b/crates/sivtr-core/src/workspace.rs index cc31309f..9aa69cff 100644 --- a/crates/sivtr-core/src/workspace.rs +++ b/crates/sivtr-core/src/workspace.rs @@ -381,11 +381,12 @@ fn scan_workspace_keys(apply: bool) -> Result { if !base.exists() { return Ok(report); } - for entry in fs::read_dir(&base)? { - let Ok(entry) = entry else { - continue; - }; - let dir = entry.path(); + // Snapshot entries first: the loop renames and deletes directories inside + // `base`, whose visibility during iteration is platform-dependent. + let entries: Vec = fs::read_dir(&base)? + .filter_map(|entry| entry.ok().map(|entry| entry.path())) + .collect(); + for dir in entries { let Some(old_key) = dir.file_name().and_then(|n| n.to_str()).map(str::to_string) else { continue; }; @@ -730,6 +731,7 @@ mod tests { None => std::env::remove_var("SIVTR_DATA_DIR"), } let _ = std::fs::remove_dir_all(main); + let _ = std::fs::remove_dir_all(worktree); let _ = std::fs::remove_dir_all(data); } diff --git a/src/commands/browse/load.rs b/src/commands/browse/load.rs index b0fc4b44..f46ecf05 100644 --- a/src/commands/browse/load.rs +++ b/src/commands/browse/load.rs @@ -4,7 +4,7 @@ //! [`SlidingPane::ensure_meta`] / [`SlidingPane::ensure_bodies`]; this module //! only fulfills those requests over workset. -use anyhow::Result; +use anyhow::{Context, Result}; use chrono::{DateTime, Utc}; use std::collections::{BTreeMap, HashMap, HashSet}; use std::path::{Path, PathBuf}; @@ -687,7 +687,7 @@ pub fn workspace_source_catalog( } // Mount aliases come from the same origin registry every query path uses; // it lists mounts only while the daemon is already running. - let registry = crate::origins::collect(cwd)?; + let registry = crate::origins::collect(cwd).context("Failed to collect the origin registry")?; for origin in registry.all() { if origin.kind != OriginKind::Remote { continue; diff --git a/src/commands/system/doctor.rs b/src/commands/system/doctor.rs index bf61452f..358539d3 100644 --- a/src/commands/system/doctor.rs +++ b/src/commands/system/doctor.rs @@ -504,6 +504,32 @@ impl Report { } } +/// `"N workspace(s) could not be migrated (dir: reason; …)"` from a migration +/// report's skipped entries, sampling the first three reasons. +fn skipped_summary(skipped: &[(PathBuf, String)]) -> String { + let reasons: Vec = skipped + .iter() + .take(3) + .map(|(path, reason)| { + let name = path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("workspace"); + format!("{name}: {reason}") + }) + .collect(); + let more = if skipped.len() > 3 { + format!(", +{} more", skipped.len() - 3) + } else { + String::new() + }; + format!( + "{} workspace(s) could not be migrated ({}{more})", + skipped.len(), + reasons.join("; ") + ) +} + /// Map a migration report onto the `workspace_keys` check. Partial success /// keeps the migrated/merged counts but must never report `Fixed` while any /// workspace was skipped — its sessions stay unreachable. @@ -518,28 +544,8 @@ fn workspace_keys_check(report: workspace::WorkspaceMigration, fix: bool) -> Che if !pending.is_empty() { let mut detail = pending.join(", "); if !report.skipped.is_empty() { - let reasons: Vec = report - .skipped - .iter() - .take(3) - .map(|(path, reason)| { - let name = path - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or("workspace"); - format!("{name}: {reason}") - }) - .collect(); - let more = if report.skipped.len() > 3 { - format!(", +{} more", report.skipped.len() - 3) - } else { - String::new() - }; - detail.push_str(&format!( - "; {} workspace(s) could not be migrated ({}{more})", - report.skipped.len(), - reasons.join("; ") - )); + detail.push_str("; "); + detail.push_str(&skipped_summary(&report.skipped)); } return Check { name: "workspace_keys", @@ -562,10 +568,7 @@ fn workspace_keys_check(report: workspace::WorkspaceMigration, fix: bool) -> Che name: "workspace_keys", label: "workspace keys", status: Status::Manual, - detail: format!( - "{} workspace(s) could not be migrated", - report.skipped.len() - ), + detail: skipped_summary(&report.skipped), hint: None, }; } @@ -756,4 +759,17 @@ mod tests { assert_eq!(check.status, Status::Fail); assert_eq!(check.hint.as_deref(), Some("run `sivtr doctor --fix`")); } + + #[test] + fn skipped_only_reports_manual_with_reasons() { + let report = WorkspaceMigration { + migrated: Vec::new(), + merged: Vec::new(), + skipped: vec![(PathBuf::from("old-b"), "rename failed: boom".into())], + current: 2, + }; + let check = workspace_keys_check(report, true); + assert_eq!(check.status, Status::Manual); + assert!(check.detail.contains("old-b: rename failed: boom")); + } }