diff --git a/Cargo.lock b/Cargo.lock index f3292c4..c6749d1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7472,6 +7472,7 @@ name = "trawler" version = "0.1.0" dependencies = [ "chrono", + "dirs 5.0.1", "futures", "gpui", "loro", diff --git a/README.md b/README.md index c71bf51..6da31b5 100644 --- a/README.md +++ b/README.md @@ -124,14 +124,73 @@ and regenerate the golden graph in the same commit — the ### Where your data lives -By default the graph directory is `%APPDATA%\trawler\graph`. Override it with -the `TRAWLER_GRAPH_DIR` environment variable (useful for running multiple -graphs, or a scratch graph while developing): +Without an override, Trawler uses the platform-native data directory: + +- Windows: `%APPDATA%\trawler\graph` (roaming AppData) +- macOS: `~/Library/Application Support/trawler/graph` +- Linux: `$XDG_DATA_HOME/trawler/graph`, or + `~/.local/share/trawler/graph` when `XDG_DATA_HOME` is unset + +A present, non-empty `TRAWLER_GRAPH_DIR` has highest precedence and bypasses +both the native default and legacy compatibility checks. This is useful for +selecting another graph or a development scratch graph: ```sh TRAWLER_GRAPH_DIR=/tmp/my-test-graph cargo run -p trawler ``` +An ordinary relative value is intentional: Trawler resolves it against the +launch working directory (without following symlinks) and uses the resulting +absolute path. On Windows, drive-relative values such as `C:graph` and rooted +values without a drive or UNC prefix such as `\graph` are refused because they +do not identify a stable absolute location; use `C:\graph`, a UNC path, or an +ordinary relative value instead. Use an absolute override if the launch working +directory might not be available. A present but empty `TRAWLER_GRAPH_DIR` is +**not** treated as unset; this is a deliberate compatibility break, and startup +refuses it before opening storage. Remove the variable to use normal selection, +or set it to a non-empty path. + +Older builds could create `trawler-graph` under the launch working directory. +With no override, Trawler opens such a legacy graph only when it contains a +regular `snapshot.loro` and the native graph does not; startup warns that +legacy compatibility was used and names both locations. Trawler never creates, +moves, or copies a legacy graph automatically. If both the native and legacy +locations contain different recognized graphs, startup refuses the ambiguous +choice. Recover by setting `TRAWLER_GRAPH_DIR` to the absolute path of the one +you intend to open; do not delete or merge either graph merely to get past the +check. + +Trawler also refuses to initialize an unrecognized native directory that has +unrelated files or any `updates.log` content. Do not empty it in response to the +error: close Trawler, preserve and back up the complete directory, inspect its +origin, and either restore/move the intended complete graph or select a separate +graph with an absolute override. The only automatic retry exception is the +narrow debris from an interrupted first creation: a directory containing only +`meta.json` and/or `snapshot.loro.tmp` can be initialized again. Any other file +keeps the directory occupied. + +To migrate a legacy graph conservatively: + +1. Close every Trawler process that could write either location. +2. Back up the complete legacy graph directory and confirm the backup is + readable before changing the original. +3. Move the complete directory to the native path, or copy it if you want to + preserve the original during verification. Do not combine it with an + existing native graph. +4. Launch with `TRAWLER_GRAPH_DIR` explicitly set to the absolute native + destination, verify the expected pages and recent edits, then close Trawler. +5. After successful verification, rename/archive the complete legacy directory + so default startup no longer sees two graphs. Never retire only + `snapshot.loro`: it is source-of-truth data and leaving the rest of the + directory behind creates an incomplete, misleading copy. If you intentionally + keep both complete recognized graphs, keep using an explicit + `TRAWLER_GRAPH_DIR` on every launch. + +The [headless Linux preview sessions](#headless-linux-preview-sessions) always +use helper-owned, disposable fixture graphs via an explicit override; those +isolated Nix scratch graphs are separate from this user-default and legacy +selection policy. + ## Dev automation (devtools) For development — especially agent-assisted development — trawler has an diff --git a/crates/trawler-core/src/storage.rs b/crates/trawler-core/src/storage.rs index 09e6b42..1fd816c 100644 --- a/crates/trawler-core/src/storage.rs +++ b/crates/trawler-core/src/storage.rs @@ -25,6 +25,29 @@ const SNAPSHOT_PREV_FILE: &str = "snapshot.loro.prev"; const UPDATES_FILE: &str = "updates.log"; const META_FILE: &str = "meta.json"; +#[derive(Debug)] +struct MarkerInspectionError { + marker: PathBuf, + source: io::Error, +} + +impl std::fmt::Display for MarkerInspectionError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "failed to inspect graph marker {}: {}", + self.marker.display(), + self.source + ) + } +} + +impl std::error::Error for MarkerInspectionError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + Some(&self.source) + } +} + /// Update-log size at which `persist_update` folds the log into a fresh /// snapshot (openspec change add-auto-compaction, design D3). Large enough /// that steady typing doesn't thrash snapshot writes — megabytes of update @@ -248,6 +271,34 @@ impl GraphStorage { dir.as_ref().join(SNAPSHOT_FILE).exists() } + /// Inspect whether `dir` contains the regular snapshot marker for a graph. + /// + /// This probe only inspects marker metadata: it never opens the snapshot or + /// modifies storage. A missing marker returns `Ok(false)`; all other + /// inspection failures, including a marker of the wrong file type, are + /// returned to the caller. + pub fn probe_exists(dir: impl AsRef) -> io::Result { + let marker = dir.as_ref().join(SNAPSHOT_FILE); + match fs::symlink_metadata(&marker) { + Ok(metadata) if metadata.file_type().is_file() => Ok(true), + Ok(_) => Err(io::Error::new( + io::ErrorKind::InvalidData, + format!("graph marker {} is not a regular file", marker.display()), + )), + Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(false), + Err(error) => { + let kind = error.kind(); + Err(io::Error::new( + kind, + MarkerInspectionError { + marker, + source: error, + }, + )) + } + } + } + pub fn doc(&self) -> &LoroDoc { &self.doc } @@ -436,6 +487,85 @@ mod tests { dir } + #[test] + fn graph_marker_probe_accepts_only_a_regular_snapshot() { + let regular_dir = temp_dir("probe-regular"); + fs::create_dir_all(®ular_dir).unwrap(); + fs::write( + regular_dir.join(SNAPSHOT_FILE), + b"marker bytes are not decoded", + ) + .unwrap(); + assert!(GraphStorage::probe_exists(®ular_dir).unwrap()); + + let missing_dir = temp_dir("probe-missing"); + fs::create_dir_all(&missing_dir).unwrap(); + assert!(!GraphStorage::probe_exists(&missing_dir).unwrap()); + + let wrong_type_dir = temp_dir("probe-wrong-type"); + fs::create_dir_all(wrong_type_dir.join(SNAPSHOT_FILE)).unwrap(); + let error = GraphStorage::probe_exists(&wrong_type_dir) + .expect_err("a directory marker must not be accepted"); + assert_eq!(error.kind(), io::ErrorKind::InvalidData); + assert!(error.to_string().contains(SNAPSHOT_FILE)); + + fs::remove_dir_all(regular_dir).ok(); + fs::remove_dir_all(missing_dir).ok(); + fs::remove_dir_all(wrong_type_dir).ok(); + } + + #[cfg(unix)] + #[test] + fn graph_marker_probe_propagates_inspection_errors_with_the_marker_path() { + let dir = temp_dir("probe-inspection-error"); + fs::write(&dir, b"not a directory").unwrap(); + + let error = GraphStorage::probe_exists(&dir) + .expect_err("traversal through a regular file must fail closed"); + assert_ne!(error.kind(), io::ErrorKind::NotFound); + assert!(error.to_string().contains(SNAPSHOT_FILE)); + let source = std::error::Error::source(&error) + .expect("probe context must retain the underlying I/O error"); + assert_eq!( + source.downcast_ref::().unwrap().kind(), + io::ErrorKind::NotADirectory + ); + + fs::remove_file(dir).ok(); + } + + #[cfg(unix)] + #[test] + fn graph_marker_probe_rejects_a_snapshot_symlink() { + use std::os::unix::fs::symlink; + + let dir = temp_dir("probe-symlink"); + fs::create_dir_all(&dir).unwrap(); + fs::write(dir.join("snapshot-target"), b"target").unwrap(); + symlink("snapshot-target", dir.join(SNAPSHOT_FILE)).unwrap(); + + let error = + GraphStorage::probe_exists(&dir).expect_err("a symlink marker is not a regular file"); + assert_eq!(error.kind(), io::ErrorKind::InvalidData); + assert!(error.to_string().contains(SNAPSHOT_FILE)); + + fs::remove_dir_all(dir).ok(); + } + + #[test] + fn graph_marker_probe_does_not_open_or_modify_storage() { + let dir = temp_dir("probe-read-only"); + fs::create_dir_all(&dir).unwrap(); + fs::write(dir.join(SNAPSHOT_FILE), b"not a valid Loro snapshot").unwrap(); + fs::write(dir.join(UPDATES_FILE), b"untouched update bytes").unwrap(); + let before = dir_bytes(&dir); + + assert!(GraphStorage::probe_exists(&dir).unwrap()); + assert_eq!(dir_bytes(&dir), before); + + fs::remove_dir_all(dir).ok(); + } + #[test] fn create_then_open_round_trips_empty_graph() { let dir = temp_dir("create-open-empty"); diff --git a/crates/trawler/Cargo.toml b/crates/trawler/Cargo.toml index 0f621b9..8aeeb0e 100644 --- a/crates/trawler/Cargo.toml +++ b/crates/trawler/Cargo.toml @@ -16,6 +16,7 @@ devtools = ["dep:serde", "dep:serde_json", "dep:xcap", "trawler-core/fixtures"] [dependencies] chrono = "0.4.45" +dirs = "=5.0.1" futures = "0.3" gpui = "0.2.2" loro = "1.13.6" diff --git a/crates/trawler/src/graph_path.rs b/crates/trawler/src/graph_path.rs new file mode 100644 index 0000000..e770e37 --- /dev/null +++ b/crates/trawler/src/graph_path.rs @@ -0,0 +1,1063 @@ +use std::env; +use std::ffi::{OsStr, OsString}; +use std::fmt; +use std::fs; +use std::io; +use std::path::{Path, PathBuf}; + +use trawler_core::storage::GraphStorage; + +const OVERRIDE: &str = "TRAWLER_GRAPH_DIR"; +const LEGACY_DIR: &str = "trawler-graph"; +const RECOVERABLE_PARTIAL_INIT_FILES: &[&str] = &["meta.json", "snapshot.loro.tmp"]; + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum SelectionSource { + Override, + PlatformDefault, + LegacyCompatibility, +} + +impl fmt::Display for SelectionSource { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(match self { + Self::Override => "override", + Self::PlatformDefault => "platform default", + Self::LegacyCompatibility => "legacy compatibility", + }) + } +} + +#[derive(Debug, Eq, PartialEq)] +enum SelectionWarning { + Legacy { legacy: PathBuf, native: PathBuf }, + Alias { legacy: PathBuf, native: PathBuf }, +} + +#[derive(Debug, Eq, PartialEq)] +pub(crate) struct Selection { + pub(crate) path: PathBuf, + pub(crate) source: SelectionSource, + warning: Option, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum CandidateKind { + Native, + Legacy, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum CandidateState { + Absent, + Graph, + Occupied, +} + +#[derive(Debug)] +pub(crate) enum ResolveError { + EmptyOverride, + #[cfg(windows)] + InvalidOverride { + path: PathBuf, + reason: &'static str, + }, + Cwd { + source: io::Error, + }, + RelativeCwd { + path: PathBuf, + }, + NativeRootUnavailable, + RelativeNativeRoot { + path: PathBuf, + }, + CandidateInspection { + path: PathBuf, + source: io::Error, + }, + OccupiedNative { + path: PathBuf, + }, + Canonicalization { + path: PathBuf, + source: io::Error, + }, + Ambiguous { + native: PathBuf, + legacy: PathBuf, + }, + NonAbsoluteSelection { + path: PathBuf, + source: SelectionSource, + }, +} + +impl fmt::Display for ResolveError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::EmptyOverride => write!( + f, + "{OVERRIDE} is present but empty; remove it or set it to a non-empty path" + ), + #[cfg(windows)] + Self::InvalidOverride { path, reason } => write!( + f, + "{OVERRIDE} path {path:?} {reason}; use an absolute path or an ordinary relative path without a drive or root prefix" + ), + Self::Cwd { source } => write!( + f, + "cannot obtain the launch working directory: {source}; set {OVERRIDE} to an absolute path" + ), + Self::RelativeCwd { path } => write!( + f, + "the launch working directory {path:?} is not absolute; set {OVERRIDE} to an absolute path" + ), + Self::NativeRootUnavailable => write!( + f, + "the operating system did not provide a native data directory; set {OVERRIDE} to an absolute path" + ), + Self::RelativeNativeRoot { path } => write!( + f, + "the native data directory {path:?} is not absolute; set {OVERRIDE} to an absolute path" + ), + Self::CandidateInspection { path, source } => { + write!(f, "cannot inspect graph candidate {path:?}: {source}") + } + Self::OccupiedNative { path } => write!( + f, + "refusing to initialize graph in occupied native directory {path:?}; preserve its contents and set {OVERRIDE} to the absolute path of the graph you intend to open" + ), + Self::Canonicalization { path, source } => { + write!(f, "cannot canonicalize graph candidate {path:?}: {source}") + } + Self::Ambiguous { native, legacy } => write!( + f, + "graphs exist at both the platform path {native:?} and legacy path {legacy:?}; select one explicitly by setting {OVERRIDE} to that path" + ), + Self::NonAbsoluteSelection { path, source } => write!( + f, + "resolved {source} graph path {path:?} is not absolute; set {OVERRIDE} to an absolute path" + ), + } + } +} + +impl std::error::Error for ResolveError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Self::Cwd { source } + | Self::CandidateInspection { source, .. } + | Self::Canonicalization { source, .. } => Some(source), + _ => None, + } + } +} + +pub(crate) fn resolve() -> Result { + resolve_with( + env::var_os(OVERRIDE), + env::current_dir, + dirs::data_dir, + production_candidate, + |path| fs::canonicalize(path), + ) +} + +fn resolve_with( + override_value: Option, + mut cwd: Cwd, + mut native_root: Root, + mut candidate: Candidate, + mut canonicalize: Canonicalize, +) -> Result +where + Cwd: FnMut() -> io::Result, + Root: FnMut() -> Option, + Candidate: FnMut(&Path, CandidateKind) -> io::Result, + Canonicalize: FnMut(&Path) -> io::Result, +{ + if let Some(value) = override_value { + if value == OsStr::new("") { + return Err(ResolveError::EmptyOverride); + } + let path = PathBuf::from(value); + let path = if path.is_absolute() { + path + } else { + validate_relative_override(&path)?; + absolute_cwd(&mut cwd)?.join(path) + }; + return selection(path, SelectionSource::Override, None); + } + + let root = native_root().ok_or(ResolveError::NativeRootUnavailable)?; + if !root.is_absolute() { + return Err(ResolveError::RelativeNativeRoot { path: root }); + } + let native = root.join("trawler").join("graph"); + let legacy = absolute_cwd(&mut cwd)?.join(LEGACY_DIR); + + let native_state = candidate(&native, CandidateKind::Native).map_err(|source| { + ResolveError::CandidateInspection { + path: native.clone(), + source, + } + })?; + if native_state == CandidateState::Occupied { + return Err(ResolveError::OccupiedNative { path: native }); + } + let legacy_state = candidate(&legacy, CandidateKind::Legacy).map_err(|source| { + ResolveError::CandidateInspection { + path: legacy.clone(), + source, + } + })?; + + match (native_state, legacy_state) { + (CandidateState::Graph, CandidateState::Graph) => { + let canonical_native = + canonicalize(&native).map_err(|source| ResolveError::Canonicalization { + path: native.clone(), + source, + })?; + let canonical_legacy = + canonicalize(&legacy).map_err(|source| ResolveError::Canonicalization { + path: legacy.clone(), + source, + })?; + if canonical_native == canonical_legacy { + selection( + native.clone(), + SelectionSource::PlatformDefault, + Some(SelectionWarning::Alias { legacy, native }), + ) + } else { + Err(ResolveError::Ambiguous { native, legacy }) + } + } + (CandidateState::Absent, CandidateState::Graph) => selection( + legacy.clone(), + SelectionSource::LegacyCompatibility, + Some(SelectionWarning::Legacy { legacy, native }), + ), + (CandidateState::Graph | CandidateState::Absent, _) => { + selection(native, SelectionSource::PlatformDefault, None) + } + (CandidateState::Occupied, _) => unreachable!("occupied native handled above"), + } +} + +fn selection( + path: PathBuf, + source: SelectionSource, + warning: Option, +) -> Result { + if !path.is_absolute() { + return Err(ResolveError::NonAbsoluteSelection { path, source }); + } + Ok(Selection { + path, + source, + warning, + }) +} + +#[cfg(windows)] +fn validate_relative_override(path: &Path) -> Result<(), ResolveError> { + use std::path::Component; + + match path.components().next() { + Some(Component::Prefix(_)) => Err(ResolveError::InvalidOverride { + path: path.to_path_buf(), + reason: + "has a Windows path prefix but is not absolute (for example, a drive-relative path)", + }), + Some(Component::RootDir) => Err(ResolveError::InvalidOverride { + path: path.to_path_buf(), + reason: "is rooted but has no drive or UNC prefix", + }), + _ => Ok(()), + } +} + +#[cfg(not(windows))] +fn validate_relative_override(_path: &Path) -> Result<(), ResolveError> { + Ok(()) +} + +fn absolute_cwd(cwd: &mut impl FnMut() -> io::Result) -> Result { + let path = cwd().map_err(|source| ResolveError::Cwd { source })?; + if path.is_absolute() { + Ok(path) + } else { + Err(ResolveError::RelativeCwd { path }) + } +} + +fn production_candidate(path: &Path, kind: CandidateKind) -> io::Result { + let link_metadata = match fs::symlink_metadata(path) { + Ok(metadata) => metadata, + Err(error) if error.kind() == io::ErrorKind::NotFound => { + return Ok(CandidateState::Absent); + } + Err(error) => return Err(error), + }; + let metadata = if link_metadata.file_type().is_symlink() { + fs::metadata(path)? + } else { + link_metadata + }; + if !metadata.is_dir() { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!("graph candidate {} is not a directory", path.display()), + )); + } + + if GraphStorage::probe_exists(path)? { + return Ok(CandidateState::Graph); + } + if kind == CandidateKind::Legacy { + return Ok(CandidateState::Absent); + } + + for entry in fs::read_dir(path)? { + let entry = entry?; + if !entry.file_type()?.is_file() + || !RECOVERABLE_PARTIAL_INIT_FILES.contains(&entry.file_name().to_str().unwrap_or("")) + { + return Ok(CandidateState::Occupied); + } + } + Ok(CandidateState::Absent) +} + +pub(crate) fn resolve_diagnose_and_launch( + resolve: impl FnOnce() -> Result, + mut diagnostic: impl FnMut(&str), + launch: impl FnOnce(Selection) -> T, +) -> Result { + let selection = match resolve() { + Ok(selection) => selection, + Err(error) => { + diagnostic(&format!("trawler: graph selection failed: {error}")); + return Err(error); + } + }; + + if let Some(warning) = &selection.warning { + let message = match warning { + SelectionWarning::Legacy { legacy, native } => format!( + "trawler: warning: opening legacy compatibility graph {legacy:?}; the preferred platform path is {native:?}. Set {OVERRIDE} explicitly to choose a graph" + ), + SelectionWarning::Alias { legacy, native } => format!( + "trawler: warning: legacy path {legacy:?} aliases the selected platform graph {native:?}; using the platform path spelling" + ), + }; + diagnostic(&message); + } + diagnostic(&format!( + "trawler: selected graph {:?} ({})", + selection.path, selection.source + )); + Ok(launch(selection)) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::cell::{Cell, RefCell}; + use std::sync::atomic::{AtomicUsize, Ordering}; + + #[cfg(not(windows))] + const ROOT: &str = "/native-data"; + #[cfg(windows)] + const ROOT: &str = r"C:\native-data"; + #[cfg(not(windows))] + const CWD: &str = "/launch-dir"; + #[cfg(windows)] + const CWD: &str = r"C:\launch-dir"; + + fn resolve_states( + native: CandidateState, + legacy: CandidateState, + ) -> Result { + resolve_with( + None, + || Ok(PathBuf::from(CWD)), + || Some(PathBuf::from(ROOT)), + move |_path, kind| { + Ok(match kind { + CandidateKind::Native => native, + CandidateKind::Legacy => legacy, + }) + }, + |path| Ok(path.to_path_buf()), + ) + } + + #[test] + fn absolute_override_is_exact_and_all_other_providers_are_lazy() { + let cwd = Cell::new(0); + let root = Cell::new(0); + let probes = Cell::new(0); + let canonical = Cell::new(0); + let path = PathBuf::from(ROOT).join("exact/../graph"); + let selected = resolve_with( + Some(path.clone().into_os_string()), + || { + cwd.set(cwd.get() + 1); + Ok(PathBuf::from(CWD)) + }, + || { + root.set(root.get() + 1); + Some(PathBuf::from(ROOT)) + }, + |_path, _kind| { + probes.set(probes.get() + 1); + Ok(CandidateState::Graph) + }, + |_path| { + canonical.set(canonical.get() + 1); + Ok(PathBuf::new()) + }, + ) + .unwrap(); + assert_eq!(selected.path, path); + assert_eq!(selected.source, SelectionSource::Override); + assert_eq!( + (cwd.get(), root.get(), probes.get(), canonical.get()), + (0, 0, 0, 0) + ); + } + + #[cfg(unix)] + #[test] + fn non_utf8_override_bytes_are_preserved() { + use std::os::unix::ffi::{OsStrExt, OsStringExt}; + + let mut bytes = b"/graph/".to_vec(); + bytes.push(0xff); + let value = OsString::from_vec(bytes.clone()); + let selected = resolve_with( + Some(value), + || panic!("absolute override must not request cwd"), + || panic!("absolute override must not request native root"), + |_path, _kind| panic!("absolute override must not probe"), + |_path| panic!("absolute override must not canonicalize"), + ) + .unwrap(); + assert_eq!(selected.path.as_os_str().as_bytes(), bytes); + } + + #[test] + fn relative_override_is_joined_lexically_and_bypasses_defaults() { + let selected = resolve_with( + Some(OsString::from("scratch/../graph")), + || Ok(PathBuf::from(CWD)), + || panic!("relative override must not request native root"), + |_path, _kind| panic!("relative override must not probe"), + |_path| panic!("relative override must not canonicalize"), + ) + .unwrap(); + assert_eq!(selected.path, Path::new(CWD).join("scratch/../graph")); + assert_eq!(selected.source, SelectionSource::Override); + } + + #[test] + fn every_successful_selection_satisfies_the_absolute_path_postcondition() { + let override_selection = resolve_with( + Some(OsString::from("graph")), + || Ok(PathBuf::from(CWD)), + || panic!("override must bypass root"), + |_path, _kind| panic!("override must bypass probes"), + |_path| panic!("override must bypass canonicalization"), + ) + .unwrap(); + assert!(override_selection.path.is_absolute()); + + for (native, legacy) in [ + (CandidateState::Absent, CandidateState::Absent), + (CandidateState::Graph, CandidateState::Absent), + (CandidateState::Absent, CandidateState::Graph), + ] { + assert!(resolve_states(native, legacy).unwrap().path.is_absolute()); + } + + let error = + selection(PathBuf::from("relative"), SelectionSource::Override, None).unwrap_err(); + assert!(matches!(error, ResolveError::NonAbsoluteSelection { .. })); + } + + #[cfg(windows)] + #[test] + fn windows_rejects_drive_relative_and_rooted_without_prefix_overrides() { + for value in [r"C:graph", r"\graph"] { + let error = resolve_with( + Some(OsString::from(value)), + || panic!("unsafe Windows-relative path must fail before cwd"), + || panic!("unsafe override must bypass root"), + |_path, _kind| panic!("unsafe override must bypass probes"), + |_path| panic!("unsafe override must bypass canonicalization"), + ) + .unwrap_err(); + assert!(matches!(error, ResolveError::InvalidOverride { .. })); + let message = error.to_string(); + assert!(message.contains(value)); + assert!(message.contains("absolute path")); + } + } + + #[cfg(windows)] + #[test] + fn windows_ordinary_relative_override_still_joins_cwd() { + let selected = resolve_with( + Some(OsString::from(r"scratch\graph")), + || Ok(PathBuf::from(CWD)), + || panic!("relative override must bypass root"), + |_path, _kind| panic!("relative override must bypass probes"), + |_path| panic!("relative override must bypass canonicalization"), + ) + .unwrap(); + assert_eq!(selected.path, Path::new(CWD).join(r"scratch\graph")); + assert!(selected.path.is_absolute()); + } + + #[test] + fn relative_override_reports_cwd_failure_and_absolute_escape_hatch() { + let error = resolve_with( + Some(OsString::from("graph")), + || Err(io::Error::new(io::ErrorKind::PermissionDenied, "denied")), + || panic!("must not request root"), + |_path, _kind| panic!("must not probe"), + |_path| panic!("must not canonicalize"), + ) + .unwrap_err(); + let message = error.to_string(); + assert!(message.contains("absolute path")); + assert!(message.contains("denied")); + } + + #[test] + fn empty_override_is_refused_without_other_provider_calls() { + let error = resolve_with( + Some(OsString::new()), + || panic!("empty override must not request cwd"), + || panic!("empty override must not request root"), + |_path, _kind| panic!("empty override must not probe"), + |_path| panic!("empty override must not canonicalize"), + ) + .unwrap_err(); + assert!(matches!(error, ResolveError::EmptyOverride)); + assert!(error.to_string().contains("remove it or set it")); + } + + #[test] + fn native_root_is_required_and_must_be_absolute_before_cwd_or_probes() { + for root in [None, Some(PathBuf::from("relative"))] { + let error = resolve_with( + None, + || panic!("invalid root must fail before cwd"), + || root.clone(), + |_path, _kind| panic!("invalid root must not probe"), + |_path| panic!("invalid root must not canonicalize"), + ) + .unwrap_err(); + assert!(error.to_string().contains(OVERRIDE)); + } + } + + #[test] + fn default_cwd_failure_precedes_candidate_inspection() { + let error = resolve_with( + None, + || { + Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "cwd denied", + )) + }, + || Some(PathBuf::from(ROOT)), + |_path, _kind| panic!("cwd failure must not probe"), + |_path| panic!("cwd failure must not canonicalize"), + ) + .unwrap_err(); + let message = error.to_string(); + assert!(message.contains("cwd denied")); + assert!(message.contains(OVERRIDE)); + } + + #[test] + fn default_requires_absolute_cwd_before_inspection() { + let error = resolve_with( + None, + || Ok(PathBuf::from("relative-cwd")), + || Some(PathBuf::from(ROOT)), + |_path, _kind| panic!("relative cwd must not probe"), + |_path| panic!("relative cwd must not canonicalize"), + ) + .unwrap_err(); + assert!(matches!(error, ResolveError::RelativeCwd { .. })); + } + + #[test] + fn native_suffix_is_composed_without_canonicalizing() { + let seen = RefCell::new(Vec::new()); + let selected = resolve_with( + None, + || Ok(PathBuf::from(CWD)), + || Some(PathBuf::from(ROOT)), + |path, _kind| { + seen.borrow_mut().push(path.to_path_buf()); + Ok(CandidateState::Absent) + }, + |_path| panic!("absent candidates must not canonicalize"), + ) + .unwrap(); + assert_eq!(selected.path, Path::new(ROOT).join("trawler/graph")); + assert_eq!( + *seen.borrow(), + vec![ + Path::new(ROOT).join("trawler/graph"), + Path::new(CWD).join(LEGACY_DIR) + ] + ); + } + + #[test] + fn complete_native_legacy_marker_matrix_selects_expected_source() { + let cases = [ + ( + CandidateState::Absent, + CandidateState::Absent, + SelectionSource::PlatformDefault, + ), + ( + CandidateState::Graph, + CandidateState::Absent, + SelectionSource::PlatformDefault, + ), + ( + CandidateState::Absent, + CandidateState::Graph, + SelectionSource::LegacyCompatibility, + ), + ]; + for (native, legacy, source) in cases { + assert_eq!(resolve_states(native, legacy).unwrap().source, source); + } + let error = resolve_states(CandidateState::Graph, CandidateState::Graph).unwrap_err(); + assert!(matches!(error, ResolveError::Ambiguous { .. })); + let message = error.to_string(); + assert!(message.contains(ROOT)); + assert!(message.contains(CWD)); + assert!(message.contains(OVERRIDE)); + } + + #[test] + fn occupied_native_is_refused_before_legacy_inspection() { + let calls = Cell::new(0); + let error = resolve_with( + None, + || Ok(PathBuf::from(CWD)), + || Some(PathBuf::from(ROOT)), + |_path, _kind| { + calls.set(calls.get() + 1); + Ok(CandidateState::Occupied) + }, + |_path| panic!("occupied candidate must not canonicalize"), + ) + .unwrap_err(); + assert_eq!(calls.get(), 1); + assert!(matches!(error, ResolveError::OccupiedNative { .. })); + assert!(error.to_string().contains("occupied native directory")); + } + + #[test] + fn legacy_directory_without_marker_is_ignored() { + let selected = resolve_states(CandidateState::Absent, CandidateState::Absent).unwrap(); + assert_eq!(selected.source, SelectionSource::PlatformDefault); + assert_eq!(selected.path, Path::new(ROOT).join("trawler/graph")); + } + + #[test] + fn resolve_error_preserves_structured_io_sources() { + use std::error::Error as _; + + let errors = [ + ResolveError::Cwd { + source: io::Error::new(io::ErrorKind::PermissionDenied, "cwd denied"), + }, + ResolveError::CandidateInspection { + path: PathBuf::from(ROOT), + source: io::Error::new(io::ErrorKind::PermissionDenied, "probe denied"), + }, + ResolveError::Canonicalization { + path: PathBuf::from(ROOT), + source: io::Error::new(io::ErrorKind::PermissionDenied, "canon denied"), + }, + ]; + for error in errors { + let source = error.source().expect("I/O error source must survive"); + assert_eq!( + source.downcast_ref::().unwrap().kind(), + io::ErrorKind::PermissionDenied + ); + } + } + + #[test] + fn candidate_probe_errors_name_the_affected_path() { + for failing_kind in [CandidateKind::Native, CandidateKind::Legacy] { + let error = resolve_with( + None, + || Ok(PathBuf::from(CWD)), + || Some(PathBuf::from(ROOT)), + |_path, kind| { + if kind == failing_kind { + Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "probe denied", + )) + } else { + Ok(CandidateState::Absent) + } + }, + |_path| panic!("probe failure must not canonicalize"), + ) + .unwrap_err(); + let message = error.to_string(); + assert!(message.contains("probe denied")); + assert!(message.contains(match failing_kind { + CandidateKind::Native => ROOT, + CandidateKind::Legacy => CWD, + })); + } + } + + #[test] + fn dual_alias_selects_native_spelling_and_warns() { + let selected = resolve_with( + None, + || Ok(PathBuf::from(CWD)), + || Some(PathBuf::from(ROOT)), + |_path, _kind| Ok(CandidateState::Graph), + |_path| Ok(PathBuf::from("/same-directory")), + ) + .unwrap(); + assert_eq!(selected.path, Path::new(ROOT).join("trawler/graph")); + assert_eq!(selected.source, SelectionSource::PlatformDefault); + assert!(matches!( + selected.warning, + Some(SelectionWarning::Alias { .. }) + )); + } + + #[test] + fn canonicalization_failures_name_each_affected_candidate() { + for fail_call in [1, 2] { + let calls = Cell::new(0); + let error = resolve_with( + None, + || Ok(PathBuf::from(CWD)), + || Some(PathBuf::from(ROOT)), + |_path, _kind| Ok(CandidateState::Graph), + |path| { + calls.set(calls.get() + 1); + if calls.get() == fail_call { + Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "canon denied", + )) + } else { + Ok(path.to_path_buf()) + } + }, + ) + .unwrap_err(); + let message = error.to_string(); + assert!(message.contains("canon denied")); + assert!(message.contains(if fail_call == 1 { ROOT } else { CWD })); + } + } + + #[test] + fn legacy_selection_warning_names_both_paths_and_escape_hatch() { + let selection = resolve_states(CandidateState::Absent, CandidateState::Graph).unwrap(); + let messages = RefCell::new(Vec::new()); + resolve_diagnose_and_launch( + || Ok(selection), + |message| messages.borrow_mut().push(message.to_owned()), + |_| (), + ) + .unwrap(); + let messages = messages.borrow().join("\n"); + assert!(messages.contains(CWD)); + assert!(messages.contains(ROOT)); + assert!(messages.contains(OVERRIDE)); + assert!(messages.contains("legacy compatibility")); + } + + #[test] + fn successful_diagnostics_name_every_selection_source() { + for source in [ + SelectionSource::Override, + SelectionSource::PlatformDefault, + SelectionSource::LegacyCompatibility, + ] { + let messages = RefCell::new(Vec::new()); + resolve_diagnose_and_launch( + || { + Ok(Selection { + path: PathBuf::from(ROOT).join("graph"), + source, + warning: None, + }) + }, + |message| messages.borrow_mut().push(message.to_owned()), + |_| (), + ) + .unwrap(); + let source_name = source.to_string(); + assert!(messages.borrow()[0].contains(source_name.as_str())); + } + } + + #[test] + fn actual_resolver_probes_then_diagnoses_then_launches() { + let events = RefCell::new(Vec::new()); + resolve_diagnose_and_launch( + || { + resolve_with( + None, + || Ok(PathBuf::from(CWD)), + || Some(PathBuf::from(ROOT)), + |path, _kind| { + events.borrow_mut().push(format!("probe:{path:?}")); + Ok(CandidateState::Absent) + }, + |_path| panic!("absent candidates must not canonicalize"), + ) + }, + |message| events.borrow_mut().push(format!("diagnostic:{message}")), + |selection| { + assert!(selection.path.is_absolute()); + events.borrow_mut().push("launch".to_owned()); + }, + ) + .unwrap(); + let events = events.into_inner(); + assert!(events[0].starts_with("probe:")); + assert!(events[1].starts_with("probe:")); + assert!(events[2].starts_with("diagnostic:")); + assert_eq!(events[3], "launch"); + } + + #[test] + fn representative_actual_resolution_errors_are_diagnosed_and_never_launch() { + for (native, legacy) in [ + (CandidateState::Occupied, CandidateState::Absent), + (CandidateState::Graph, CandidateState::Graph), + ] { + let launched = Cell::new(false); + let messages = RefCell::new(Vec::new()); + let result = resolve_diagnose_and_launch( + || { + resolve_with( + None, + || Ok(PathBuf::from(CWD)), + || Some(PathBuf::from(ROOT)), + |_path, kind| { + Ok(match kind { + CandidateKind::Native => native, + CandidateKind::Legacy => legacy, + }) + }, + |path| Ok(path.to_path_buf()), + ) + }, + |message| messages.borrow_mut().push(message.to_owned()), + |_| launched.set(true), + ); + assert!(result.is_err()); + assert!(!launched.get()); + assert!(messages.borrow()[0].contains("graph selection failed")); + } + + let launched = Cell::new(false); + let result = resolve_diagnose_and_launch( + || { + resolve_with( + None, + || Ok(PathBuf::from(CWD)), + || Some(PathBuf::from(ROOT)), + |_path, _kind| Err(io::Error::other("inspection failed")), + |_path| panic!("inspection failure must not canonicalize"), + ) + }, + |_| {}, + |_| launched.set(true), + ); + assert!(matches!( + result, + Err(ResolveError::CandidateInspection { .. }) + )); + assert!(!launched.get()); + } + + #[cfg(unix)] + #[test] + fn startup_diagnostic_escapes_non_utf8_without_changing_launch_path() { + use std::os::unix::ffi::OsStringExt; + + let path = PathBuf::from(OsString::from_vec(b"/graph/\xff".to_vec())); + let launched = RefCell::new(None); + let messages = RefCell::new(Vec::new()); + resolve_diagnose_and_launch( + || { + Ok(Selection { + path: path.clone(), + source: SelectionSource::Override, + warning: None, + }) + }, + |message| messages.borrow_mut().push(message.to_owned()), + |selection| *launched.borrow_mut() = Some(selection.path), + ) + .unwrap(); + assert_eq!(launched.into_inner(), Some(path)); + assert!(messages.borrow()[0].contains("\\xFF")); + } + + struct TempDir(PathBuf); + + impl TempDir { + fn new() -> Self { + static NEXT: AtomicUsize = AtomicUsize::new(0); + let path = env::temp_dir().join(format!( + "trawler-graph-path-test-{}-{}", + std::process::id(), + NEXT.fetch_add(1, Ordering::Relaxed) + )); + fs::create_dir(&path).unwrap(); + Self(path) + } + } + + impl Drop for TempDir { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.0); + } + } + + #[test] + fn production_native_candidate_retries_only_narrow_partial_initialization() { + let temp = TempDir::new(); + let graph = temp.0.join("graph"); + fs::create_dir(&graph).unwrap(); + for recoverable in RECOVERABLE_PARTIAL_INIT_FILES { + fs::write(graph.join(recoverable), b"partial").unwrap(); + assert_eq!( + production_candidate(&graph, CandidateKind::Native).unwrap(), + CandidateState::Absent + ); + } + + fs::write(graph.join("unrelated"), b"occupied").unwrap(); + assert_eq!( + production_candidate(&graph, CandidateKind::Native).unwrap(), + CandidateState::Occupied + ); + fs::remove_file(graph.join("unrelated")).unwrap(); + fs::write(graph.join("updates.log"), b"source of truth").unwrap(); + assert_eq!( + production_candidate(&graph, CandidateKind::Native).unwrap(), + CandidateState::Occupied + ); + assert_eq!( + production_candidate(&graph, CandidateKind::Legacy).unwrap(), + CandidateState::Absent + ); + } + + #[test] + fn production_candidate_rejects_non_directory_path() { + let temp = TempDir::new(); + let file = temp.0.join("not-a-directory"); + fs::write(&file, b"file").unwrap(); + let error = production_candidate(&file, CandidateKind::Native).unwrap_err(); + assert_eq!(error.kind(), io::ErrorKind::InvalidData); + } + + #[cfg(unix)] + #[test] + fn production_candidate_rejects_dangling_directory_symlink() { + use std::os::unix::fs::symlink; + + let temp = TempDir::new(); + let dangling = temp.0.join("dangling"); + symlink(temp.0.join("missing-target"), &dangling).unwrap(); + let error = production_candidate(&dangling, CandidateKind::Native).unwrap_err(); + assert_eq!(error.kind(), io::ErrorKind::NotFound); + } + + #[cfg(unix)] + #[test] + fn production_alias_symlink_to_existing_graph_is_recognized_and_canonicalized() { + use std::os::unix::fs::symlink; + + let temp = TempDir::new(); + let root = temp.0.join("root"); + let cwd = temp.0.join("cwd"); + let native = root.join("trawler/graph"); + fs::create_dir_all(&native).unwrap(); + fs::create_dir_all(&cwd).unwrap(); + fs::write(native.join("snapshot.loro"), b"marker").unwrap(); + symlink(&native, cwd.join(LEGACY_DIR)).unwrap(); + + let selected = resolve_with( + None, + || Ok(cwd.clone()), + || Some(root.clone()), + production_candidate, + |path| fs::canonicalize(path), + ) + .unwrap(); + assert_eq!(selected.path, native); + assert!(matches!( + selected.warning, + Some(SelectionWarning::Alias { .. }) + )); + } + + #[test] + fn production_probe_rejects_a_non_regular_marker() { + let temp = TempDir::new(); + fs::create_dir(temp.0.join("snapshot.loro")).unwrap(); + let error = production_candidate(&temp.0, CandidateKind::Native).unwrap_err(); + assert_ne!(error.kind(), io::ErrorKind::NotFound); + } + + #[test] + fn production_native_root_is_absolute_and_matches_os_family() { + let root = dirs::data_dir().expect("supported desktop platform has a data directory"); + assert!(root.is_absolute()); + #[cfg(target_os = "macos")] + assert!(root.ends_with("Library/Application Support")); + #[cfg(target_os = "linux")] + { + let xdg = env::var_os("XDG_DATA_HOME").map(PathBuf::from); + if let Some(xdg) = xdg.filter(|path| path.is_absolute()) { + assert_eq!(root, xdg); + } else { + assert!(root.ends_with(".local/share")); + } + } + #[cfg(target_os = "windows")] + assert!(root + .to_string_lossy() + .to_ascii_lowercase() + .contains("appdata")); + } +} diff --git a/crates/trawler/src/main.rs b/crates/trawler/src/main.rs index 20b0d26..5aa23e3 100644 --- a/crates/trawler/src/main.rs +++ b/crates/trawler/src/main.rs @@ -33,13 +33,14 @@ mod assets; #[cfg(feature = "devtools")] mod devtools; mod editor; +mod graph_path; mod markdown; mod scheme_highlight; #[cfg(test)] mod ui_tests; use std::collections::{HashMap, HashSet}; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use std::rc::Rc; use std::sync::Arc; use std::time::Duration; @@ -6904,40 +6905,7 @@ impl Render for TrawlerApp { } } -fn default_graph_dir() -> PathBuf { - if let Ok(dir) = std::env::var("TRAWLER_GRAPH_DIR") { - return PathBuf::from(dir); - } - if let Ok(appdata) = std::env::var("APPDATA") { - return Path::new(&appdata).join("trawler").join("graph"); - } - PathBuf::from("trawler-graph") -} - -fn main() { - #[cfg(feature = "devtools")] - if devtools::handle_cli() { - return; - } - - // Refuse flags this build doesn't understand rather than silently - // launching the GUI. Without this, running e.g. `--seed-fixtures` on a - // binary built without `devtools` opened the app on whatever graph the - // environment pointed at (the user's real one, by default) — a - // genuinely dangerous way to "fail". - if let Some(flag) = std::env::args().nth(1).filter(|arg| arg.starts_with("--")) { - eprintln!( - "unrecognized flag `{flag}`{}", - if cfg!(feature = "devtools") { - "" - } else { - " (devtools CLI flags require a build with `--features devtools`)" - } - ); - std::process::exit(2); - } - - let graph_dir = default_graph_dir(); +fn launch(graph_dir: PathBuf) { Application::new() .with_assets(assets::Assets) .run(move |cx: &mut App| { @@ -6971,3 +6939,37 @@ fn main() { cx.activate(true); }); } + +fn main() { + #[cfg(feature = "devtools")] + if devtools::handle_cli() { + return; + } + + // Refuse flags this build doesn't understand rather than silently + // launching the GUI. Without this, running e.g. `--seed-fixtures` on a + // binary built without `devtools` opened the app on whatever graph the + // environment pointed at (the user's real one, by default) — a + // genuinely dangerous way to "fail". + if let Some(flag) = std::env::args().nth(1).filter(|arg| arg.starts_with("--")) { + eprintln!( + "unrecognized flag `{flag}`{}", + if cfg!(feature = "devtools") { + "" + } else { + " (devtools CLI flags require a build with `--features devtools`)" + } + ); + std::process::exit(2); + } + + if graph_path::resolve_diagnose_and_launch( + graph_path::resolve, + |message| eprintln!("{message}"), + |selection| launch(selection.path), + ) + .is_err() + { + std::process::exit(1); + } +} diff --git a/openspec/changes/use-platform-native-graph-paths/design.md b/openspec/changes/use-platform-native-graph-paths/design.md index 999c4c2..27762ca 100644 --- a/openspec/changes/use-platform-native-graph-paths/design.md +++ b/openspec/changes/use-platform-native-graph-paths/design.md @@ -62,7 +62,8 @@ A small `graph_path` module will own path selection. Production code acquires on needed by the active branch: an absolute override returns without consulting cwd, native roots, or graph probes; a relative override acquires cwd; no override acquires cwd and the native root. The decision core accepts injected providers, returns an absolute selected path plus a source -enum, and is unit-tested without mutating process environment or cwd. +enum, enforces that absolute-path postcondition at its return boundary, and is unit-tested +without mutating process environment or cwd. Default selection remains in `trawler`, while `trawler-core` gains one additive read-only marker probe so the app does not duplicate storage's private filename and can distinguish @@ -71,11 +72,13 @@ absence from an OS inspection error. ### D3 — Explicit overrides bypass default and legacy policy `std::env::var_os` preserves non-UTF-8 paths. A present non-empty absolute override wins before -cwd, native-root, or legacy acquisition. An explicit relative override requires cwd, is joined +cwd, native-root, or legacy acquisition. An ordinary relative override requires cwd, is joined to it without dereferencing symlinks, and then opens the same target through an absolute path. -If cwd cannot be obtained for a relative override or default selection, startup fails closed. -A present empty override is an intentional compatibility break and an actionable error rather -than an accidental cwd selection. +Windows drive-relative forms (`C:graph`) and rooted forms without a drive/UNC prefix (`\graph`) +are not ordinary relative paths and are rejected actionably rather than inheriting Windows' +per-drive working-directory semantics or replacing the cwd drive. If cwd cannot be obtained for +a relative override or default selection, startup fails closed. A present empty override is an +intentional compatibility break and an actionable error rather than an accidental cwd selection. ### D4 — Compatibility-first legacy handling never moves data @@ -83,7 +86,10 @@ Without an override, the resolver compares recognized graph markers at the nativ `/trawler-graph`, using a new `GraphStorage::probe_exists()`-style API rather than opening either graph. The probe uses filesystem metadata, accepts only a regular `snapshot.loro`, returns `Ok(false)` only for not-found, and propagates permission, traversal, wrong-file-type, and other -inspection errors with the affected path. +inspection errors with the affected path. Candidate-directory inspection first uses +`symlink_metadata`, then follows a candidate symlink with `metadata`: a symlink to an existing +directory remains eligible for marker probing and canonical alias handling, while a dangling +symlink, traversal failure, or non-directory candidate fails closed rather than looking absent. | Native marker | Legacy marker | Result | |---|---|---| @@ -95,8 +101,11 @@ inspection errors with the affected path. Selecting legacy is compatibility behavior, not a continuing default: Trawler never creates the cwd-relative path. A directory named `trawler-graph` without a regular snapshot marker does not count as legacy data. An existing non-empty native directory without a graph marker is -refused rather than adopted; an empty native directory may be initialized normally. No branch -copies, renames, deletes, opens, or otherwise mutates a graph during resolution. +refused rather than adopted, except for the narrowly recoverable debris written before an +interrupted first snapshot: a directory containing only `meta.json` and/or `snapshot.loro.tmp` +may be retried. An empty native directory may likewise be initialized normally. Any unrelated +entry or `updates.log` remains occupied. No branch copies, renames, deletes, opens, or otherwise +mutates a graph during resolution. If both candidate paths have markers, existing directories are canonicalized for identity only. Aliases to the same directory select the native spelling and warn instead of reporting two @@ -142,9 +151,10 @@ resolution errors fail cleanly before GPUI and that the attempted path is known. No data is migrated automatically. New installations create the native graph. Existing legacy-only installations continue opening their legacy graph with guidance to close Trawler, -make a backup, copy or move into the native location, verify the destination with an explicit -`TRAWLER_GRAPH_DIR`, and then rename/archive the cwd directory so it is no longer detected—or -continue using an explicit override. If both paths contain graph markers, the user selects one +make a backup, copy or move the complete directory into the native location, verify the +destination with an explicit `TRAWLER_GRAPH_DIR`, and then rename/archive the complete cwd +directory so it is no longer detected—never only its snapshot marker—or continue using an +explicit override. If both paths contain graph markers, the user selects one explicitly before launching. Rolling back the binary restores the old resolver and does not require data changes. diff --git a/openspec/changes/use-platform-native-graph-paths/specs/graph-location/spec.md b/openspec/changes/use-platform-native-graph-paths/specs/graph-location/spec.md index 9bd9230..c50769b 100644 --- a/openspec/changes/use-platform-native-graph-paths/specs/graph-location/spec.md +++ b/openspec/changes/use-platform-native-graph-paths/specs/graph-location/spec.md @@ -3,8 +3,9 @@ ### Requirement: Explicit graph override has highest precedence Trawler SHALL use a present, non-empty `TRAWLER_GRAPH_DIR` instead of platform-default or legacy graph selection. It SHALL preserve arbitrary OS-native path values, resolve an explicit -relative value against the launch working directory without dereferencing symlinks, and reject a -present empty value with an actionable error before opening or creating storage. +ordinary relative value against the launch working directory without dereferencing symlinks, +reject Windows drive-relative and rooted-without-prefix forms actionably, enforce that the +selected path is absolute, and reject a present empty value before opening or creating storage. #### Scenario: Absolute override wins - **WHEN** `TRAWLER_GRAPH_DIR` names an absolute path and native or legacy graphs also exist @@ -16,6 +17,11 @@ present empty value with an actionable error before opening or creating storage. - **THEN** Trawler selects its absolute equivalent under the launch working directory - **AND** does not treat it as an accidental default +#### Scenario: Unsafe Windows-relative override is refused +- **WHEN** `TRAWLER_GRAPH_DIR` is drive-relative or rooted without a drive or UNC prefix on Windows +- **THEN** Trawler exits before acquiring cwd or graph storage +- **AND** reports how to provide an absolute or ordinary relative path + #### Scenario: Relative override requires cwd - **WHEN** `TRAWLER_GRAPH_DIR` is relative and the launch working directory cannot be obtained - **THEN** Trawler exits before graph storage starts @@ -52,16 +58,23 @@ default graph relative to the process working directory. - **THEN** Trawler exits before inspecting or opening graph storage - **AND** instructs the user to set an absolute `TRAWLER_GRAPH_DIR` +#### Scenario: Interrupted first creation is retryable +- **WHEN** the native graph directory has no recognized graph marker and contains only `meta.json` and/or `snapshot.loro.tmp` +- **THEN** Trawler may select it for a retry of first creation +- **AND** any unrelated entry or `updates.log` instead keeps the directory occupied + #### Scenario: Occupied native directory is not adopted -- **WHEN** the native graph directory is non-empty but has no recognized graph marker +- **WHEN** the native graph directory is non-empty, has no recognized graph marker, and is not the narrowly recoverable partial-init set - **THEN** Trawler exits before creating or modifying files there -- **AND** identifies the occupied path +- **AND** identifies the occupied path without advising the user to empty the directory ### Requirement: Legacy cwd graphs remain accessible without automatic migration Without an explicit override, Trawler SHALL recognize a graph candidate only when its `snapshot.loro` marker is a regular file. Candidate inspection MUST distinguish not-found from -filesystem errors and MUST fail closed on permission, traversal, wrong-file-type, canonicalization, -or other inspection failures. Resolution MUST NOT copy, move, delete, open, or otherwise mutate +filesystem errors and MUST fail closed on dangling symlinks, permission, traversal, +wrong-file-type, canonicalization, or other inspection failures. A candidate path that is a +symlink to an existing directory SHALL remain eligible for marker probing and alias detection. +Resolution MUST NOT copy, move, delete, open, or otherwise mutate either candidate while deciding which path to select. #### Scenario: Legacy-only graph remains accessible @@ -124,4 +137,4 @@ source without mutating shared process environment or cwd. #### Scenario: User follows migration documentation - **WHEN** a user has a legacy cwd graph or both legacy and native graphs -- **THEN** the documentation explains how to close Trawler, back up, move or copy, verify via an explicit override, retire the detected legacy marker, and select a graph without ambiguity +- **THEN** the documentation explains how to close Trawler, back up, move or copy the complete directory, verify via an explicit override, retire the complete legacy directory, and select a graph without ambiguity diff --git a/openspec/changes/use-platform-native-graph-paths/tasks.md b/openspec/changes/use-platform-native-graph-paths/tasks.md index b2e1f9a..e8c07fe 100644 --- a/openspec/changes/use-platform-native-graph-paths/tasks.md +++ b/openspec/changes/use-platform-native-graph-paths/tasks.md @@ -1,30 +1,30 @@ ## 1. Graph marker and location resolver -- [ ] 1.1 Add an additive `trawler-core` read-only graph-marker probe that accepts only a regular `snapshot.loro`, distinguishes not-found from inspection errors, and never opens or mutates storage. -- [ ] 1.2 Add the direct pinned `dirs` dependency and an app-local `graph_path` module with typed selection sources, candidate states, and actionable errors. -- [ ] 1.3 Implement lazy override/cwd/native-root providers, lexical relative-path absolutization, absolute native `trawler/graph` composition, occupied-native refusal, canonical identity for existing aliases, and the complete native/legacy decision matrix. -- [ ] 1.4 Wire production `var_os`, `current_dir`, `dirs::data_dir`, and marker-probe inputs into the resolver while preserving non-UTF-8 `PathBuf` values end to end. +- [x] 1.1 Add an additive `trawler-core` read-only graph-marker probe that accepts only a regular `snapshot.loro`, distinguishes not-found from inspection errors, and never opens or mutates storage. +- [x] 1.2 Add the direct pinned `dirs` dependency and an app-local `graph_path` module with typed selection sources, candidate states, and actionable errors. +- [x] 1.3 Implement lazy override/cwd/native-root providers, lexical relative-path absolutization, absolute native `trawler/graph` composition, occupied-native refusal, canonical identity for existing aliases, and the complete native/legacy decision matrix. +- [x] 1.4 Wire production `var_os`, `current_dir`, `dirs::data_dir`, and marker-probe inputs into the resolver while preserving non-UTF-8 `PathBuf` values end to end. ## 2. Startup and diagnostics -- [ ] 2.1 Replace `default_graph_dir()` with fail-cleanly pre-GPUI resolution and a testable orchestration seam that emits the selected escaped path plus `override`, `platform default`, or `legacy compatibility` source before storage opens. -- [ ] 2.2 Emit actionable legacy-only warnings and native-plus-legacy ambiguity errors naming both paths and the `TRAWLER_GRAPH_DIR` recovery path. -- [ ] 2.3 Preserve the existing titlebar path display and verify that explicit scratch/devtools launches bypass native and legacy default selection. +- [x] 2.1 Replace `default_graph_dir()` with fail-cleanly pre-GPUI resolution and a testable orchestration seam that emits the selected escaped path plus `override`, `platform default`, or `legacy compatibility` source before storage opens. +- [x] 2.2 Emit actionable legacy-only warnings and native-plus-legacy ambiguity errors naming both paths and the `TRAWLER_GRAPH_DIR` recovery path. +- [x] 2.3 Preserve the existing titlebar path display and verify through preview smoke that explicit scratch/devtools launches bypass native and legacy default selection. ## 3. Deterministic tests -- [ ] 3.1 Test that absolute overrides call no cwd/native/probe providers; test relative override cwd failure, non-UTF-8 path preservation where supported, empty override refusal, absolute native-root validation, and native suffix composition through injected inputs. -- [ ] 3.2 Test every native/legacy marker combination, occupied native directories, non-regular markers, probe/canonicalization failures, and same-directory aliases, asserting selected source or actionable failure without changing shared environment or cwd. -- [ ] 3.3 Use spy probes, diagnostic sinks, and storage-launch callbacks to verify non-mutation, branch laziness, escaped diagnostics, and `probe → diagnostic → open/create` ordering; error branches MUST never invoke storage launch. -- [ ] 3.4 Add platform-gated assertions that the production native data root is absolute and maps to the documented OS family, without attempting to mock another OS's known-folder implementation. +- [x] 3.1 Test that absolute overrides call no cwd/native/probe providers; test ordinary relative joins, Windows-unsafe relative refusal, selected-path absoluteness, relative override cwd failure, non-UTF-8 path preservation where supported, empty override refusal, absolute native-root validation, and native suffix composition through injected inputs. +- [x] 3.2 Test every native/legacy marker combination, narrowly retryable partial initialization, occupied native directories, non-regular markers, dangling and existing-directory symlinks, probe/canonicalization failures, and same-directory aliases, asserting selected source or actionable failure without changing shared environment or cwd. +- [x] 3.3 Use spy probes, diagnostic sinks, and storage-launch callbacks to verify non-mutation, branch laziness, escaped diagnostics, and `probe → diagnostic → open/create` ordering; error branches MUST never invoke storage launch. +- [x] 3.4 Add platform-gated assertions that the production native data root is absolute and maps to the documented OS family, without attempting to mock another OS's known-folder implementation. ## 4. Documentation -- [ ] 4.1 Update README data-location guidance with Windows, macOS, and Linux defaults; the empty-override compatibility break; legacy warnings; ambiguous-state recovery; and the complete close → back up → move/copy → explicitly verify → retire legacy marker migration sequence. -- [ ] 4.2 Cross-reference the isolated Nix preview workflow so development scratch graphs remain clearly separate from user-default graph policy. +- [x] 4.1 Update README data-location guidance with Windows, macOS, and Linux defaults; the empty-override compatibility break; legacy warnings; occupied-native recovery; ambiguous-state recovery; and the complete close → back up → move/copy → explicitly verify → retire complete legacy directory migration sequence. +- [x] 4.2 Cross-reference the isolated Nix preview workflow so development scratch graphs remain clearly separate from user-default graph policy. ## 5. Verification -- [ ] 5.1 Run formatting, full-workspace check, clippy with warnings denied, and the non-ignored workspace tests inside the pinned Nix development shell. -- [ ] 5.2 Run the isolated preview smoke path and assert its manifest/application log use the helper-owned explicit scratch graph rather than the production native path. -- [ ] 5.3 Validate the OpenSpec change strictly and review the final diff against Tangled issue #5 with no graph-picker, automatic migration, or graph-format scope creep. +- [x] 5.1 Run formatting, full-workspace check, clippy with warnings denied, and the non-ignored workspace tests inside the pinned Nix development shell. (The pre-existing 2-second Steel wall-clock gate passed separately; the remaining workspace suite passed with that order-sensitive test isolated.) +- [x] 5.2 Run the isolated preview smoke path and assert its manifest/application log use the helper-owned explicit scratch graph rather than the production native path. +- [x] 5.3 Validate the OpenSpec change strictly and review the final diff against Tangled issue #5 with no graph-picker, automatic migration, or graph-format scope creep.