From 242b09125a95e7ab3d287fcfddb4a4df42328007 Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Thu, 9 Jul 2026 22:39:28 +0200 Subject: [PATCH 1/6] feat(cli, workspace): Add `jp workspace` and RFD 087 bootstrap Adds `jp workspace` (alias `jp w`), with `use`, `ls`, and `show` subcommands, plus a global `--workspace` grammar (ID, path, `cwd`, `-` for stdin, and interactive keywords like `?`, `s`, `l`). Selecting a workspace with `jp w use` makes it the session's active workspace, so subsequent commands in that terminal session run against it from any directory, the way an active conversation already follows the session (RFD 020). Standing inside a different workspace than the session's active one now prompts to resolve the conflict (`c`/`C`/ `a`/`A`/`q`), with `A` pinning the session sticky to its active workspace across cwd changes. A new pre-workspace bootstrap step in `jp_cli` resolves the launch cwd, the selected checkout root, and the working directory for spawned children once, before any `Workspace` is constructed. When JP operates on a workspace other than the launch cwd's own, MCP servers, plugins, and the `.jp.toml` config chain all run as if launched from the selected workspace root instead of the process cwd. Commands that only need to select or inspect a workspace (`jp w ls`, `jp w use`) no longer pay for workspace construction or conversation indexing. Workspace checkouts are now tracked in a per-workspace roots registry (`workspace/-/roots/`) instead of the single `storage` symlink, so a workspace with multiple live checkouts (e.g. git worktrees) can be targeted and listed by any of them; a one-time migration folds the legacy symlink into the registry. A new user-global `sessions/` store maps each terminal session to its active workspace, cleaned up on the same source-split liveness rules as the existing per-workspace session-to-conversation mapping (RFD 020). `WorkspaceIdOrPath` and the ad hoc `-w ` directory scan are removed in favor of the shared `WorkspaceTarget` grammar and roots registry, and the "silo" terminology in docs and code comments is renamed to "user-workspace directory" for clarity. Signed-off-by: Jean Mertz --- Cargo.lock | 2 +- crates/jp_cli/src/bootstrap.rs | 557 ++++++++++++++++ crates/jp_cli/src/bootstrap_tests.rs | 613 ++++++++++++++++++ crates/jp_cli/src/cmd.rs | 39 +- crates/jp_cli/src/cmd/attachment_tests.rs | 1 + crates/jp_cli/src/cmd/config/set_tests.rs | 1 + .../jp_cli/src/cmd/conversation/fork_tests.rs | 3 + .../jp_cli/src/cmd/conversation/grep_tests.rs | 2 + .../jp_cli/src/cmd/conversation/path_tests.rs | 1 + .../src/cmd/conversation/print_tests.rs | 1 + .../jp_cli/src/cmd/conversation/use_tests.rs | 2 + crates/jp_cli/src/cmd/plugin/dispatch.rs | 12 + crates/jp_cli/src/cmd/workspace.rs | 69 ++ crates/jp_cli/src/cmd/workspace/ls.rs | 94 +++ crates/jp_cli/src/cmd/workspace/ls_tests.rs | 123 ++++ crates/jp_cli/src/cmd/workspace/show.rs | 369 +++++++++++ crates/jp_cli/src/cmd/workspace/show_tests.rs | 152 +++++ crates/jp_cli/src/cmd/workspace/target.rs | 549 ++++++++++++++++ .../jp_cli/src/cmd/workspace/target_tests.rs | 317 +++++++++ crates/jp_cli/src/cmd/workspace/use_.rs | 123 ++++ crates/jp_cli/src/cmd/workspace/use_tests.rs | 232 +++++++ crates/jp_cli/src/ctx.rs | 11 +- crates/jp_cli/src/lib.rs | 216 +++--- crates/jp_cli/src/lib_tests.rs | 4 + crates/jp_cli/src/shared/search_tests.rs | 1 + crates/jp_inquire/src/lib.rs | 2 + crates/jp_mcp/src/client.rs | 36 +- crates/jp_mcp/src/lib.rs | 2 + crates/jp_storage/src/backend/fs.rs | 12 +- crates/jp_storage/src/error.rs | 3 - crates/jp_storage/src/lib.rs | 143 ++-- crates/jp_storage/src/lib_tests.rs | 39 +- crates/jp_workspace/Cargo.toml | 2 +- crates/jp_workspace/src/lib.rs | 7 + crates/jp_workspace/src/roots.rs | 354 ++++++++++ crates/jp_workspace/src/roots_tests.rs | 429 ++++++++++++ crates/jp_workspace/src/session_mapping.rs | 4 +- crates/jp_workspace/src/session_store.rs | 322 +++++++++ .../jp_workspace/src/session_store_tests.rs | 358 ++++++++++ docs/.vitepress/rfd-summaries.json | 4 +- docs/architecture/ubiquitous-language.md | 16 + ...ation-storage-with-workspace-projection.md | 39 +- .../087-session-scoped-active-workspace.md | 18 +- 43 files changed, 5046 insertions(+), 238 deletions(-) create mode 100644 crates/jp_cli/src/bootstrap.rs create mode 100644 crates/jp_cli/src/bootstrap_tests.rs create mode 100644 crates/jp_cli/src/cmd/workspace.rs create mode 100644 crates/jp_cli/src/cmd/workspace/ls.rs create mode 100644 crates/jp_cli/src/cmd/workspace/ls_tests.rs create mode 100644 crates/jp_cli/src/cmd/workspace/show.rs create mode 100644 crates/jp_cli/src/cmd/workspace/show_tests.rs create mode 100644 crates/jp_cli/src/cmd/workspace/target.rs create mode 100644 crates/jp_cli/src/cmd/workspace/target_tests.rs create mode 100644 crates/jp_cli/src/cmd/workspace/use_.rs create mode 100644 crates/jp_cli/src/cmd/workspace/use_tests.rs create mode 100644 crates/jp_workspace/src/roots.rs create mode 100644 crates/jp_workspace/src/roots_tests.rs create mode 100644 crates/jp_workspace/src/session_store.rs create mode 100644 crates/jp_workspace/src/session_store_tests.rs diff --git a/Cargo.lock b/Cargo.lock index 816d85b50..22561d6df 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -155,7 +155,7 @@ checksum = "9b34d609dfbaf33d6889b2b7106d3ca345eacad44200913df5ba02bfd31d2ba9" [[package]] name = "async-anthropic" version = "0.6.0" -source = "git+https://github.com/JeanMertz/async-anthropic#140c11637550c86273136f7eee076fb94cd1798b" +source = "git+https://github.com/JeanMertz/async-anthropic#6d2714bf7abec18e63daef27172978b695c92082" dependencies = [ "backon", "derive_builder", diff --git a/crates/jp_cli/src/bootstrap.rs b/crates/jp_cli/src/bootstrap.rs new file mode 100644 index 000000000..3fa6d6cf0 --- /dev/null +++ b/crates/jp_cli/src/bootstrap.rs @@ -0,0 +1,557 @@ +//! Pre-workspace bootstrap (RFD 087). +//! +//! Selecting a workspace by ID or path happens *before* a [`Workspace`] exists, +//! so a dedicated bootstrap step owns the pre-workspace resolution: it resolves +//! the launch cwd, selects a concrete checkout root, and derives the working +//! directory for spawned children — once, up front. +//! +//! Consumers (workspace construction, config loading, MCP and plugin spawns, +//! local tools) receive the resolved [`ExecutionContext`] explicitly instead of +//! re-deriving values from the process cwd at each call site, which is what +//! keeps the launch-cwd / root / child-cwd distinction from collapsing. + +use std::io; + +use camino::{Utf8Path, Utf8PathBuf}; +use chrono::Utc; +use crossterm::style::Stylize as _; +use inquire::Select; +use jp_workspace::{ + Id, Workspace, + roots::{self, RootEntry}, + session::Session, + session_store::WorkspaceSelection, +}; +use tracing::{debug, warn}; + +use crate::{ + DEFAULT_STORAGE_DIR, + cmd::{ + self, + workspace::target::{self as workspace_target, ResolvedTarget, TargetEnv, WorkspaceTarget}, + }, + error::Result, +}; + +/// What a command needs from the workspace bootstrap. +/// +/// The workspace-level analog of a command's conversation load request +/// (`conversation_load_request`): the bootstrap step reads this declaration and +/// only runs workspace resolution when the command asks for it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum WorkspaceRequirement { + /// No workspace is bootstrapped. + /// + /// The downstream consumers that assume a root — config loading, MCP and + /// plugin child cwd, path parsing — simply do not run. + None, + + /// Resolve and validate a target root, without loading the conversation + /// index. + Resolve, + + /// Resolve, construct the [`Workspace`], and load the conversation index. + Load, +} + +/// How the workspace root was selected. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum RootSource { + /// Derived from the launch cwd by walking up the directory tree. + Cwd, + + /// An explicit `--workspace ` target. + CliPath, + + /// An explicit `--workspace ` target, expanded through the workspace's + /// roots registry. + CliId, + + /// An explicit `--workspace` keyword target (`s`, `l`, `?`, free text). + CliSelector, + + /// The session's active workspace, from the user-global session store (`jp + /// w use`). + SessionActive, + + /// The interactive picker fallback, recorded as the session's new active + /// workspace. + Picker, +} + +/// The bootstrap-resolved execution context. +/// +/// A from-anywhere run keeps three directories distinct, which coincide only +/// when JP runs from inside the workspace it operates on: +/// +/// - **launch cwd** — where the user invoked `jp`. +/// - **workspace root** — the selected checkout root. +/// - **child cwd** — the working directory spawned children inherit. +#[derive(Debug)] +pub(crate) struct ExecutionContext { + /// Where the user invoked `jp`. + /// + /// The shell completed any relative path argument against this directory, + /// so user-typed relative paths resolve against it — never against the + /// workspace root. + pub(crate) launch_cwd: Utf8PathBuf, + + /// The selected checkout root. + pub(crate) root: Utf8PathBuf, + + /// The working directory for spawned children, when it differs from the + /// process cwd. + /// + /// See [`Self::child_cwd`]. + child_cwd: Option, + + /// How [`Self::root`] was selected. + pub(crate) source: RootSource, +} + +impl ExecutionContext { + /// The working directory spawned MCP servers, plugins, and local tools + /// inherit. + /// + /// `Some` — holding the workspace root — when JP operates on a workspace + /// whose root is not the launch cwd's own workspace: children then run as + /// if launched from the selected root. + /// `None` when JP runs from inside the selected workspace: children inherit + /// the process cwd unchanged. + pub(crate) fn child_cwd(&self) -> Option<&Utf8Path> { + self.child_cwd.as_deref() + } + + /// The directory config loading treats as the invocation directory. + /// + /// The `$CWD/.jp.{toml,json,yaml}` chain loads from here: the selected + /// workspace root for a from-anywhere run (config loads as if launched from + /// there), the launch cwd otherwise (so a subdirectory's `.jp.toml` chain + /// keeps loading as it does today). + pub(crate) fn config_cwd(&self) -> &Utf8Path { + self.child_cwd().unwrap_or(&self.launch_cwd) + } + + /// An execution context for a test-constructed workspace: as if `jp` was + /// launched from the workspace root itself. + #[cfg(test)] + pub(crate) fn for_workspace(workspace: &Workspace) -> Self { + Self { + launch_cwd: workspace.root().to_owned(), + root: workspace.root().to_owned(), + child_cwd: None, + source: RootSource::Cwd, + } + } +} + +/// Resolve the execution context for this invocation. +/// +/// The interactive precedence ladder (RFD 087): +/// +/// 1. An explicit `--workspace` target wins. +/// 2. Else a session **sticky** to its active workspace keeps using it, even +/// when the cwd resolves elsewhere. +/// 3. Else, when the cwd and the session-active workspace disagree, a prompt +/// decides — and can record the cwd as the new selection (`C`) or pin the +/// session sticky (`A`). +/// 4. Else the launch cwd's own workspace. +/// 5. Else the session-active workspace, while the workspace still has a live +/// checkout — recovering through surviving checkouts when the recorded one +/// is gone. +/// 6. Else the picker, recorded as the new session-active workspace. +/// +/// Non-interactive runs ignore the session layer entirely (steps 2–3 and +/// 5–6), so scripts never depend on hidden per-session state. +pub(crate) fn resolve( + target: Option<&WorkspaceTarget>, + session: Option<&Session>, +) -> Result { + let env = TargetEnv::new(session)?; + resolve_from(&env, target) +} + +/// [`resolve`], with the environment passed explicitly. +fn resolve_from(env: &TargetEnv<'_>, target: Option<&WorkspaceTarget>) -> Result { + let (root, source) = match target { + // An explicit target wins. + Some(target) => match workspace_target::resolve(target, env)? { + ResolvedTarget::Help => { + return Err(cmd::Error::from(workspace_target::help()).into()); + } + // `-w cwd`: resolve from the launch directory — explicitly, so + // the session layer does not apply. + ResolvedTarget::Cwd => (cwd_root(env)?, RootSource::Cwd), + ResolvedTarget::Root(selected) => (selected.root, source_for(target)), + }, + + // No explicit target: the session layer applies only to interactive + // runs with a session identity — scripts and identity-less sessions + // resolve from the cwd or error with guidance. + None => match (env.session, env.interactive) { + (Some(session), true) => ladder(env, session)?, + _ => (cwd_root(env)?, RootSource::Cwd), + }, + }; + + // The root-as-working-directory invariant: children run as if launched + // from the selected root whenever that root is not the launch cwd's own + // workspace. Runs from inside the selected workspace leave the child cwd + // untouched. + let child_cwd = if matches!(source, RootSource::Cwd) { + None + } else { + let launch_root = Workspace::find_root(env.launch_cwd.clone(), DEFAULT_STORAGE_DIR); + + (!launch_root.is_some_and(|launch_root| same_dir(&launch_root, &root))) + .then(|| root.clone()) + }; + + Ok(ExecutionContext { + launch_cwd: env.launch_cwd.clone(), + root, + child_cwd, + source, + }) +} + +/// The `RootSource` an explicit, root-producing target maps to. +fn source_for(target: &WorkspaceTarget) -> RootSource { + match target { + WorkspaceTarget::Path(_) => RootSource::CliPath, + WorkspaceTarget::Id(_) | WorkspaceTarget::Stdin => RootSource::CliId, + WorkspaceTarget::Session + | WorkspaceTarget::SessionPicker + | WorkspaceTarget::Picker + | WorkspaceTarget::Latest + | WorkspaceTarget::Fuzzy(_) => RootSource::CliSelector, + WorkspaceTarget::Cwd | WorkspaceTarget::Help => { + unreachable!("resolved before source mapping") + } + } +} + +/// The launch cwd's own workspace, or the no-workspace error. +fn cwd_root(env: &TargetEnv<'_>) -> Result { + Workspace::find_root(env.launch_cwd.clone(), DEFAULT_STORAGE_DIR) + .ok_or_else(|| no_workspace_error(env)) +} + +/// The no-target precedence ladder (RFD 087 steps 2–6): sticky pin, conflict +/// prompt, cwd, session-active workspace, picker. +/// +/// Only reached interactively with a session identity: a choice that cannot be +/// prompted for or recorded is not made at all. +fn ladder(env: &TargetEnv<'_>, session: &Session) -> Result<(Utf8PathBuf, RootSource)> { + let cwd = Workspace::find_root(env.launch_cwd.clone(), DEFAULT_STORAGE_DIR); + + let mapping = env.store.load(session); + let sticky = mapping.as_ref().is_some_and(|mapping| mapping.sticky); + let active = mapping + .and_then(|mapping| mapping.history.into_iter().next()) + .and_then(|entry| active_workspace(env, entry)); + + match (active, cwd) { + // Step 2: a sticky session keeps its active workspace, even when the + // cwd resolves elsewhere. + (Some(active), _) if sticky => { + active_root(env, session, active).map(|root| (root, RootSource::SessionActive)) + } + + // Step 3: the cwd and the active workspace disagree — prompt. + (Some(active), Some(cwd)) if !active.covers(&cwd) => { + let choice = conflict_choice(&active, &cwd)?; + apply_conflict_choice(env, session, choice, active, cwd) + } + + // Step 4: cwd wins when present. + (_, Some(cwd)) => Ok((cwd, RootSource::Cwd)), + + // Step 5: the session-active workspace, while it has a live checkout. + (Some(active), None) => { + active_root(env, session, active).map(|root| (root, RootSource::SessionActive)) + } + + // Step 6: the picker. + (None, None) => picker(env, session), + } +} + +/// The session's active workspace selection, resolved against liveness. +#[derive(Debug, Clone, PartialEq, Eq)] +enum ActiveWorkspace { + /// The recorded checkout root is still live. + Live { + /// The recorded checkout root. + root: Utf8PathBuf, + }, + + /// The recorded checkout is gone, but the workspace ID still has live + /// checkouts to recover through (RFD 087's reprompt). + Recoverable { + /// The workspace ID recovery expands. + id: Id, + + /// The ID's surviving live checkouts. + candidates: Vec, + }, +} + +impl ActiveWorkspace { + /// Whether `cwd_root` is a checkout this selection already denotes, so + /// standing inside the active workspace never prompts. + fn covers(&self, cwd_root: &Utf8Path) -> bool { + match self { + Self::Live { root } => same_dir(root, cwd_root), + // The recorded checkout is gone; when the cwd is one of the + // workspace's surviving checkouts, a prompt would offer two + // spellings of the same recovery answer — cwd wins instead. + Self::Recoverable { candidates, .. } => candidates + .iter() + .any(|entry| same_dir(&entry.path, cwd_root)), + } + } + + /// The prompt-facing description of the active side. + fn display(&self) -> String { + match self { + Self::Live { root } => root.to_string(), + Self::Recoverable { id, candidates } => { + format!("{id} (recorded checkout gone; {} live)", candidates.len()) + } + } + } +} + +/// Resolve the session's recorded selection to its live state. +/// +/// `None` when nothing usable remains — no parseable ID, or a workspace with +/// no live checkout left — which drops the selection out of the ladder; the +/// source-split cleanup pass prunes the record itself. +fn active_workspace(env: &TargetEnv<'_>, entry: WorkspaceSelection) -> Option { + let Some(id) = entry.id() else { + debug!( + root = %entry.root, + "Session-active record holds an unparseable workspace ID; ignoring it." + ); + return None; + }; + + if roots::is_live(&entry.root, &id, DEFAULT_STORAGE_DIR) { + return Some(ActiveWorkspace::Live { root: entry.root }); + } + + // The recorded checkout is gone: recover through the ID's surviving + // checkouts (RFD 087's reprompt-on-missing-workspace). + let candidates = roots::resolve_live_roots(&env.workspaces_dir, &id, DEFAULT_STORAGE_DIR); + if candidates.is_empty() { + debug!( + root = %entry.root, + workspace = %id, + "Session-active workspace has no live checkout left; falling through." + ); + return None; + } + + debug!( + root = %entry.root, + workspace = %id, + "Session-active checkout is gone; recovering through the workspace's surviving checkouts." + ); + Some(ActiveWorkspace::Recoverable { id, candidates }) +} + +/// The concrete checkout root of the active workspace. +/// +/// A live recorded root is used directly. +/// Recovery uses one surviving checkout directly and prompts among several (RFD +/// 087); the recovered choice repairs the session record, so the next run does +/// not recover again. +fn active_root( + env: &TargetEnv<'_>, + session: &Session, + active: ActiveWorkspace, +) -> Result { + match active { + ActiveWorkspace::Live { root } => Ok(root), + ActiveWorkspace::Recoverable { id, candidates } => { + let selected = workspace_target::select_root(&id, candidates, env.interactive)?; + + if let Err(error) = env + .store + .record_selection(session, &id, &selected.root, Utc::now()) + { + warn!(%error, "Failed to repair the session's active workspace record."); + } + + Ok(selected.root) + } + } +} + +/// A resolution of the cwd-vs-active conflict (RFD 087's `[c/C/a/A/q]`). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ConflictChoice { + /// `c` — use the cwd workspace for this run. + Current, + + /// `C` — use the cwd workspace and record it as the session's active one. + CurrentAndSelect, + + /// `a` — use the active workspace for this run. + Active, + + /// `A` — use the active workspace and pin the session **sticky** to it. + ActiveAndStick, + + /// `q` — quit without running the command. + Quit, +} + +/// The conflict prompt's rows, mirroring the RFD's `[c/C/a/A/q]` sketch. +const CONFLICT_ROWS: [(&str, ConflictChoice); 5] = [ + ("c - use current workspace", ConflictChoice::Current), + ( + "C - use current workspace and make it session-active", + ConflictChoice::CurrentAndSelect, + ), + ("a - use active workspace", ConflictChoice::Active), + ( + "A - use active workspace and keep the session sticky to it", + ConflictChoice::ActiveAndStick, + ), + ("q - quit without running command", ConflictChoice::Quit), +]; + +/// Prompt for a conflict resolution, writing to stderr. +fn conflict_choice(active: &ActiveWorkspace, cwd: &Utf8Path) -> Result { + let message = format!( + "The current directory is workspace `{cwd}`, but this session's active workspace is `{}`. \ + How to proceed?", + active.display() + ); + + let labels: Vec<&str> = CONFLICT_ROWS.iter().map(|(label, _)| *label).collect(); + let mut writer = io::stderr(); + let selected = Select::new(&message, labels).prompt_with_writer(&mut writer)?; + + Ok(CONFLICT_ROWS + .iter() + .find(|(label, _)| *label == selected) + .expect("selected label came from the list") + .1) +} + +/// Apply a conflict resolution: the run's (root, source), plus the `C` / `A` +/// store effects. +fn apply_conflict_choice( + env: &TargetEnv<'_>, + session: &Session, + choice: ConflictChoice, + active: ActiveWorkspace, + cwd: Utf8PathBuf, +) -> Result<(Utf8PathBuf, RootSource)> { + match choice { + ConflictChoice::Current => Ok((cwd, RootSource::Cwd)), + + ConflictChoice::CurrentAndSelect => { + let id = Id::load(cwd.join(DEFAULT_STORAGE_DIR)).and_then(std::result::Result::ok); + + if let Some(id) = id { + if let Err(error) = env.store.record_selection(session, &id, &cwd, Utc::now()) { + warn!(%error, "Failed to record the workspace selection."); + } + } else { + warn!( + root = %cwd, + "The current workspace has no readable ID; selection not recorded." + ); + } + + Ok((cwd, RootSource::Cwd)) + } + + ConflictChoice::Active => { + active_root(env, session, active).map(|root| (root, RootSource::SessionActive)) + } + + ConflictChoice::ActiveAndStick => { + let root = active_root(env, session, active)?; + + if let Err(error) = env.store.set_sticky(session, true) { + warn!(%error, "Failed to pin the session to its active workspace."); + } + + Ok((root, RootSource::SessionActive)) + } + + ConflictChoice::Quit => { + Err(cmd::Error::from("Aborted: quit without running the command.").into()) + } + } +} + +/// The ladder's last step: pick from every known workspace. +/// +/// The choice is recorded as the session's new active workspace: an +/// unrecordable choice would be re-made on every invocation, which is why the +/// session layer requires a session identity at all. +fn picker(env: &TargetEnv<'_>, session: &Session) -> Result<(Utf8PathBuf, RootSource)> { + let Some(selected) = workspace_target::pick_known_workspace(env, "Select a workspace")? else { + return Err(no_workspace_error(env)); + }; + + if let Some(id) = &selected.id + && let Err(error) = env + .store + .record_selection(session, id, &selected.root, Utc::now()) + { + warn!(%error, "Failed to record the workspace selection."); + } + + Ok((selected.root, RootSource::Picker)) +} + +/// The no-workspace error, with guidance matching how the run fell through: +/// non-interactive runs and identity-less sessions each get their way out. +fn no_workspace_error(env: &TargetEnv<'_>) -> crate::error::Error { + let jp_init = "jp init".bold().yellow(); + let workspace_flag = "--workspace ".bold().yellow(); + + let message = if !env.interactive { + format!( + "Could not locate workspace. Run from inside a workspace, pass `{workspace_flag}`, or \ + create one with `{jp_init}`." + ) + } else if env.session.is_none() { + format!( + "Could not locate workspace, and no session identity is available to select one. Pass \ + `{workspace_flag}`, set $JP_SESSION (or run in a terminal with automatic session \ + detection), or create a workspace with `{jp_init}`." + ) + } else { + format!("Could not locate workspace. Use `{jp_init}` to create a new workspace.") + }; + + cmd::Error::from(message).into() +} + +/// Whether two directory paths refer to the same location. +/// +/// Canonicalizes both sides to tolerate symlinked spellings of the same +/// checkout, falling back to literal equality when either side cannot be +/// canonicalized. +fn same_dir(a: &Utf8Path, b: &Utf8Path) -> bool { + if a == b { + return true; + } + + match (a.canonicalize(), b.canonicalize()) { + (Ok(a), Ok(b)) => a == b, + _ => false, + } +} + +#[cfg(test)] +#[path = "bootstrap_tests.rs"] +mod tests; diff --git a/crates/jp_cli/src/bootstrap_tests.rs b/crates/jp_cli/src/bootstrap_tests.rs new file mode 100644 index 000000000..132f16a48 --- /dev/null +++ b/crates/jp_cli/src/bootstrap_tests.rs @@ -0,0 +1,613 @@ +use std::str::FromStr as _; + +use camino::Utf8PathBuf; +use camino_tempfile::tempdir; +use jp_workspace::{ + Id, + session::{Session, SessionId, SessionSource}, + session_store::WorkspaceSessionStore, +}; + +use super::*; + +/// Create a workspace root (a directory containing `.jp/`) under `base`. +fn make_workspace(base: &Utf8Path, name: &str) -> Utf8PathBuf { + let root = base.join(name); + std::fs::create_dir_all(root.join(DEFAULT_STORAGE_DIR)).unwrap(); + root +} + +/// Create a workspace root whose storage directory carries a readable ID, so +/// liveness checks (`roots::is_live`) can match it. +fn make_workspace_with_id(base: &Utf8Path, name: &str, id: &str) -> Utf8PathBuf { + let root = make_workspace(base, name); + std::fs::write(root.join(DEFAULT_STORAGE_DIR).join(".id"), id).unwrap(); + root +} + +/// A `TargetEnv` with explicit launch cwd, user-data directory, session, and +/// interactivity, so tests never touch the real process environment. +fn env_at<'a>( + launch_cwd: Utf8PathBuf, + data_dir: &Utf8Path, + session: Option<&'a Session>, + interactive: bool, +) -> TargetEnv<'a> { + TargetEnv { + launch_cwd, + workspaces_dir: data_dir.join(crate::USER_WORKSPACES_DIR), + store: WorkspaceSessionStore::at_user_data_dir(data_dir), + session, + interactive, + } +} + +/// An `Env`-sourced session identity. +fn env_session() -> Session { + Session { + id: SessionId::new("42").expect("non-empty"), + source: SessionSource::env("JP_SESSION"), + } +} + +/// Render an error with its full source chain, so assertions can match the +/// message of a wrapped `cmd::Error`. +fn error_message(error: &crate::Error) -> String { + let mut message = error.to_string(); + let mut source = std::error::Error::source(error); + while let Some(inner) = source { + message.push_str(": "); + message.push_str(&inner.to_string()); + source = inner.source(); + } + message +} + +#[test] +fn cwd_inside_workspace_resolves_root_without_child_cwd() { + let tmp = tempdir().unwrap(); + let root = make_workspace(tmp.path(), "ws"); + + let env = env_at(root.clone(), tmp.path(), None, false); + let exec = resolve_from(&env, None).unwrap(); + + assert_eq!(exec.root, root); + assert_eq!(exec.launch_cwd, root); + assert_eq!(exec.source, RootSource::Cwd); + assert_eq!(exec.child_cwd(), None); + assert_eq!(exec.config_cwd(), root); +} + +#[test] +fn cwd_in_subdirectory_keeps_launch_cwd_and_config_cwd() { + let tmp = tempdir().unwrap(); + let root = make_workspace(tmp.path(), "ws"); + let subdir = root.join("crates").join("foo"); + std::fs::create_dir_all(&subdir).unwrap(); + + let env = env_at(subdir.clone(), tmp.path(), None, false); + let exec = resolve_from(&env, None).unwrap(); + + assert_eq!(exec.root, root); + assert_eq!(exec.launch_cwd, subdir); + // Launched from inside the workspace: children inherit the process cwd, + // and the subdirectory's `.jp.toml` chain keeps loading from the launch + // cwd. + assert_eq!(exec.child_cwd(), None); + assert_eq!(exec.config_cwd(), subdir); +} + +#[test] +fn cwd_outside_any_workspace_errors() { + let tmp = tempdir().unwrap(); + let scratch = tmp.path().join("scratch"); + std::fs::create_dir_all(&scratch).unwrap(); + + let env = env_at(scratch, tmp.path(), None, false); + let error = error_message(&resolve_from(&env, None).unwrap_err()); + + assert!( + error.contains("Could not locate workspace"), + "unexpected error: {error}" + ); +} + +#[test] +fn cwd_target_resolves_launch_directory_workspace() { + let tmp = tempdir().unwrap(); + let root = make_workspace(tmp.path(), "ws"); + + // `-w cwd`: explicit, but resolves exactly like no target. + let env = env_at(root.clone(), tmp.path(), None, false); + let exec = resolve_from(&env, Some(&WorkspaceTarget::Cwd)).unwrap(); + + assert_eq!(exec.root, root); + assert_eq!(exec.source, RootSource::Cwd); + assert_eq!(exec.child_cwd(), None); +} + +#[test] +fn path_target_from_outside_runs_children_at_root() { + let tmp = tempdir().unwrap(); + let root = make_workspace(tmp.path(), "ws"); + let scratch = tmp.path().join("scratch"); + std::fs::create_dir_all(&scratch).unwrap(); + + let target = WorkspaceTarget::Path(root.clone()); + let env = env_at(scratch.clone(), tmp.path(), None, false); + let exec = resolve_from(&env, Some(&target)).unwrap(); + + assert_eq!(exec.root, root); + assert_eq!(exec.launch_cwd, scratch); + assert_eq!(exec.source, RootSource::CliPath); + // The root-as-working-directory invariant: children run as if launched + // from the selected root, and config loads as if launched from there. + assert_eq!(exec.child_cwd(), Some(root.as_path())); + assert_eq!(exec.config_cwd(), root); +} + +#[test] +fn path_target_to_subdirectory_resolves_containing_root() { + let tmp = tempdir().unwrap(); + let root = make_workspace(tmp.path(), "ws"); + let subdir = root.join("docs"); + std::fs::create_dir_all(&subdir).unwrap(); + let scratch = tmp.path().join("scratch"); + std::fs::create_dir_all(&scratch).unwrap(); + + let target = WorkspaceTarget::Path(subdir); + let env = env_at(scratch, tmp.path(), None, false); + let exec = resolve_from(&env, Some(&target)).unwrap(); + + assert_eq!(exec.root, root); + assert_eq!(exec.child_cwd(), Some(root.as_path())); +} + +#[test] +fn path_target_into_own_workspace_leaves_child_cwd_unchanged() { + let tmp = tempdir().unwrap(); + let root = make_workspace(tmp.path(), "ws"); + let subdir = root.join("crates"); + std::fs::create_dir_all(&subdir).unwrap(); + + // Standing inside the workspace and explicitly targeting it: children + // inherit the process cwd, as if no target was given. + let target = WorkspaceTarget::Path(root.clone()); + let env = env_at(subdir.clone(), tmp.path(), None, false); + let exec = resolve_from(&env, Some(&target)).unwrap(); + + assert_eq!(exec.root, root); + assert_eq!(exec.source, RootSource::CliPath); + assert_eq!(exec.child_cwd(), None); + assert_eq!(exec.config_cwd(), subdir); +} + +#[test] +fn path_target_from_different_workspace_runs_children_at_target_root() { + let tmp = tempdir().unwrap(); + let target_root = make_workspace(tmp.path(), "target"); + let other_root = make_workspace(tmp.path(), "other"); + + let target = WorkspaceTarget::Path(target_root.clone()); + let env = env_at(other_root.clone(), tmp.path(), None, false); + let exec = resolve_from(&env, Some(&target)).unwrap(); + + assert_eq!(exec.root, target_root); + assert_eq!(exec.launch_cwd, other_root); + assert_eq!(exec.child_cwd(), Some(target_root.as_path())); + assert_eq!(exec.config_cwd(), target_root); +} + +#[test] +fn nonexistent_path_target_errors() { + let tmp = tempdir().unwrap(); + let scratch = tmp.path().join("scratch"); + std::fs::create_dir_all(&scratch).unwrap(); + + let target = WorkspaceTarget::Path(tmp.path().join("missing")); + let env = env_at(scratch, tmp.path(), None, false); + let error = error_message(&resolve_from(&env, Some(&target)).unwrap_err()); + + assert!( + error.contains("No workspace found"), + "unexpected error: {error}" + ); +} + +#[test] +fn session_active_workspace_wins_from_outside_when_interactive() { + let tmp = tempdir().unwrap(); + let root = make_workspace_with_id(tmp.path(), "ws", "ws123"); + let scratch = tmp.path().join("scratch"); + std::fs::create_dir_all(&scratch).unwrap(); + + let session = env_session(); + let env = env_at(scratch.clone(), tmp.path(), Some(&session), true); + env.store + .record_selection(&session, &Id::from_str("ws123").unwrap(), &root, Utc::now()) + .unwrap(); + + let exec = resolve_from(&env, None).unwrap(); + + assert_eq!(exec.root, root); + assert_eq!(exec.launch_cwd, scratch); + assert_eq!(exec.source, RootSource::SessionActive); + // A from-anywhere run: children run at the selected root. + assert_eq!(exec.child_cwd(), Some(root.as_path())); + assert_eq!(exec.config_cwd(), root); +} + +#[test] +fn cwd_matching_the_active_workspace_wins_without_prompt() { + let tmp = tempdir().unwrap(); + let root = make_workspace_with_id(tmp.path(), "ws", "ws123"); + + let session = env_session(); + let env = env_at(root.clone(), tmp.path(), Some(&session), true); + env.store + .record_selection(&session, &Id::from_str("ws123").unwrap(), &root, Utc::now()) + .unwrap(); + + // Standing inside the active workspace is not a conflict: cwd resolution + // applies without prompting. + let exec = resolve_from(&env, None).unwrap(); + + assert_eq!(exec.root, root); + assert_eq!(exec.source, RootSource::Cwd); + assert_eq!(exec.child_cwd(), None); +} + +#[test] +fn sticky_session_keeps_its_active_workspace_over_a_different_cwd() { + let tmp = tempdir().unwrap(); + let active = make_workspace_with_id(tmp.path(), "active", "ws123"); + let current = make_workspace_with_id(tmp.path(), "current", "ws456"); + + let session = env_session(); + let env = env_at(current.clone(), tmp.path(), Some(&session), true); + env.store + .record_selection( + &session, + &Id::from_str("ws123").unwrap(), + &active, + Utc::now(), + ) + .unwrap(); + env.store.set_sticky(&session, true).unwrap(); + + // The persisted `A` choice: the active workspace wins without a prompt, + // even though the cwd resolves to a different workspace. + let exec = resolve_from(&env, None).unwrap(); + + assert_eq!(exec.root, active); + assert_eq!(exec.source, RootSource::SessionActive); + // A from-anywhere run: children run at the selected root. + assert_eq!(exec.child_cwd(), Some(active.as_path())); + assert_eq!(exec.config_cwd(), active); +} + +#[test] +fn dead_active_root_recovers_through_the_surviving_checkout() { + let tmp = tempdir().unwrap(); + // The registry canonicalizes recorded paths; keep expectations literal. + let base = tmp.path().canonicalize_utf8().unwrap(); + let dead = make_workspace_with_id(&base, "dead", "ws123"); + let survivor = make_workspace_with_id(&base, "survivor", "ws123"); + let scratch = base.join("scratch"); + std::fs::create_dir_all(&scratch).unwrap(); + + let session = env_session(); + let env = env_at(scratch, &base, Some(&session), true); + env.store + .record_selection(&session, &Id::from_str("ws123").unwrap(), &dead, Utc::now()) + .unwrap(); + jp_workspace::roots::upsert_root(&env.workspaces_dir.join("ws-ws123"), &survivor).unwrap(); + std::fs::remove_dir_all(&dead).unwrap(); + + // One surviving checkout: recovery uses it directly and repairs the + // session record (RFD 087's reprompt-on-missing-workspace). + let exec = resolve_from(&env, None).unwrap(); + + assert_eq!(exec.root, survivor); + assert_eq!(exec.source, RootSource::SessionActive); + assert_eq!(env.store.active(&session).unwrap().root, survivor); +} + +#[test] +fn sticky_recovery_survives_a_different_cwd() { + let tmp = tempdir().unwrap(); + let base = tmp.path().canonicalize_utf8().unwrap(); + let dead = make_workspace_with_id(&base, "dead", "ws123"); + let survivor = make_workspace_with_id(&base, "survivor", "ws123"); + let current = make_workspace_with_id(&base, "current", "ws456"); + + let session = env_session(); + let env = env_at(current, &base, Some(&session), true); + env.store + .record_selection(&session, &Id::from_str("ws123").unwrap(), &dead, Utc::now()) + .unwrap(); + env.store.set_sticky(&session, true).unwrap(); + jp_workspace::roots::upsert_root(&env.workspaces_dir.join("ws-ws123"), &survivor).unwrap(); + std::fs::remove_dir_all(&dead).unwrap(); + + // A sticky session whose recorded checkout died recovers through the + // workspace's surviving checkout — the pin is not dropped (RFD 087). + let exec = resolve_from(&env, None).unwrap(); + + assert_eq!(exec.root, survivor); + assert_eq!(exec.source, RootSource::SessionActive); + let mapping = env.store.load(&session).unwrap(); + assert!(mapping.sticky); + assert_eq!(mapping.history[0].root, survivor); +} + +#[test] +fn cwd_sibling_of_a_dead_active_checkout_wins_without_prompt() { + let tmp = tempdir().unwrap(); + let base = tmp.path().canonicalize_utf8().unwrap(); + let dead = make_workspace_with_id(&base, "dead", "ws123"); + let survivor = make_workspace_with_id(&base, "survivor", "ws123"); + + let session = env_session(); + let env = env_at(survivor.clone(), &base, Some(&session), true); + env.store + .record_selection(&session, &Id::from_str("ws123").unwrap(), &dead, Utc::now()) + .unwrap(); + jp_workspace::roots::upsert_root(&env.workspaces_dir.join("ws-ws123"), &survivor).unwrap(); + std::fs::remove_dir_all(&dead).unwrap(); + + // The recorded checkout is gone and the cwd is one of the workspace's + // surviving checkouts: not a conflict — cwd wins without a prompt. + let exec = resolve_from(&env, None).unwrap(); + + assert_eq!(exec.root, survivor); + assert_eq!(exec.source, RootSource::Cwd); + assert_eq!(exec.child_cwd(), None); +} + +#[test] +fn non_interactive_run_ignores_session_active_workspace() { + let tmp = tempdir().unwrap(); + let root = make_workspace_with_id(tmp.path(), "ws", "ws123"); + let scratch = tmp.path().join("scratch"); + std::fs::create_dir_all(&scratch).unwrap(); + + let session = env_session(); + let env = env_at(scratch, tmp.path(), Some(&session), false); + env.store + .record_selection(&session, &Id::from_str("ws123").unwrap(), &root, Utc::now()) + .unwrap(); + + // Scripts never depend on hidden per-session state: outside a workspace + // and without `--workspace`, a non-interactive run errors. + let error = error_message(&resolve_from(&env, None).unwrap_err()); + + assert!( + error.contains("Could not locate workspace"), + "unexpected error: {error}" + ); + assert!( + error.contains("--workspace"), + "expected `--workspace` guidance: {error}" + ); +} + +#[test] +fn dead_session_active_root_falls_through_to_error_without_candidates() { + let tmp = tempdir().unwrap(); + let root = make_workspace_with_id(tmp.path(), "ws", "ws123"); + let scratch = tmp.path().join("scratch"); + std::fs::create_dir_all(&scratch).unwrap(); + + let session = env_session(); + let env = env_at(scratch, tmp.path(), Some(&session), true); + env.store + .record_selection(&session, &Id::from_str("ws123").unwrap(), &root, Utc::now()) + .unwrap(); + + // The recorded root dies and no other workspace is known: the ladder + // falls through the (empty) picker to the no-workspace error. + std::fs::remove_dir_all(&root).unwrap(); + + let error = error_message(&resolve_from(&env, None).unwrap_err()); + + assert!( + error.contains("Could not locate workspace"), + "unexpected error: {error}" + ); +} + +#[test] +fn interactive_run_without_session_identity_points_at_session_setup() { + let tmp = tempdir().unwrap(); + let scratch = tmp.path().join("scratch"); + std::fs::create_dir_all(&scratch).unwrap(); + + let env = env_at(scratch, tmp.path(), None, true); + let error = error_message(&resolve_from(&env, None).unwrap_err()); + + assert!( + error.contains("no session identity"), + "unexpected error: {error}" + ); + assert!(error.contains("JP_SESSION"), "unexpected error: {error}"); +} + +#[test] +fn same_dir_tolerates_symlinked_spellings() { + let tmp = tempdir().unwrap(); + let root = make_workspace(tmp.path(), "ws"); + + assert!(same_dir(&root, &root)); + assert!(!same_dir(&root, tmp.path())); + + #[cfg(unix)] + { + let link = tmp.path().join("link"); + std::os::unix::fs::symlink(&root, &link).unwrap(); + assert!(same_dir(&root, &link)); + } +} + +#[test] +fn symlinked_target_into_own_workspace_leaves_child_cwd_unchanged() { + #[cfg(unix)] + { + let tmp = tempdir().unwrap(); + let root = make_workspace(tmp.path(), "ws"); + let link = tmp.path().join("link"); + std::os::unix::fs::symlink(&root, &link).unwrap(); + + // `-w` targeting a symlinked spelling of the workspace we are + // standing in: same checkout, so children inherit the process cwd. + let target = WorkspaceTarget::Path(link); + let env = env_at(root.clone(), tmp.path(), None, false); + let exec = resolve_from(&env, Some(&target)).unwrap(); + + assert_eq!(exec.child_cwd(), None); + } +} + +/// A (env, session, active-root, cwd-root) fixture for conflict-choice tests: +/// `ws123` at `active/` is the recorded selection, `ws456` at `current/` is the +/// cwd workspace. +fn conflict_fixture( + tmp: &Utf8Path, + session: &Session, +) -> (TargetEnv<'static>, Utf8PathBuf, Utf8PathBuf) { + let active = make_workspace_with_id(tmp, "active", "ws123"); + let current = make_workspace_with_id(tmp, "current", "ws456"); + + let env = env_at(current.clone(), tmp, None, true); + env.store + .record_selection( + session, + &Id::from_str("ws123").unwrap(), + &active, + Utc::now(), + ) + .unwrap(); + + (env, active, current) +} + +#[test] +fn conflict_choice_current_keeps_the_cwd_and_the_record() { + let tmp = tempdir().unwrap(); + let session = env_session(); + let (env, active, current) = conflict_fixture(tmp.path(), &session); + + let (root, source) = apply_conflict_choice( + &env, + &session, + ConflictChoice::Current, + ActiveWorkspace::Live { + root: active.clone(), + }, + current.clone(), + ) + .unwrap(); + + assert_eq!(root, current); + assert_eq!(source, RootSource::Cwd); + // `c` is one-shot: the session record is untouched. + let mapping = env.store.load(&session).unwrap(); + assert_eq!(mapping.history[0].root, active); + assert!(!mapping.sticky); +} + +#[test] +fn conflict_choice_current_and_select_records_the_cwd_workspace() { + let tmp = tempdir().unwrap(); + let session = env_session(); + let (env, active, current) = conflict_fixture(tmp.path(), &session); + + let (root, source) = apply_conflict_choice( + &env, + &session, + ConflictChoice::CurrentAndSelect, + ActiveWorkspace::Live { + root: active.clone(), + }, + current.clone(), + ) + .unwrap(); + + assert_eq!(root, current); + assert_eq!(source, RootSource::Cwd); + // `C` records the cwd workspace as the new selection; the former active + // workspace becomes the `s` target. + let mapping = env.store.load(&session).unwrap(); + assert_eq!(mapping.history[0].workspace_id, "ws456"); + assert_eq!(mapping.history[0].root, current); + assert_eq!(mapping.history[1].root, active); + assert!(!mapping.sticky); +} + +#[test] +fn conflict_choice_active_resolves_the_recorded_root_without_pinning() { + let tmp = tempdir().unwrap(); + let session = env_session(); + let (env, active, current) = conflict_fixture(tmp.path(), &session); + + let (root, source) = apply_conflict_choice( + &env, + &session, + ConflictChoice::Active, + ActiveWorkspace::Live { + root: active.clone(), + }, + current, + ) + .unwrap(); + + assert_eq!(root, active); + assert_eq!(source, RootSource::SessionActive); + // `a` is one-shot: no sticky pin. + assert!(!env.store.load(&session).unwrap().sticky); +} + +#[test] +fn conflict_choice_active_and_stick_pins_the_session() { + let tmp = tempdir().unwrap(); + let session = env_session(); + let (env, active, current) = conflict_fixture(tmp.path(), &session); + + let (root, source) = apply_conflict_choice( + &env, + &session, + ConflictChoice::ActiveAndStick, + ActiveWorkspace::Live { + root: active.clone(), + }, + current, + ) + .unwrap(); + + assert_eq!(root, active); + assert_eq!(source, RootSource::SessionActive); + // `A` persists on the session record. + let mapping = env.store.load(&session).unwrap(); + assert!(mapping.sticky); + assert_eq!(mapping.history[0].root, active); +} + +#[test] +fn conflict_choice_quit_aborts_the_run() { + let tmp = tempdir().unwrap(); + let session = env_session(); + let (env, active, current) = conflict_fixture(tmp.path(), &session); + + let error = error_message( + &apply_conflict_choice( + &env, + &session, + ConflictChoice::Quit, + ActiveWorkspace::Live { root: active }, + current, + ) + .unwrap_err(), + ); + + assert!(error.contains("Aborted"), "unexpected error: {error}"); +} diff --git a/crates/jp_cli/src/cmd.rs b/crates/jp_cli/src/cmd.rs index faaf78b70..b6519e6b2 100644 --- a/crates/jp_cli/src/cmd.rs +++ b/crates/jp_cli/src/cmd.rs @@ -10,6 +10,7 @@ mod query; pub(crate) mod target; pub(crate) mod time; pub(crate) mod turn_range; +pub(crate) mod workspace; use std::{fmt, num::NonZeroU8}; @@ -19,7 +20,7 @@ use serde_json::Value; pub(crate) use target::ConversationLoadRequest; use super::cmd::conversation_id::format_target_help; -use crate::{Ctx, ctx::IntoPartialAppConfig}; +use crate::{Ctx, bootstrap::WorkspaceRequirement, ctx::IntoPartialAppConfig}; #[derive(Debug, clap::Subcommand)] #[command(disable_help_subcommand = true, allow_external_subcommands = true)] @@ -50,6 +51,10 @@ pub(crate) enum Commands { /// Manage plugins. Plugin(plugin::PluginManagement), + /// Manage workspaces. + #[command(visible_alias = "w", alias = "workspaces")] + Workspace(workspace::Workspace), + /// External plugin subcommand (`jp-` on $PATH or registry). #[command(external_subcommand)] External(Vec), @@ -83,7 +88,9 @@ impl Commands { } Commands::Plugin(args) => args.run(ctx).await, Commands::External(args) => plugin::dispatch::run_external(&args, ctx).await, - Commands::Init(_) => unreachable!("handled before workspace initialization"), + Commands::Init(_) | Commands::Workspace(_) => { + unreachable!("handled before workspace initialization") + } } } @@ -98,10 +105,30 @@ impl Commands { | Commands::Attachment(_) | Commands::AttachmentAdd(_) | Commands::Plugin(_) + | Commands::Workspace(_) | Commands::External(_) => ConversationLoadRequest::none(), } } + /// Declare what this command needs from the workspace bootstrap (RFD 087). + /// + /// The workspace-level analog of [`Self::conversation_load_request`]: the + /// bootstrap step only runs workspace resolution when the command asks for + /// it. + pub(crate) fn workspace_requirement(&self) -> WorkspaceRequirement { + match self { + Commands::Init(_) => WorkspaceRequirement::None, + Commands::Workspace(args) => args.workspace_requirement(), + Commands::Query(_) + | Commands::Config(_) + | Commands::Conversation(_) + | Commands::Attachment(_) + | Commands::AttachmentAdd(_) + | Commands::Plugin(_) + | Commands::External(_) => WorkspaceRequirement::Load, + } + } + /// Whether the interactive conversation picker should offer a "start a new /// conversation" item for this command. pub(crate) fn allows_new_from_picker(&self) -> bool { @@ -120,6 +147,7 @@ impl Commands { Commands::Init(_) => "init", Commands::Conversation(_) => "conversation", Commands::Plugin(_) => "plugin", + Commands::Workspace(_) => "workspace", Commands::External(args) => { // Use first arg as the command name (it's the subcommand name). // Clap puts the subcommand name as the first element. @@ -150,6 +178,7 @@ impl IntoPartialAppConfig for Commands { | Commands::Conversation(_) | Commands::Init(_) | Commands::Plugin(_) + | Commands::Workspace(_) | Commands::External(_) => Ok(partial), } } @@ -171,6 +200,7 @@ impl IntoPartialAppConfig for Commands { | Commands::Conversation(_) | Commands::Init(_) | Commands::Plugin(_) + | Commands::Workspace(_) | Commands::External(_) => Ok(partial), } } @@ -697,11 +727,6 @@ impl From for Error { ("path", path.to_string().into()), ] .into(), - Error::NotSymlink(path) => [ - ("message", "Path is not a symlink.".into()), - ("path", path.to_string().into()), - ] - .into(), Error::ConversationNotFound(id) => [ ("message", "Conversation not found.".into()), ("id", id.to_string().into()), diff --git a/crates/jp_cli/src/cmd/attachment_tests.rs b/crates/jp_cli/src/cmd/attachment_tests.rs index b10968f43..818085561 100644 --- a/crates/jp_cli/src/cmd/attachment_tests.rs +++ b/crates/jp_cli/src/cmd/attachment_tests.rs @@ -26,6 +26,7 @@ fn empty_ctx() -> (Ctx, Runtime) { let (printer, _out, _err) = Printer::memory(OutputFormat::Text); let runtime = Runtime::new().unwrap(); let ctx = Ctx::new( + crate::bootstrap::ExecutionContext::for_workspace(&workspace), workspace, None, Runtime::new().unwrap(), diff --git a/crates/jp_cli/src/cmd/config/set_tests.rs b/crates/jp_cli/src/cmd/config/set_tests.rs index ccc32e9a6..f464fb1d7 100644 --- a/crates/jp_cli/src/cmd/config/set_tests.rs +++ b/crates/jp_cli/src/cmd/config/set_tests.rs @@ -57,6 +57,7 @@ fn setup( }; let fs_backend = Some(fs); let ctx = Ctx::new( + crate::bootstrap::ExecutionContext::for_workspace(&workspace), workspace, fs_backend, Runtime::new().unwrap(), diff --git a/crates/jp_cli/src/cmd/conversation/fork_tests.rs b/crates/jp_cli/src/cmd/conversation/fork_tests.rs index db943db15..9541a56d7 100644 --- a/crates/jp_cli/src/cmd/conversation/fork_tests.rs +++ b/crates/jp_cli/src/cmd/conversation/fork_tests.rs @@ -923,6 +923,7 @@ fn test_conversation_fork() { ); let workspace = Workspace::new(tmp.path()).with_backend(fs); let mut ctx = Ctx::new( + crate::bootstrap::ExecutionContext::for_workspace(&workspace), workspace, None, Runtime::new().unwrap(), @@ -992,6 +993,7 @@ fn fork_targets_correct_source() { ); let workspace = Workspace::new(tmp.path()).with_backend(fs); let mut ctx = Ctx::new( + crate::bootstrap::ExecutionContext::for_workspace(&workspace), workspace, None, Runtime::new().unwrap(), @@ -1124,6 +1126,7 @@ fn fork_inherits_local_only_projection() { ); let workspace = Workspace::new(tmp.path()).with_backend(fs); let mut ctx = Ctx::new( + crate::bootstrap::ExecutionContext::for_workspace(&workspace), workspace, None, Runtime::new().unwrap(), diff --git a/crates/jp_cli/src/cmd/conversation/grep_tests.rs b/crates/jp_cli/src/cmd/conversation/grep_tests.rs index 89aef5142..14191eed9 100644 --- a/crates/jp_cli/src/cmd/conversation/grep_tests.rs +++ b/crates/jp_cli/src/cmd/conversation/grep_tests.rs @@ -36,6 +36,7 @@ fn setup_ctx_with_conversations( let workspace = Workspace::new(tmp.path()); let (printer, out, _err) = Printer::memory(OutputFormat::TextPretty); let mut ctx = Ctx::new( + crate::bootstrap::ExecutionContext::for_workspace(&workspace), workspace, None, Runtime::new().unwrap(), @@ -90,6 +91,7 @@ fn setup_ctx_with_conversations_and_format( let workspace = Workspace::new(tmp.path()); let (printer, out, _err) = Printer::memory(format); let mut ctx = Ctx::new( + crate::bootstrap::ExecutionContext::for_workspace(&workspace), workspace, None, Runtime::new().unwrap(), diff --git a/crates/jp_cli/src/cmd/conversation/path_tests.rs b/crates/jp_cli/src/cmd/conversation/path_tests.rs index 0eea75d07..d55ae5663 100644 --- a/crates/jp_cli/src/cmd/conversation/path_tests.rs +++ b/crates/jp_cli/src/cmd/conversation/path_tests.rs @@ -33,6 +33,7 @@ fn setup(id: ConversationId) -> (Ctx, SharedBuffer, Utf8TempDir) { let (printer, out, _err) = Printer::memory(OutputFormat::Text); let ctx = Ctx::new( + crate::bootstrap::ExecutionContext::for_workspace(&workspace), workspace, Some(fs), Runtime::new().unwrap(), diff --git a/crates/jp_cli/src/cmd/conversation/print_tests.rs b/crates/jp_cli/src/cmd/conversation/print_tests.rs index 55c069b41..bbe0712bc 100644 --- a/crates/jp_cli/src/cmd/conversation/print_tests.rs +++ b/crates/jp_cli/src/cmd/conversation/print_tests.rs @@ -50,6 +50,7 @@ fn setup_ctx_with_config( let runtime = Runtime::new().unwrap(); let mut ctx = Ctx::new( + crate::bootstrap::ExecutionContext::for_workspace(&workspace), workspace, None, Runtime::new().unwrap(), diff --git a/crates/jp_cli/src/cmd/conversation/use_tests.rs b/crates/jp_cli/src/cmd/conversation/use_tests.rs index 76599510c..899e325ab 100644 --- a/crates/jp_cli/src/cmd/conversation/use_tests.rs +++ b/crates/jp_cli/src/cmd/conversation/use_tests.rs @@ -53,6 +53,7 @@ fn setup(id: ConversationId) -> Ctx { let (printer, _, _) = Printer::memory(OutputFormat::TextPretty); let mut ctx = Ctx::new( + crate::bootstrap::ExecutionContext::for_workspace(&workspace), workspace, None, Runtime::new().unwrap(), @@ -161,6 +162,7 @@ fn setup_multi(entries: Vec<(ConversationId, Conversation, Vec, storage_path: Option<&Utf8Path>, user_storage_path: Option<&Utf8Path>, config: &Arc, @@ -94,6 +98,13 @@ pub(crate) fn run_plugin( .stdout(Stdio::piped()) .stderr(Stdio::piped()); + // The root-as-working-directory invariant (RFD 087): when JP operates on + // a workspace other than the launch cwd's own, plugins run as if launched + // from the selected workspace root. + if let Some(cwd) = child_cwd { + cmd.current_dir(cwd); + } + // Prevent the child from receiving SIGINT/SIGTERM directly. The host // sends `Shutdown` over the protocol instead, giving the plugin a // chance to exit gracefully. @@ -894,6 +905,7 @@ pub(crate) async fn run_external(args: &[String], ctx: &Ctx) -> cmd::Output { &binary, plugin_args, &ctx.workspace, + ctx.exec.child_cwd(), ctx.storage_path(), ctx.user_storage_path(), &config, diff --git a/crates/jp_cli/src/cmd/workspace.rs b/crates/jp_cli/src/cmd/workspace.rs new file mode 100644 index 000000000..7e3aab237 --- /dev/null +++ b/crates/jp_cli/src/cmd/workspace.rs @@ -0,0 +1,69 @@ +//! The `jp workspace` (`jp w`) command surface (RFD 087). +//! +//! Mirrors `jp conversation` one level up: `use` selects the session's active +//! workspace, `ls` lists known workspaces, `show` reports one. +//! +//! These commands run on a dedicated pre-workspace path: selecting or +//! inspecting a workspace must work from outside every workspace — including +//! resolving to *no* workspace — so they receive the pre-workspace +//! [`TargetEnv`] instead of a `Ctx`, and never construct a +//! [`jp_workspace::Workspace`] except where their own semantics load one +//! (`show`'s conversation count). + +mod ls; +mod show; +pub(crate) mod target; +mod use_; + +use jp_printer::Printer; +use jp_workspace::session::Session; +use target::TargetEnv; + +use crate::{bootstrap::WorkspaceRequirement, cmd::Output}; + +/// Manage workspaces. +#[derive(Debug, clap::Args)] +pub(crate) struct Workspace { + #[command(subcommand)] + command: Commands, +} + +#[derive(Debug, clap::Subcommand)] +enum Commands { + /// Select the session's active workspace. + #[command(name = "use", visible_alias = "u")] + Use(use_::Use), + + /// List known workspaces and their checkouts. + #[command(name = "ls", alias = "list")] + Ls(ls::Ls), + + /// Show a workspace: identity, checkouts, and how it resolves. + #[command(name = "show", visible_alias = "s")] + Show(show::Show), +} + +impl Workspace { + pub(crate) fn run(self, printer: &Printer, session: Option<&Session>, persist: bool) -> Output { + let env = TargetEnv::new(session)?; + + match self.command { + Commands::Use(args) => args.run(printer, &env), + Commands::Ls(args) => args.run(printer, &env), + Commands::Show(args) => args.run(printer, &env, persist), + } + } + + /// What each subcommand needs from the workspace bootstrap (RFD 087). + /// + /// `ls` reads the user-global registries only; `use` resolves and validates + /// a target root to record a selection; `show` additionally loads + /// conversation indexes for its count. + pub(crate) fn workspace_requirement(&self) -> WorkspaceRequirement { + match &self.command { + Commands::Ls(_) => WorkspaceRequirement::None, + Commands::Use(_) => WorkspaceRequirement::Resolve, + Commands::Show(_) => WorkspaceRequirement::Load, + } + } +} diff --git a/crates/jp_cli/src/cmd/workspace/ls.rs b/crates/jp_cli/src/cmd/workspace/ls.rs new file mode 100644 index 000000000..0342e28da --- /dev/null +++ b/crates/jp_cli/src/cmd/workspace/ls.rs @@ -0,0 +1,94 @@ +use chrono::{DateTime, Local, Utc}; +use comfy_table::Row; +use jp_printer::Printer; +use jp_workspace::roots; + +use crate::{ + DEFAULT_STORAGE_DIR, + cmd::{Output, workspace::target::TargetEnv}, + output::print_table, +}; + +/// List known workspaces and their checkouts. +/// +/// Reads the user-global registries only — no workspace is bootstrapped, so +/// the listing works from anywhere (RFD 087). +/// One row per live checkout; the session-active checkout is marked with `*`. +#[derive(Debug, clap::Args)] +pub(crate) struct Ls {} + +impl Ls { + #[expect(clippy::unused_self, clippy::unnecessary_wraps)] + pub(crate) fn run(self, printer: &Printer, env: &TargetEnv<'_>) -> Output { + let known = roots::known_workspaces(&env.workspaces_dir, DEFAULT_STORAGE_DIR); + if known.is_empty() { + printer.println( + "No known workspaces. JP registers a workspace when a command runs from inside it." + .to_owned(), + ); + return Ok(()); + } + + let active = env.session.and_then(|session| env.store.active(session)); + let is_active = |id: &jp_workspace::Id, root: &camino::Utf8Path| { + active.as_ref().is_some_and(|entry| { + entry.id().is_some_and(|active_id| active_id == *id) && entry.root == root + }) + }; + + let header = Row::from(vec!["", "ID", "Name", "Checkout", "Last used"]); + let mut rows = Vec::new(); + + for workspace in known { + let name = workspace.slug.as_deref().unwrap_or("").to_owned(); + + if workspace.roots.is_empty() { + rows.push(Row::from(vec![ + String::new(), + workspace.id.to_string(), + name, + "(no live checkouts)".to_owned(), + String::new(), + ])); + continue; + } + + for (index, entry) in workspace.roots.into_iter().enumerate() { + // Repeat the ID and name only on the workspace's first row; + // further checkouts read as a continuation. + let (id, name) = if index == 0 { + (workspace.id.to_string(), name.clone()) + } else { + (String::new(), String::new()) + }; + let marker = if is_active(&workspace.id, &entry.path) { + "*".to_owned() + } else { + String::new() + }; + + rows.push(Row::from(vec![ + marker, + id, + name, + entry.path.to_string(), + humanize(entry.last_used), + ])); + } + } + + print_table(printer, header, rows, false); + Ok(()) + } +} + +/// A stable local-time rendering for the listing. +fn humanize(at: DateTime) -> String { + at.with_timezone(&Local) + .format("%Y-%m-%d %H:%M") + .to_string() +} + +#[cfg(test)] +#[path = "ls_tests.rs"] +mod tests; diff --git a/crates/jp_cli/src/cmd/workspace/ls_tests.rs b/crates/jp_cli/src/cmd/workspace/ls_tests.rs new file mode 100644 index 000000000..20128bede --- /dev/null +++ b/crates/jp_cli/src/cmd/workspace/ls_tests.rs @@ -0,0 +1,123 @@ +use std::str::FromStr as _; + +use camino::{Utf8Path, Utf8PathBuf}; +use camino_tempfile::tempdir; +use chrono::Utc; +use jp_printer::{OutputFormat, Printer}; +use jp_workspace::{ + Id, + session::{Session, SessionId, SessionSource}, + session_store::WorkspaceSessionStore, +}; + +use super::*; +use crate::cmd::workspace::target::TargetEnv; + +fn env_at<'a>( + launch_cwd: Utf8PathBuf, + data_dir: &Utf8Path, + session: Option<&'a Session>, +) -> TargetEnv<'a> { + TargetEnv { + launch_cwd, + workspaces_dir: data_dir.join(crate::USER_WORKSPACES_DIR), + store: WorkspaceSessionStore::at_user_data_dir(data_dir), + session, + interactive: false, + } +} + +fn make_workspace(base: &Utf8Path, name: &str, id: &str) -> Utf8PathBuf { + let root = base.join(name); + std::fs::create_dir_all(root.join(crate::DEFAULT_STORAGE_DIR).as_std_path()).unwrap(); + std::fs::write( + root.join(crate::DEFAULT_STORAGE_DIR) + .join(".id") + .as_std_path(), + id, + ) + .unwrap(); + root +} + +fn register(env: &TargetEnv<'_>, slug: &str, id: &str, root: &Utf8Path) { + let silo = env.workspaces_dir.join(format!("{slug}-{id}")); + jp_workspace::roots::upsert_root(&silo, root).unwrap(); +} + +fn env_session() -> Session { + Session { + id: SessionId::new("42").expect("non-empty"), + source: SessionSource::env("JP_SESSION"), + } +} + +/// Flush the async printer, then read what reached the buffer. +fn stdout_of(printer: &Printer, buffer: &jp_printer::SharedBuffer) -> String { + printer.flush(); + buffer.lock().clone() +} + +#[test] +fn empty_registry_prints_a_hint() { + let tmp = tempdir().unwrap(); + let env = env_at(tmp.path().to_owned(), tmp.path(), None); + let (printer, out, _err) = Printer::memory(OutputFormat::Text); + + Ls {}.run(&printer, &env).unwrap(); + + let stdout = stdout_of(&printer, &out); + assert!( + stdout.contains("No known workspaces"), + "unexpected output: {stdout}" + ); +} + +#[test] +fn lists_workspaces_with_their_checkouts() { + let tmp = tempdir().unwrap(); + let root = make_workspace(tmp.path(), "proj", "ws123"); + let env = env_at(tmp.path().to_owned(), tmp.path(), None); + register(&env, "proj", "ws123", &root); + + let (printer, out, _err) = Printer::memory(OutputFormat::Text); + Ls {}.run(&printer, &env).unwrap(); + + let stdout = stdout_of(&printer, &out); + assert!(stdout.contains("ws123"), "unexpected output: {stdout}"); + assert!(stdout.contains("proj"), "unexpected output: {stdout}"); + assert!( + stdout.contains(root.canonicalize_utf8().unwrap().as_str()), + "unexpected output: {stdout}" + ); +} + +#[test] +fn marks_the_session_active_checkout() { + let tmp = tempdir().unwrap(); + let root = make_workspace(tmp.path(), "proj", "ws123"); + let session = env_session(); + let env = env_at(tmp.path().to_owned(), tmp.path(), Some(&session)); + register(&env, "proj", "ws123", &root); + + // The registry stores the canonical path; record the same value so the + // active marker matches the listed checkout. + let canonical = root.canonicalize_utf8().unwrap(); + env.store + .record_selection( + &session, + &Id::from_str("ws123").unwrap(), + &canonical, + Utc::now(), + ) + .unwrap(); + + let (printer, out, _err) = Printer::memory(OutputFormat::Text); + Ls {}.run(&printer, &env).unwrap(); + + let stdout = stdout_of(&printer, &out); + let marked = stdout + .lines() + .any(|line| line.contains('*') && line.contains(canonical.as_str())); + assert!(marked, "no active marker in output: {stdout}"); +} diff --git a/crates/jp_cli/src/cmd/workspace/show.rs b/crates/jp_cli/src/cmd/workspace/show.rs new file mode 100644 index 000000000..a7b94e078 --- /dev/null +++ b/crates/jp_cli/src/cmd/workspace/show.rs @@ -0,0 +1,369 @@ +use std::collections::BTreeSet; + +use camino::{Utf8Path, Utf8PathBuf}; +use crossterm::style::Stylize as _; +use jp_conversation::ConversationId; +use jp_printer::Printer; +use jp_workspace::{Id, Workspace, roots, session_store::WorkspaceSelection}; +use tracing::warn; + +use crate::{ + DEFAULT_STORAGE_DIR, + cmd::{ + Output, + workspace::target::{self, ResolvedTarget, TargetEnv, WorkspaceTarget}, + }, +}; + +/// Show a workspace: identity, checkouts, and how it resolves. +/// +/// With no target, reports the session's active workspace, falling back to the +/// cwd-derived one; "no workspace selected" is a first-class outcome, not an +/// error (RFD 087). +/// `show` stays read-only and script-friendly: concrete targets never prompt — +/// a multi-checkout workspace lists every live root — and only the picker +/// targets (`?`, `?s`) require interactivity. +#[derive(Debug, clap::Args)] +pub(crate) struct Show { + /// The workspace to show. + /// See `jp w use help` for the grammar. + /// + /// Defaults to the session's active workspace, then the cwd-derived one. + target: Option, +} + +/// What `show` reports on: a workspace and how it was reached. +struct Subject { + id: Id, + slug: Option, + /// Live checkout roots, most recently used first. + roots: Vec, + /// How the subject was resolved, for the readout. + resolved: &'static str, +} + +impl Show { + pub(crate) fn run(self, printer: &Printer, env: &TargetEnv<'_>, persist: bool) -> Output { + if matches!(self.target, Some(WorkspaceTarget::Help)) { + printer.println(target::help()); + return Ok(()); + } + + let active = env.session.and_then(|session| env.store.active(session)); + let cwd_root = Workspace::find_root(env.launch_cwd.clone(), DEFAULT_STORAGE_DIR); + + let Some(subject) = self.subject(env, active.as_ref(), cwd_root.as_deref())? else { + printer.println(format!( + "No workspace selected. Select one with `{}`, or run from inside a workspace.", + "jp w use ?".bold().yellow(), + )); + return Ok(()); + }; + + render( + printer, + env, + &subject, + active.as_ref(), + cwd_root.as_deref(), + persist, + ); + Ok(()) + } + + /// Resolve the readout subject. + /// + /// `Ok(None)` is the no-target, nothing-selected outcome. + #[expect(clippy::too_many_lines)] + fn subject( + &self, + env: &TargetEnv<'_>, + active: Option<&WorkspaceSelection>, + cwd_root: Option<&Utf8Path>, + ) -> Result, crate::cmd::Error> { + let Some(target) = &self.target else { + // No target: the session's active workspace, then cwd. + if let Some(id) = active.and_then(WorkspaceSelection::id) { + return Ok(Some(subject_for(env, id, "session-active"))); + } + return Ok(cwd_root + .and_then(root_id) + .map(|id| subject_for(env, id, "current directory"))); + }; + + match target { + WorkspaceTarget::Help => unreachable!("handled before subject resolution"), + + WorkspaceTarget::Id(id) => Ok(Some(subject_for(env, id.clone(), "explicit target"))), + + WorkspaceTarget::Path(path) => { + let base = if path.is_absolute() { + path.clone() + } else { + env.launch_cwd.join(path) + }; + let root = Workspace::find_root(base, DEFAULT_STORAGE_DIR) + .ok_or_else(|| format!("No workspace found at `{path}`."))?; + let id = root_id(&root) + .ok_or_else(|| format!("`{root}` has no readable workspace ID."))?; + + Ok(Some(subject_for(env, id, "explicit target"))) + } + + WorkspaceTarget::Cwd => Ok(cwd_root + .and_then(root_id) + .map(|id| subject_for(env, id, "current directory"))), + + WorkspaceTarget::Session => { + let session = env.session.ok_or( + "No session identity available. Set $JP_SESSION or run in a terminal with \ + automatic session detection.", + )?; + let entry = env + .store + .previous(session) + .ok_or("No previously active workspace recorded for this session.")?; + let id = entry + .id() + .ok_or("The session history entry holds an invalid workspace ID.")?; + + Ok(Some(subject_for(env, id, "session history"))) + } + + WorkspaceTarget::Latest => Ok(roots::known_workspaces( + &env.workspaces_dir, + DEFAULT_STORAGE_DIR, + ) + .into_iter() + .find(|workspace| !workspace.roots.is_empty()) + .map(|workspace| subject_for(env, workspace.id, "most recently used"))), + + WorkspaceTarget::Stdin => { + let id = target::stdin_id(std::io::stdin().lock())?; + Ok(Some(subject_for(env, id, "explicit target"))) + } + + // Free text stays promptless here: a unique match resolves, an + // ambiguous one errors with the candidates. `show` is the + // scriptable readout; interactive exploration is `jp w use ?`. + WorkspaceTarget::Fuzzy(text) => { + let needle = text.to_lowercase(); + let matches: Vec<_> = + roots::known_workspaces(&env.workspaces_dir, DEFAULT_STORAGE_DIR) + .into_iter() + .filter(|workspace| { + workspace + .slug + .as_deref() + .is_some_and(|slug| slug.to_lowercase().contains(&needle)) + || workspace.id.to_lowercase().contains(&needle) + || workspace.roots.iter().any(|entry| { + entry.path.as_str().to_lowercase().contains(&needle) + }) + }) + .collect(); + + match matches.len() { + 0 => Err(format!("No known workspace matches `{text}`.").into()), + 1 => Ok(matches + .into_iter() + .next() + .map(|workspace| subject_for(env, workspace.id, "fuzzy match"))), + _ => { + let candidates = matches + .iter() + .map(|workspace| { + format!( + " {} ({})", + workspace.slug.as_deref().unwrap_or("-"), + workspace.id, + ) + }) + .collect::>() + .join("\n"); + + Err(format!( + "`{text}` matches multiple workspaces:\n{candidates}\nNarrow the \ + match or use an ID." + ) + .into()) + } + } + } + + // The pickers prompt, which `resolve` gates on interactivity. + WorkspaceTarget::Picker | WorkspaceTarget::SessionPicker => { + match target::resolve(target, env)? { + ResolvedTarget::Root(selected) => { + Ok(selected.id.map(|id| subject_for(env, id, "picked"))) + } + ResolvedTarget::Cwd | ResolvedTarget::Help => { + unreachable!("pickers resolve to a root") + } + } + } + } + } +} + +/// Build the subject for a workspace ID: its slug and live checkouts. +fn subject_for(env: &TargetEnv<'_>, id: Id, resolved: &'static str) -> Subject { + let slug = roots::known_workspaces(&env.workspaces_dir, DEFAULT_STORAGE_DIR) + .into_iter() + .find(|workspace| workspace.id == id) + .and_then(|workspace| workspace.slug); + let roots = roots::resolve_live_roots(&env.workspaces_dir, &id, DEFAULT_STORAGE_DIR) + .into_iter() + .map(|entry| entry.path) + .collect(); + + Subject { + id, + slug, + roots, + resolved, + } +} + +/// The workspace ID stored at a checkout root, when readable. +fn root_id(root: &Utf8Path) -> Option { + Id::load(root.join(DEFAULT_STORAGE_DIR)).and_then(Result::ok) +} + +/// Print the readout. +fn render( + printer: &Printer, + env: &TargetEnv<'_>, + subject: &Subject, + active: Option<&WorkspaceSelection>, + cwd_root: Option<&Utf8Path>, + persist: bool, +) { + let name = match &subject.slug { + Some(slug) => format!("{} ({})", slug.clone().bold(), subject.id), + None => subject.id.to_string().bold().to_string(), + }; + printer.println(format!("Workspace: {name}")); + printer.println(format!("Resolved from: {}", subject.resolved)); + + // Sticky is session-level state about the *active* workspace, so it only + // renders when the subject is the active one. + let subject_is_active = active.is_some_and(|entry| entry.workspace_id == *subject.id); + let sticky = if subject_is_active + && let Some(session) = env.session + && let Some(mapping) = env.store.load(session) + { + printer.println(format!( + "Sticky: {}", + if mapping.sticky { "yes" } else { "no" }, + )); + mapping.sticky + } else { + false + }; + + if subject.roots.is_empty() { + printer.println("Checkouts: (no live checkouts)".to_owned()); + } else { + printer.println("Checkouts:".to_owned()); + for root in &subject.roots { + let marker = if active.is_some_and(|entry| entry.root == *root) { + "*" + } else { + " " + }; + printer.println(format!(" {marker} {root}")); + } + } + + // The one multi-root read: union the conversation index across the + // user-local durable store and every live checkout, deduplicated by ID — + // accurate rather than cheap (RFD 087). + if let Some(stats) = conversation_stats(env, &subject.roots, persist) { + printer.println(format!("Conversations: {}", stats.count)); + + if let Some((id, title)) = stats.active { + let title = title.map(|t| format!(": {t}")).unwrap_or_default(); + printer.println(format!("Active conversation: {id}{title}")); + } + } + + // The cwd-vs-active tension, surfaced instead of silently resolved (RFD + // 087's precedence ladder): a sticky session keeps the active workspace; + // otherwise commands prompt when the two disagree. + if subject_is_active + && let Some(cwd_root) = cwd_root + && active.is_some_and(|entry| entry.root != cwd_root) + { + let note = if sticky { + format!( + "Note: the current directory resolves to `{cwd_root}`, but this session is sticky \ + to its active workspace, which takes precedence for commands run here." + ) + } else { + format!( + "Note: the current directory resolves to `{cwd_root}`; commands run here prompt \ + between it and the active workspace." + ) + }; + printer.println(note); + } +} + +/// The union conversation count, and the session's active conversation there. +struct ConversationStats { + count: usize, + active: Option<(ConversationId, Option)>, +} + +/// Load each live root's conversation index (index only, not event contents) +/// and deduplicate by conversation ID. +/// +/// Roots that fail to load are skipped with a warning; `None` when no root +/// produced an index. +fn conversation_stats( + env: &TargetEnv<'_>, + roots: &[Utf8PathBuf], + persist: bool, +) -> Option { + let mut ids: BTreeSet = BTreeSet::new(); + let mut active = None; + let mut loaded_any = false; + + for root in roots { + let (mut workspace, _backend) = match crate::load_workspace(root, persist) { + Ok(loaded) => loaded, + Err(error) => { + warn!(%error, %root, "Skipping unloadable checkout in the conversation count."); + continue; + } + }; + + workspace.load_conversation_index(); + ids.extend(workspace.conversations().map(|(id, _)| *id)); + loaded_any = true; + + // The session → conversation mapping is per workspace ID, so any + // loaded checkout answers it; take the first. + if active.is_none() + && let Some(session) = env.session + && let Some(id) = workspace.session_active_conversation(session) + { + let title = workspace + .acquire_conversation(&id) + .ok() + .and_then(|handle| workspace.metadata(&handle).ok()) + .and_then(|metadata| metadata.title.clone()); + active = Some((id, title)); + } + } + + loaded_any.then_some(ConversationStats { + count: ids.len(), + active, + }) +} + +#[cfg(test)] +#[path = "show_tests.rs"] +mod tests; diff --git a/crates/jp_cli/src/cmd/workspace/show_tests.rs b/crates/jp_cli/src/cmd/workspace/show_tests.rs new file mode 100644 index 000000000..909e4f8e4 --- /dev/null +++ b/crates/jp_cli/src/cmd/workspace/show_tests.rs @@ -0,0 +1,152 @@ +use std::str::FromStr as _; + +use camino::{Utf8Path, Utf8PathBuf}; +use camino_tempfile::tempdir; +use chrono::Utc; +use jp_printer::{OutputFormat, Printer}; +use jp_workspace::{ + session::{Session, SessionId, SessionSource}, + session_store::WorkspaceSessionStore, +}; + +use super::*; + +fn env_at<'a>( + launch_cwd: Utf8PathBuf, + data_dir: &Utf8Path, + session: Option<&'a Session>, +) -> TargetEnv<'a> { + TargetEnv { + launch_cwd, + workspaces_dir: data_dir.join(crate::USER_WORKSPACES_DIR), + store: WorkspaceSessionStore::at_user_data_dir(data_dir), + session, + interactive: false, + } +} + +fn make_workspace(base: &Utf8Path, name: &str, id: &str) -> Utf8PathBuf { + let root = base.join(name); + std::fs::create_dir_all(root.join(crate::DEFAULT_STORAGE_DIR).as_std_path()).unwrap(); + std::fs::write( + root.join(crate::DEFAULT_STORAGE_DIR) + .join(".id") + .as_std_path(), + id, + ) + .unwrap(); + root +} + +fn env_session() -> Session { + Session { + id: SessionId::new("42").expect("non-empty"), + source: SessionSource::env("JP_SESSION"), + } +} + +/// Flush the async printer, then read what reached the buffer. +fn stdout_of(printer: &Printer, buffer: &jp_printer::SharedBuffer) -> String { + printer.flush(); + buffer.lock().clone() +} + +#[test] +fn no_selection_is_a_first_class_outcome() { + let tmp = tempdir().unwrap(); + let env = env_at(tmp.path().to_owned(), tmp.path(), None); + let (printer, out, _err) = Printer::memory(OutputFormat::Text); + + Show { target: None }.run(&printer, &env, false).unwrap(); + + let stdout = stdout_of(&printer, &out); + assert!( + stdout.contains("No workspace selected"), + "unexpected output: {stdout}" + ); +} + +#[test] +fn explicit_id_reports_missing_live_checkouts() { + let tmp = tempdir().unwrap(); + let env = env_at(tmp.path().to_owned(), tmp.path(), None); + let (printer, out, _err) = Printer::memory(OutputFormat::Text); + + Show { + target: Some(WorkspaceTarget::Id(Id::from_str("ws123").unwrap())), + } + .run(&printer, &env, false) + .unwrap(); + + let stdout = stdout_of(&printer, &out); + assert!(stdout.contains("ws123"), "unexpected output: {stdout}"); + assert!( + stdout.contains("(no live checkouts)"), + "unexpected output: {stdout}" + ); + assert!( + stdout.contains("explicit target"), + "unexpected output: {stdout}" + ); +} + +#[test] +fn ambiguous_fuzzy_match_errors_with_candidates() { + let tmp = tempdir().unwrap(); + let env = env_at(tmp.path().to_owned(), tmp.path(), None); + + // Two known workspaces whose slugs share a prefix. No live checkouts are + // needed: `known_workspaces` lists them regardless. + std::fs::create_dir_all(env.workspaces_dir.join("alpha-one-aaa11").as_std_path()).unwrap(); + std::fs::create_dir_all(env.workspaces_dir.join("alpha-two-bbb22").as_std_path()).unwrap(); + + let (printer, _out, _err) = Printer::memory(OutputFormat::Text); + let error = Show { + target: Some(WorkspaceTarget::Fuzzy("alpha".to_owned())), + } + .run(&printer, &env, false) + .unwrap_err(); + + let message = format!("{error:?}"); + assert!( + message.contains("matches multiple workspaces"), + "unexpected error: {message}" + ); +} + +#[test] +fn session_active_subject_notes_cwd_precedence() { + let tmp = tempdir().unwrap(); + // The cwd resolves to a live workspace... + let cwd_root = make_workspace(tmp.path(), "here", "ccc33"); + // ...while the session-active workspace points at a removed checkout. + let session = env_session(); + let env = env_at(cwd_root.clone(), tmp.path(), Some(&session)); + env.store + .record_selection( + &session, + &Id::from_str("aaa11").unwrap(), + &tmp.path().join("gone"), + Utc::now(), + ) + .unwrap(); + + let (printer, out, _err) = Printer::memory(OutputFormat::Text); + Show { target: None }.run(&printer, &env, false).unwrap(); + + let stdout = stdout_of(&printer, &out); + // The subject is the session-active workspace, not the cwd one. + assert!(stdout.contains("aaa11"), "unexpected output: {stdout}"); + assert!( + stdout.contains("session-active"), + "unexpected output: {stdout}" + ); + // Session-level sticky state renders for the active subject. + assert!(stdout.contains("Sticky"), "unexpected output: {stdout}"); + // The cwd-vs-active tension is surfaced: without a sticky pin, commands + // run here prompt between the two (RFD 087). + assert!( + stdout.contains("prompt between"), + "unexpected output: {stdout}" + ); +} diff --git a/crates/jp_cli/src/cmd/workspace/target.rs b/crates/jp_cli/src/cmd/workspace/target.rs new file mode 100644 index 000000000..49b26096f --- /dev/null +++ b/crates/jp_cli/src/cmd/workspace/target.rs @@ -0,0 +1,549 @@ +//! The workspace targeting grammar (RFD 087). +//! +//! `jp w use`, `jp w show`, and the global `--workspace` flag share one +//! grammar, modeled on `ConversationTarget` (`jp_cli::cmd::target`) with the +//! keywords that carry over to workspaces: +//! +//! | Target | Meaning | +//! | ---------------- | ---------------------------------------------------- | +//! | `` | a literal workspace ID | +//! | `` | an existing path (shadows an ID of the same name) | +//! | free text | fuzzy-match known workspaces by slug / path / ID | +//! | `?` | pick from all known workspaces | +//! | `?s`, `?session` | pick from this session's workspace history | +//! | `s`, `session` | the previously active workspace (like `cd -`) | +//! | `l`, `latest` | the most recently used known workspace | +//! | `cwd`, `.` | the cwd-derived workspace (as a `use` target: clear) | +//! | `-` | read a workspace ID from stdin | +//! | `help` | print keyword help | +//! +//! Keywords are matched before paths, so a literal directory named like a +//! keyword needs a path spelling (`./s`). +//! Session-derived targets (`s`, `?s`) and pickers (`?`, free text) resolve +//! against hidden per-session state or need a prompt, so they error when no +//! prompt is possible; scripts stay deterministic by targeting IDs or paths. + +use std::{ + io::{self, BufRead, IsTerminal as _}, + str::FromStr, +}; + +use camino::{Utf8Path, Utf8PathBuf, absolute_utf8}; +use crossterm::style::Stylize as _; +use inquire::Select; +use jp_workspace::{ + Id, Workspace, + roots::{self, RootEntry}, + session::Session, + session_store::WorkspaceSessionStore, + user_data_dir, +}; + +use crate::{DEFAULT_STORAGE_DIR, USER_WORKSPACES_DIR, cmd, error::Result}; + +/// A parsed workspace target. +/// +/// See the module documentation for the grammar. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum WorkspaceTarget { + /// A literal workspace ID. + Id(Id), + + /// An existing filesystem path. + /// + /// A bare target is treated as a path when it resolves to an existing path, + /// so a local directory whose name matches a workspace ID shadows the ID. + Path(Utf8PathBuf), + + /// `?` — pick from all known workspaces. + Picker, + + /// `?s` / `?session` — pick from this session's workspace history. + SessionPicker, + + /// `s` / `session` — the session's previously active workspace. + Session, + + /// `l` / `latest` — the live root with the newest `last_used` across the + /// roots registry (global recency, distinct from `s`). + Latest, + + /// `cwd` / `.` — the cwd-derived workspace. + /// + /// As a `jp w use` target this clears the session selection; as a + /// `--workspace` target it resolves from the invocation directory. + Cwd, + + /// `-` — read a workspace ID from stdin. + Stdin, + + /// `help` — print keyword help. + Help, + + /// Free text — fuzzy-match known workspaces by slug, path, and ID. + Fuzzy(String), +} + +impl FromStr for WorkspaceTarget { + type Err = crate::error::Error; + + fn from_str(s: &str) -> Result { + Ok(match s { + "" => { + return Err(crate::error::Error::NotFound( + "workspace", + "empty target".into(), + )); + } + "?" => Self::Picker, + "?s" | "?session" => Self::SessionPicker, + "s" | "session" => Self::Session, + "l" | "latest" => Self::Latest, + "cwd" | "." => Self::Cwd, + "-" => Self::Stdin, + "help" => Self::Help, + _ if Utf8Path::new(s).exists() => Self::Path(s.into()), + _ => Id::from_str(s).map_or_else(|_| Self::Fuzzy(s.to_owned()), Self::Id), + }) + } +} + +/// The keyword help table, printed for the `help` target. +pub(crate) fn help() -> String { + indoc::indoc! {" + Workspace targets: + + a literal workspace ID + an existing path (shadows an ID of the same name) + free text fuzzy-match known workspaces by slug / path / ID + ? pick from all known workspaces + ?s, ?session pick from this session's workspace history + s, session the previously active workspace (like `cd -`) + l, latest the most recently used known workspace + cwd, . the cwd-derived workspace (as a `use` target: clears + the session selection) + - read a workspace ID from stdin + help print this help + + Keywords are matched before paths; spell a directory named like a + keyword as a path (`./s`). Session-derived targets and pickers are + interactive-only; scripts target IDs or paths."} + .to_owned() +} + +/// The pre-workspace dependencies target resolution runs against. +/// +/// Bundled once by the bootstrap (or a `jp workspace` command) and passed +/// explicitly, so resolution never re-derives state from the process +/// environment at each call site. +#[derive(Debug)] +pub(crate) struct TargetEnv<'a> { + /// Where the user invoked `jp`; relative path targets resolve against it. + pub(crate) launch_cwd: Utf8PathBuf, + + /// The per-user `workspace/` data directory holding the registries. + pub(crate) workspaces_dir: Utf8PathBuf, + + /// The user-global session → active-workspace store. + pub(crate) store: WorkspaceSessionStore, + + /// The resolved session identity, if any. + pub(crate) session: Option<&'a Session>, + + /// Whether interactive prompting is possible. + pub(crate) interactive: bool, +} + +impl<'a> TargetEnv<'a> { + /// The environment for this invocation. + pub(crate) fn new(session: Option<&'a Session>) -> Result { + let data_dir = user_data_dir()?; + + Ok(Self { + launch_cwd: absolute_utf8(".")?, + workspaces_dir: data_dir.join(USER_WORKSPACES_DIR), + store: WorkspaceSessionStore::at_user_data_dir(&data_dir), + session, + interactive: io::stdin().is_terminal(), + }) + } +} + +/// A concrete checkout root a target resolved to. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct SelectedRoot { + /// The workspace ID, when readable. + /// + /// `None` only for path targets whose checkout has no readable ID file; + /// registry- and session-derived roots always carry one. + pub(crate) id: Option, + + /// The checkout root. + pub(crate) root: Utf8PathBuf, +} + +/// What a workspace target resolved to. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum ResolvedTarget { + /// A concrete, validated checkout root. + Root(SelectedRoot), + + /// The cwd-derived workspace. + /// + /// Symbolic: `jp w use` clears the session selection, the bootstrap + /// resolves from the launch cwd. + Cwd, + + /// Print targeting help. + Help, +} + +/// Resolve a target to a concrete checkout root. +/// +/// Selection prompts (multi-root IDs, pickers, fuzzy matches) write to stderr; +/// when `env.interactive` is `false`, targets that would prompt — and +/// session-derived targets, which resolve against hidden per-session state — +/// error instead. +#[expect(clippy::too_many_lines)] +pub(crate) fn resolve(target: &WorkspaceTarget, env: &TargetEnv<'_>) -> Result { + match target { + WorkspaceTarget::Help => Ok(ResolvedTarget::Help), + WorkspaceTarget::Cwd => Ok(ResolvedTarget::Cwd), + + WorkspaceTarget::Id(id) => { + select_root(id, live_roots(env, id), env.interactive).map(ResolvedTarget::Root) + } + + WorkspaceTarget::Path(path) => { + let base = if path.is_absolute() { + path.clone() + } else { + env.launch_cwd.join(path) + }; + + let root = Workspace::find_root(base, DEFAULT_STORAGE_DIR) + .ok_or(cmd::Error::from(format!("No workspace found at `{path}`.")))?; + let id = Id::load(root.join(DEFAULT_STORAGE_DIR)).and_then(std::result::Result::ok); + + Ok(ResolvedTarget::Root(SelectedRoot { id, root })) + } + + WorkspaceTarget::Stdin => { + let id = stdin_id(io::stdin().lock())?; + select_root(&id, live_roots(env, &id), env.interactive).map(ResolvedTarget::Root) + } + + WorkspaceTarget::Session => { + require_interactive(env, "s / session")?; + let session = require_session(env)?; + + let entry = env.store.previous(session).ok_or(cmd::Error::from( + "No previously active workspace recorded for this session.", + ))?; + + let live = entry + .id() + .filter(|id| roots::is_live(&entry.root, id, DEFAULT_STORAGE_DIR)); + let Some(id) = live else { + return Err(cmd::Error::from(format!( + "The previously active workspace checkout is gone ({}). Pick one with `{}`.", + entry.root, + "jp w use '?s'".bold().yellow(), + )) + .into()); + }; + + Ok(ResolvedTarget::Root(SelectedRoot { + id: Some(id), + root: entry.root, + })) + } + + WorkspaceTarget::SessionPicker => { + require_interactive(env, "?s / ?session")?; + let session = require_session(env)?; + + let slugs = slug_index(env); + let rows: Vec = env + .store + .load(session) + .map(|mapping| mapping.history) + .unwrap_or_default() + .into_iter() + .filter_map(|entry| { + let id = entry + .id() + .filter(|id| roots::is_live(&entry.root, id, DEFAULT_STORAGE_DIR))?; + let slug = slugs + .iter() + .find(|(known, _)| *known == id) + .and_then(|(_, slug)| slug.clone()); + + Some(WorkspaceRow { + id, + slug, + root: entry.root, + }) + }) + .collect(); + + if rows.is_empty() { + return Err( + cmd::Error::from("No live workspaces in this session's history.").into(), + ); + } + + pick("Select a workspace from this session's history", rows).map(ResolvedTarget::Root) + } + + WorkspaceTarget::Picker => { + require_interactive(env, "?")?; + + let rows = known_rows(env); + if rows.is_empty() { + return Err(no_known_workspaces().into()); + } + + pick("Select a workspace", rows).map(ResolvedTarget::Root) + } + + WorkspaceTarget::Latest => latest_root(env) + .ok_or(no_known_workspaces().into()) + .map(ResolvedTarget::Root), + + WorkspaceTarget::Fuzzy(text) => { + require_interactive(env, "free-text matching")?; + + let needle = text.to_lowercase(); + let rows: Vec = known_rows(env) + .into_iter() + .filter(|row| row.matches(&needle)) + .collect(); + + match rows.len() { + 0 => Err(cmd::Error::from(format!("No known workspace matches `{text}`.")).into()), + 1 => Ok(ResolvedTarget::Root( + rows.into_iter().next().expect("one row").into_selected(), + )), + _ => pick(&format!("Select a workspace matching `{text}`"), rows) + .map(ResolvedTarget::Root), + } + } + } +} + +/// One pickable (workspace, live checkout) pair. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct WorkspaceRow { + /// The workspace ID. + pub(crate) id: Id, + + /// The cosmetic display name, when the user-workspace directory has one. + pub(crate) slug: Option, + + /// The live checkout root. + pub(crate) root: Utf8PathBuf, +} + +impl WorkspaceRow { + /// The display name: the slug when present, the ID otherwise. + fn name(&self) -> &str { + self.slug.as_deref().unwrap_or(&self.id) + } + + /// Case-insensitive substring match over slug, ID, and root path. + fn matches(&self, needle: &str) -> bool { + self.slug + .as_deref() + .is_some_and(|slug| slug.to_lowercase().contains(needle)) + || self.id.to_lowercase().contains(needle) + || self.root.as_str().to_lowercase().contains(needle) + } + + fn into_selected(self) -> SelectedRoot { + SelectedRoot { + id: Some(self.id), + root: self.root, + } + } +} + +/// Every known (workspace, live checkout) pair, one row per checkout, +/// preserving the registry's most-recently-used-first workspace order. +pub(crate) fn known_rows(env: &TargetEnv<'_>) -> Vec { + roots::known_workspaces(&env.workspaces_dir, DEFAULT_STORAGE_DIR) + .into_iter() + .flat_map(|workspace| { + workspace.roots.into_iter().map(move |entry| WorkspaceRow { + id: workspace.id.clone(), + slug: workspace.slug.clone(), + root: entry.path, + }) + }) + .collect() +} + +/// Prompt among all known workspaces, for the bootstrap's picker fallback. +/// +/// `Ok(None)` when no workspace is known — the caller composes its own +/// no-workspace error. +pub(crate) fn pick_known_workspace( + env: &TargetEnv<'_>, + message: &str, +) -> Result> { + let rows = known_rows(env); + if rows.is_empty() { + return Ok(None); + } + + pick(message, rows).map(Some) +} + +/// The live root with the newest `last_used` across every known workspace. +fn latest_root(env: &TargetEnv<'_>) -> Option { + // `known_workspaces` orders by most recently used checkout, rootless + // workspaces last, so the first workspace with a root holds the answer. + roots::known_workspaces(&env.workspaces_dir, DEFAULT_STORAGE_DIR) + .into_iter() + .find_map(|workspace| { + let root = workspace.roots.into_iter().next()?; + Some(SelectedRoot { + id: Some(workspace.id), + root: root.path, + }) + }) +} + +/// Choose a checkout root among a workspace's live roots. +/// +/// One live root is used directly. +/// Several open an interactive picker, or — when no prompt is possible — fail +/// with the candidates listed. +/// None is an error pointing the user at a checkout. +pub(crate) fn select_root( + id: &Id, + mut roots: Vec, + interactive: bool, +) -> Result { + match roots.len() { + 0 => Err(cmd::Error::from(format!( + "Workspace '{id}' has no known live checkouts. Run a JP command from inside a \ + checkout of this workspace to register it, or target it by path with `{}`.", + "--workspace ".bold().yellow(), + )) + .into()), + 1 => Ok(SelectedRoot { + id: Some(id.clone()), + root: roots.remove(0).path, + }), + _ if !interactive => { + let candidates = roots + .iter() + .map(|entry| format!(" {}", entry.path)) + .collect::>() + .join("\n"); + + Err(cmd::Error::from(format!( + "Workspace '{id}' has multiple checkouts:\n{candidates}\nTarget one with \ + `--workspace `." + )) + .into()) + } + _ => { + let message = format!("Select a checkout of workspace '{id}'"); + let labels: Vec = roots.iter().map(|entry| entry.path.to_string()).collect(); + let mut writer = io::stderr(); + let selected = Select::new(&message, labels).prompt_with_writer(&mut writer)?; + + Ok(SelectedRoot { + id: Some(id.clone()), + root: Utf8PathBuf::from(selected), + }) + } + } +} + +/// Prompt among rows, mirroring the RFD's picker sketch: the display name, +/// padded, then the checkout path. +fn pick(message: &str, rows: Vec) -> Result { + let width = rows.iter().map(|row| row.name().len()).max().unwrap_or(0); + let labels: Vec = rows + .iter() + .map(|row| format!("{:) -> Vec<(Id, Option)> { + roots::known_workspaces(&env.workspaces_dir, DEFAULT_STORAGE_DIR) + .into_iter() + .map(|workspace| (workspace.id, workspace.slug)) + .collect() +} + +/// Read a workspace ID from a stdin-style reader (the `-` target). +pub(crate) fn stdin_id(mut reader: impl BufRead) -> Result { + let mut line = String::new(); + reader.read_line(&mut line)?; + + let value = line.trim(); + if value.is_empty() { + return Err(cmd::Error::from("No workspace ID on stdin.").into()); + } + + Ok(Id::from_str(value)?) +} + +/// Expand an ID through the roots registry, pruning dead entries. +fn live_roots(env: &TargetEnv<'_>, id: &Id) -> Vec { + roots::resolve_live_roots(&env.workspaces_dir, id, DEFAULT_STORAGE_DIR) +} + +/// The shared "nothing registered yet" error. +fn no_known_workspaces() -> cmd::Error { + cmd::Error::from( + "No known workspaces. JP registers a workspace when a command runs from inside it.", + ) +} + +/// Error unless a prompt is possible: `target` resolves against hidden +/// per-session state or needs a picker, so scripts must not depend on it. +fn require_interactive(env: &TargetEnv<'_>, target: &str) -> Result<()> { + if env.interactive { + return Ok(()); + } + + Err(cmd::Error::from(format!( + "Workspace target `{target}` is interactive-only. Non-interactive runs target a workspace \ + by ID or path." + )) + .into()) +} + +/// Error unless a session identity exists to resolve session state against. +fn require_session<'a>(env: &TargetEnv<'a>) -> Result<&'a Session> { + env.session.ok_or( + cmd::Error::from( + "No session identity available. Set $JP_SESSION or run in a terminal with automatic \ + session detection.", + ) + .into(), + ) +} + +#[cfg(test)] +#[path = "target_tests.rs"] +mod tests; diff --git a/crates/jp_cli/src/cmd/workspace/target_tests.rs b/crates/jp_cli/src/cmd/workspace/target_tests.rs new file mode 100644 index 000000000..f8dd07dd5 --- /dev/null +++ b/crates/jp_cli/src/cmd/workspace/target_tests.rs @@ -0,0 +1,317 @@ +use std::str::FromStr as _; + +use camino::{Utf8Path, Utf8PathBuf}; +use camino_tempfile::tempdir; +use chrono::{TimeZone as _, Utc}; +use jp_workspace::{ + roots::RootEntry, + session::{Session, SessionId, SessionSource}, + session_store::WorkspaceSessionStore, +}; + +use super::*; + +/// Build a [`TargetEnv`] rooted at `data_dir`, launched from `launch_cwd`. +fn env_at<'a>( + launch_cwd: Utf8PathBuf, + data_dir: &Utf8Path, + session: Option<&'a Session>, + interactive: bool, +) -> TargetEnv<'a> { + TargetEnv { + launch_cwd, + workspaces_dir: data_dir.join(crate::USER_WORKSPACES_DIR), + store: WorkspaceSessionStore::at_user_data_dir(data_dir), + session, + interactive, + } +} + +/// Create a minimal on-disk workspace at `base/name` with the given ID. +fn make_workspace(base: &Utf8Path, name: &str, id: &str) -> Utf8PathBuf { + let root = base.join(name); + std::fs::create_dir_all(root.join(crate::DEFAULT_STORAGE_DIR).as_std_path()).unwrap(); + std::fs::write( + root.join(crate::DEFAULT_STORAGE_DIR) + .join(".id") + .as_std_path(), + id, + ) + .unwrap(); + root +} + +/// Record `root` as a checkout of workspace `id` in the roots registry. +fn register(env: &TargetEnv<'_>, slug: &str, id: &str, root: &Utf8Path) { + let silo = env.workspaces_dir.join(format!("{slug}-{id}")); + jp_workspace::roots::upsert_root(&silo, root).unwrap(); +} + +/// Record `root` with an explicit `last_used` timestamp, for recency tests. +fn register_at(env: &TargetEnv<'_>, slug: &str, id: &str, root: &Utf8Path, last_used_secs: i64) { + let dir = env + .workspaces_dir + .join(format!("{slug}-{id}")) + .join("roots"); + std::fs::create_dir_all(dir.as_std_path()).unwrap(); + let entry = RootEntry { + path: root.to_owned(), + last_used: Utc.timestamp_opt(last_used_secs, 0).unwrap(), + }; + std::fs::write( + dir.join(format!("{slug}.json")).as_std_path(), + serde_json::to_vec(&entry).unwrap(), + ) + .unwrap(); +} + +fn env_session() -> Session { + Session { + id: SessionId::new("42").expect("non-empty"), + source: SessionSource::env("JP_SESSION"), + } +} + +/// Render an error via `Debug`, which includes wrapped messages and sources. +fn message_of(error: &impl std::fmt::Debug) -> String { + format!("{error:?}") +} + +#[test] +fn keywords_parse() { + assert!(matches!( + WorkspaceTarget::from_str("?").unwrap(), + WorkspaceTarget::Picker + )); + assert!(matches!( + WorkspaceTarget::from_str("?s").unwrap(), + WorkspaceTarget::SessionPicker + )); + assert!(matches!( + WorkspaceTarget::from_str("?session").unwrap(), + WorkspaceTarget::SessionPicker + )); + assert!(matches!( + WorkspaceTarget::from_str("s").unwrap(), + WorkspaceTarget::Session + )); + assert!(matches!( + WorkspaceTarget::from_str("session").unwrap(), + WorkspaceTarget::Session + )); + assert!(matches!( + WorkspaceTarget::from_str("l").unwrap(), + WorkspaceTarget::Latest + )); + assert!(matches!( + WorkspaceTarget::from_str("latest").unwrap(), + WorkspaceTarget::Latest + )); + assert!(matches!( + WorkspaceTarget::from_str("cwd").unwrap(), + WorkspaceTarget::Cwd + )); + assert!(matches!( + WorkspaceTarget::from_str(".").unwrap(), + WorkspaceTarget::Cwd + )); + assert!(matches!( + WorkspaceTarget::from_str("-").unwrap(), + WorkspaceTarget::Stdin + )); + assert!(matches!( + WorkspaceTarget::from_str("help").unwrap(), + WorkspaceTarget::Help + )); + assert!(WorkspaceTarget::from_str("").is_err()); +} + +#[test] +fn id_path_and_fuzzy_parse() { + // A well-formed workspace ID parses as an ID target. + assert!(matches!( + WorkspaceTarget::from_str("ws123").unwrap(), + WorkspaceTarget::Id(id) if &*id == "ws123" + )); + + // Free text that is not a valid ID parses as a fuzzy query. + assert!(matches!( + WorkspaceTarget::from_str("my project").unwrap(), + WorkspaceTarget::Fuzzy(text) if text == "my project" + )); + + // An existing path shadows everything else. Unit tests run from the + // package root, where `src` exists. + assert!(matches!( + WorkspaceTarget::from_str("src").unwrap(), + WorkspaceTarget::Path(path) if path == "src" + )); +} + +#[test] +fn stdin_id_parses_and_validates() { + let id = stdin_id("ws123\n".as_bytes()).unwrap(); + assert_eq!(&*id, "ws123"); + + let error = stdin_id("\n".as_bytes()).unwrap_err(); + assert!( + message_of(&error).contains("No workspace ID on stdin"), + "unexpected error: {error:?}" + ); + + assert!(stdin_id("definitely not an id\n".as_bytes()).is_err()); +} + +#[test] +fn id_with_no_live_roots_errors() { + let tmp = tempdir().unwrap(); + let env = env_at(tmp.path().to_owned(), tmp.path(), None, false); + + let target = WorkspaceTarget::Id(Id::from_str("ws123").unwrap()); + let error = resolve(&target, &env).unwrap_err(); + + assert!( + message_of(&error).contains("no known live checkouts"), + "unexpected error: {error:?}" + ); +} + +#[test] +fn id_with_one_live_root_resolves() { + let tmp = tempdir().unwrap(); + let root = make_workspace(tmp.path(), "ws", "ws123"); + let env = env_at(tmp.path().to_owned(), tmp.path(), None, false); + register(&env, "ws", "ws123", &root); + + let target = WorkspaceTarget::Id(Id::from_str("ws123").unwrap()); + let ResolvedTarget::Root(selected) = resolve(&target, &env).unwrap() else { + panic!("expected a resolved root"); + }; + + // The registry stores the canonicalized checkout path. + assert_eq!(selected.root, root.canonicalize_utf8().unwrap()); + assert_eq!(selected.id, Some(Id::from_str("ws123").unwrap())); +} + +#[test] +fn id_with_multiple_roots_is_ambiguous_non_interactively() { + let tmp = tempdir().unwrap(); + let a = make_workspace(tmp.path(), "a", "ws123"); + let b = make_workspace(tmp.path(), "b", "ws123"); + let env = env_at(tmp.path().to_owned(), tmp.path(), None, false); + register(&env, "ws", "ws123", &a); + register(&env, "ws", "ws123", &b); + + let target = WorkspaceTarget::Id(Id::from_str("ws123").unwrap()); + let error = resolve(&target, &env).unwrap_err(); + + let message = message_of(&error); + assert!( + message.contains("multiple checkouts"), + "unexpected error: {message}" + ); + // Both candidate roots are listed. + assert!(message.contains(a.canonicalize_utf8().unwrap().as_str())); + assert!(message.contains(b.canonicalize_utf8().unwrap().as_str())); +} + +#[test] +fn session_and_picker_targets_are_interactive_only() { + let tmp = tempdir().unwrap(); + let session = env_session(); + let env = env_at(tmp.path().to_owned(), tmp.path(), Some(&session), false); + + for target in [ + WorkspaceTarget::Session, + WorkspaceTarget::SessionPicker, + WorkspaceTarget::Picker, + WorkspaceTarget::Fuzzy("anything".to_owned()), + ] { + let error = resolve(&target, &env).unwrap_err(); + assert!( + message_of(&error).contains("interactive-only"), + "target {target:?}: unexpected error: {error:?}" + ); + } +} + +#[test] +fn session_target_requires_a_session_identity() { + let tmp = tempdir().unwrap(); + let env = env_at(tmp.path().to_owned(), tmp.path(), None, true); + + let error = resolve(&WorkspaceTarget::Session, &env).unwrap_err(); + assert!( + message_of(&error).contains("No session identity"), + "unexpected error: {error:?}" + ); +} + +#[test] +fn session_target_resolves_the_previous_checkout() { + let tmp = tempdir().unwrap(); + let a = make_workspace(tmp.path(), "a", "aaa11"); + let b = make_workspace(tmp.path(), "b", "bbb22"); + let session = env_session(); + let env = env_at(tmp.path().to_owned(), tmp.path(), Some(&session), true); + + env.store + .record_selection(&session, &Id::from_str("aaa11").unwrap(), &a, Utc::now()) + .unwrap(); + env.store + .record_selection(&session, &Id::from_str("bbb22").unwrap(), &b, Utc::now()) + .unwrap(); + + // `s` is `cd -`: the previously active checkout, not the current one. + let ResolvedTarget::Root(selected) = resolve(&WorkspaceTarget::Session, &env).unwrap() else { + panic!("expected a resolved root"); + }; + assert_eq!(selected.root, a); + assert_eq!(selected.id, Some(Id::from_str("aaa11").unwrap())); +} + +#[test] +fn session_target_errors_without_a_previous_entry() { + let tmp = tempdir().unwrap(); + let a = make_workspace(tmp.path(), "a", "aaa11"); + let session = env_session(); + let env = env_at(tmp.path().to_owned(), tmp.path(), Some(&session), true); + + env.store + .record_selection(&session, &Id::from_str("aaa11").unwrap(), &a, Utc::now()) + .unwrap(); + + let error = resolve(&WorkspaceTarget::Session, &env).unwrap_err(); + assert!( + message_of(&error).contains("No previously active workspace"), + "unexpected error: {error:?}" + ); +} + +#[test] +fn latest_resolves_the_newest_live_checkout() { + let tmp = tempdir().unwrap(); + let a = make_workspace(tmp.path(), "a", "aaa11"); + let b = make_workspace(tmp.path(), "b", "bbb22"); + let env = env_at(tmp.path().to_owned(), tmp.path(), None, false); + register_at(&env, "a", "aaa11", &a, 1_000); + register_at(&env, "b", "bbb22", &b, 2_000); + + let ResolvedTarget::Root(selected) = resolve(&WorkspaceTarget::Latest, &env).unwrap() else { + panic!("expected a resolved root"); + }; + assert_eq!(selected.root, b); + assert_eq!(selected.id, Some(Id::from_str("bbb22").unwrap())); +} + +#[test] +fn latest_errors_with_an_empty_registry() { + let tmp = tempdir().unwrap(); + let env = env_at(tmp.path().to_owned(), tmp.path(), None, false); + + let error = resolve(&WorkspaceTarget::Latest, &env).unwrap_err(); + assert!( + message_of(&error).contains("No known workspaces"), + "unexpected error: {error:?}" + ); +} diff --git a/crates/jp_cli/src/cmd/workspace/use_.rs b/crates/jp_cli/src/cmd/workspace/use_.rs new file mode 100644 index 000000000..cd0d64c32 --- /dev/null +++ b/crates/jp_cli/src/cmd/workspace/use_.rs @@ -0,0 +1,123 @@ +use chrono::Utc; +use crossterm::style::Stylize as _; +use jp_printer::Printer; + +use crate::cmd::{ + Output, + workspace::target::{self, ResolvedTarget, TargetEnv, WorkspaceTarget}, +}; + +/// Select the session's active workspace. +/// +/// After `jp w use`, workspace-consuming commands run against the selection +/// from anywhere, the way an active conversation follows the session (RFD 020). +/// `jp w use ?` opens a picker; `jp w use cwd` drops the selection and returns +/// to cwd resolution. +/// +/// Interactive-only in every form — including `cwd` — because it mutates +/// session state; scripts target a workspace per invocation with `jp +/// --workspace` instead. +#[derive(Debug, clap::Args)] +pub(crate) struct Use { + /// The workspace to select. + /// See `jp w use help` for the grammar. + /// + /// Defaults to the picker (`?`). + target: Option, +} + +impl Use { + pub(crate) fn run(self, printer: &Printer, env: &TargetEnv<'_>) -> Output { + let target = self.target.unwrap_or(WorkspaceTarget::Picker); + + if matches!(target, WorkspaceTarget::Help) { + printer.println(target::help()); + return Ok(()); + } + + // Interactive-only: the selection is hidden per-session state, and a + // script that mutated it would stop being deterministic. Scripts + // return to cwd behavior by not setting $JP_SESSION, not by running + // `jp w use cwd` (RFD 087). + if !env.interactive { + return Err(format!( + "`jp workspace use` is interactive-only. Scripts target a workspace per \ + invocation with `{}` instead.", + "--workspace ".bold().yellow(), + ) + .into()); + } + + let Some(session) = env.session else { + return Err( + "No session identity available. Set $JP_SESSION or run in a terminal with \ + automatic session detection." + .into(), + ); + }; + + let previous = env.store.active(session); + + match target::resolve(&target, env)? { + ResolvedTarget::Help => unreachable!("handled before resolution"), + + // Clearing is just selecting the cwd-derived workspace: the + // record — history and sticky flag included — is dropped, and + // resolution falls back to the directory the command runs from. + ResolvedTarget::Cwd => { + env.store.clear(session)?; + + match previous { + Some(entry) => printer.println(format!( + "Cleared the session-active workspace ({}); falling back to cwd \ + resolution.", + entry.root.to_string().bold().grey(), + )), + None => printer.println( + "No session-active workspace was set; using cwd resolution.".to_owned(), + ), + } + } + + ResolvedTarget::Root(selected) => { + let Some(id) = selected.id else { + return Err(format!( + "`{}` is not a recognizable JP workspace: its `{}` ID file is missing or \ + unreadable.", + selected.root, + crate::DEFAULT_STORAGE_DIR, + ) + .into()); + }; + + if previous.as_ref().is_some_and(|entry| { + entry.id().is_some_and(|prev| prev == id) && entry.root == selected.root + }) { + printer.println(format!( + "Already the session-active workspace: {}", + selected.root.to_string().bold().yellow(), + )); + return Ok(()); + } + + env.store + .record_selection(session, &id, &selected.root, Utc::now())?; + + let to = selected.root.to_string().bold().yellow(); + match previous { + Some(entry) => printer.println(format!( + "Switched the session-active workspace from {} to {to}", + entry.root.to_string().bold().grey(), + )), + None => printer.println(format!("Session-active workspace set to {to}")), + } + } + } + + Ok(()) + } +} + +#[cfg(test)] +#[path = "use_tests.rs"] +mod tests; diff --git a/crates/jp_cli/src/cmd/workspace/use_tests.rs b/crates/jp_cli/src/cmd/workspace/use_tests.rs new file mode 100644 index 000000000..f5813d289 --- /dev/null +++ b/crates/jp_cli/src/cmd/workspace/use_tests.rs @@ -0,0 +1,232 @@ +use std::str::FromStr as _; + +use camino::{Utf8Path, Utf8PathBuf}; +use camino_tempfile::tempdir; +use jp_printer::{OutputFormat, Printer}; +use jp_workspace::{ + Id, + session::{Session, SessionId, SessionSource}, + session_store::WorkspaceSessionStore, +}; + +use super::*; +use crate::cmd::workspace::target::TargetEnv; + +fn env_at<'a>( + launch_cwd: Utf8PathBuf, + data_dir: &Utf8Path, + session: Option<&'a Session>, + interactive: bool, +) -> TargetEnv<'a> { + TargetEnv { + launch_cwd, + workspaces_dir: data_dir.join(crate::USER_WORKSPACES_DIR), + store: WorkspaceSessionStore::at_user_data_dir(data_dir), + session, + interactive, + } +} + +fn make_workspace(base: &Utf8Path, name: &str, id: &str) -> Utf8PathBuf { + let root = base.join(name); + std::fs::create_dir_all(root.join(crate::DEFAULT_STORAGE_DIR).as_std_path()).unwrap(); + std::fs::write( + root.join(crate::DEFAULT_STORAGE_DIR) + .join(".id") + .as_std_path(), + id, + ) + .unwrap(); + root +} + +fn env_session() -> Session { + Session { + id: SessionId::new("42").expect("non-empty"), + source: SessionSource::env("JP_SESSION"), + } +} + +/// Flush the async printer, then read what reached the buffer. +fn stdout_of(printer: &Printer, buffer: &jp_printer::SharedBuffer) -> String { + printer.flush(); + buffer.lock().clone() +} + +/// Render an error via `Debug`, which includes wrapped messages and sources. +fn message_of(error: &impl std::fmt::Debug) -> String { + format!("{error:?}") +} + +#[test] +fn non_interactive_use_is_rejected() { + let tmp = tempdir().unwrap(); + let session = env_session(); + let env = env_at(tmp.path().to_owned(), tmp.path(), Some(&session), false); + let (printer, _out, _err) = Printer::memory(OutputFormat::Text); + + let error = Use { target: None }.run(&printer, &env).unwrap_err(); + assert!( + message_of(&error).contains("interactive-only"), + "unexpected error: {error:?}" + ); +} + +#[test] +fn use_without_a_session_identity_is_rejected() { + let tmp = tempdir().unwrap(); + let env = env_at(tmp.path().to_owned(), tmp.path(), None, true); + let (printer, _out, _err) = Printer::memory(OutputFormat::Text); + + let error = Use { target: None }.run(&printer, &env).unwrap_err(); + assert!( + message_of(&error).contains("No session identity"), + "unexpected error: {error:?}" + ); +} + +#[test] +fn selecting_a_path_records_the_selection() { + let tmp = tempdir().unwrap(); + let root = make_workspace(tmp.path(), "proj", "ws123"); + let session = env_session(); + let env = env_at(tmp.path().to_owned(), tmp.path(), Some(&session), true); + let (printer, out, _err) = Printer::memory(OutputFormat::Text); + + Use { + target: Some(WorkspaceTarget::Path(root.clone())), + } + .run(&printer, &env) + .unwrap(); + + let stdout = stdout_of(&printer, &out); + assert!( + stdout.contains("Session-active workspace set to"), + "unexpected output: {stdout}" + ); + + let active = env.store.active(&session).expect("active entry"); + assert_eq!(active.workspace_id, "ws123"); + assert_eq!(active.root, root); +} + +#[test] +fn reselecting_the_active_workspace_is_a_noop() { + let tmp = tempdir().unwrap(); + let root = make_workspace(tmp.path(), "proj", "ws123"); + let session = env_session(); + let env = env_at(tmp.path().to_owned(), tmp.path(), Some(&session), true); + + let (printer, _out, _err) = Printer::memory(OutputFormat::Text); + Use { + target: Some(WorkspaceTarget::Path(root.clone())), + } + .run(&printer, &env) + .unwrap(); + + let (printer, out, _err) = Printer::memory(OutputFormat::Text); + Use { + target: Some(WorkspaceTarget::Path(root.clone())), + } + .run(&printer, &env) + .unwrap(); + + let stdout = stdout_of(&printer, &out); + assert!( + stdout.contains("Already the session-active workspace"), + "unexpected output: {stdout}" + ); +} + +#[test] +fn use_cwd_clears_the_selection() { + let tmp = tempdir().unwrap(); + let root = make_workspace(tmp.path(), "proj", "ws123"); + let session = env_session(); + let env = env_at(tmp.path().to_owned(), tmp.path(), Some(&session), true); + + let (printer, _out, _err) = Printer::memory(OutputFormat::Text); + Use { + target: Some(WorkspaceTarget::Path(root)), + } + .run(&printer, &env) + .unwrap(); + assert!(env.store.load(&session).is_some()); + + let (printer, out, _err) = Printer::memory(OutputFormat::Text); + Use { + target: Some(WorkspaceTarget::Cwd), + } + .run(&printer, &env) + .unwrap(); + + let stdout = stdout_of(&printer, &out); + assert!(stdout.contains("Cleared"), "unexpected output: {stdout}"); + assert!(env.store.load(&session).is_none()); +} + +#[test] +fn a_path_without_a_workspace_id_is_rejected() { + let tmp = tempdir().unwrap(); + let session = env_session(); + let env = env_at(tmp.path().to_owned(), tmp.path(), Some(&session), true); + let (printer, _out, _err) = Printer::memory(OutputFormat::Text); + + // A directory with no `.jp` storage anywhere up the tree: target + // resolution finds no workspace at all. + let plain = tmp.path().join("plain"); + std::fs::create_dir_all(plain.as_std_path()).unwrap(); + + let error = Use { + target: Some(WorkspaceTarget::Path(plain)), + } + .run(&printer, &env) + .unwrap_err(); + assert!( + message_of(&error).contains("No workspace found"), + "unexpected error: {error:?}" + ); + + // A `.jp` directory without an `.id` file: a root is found, but it is + // not a recognizable workspace, so nothing is recorded. + let no_id = tmp.path().join("no-id"); + std::fs::create_dir_all(no_id.join(crate::DEFAULT_STORAGE_DIR).as_std_path()).unwrap(); + + let error = Use { + target: Some(WorkspaceTarget::Path(no_id)), + } + .run(&printer, &env) + .unwrap_err(); + assert!( + message_of(&error).contains("recognizable JP workspace"), + "unexpected error: {error:?}" + ); +} + +#[test] +fn selections_are_scoped_to_the_session() { + let tmp = tempdir().unwrap(); + let root = make_workspace(tmp.path(), "proj", "ws123"); + let session = env_session(); + let env = env_at(tmp.path().to_owned(), tmp.path(), Some(&session), true); + + let (printer, _out, _err) = Printer::memory(OutputFormat::Text); + Use { + target: Some(WorkspaceTarget::Path(root)), + } + .run(&printer, &env) + .unwrap(); + + // A different session sees no selection. + let other = Session { + id: SessionId::new("43").expect("non-empty"), + source: SessionSource::env("JP_SESSION"), + }; + assert!(env.store.load(&other).is_none()); + + let active = env.store.active(&session).expect("active entry"); + assert_eq!( + active.workspace_id, + Id::from_str("ws123").unwrap().to_string() + ); +} diff --git a/crates/jp_cli/src/ctx.rs b/crates/jp_cli/src/ctx.rs index c197a1fea..91c481966 100644 --- a/crates/jp_cli/src/ctx.rs +++ b/crates/jp_cli/src/ctx.rs @@ -19,10 +19,14 @@ use tokio::{ task::JoinSet, }; -use crate::{Globals, Result, signals::SignalRouter}; +use crate::{Globals, Result, bootstrap::ExecutionContext, signals::SignalRouter}; /// Context for the CLI application pub(crate) struct Ctx { + /// The bootstrap-resolved execution context: launch cwd, selected checkout + /// root, and the working directory for spawned children (RFD 087). + pub(crate) exec: ExecutionContext, + /// The workspace. pub(crate) workspace: Workspace, @@ -82,6 +86,7 @@ pub(crate) struct Term { impl Ctx { /// Create a new context with the given workspace pub(crate) fn new( + exec: ExecutionContext, workspace: Workspace, fs_backend: Option>, runtime: Runtime, @@ -93,7 +98,8 @@ impl Ctx { let config = config.into(); let escalation_cooldown = Duration::from_secs(config.interrupt.escalation_cooldown_secs.into()); - let mcp_client = jp_mcp::Client::new(config.providers.mcp.clone()); + let mcp_client = jp_mcp::Client::new(config.providers.mcp.clone()) + .with_child_cwd(exec.child_cwd().map(|cwd| cwd.as_std_path().to_path_buf())); let is_tty = io::stdout().is_terminal(); let width = if is_tty { @@ -103,6 +109,7 @@ impl Ctx { }; Self { + exec, workspace, fs_backend, config, diff --git a/crates/jp_cli/src/lib.rs b/crates/jp_cli/src/lib.rs index a1a2c55ee..d4130dabb 100644 --- a/crates/jp_cli/src/lib.rs +++ b/crates/jp_cli/src/lib.rs @@ -1,4 +1,5 @@ mod access; +mod bootstrap; mod cmd; mod config_pipeline; mod ctx; @@ -28,14 +29,13 @@ use std::{ time::Duration, }; -use camino::{FromPathBufError, Utf8PathBuf, absolute_utf8}; +use camino::{Utf8Path, Utf8PathBuf}; use camino_tempfile::NamedUtf8TempFile; use clap::{ ArgAction, Parser, builder::{BoolValueParser, TypedValueParser as _}, }; -use cmd::Commands; -use crossterm::style::Stylize as _; +use cmd::{Commands, workspace::target::WorkspaceTarget}; use ctx::{Ctx, IntoPartialAppConfig}; use error::{Error, Result}; use jp_config::{ @@ -50,13 +50,14 @@ use jp_config::{ use jp_printer::{OutputFormat, Printer}; use jp_storage::backend::{FsStorageBackend, NullLockBackend, NullPersistBackend}; use jp_term::table::{DetailRow, details, details_markdown}; -use jp_workspace::{Workspace, user_data_dir}; +use jp_workspace::{Workspace, roots, session_store::WorkspaceSessionStore, user_data_dir}; use relative_path::RelativePath; use serde_json::Value; use tokio::runtime::{self, Runtime}; use tracing::{debug, info, trace, warn}; use crate::{ + bootstrap::WorkspaceRequirement, cmd::{ plugin::dispatch::{describe_plugin, discover_plugins}, target::resolve_request, @@ -69,6 +70,10 @@ static WORKER_THREADS: AtomicUsize = AtomicUsize::new(0); const DEFAULT_STORAGE_DIR: &str = ".jp"; +/// The per-user data subdirectory holding one directory per known workspace +/// (`-`), each with its roots registry (RFD 087). +const USER_WORKSPACES_DIR: &str = "workspace"; + #[expect(dead_code)] const DEFAULT_VARIABLE_PREFIX: &str = "JP_"; @@ -157,9 +162,15 @@ struct Globals { /// The workspace to use for the command. /// - /// This can be either a path to a workspace directory, or a workspace ID. + /// Accepts the workspace targeting grammar (see `jp w use help`): a + /// workspace ID, a path, `cwd` / `.`, or `-` to read an ID from stdin. + /// Interactive runs can also use the session keywords (`s`, `?s`), the + /// pickers (`?`), and free-text matching. + /// + /// Selects the workspace for this invocation only; it does not change the + /// session's active workspace (that is `jp w use`). #[arg(short = 'w', long, global = true)] - workspace: Option, + workspace: Option, /// The format of the log output written to stderr. /// @@ -248,24 +259,6 @@ impl FromStr for KeyValueOrPath { } } -#[derive(Debug, Clone)] -pub(crate) enum WorkspaceIdOrPath { - Id(jp_workspace::Id), - Path(Utf8PathBuf), -} - -impl FromStr for WorkspaceIdOrPath { - type Err = Error; - - fn from_str(s: &str) -> Result { - if Utf8PathBuf::from(s).exists() { - return Ok(Self::Path(Utf8PathBuf::from(s))); - } - - Ok(Self::Id(jp_workspace::Id::from_str(s)?)) - } -} - /// The format of the CLI output written to stdout. #[derive(Debug, Default, Clone, Copy, PartialEq, Eq, clap::ValueEnum)] pub(crate) enum CliFormat { @@ -385,34 +378,78 @@ pub fn run() -> ExitCode { fn run_inner(cli: Cli, format: OutputFormat) -> Result<()> { let printer = Printer::terminal(format); - // `jp init` is a special case that doesn't need the full startup pipeline. - if let Commands::Init(args) = &cli.command { - return args.run(&printer).map_err(Into::into); + // `jp workspace` runs on a dedicated pre-workspace path: selecting or + // inspecting a workspace must work from outside every workspace — + // including resolving to *no* workspace — so its subcommands never + // construct a `Ctx`. Each declares what it pays for through + // `workspace_requirement` (`ls`: registries only; `use`: resolve and + // validate a target root; `show`: additionally loads conversation + // indexes). + if let Commands::Workspace(args) = cli.command { + trace!("Resolving session identity."); + let session = session::resolve(); + + let output = args + .run(&printer, session.as_ref(), cli.globals.persist) + .map_err(Into::into); + + // `jp w use` and friends mutate the user-global records, so they get + // the same hygiene pass as a workspace-consuming run. + cleanup_workspace_session_records(); + + return output; } - let (mut workspace, fs_backend) = - load_workspace(cli.globals.workspace.as_ref(), cli.globals.persist)?; - - trace!("Sanitizing workspace."); - let report = workspace.sanitize()?; - if report.has_repairs() { - for trashed in &report.trashed { - warn!( - dirname = trashed.dirname, - error = %trashed.error, - "Trashed corrupt conversation" - ); - } + // The per-command workspace bootstrap requirement (RFD 087): commands + // declaring `None` run without any workspace resolution or construction, + // so the downstream consumers that assume a root do not run. + let requirement = cli.command.workspace_requirement(); + if requirement == WorkspaceRequirement::None { + let Commands::Init(args) = &cli.command else { + unreachable!("every workspace-free command has a dedicated run path"); + }; + + return args.run(&printer).map_err(Into::into); } + // The pre-workspace bootstrap (RFD 087): session identity and the + // execution context — launch cwd, selected checkout root, child cwd — + // are resolved once, before any `Workspace` is constructed, and passed + // explicitly to their consumers below. trace!("Resolving session identity."); let session = session::resolve(); - // Populate the conversation index. This does NOT load the contents of - // individual conversations, this is done lazily as needed. - workspace.load_conversation_index(); + let exec = bootstrap::resolve(cli.globals.workspace.as_ref(), session.as_ref())?; + trace!( + root = %exec.root, + source = ?exec.source, + child_cwd = ?exec.child_cwd(), + "Bootstrapped workspace selection." + ); + + let (mut workspace, fs_backend) = load_workspace(&exec.root, cli.globals.persist)?; + + // `Resolve` commands stop at a validated root; only `Load` commands pay + // for sanitization and the conversation index. + if requirement == WorkspaceRequirement::Load { + trace!("Sanitizing workspace."); + let report = workspace.sanitize()?; + if report.has_repairs() { + for trashed in &report.trashed { + warn!( + dirname = trashed.dirname, + error = %trashed.error, + "Trashed corrupt conversation" + ); + } + } + + // Populate the conversation index. This does NOT load the contents of + // individual conversations, this is done lazily as needed. + workspace.load_conversation_index(); + } - let base = load_base_partial(fs_backend.as_deref())?; + let base = load_base_partial(fs_backend.as_deref(), exec.config_cwd().to_owned())?; let (config, handles, start_new) = resolve_config( &cli.command, base, @@ -424,6 +461,7 @@ fn run_inner(cli: Cli, format: OutputFormat) -> Result<()> { let config = Arc::new(config); let runtime = build_runtime(cli.root.threads, "jp-worker")?; let mut ctx = Ctx::new( + exec, workspace, fs_backend, runtime, @@ -482,9 +520,32 @@ fn run_inner(cli: Cli, format: OutputFormat) -> Result<()> { // Remove orphaned lock files and stale session mappings. ctx.workspace.cleanup_stale_files(ctx.fs_backend.as_deref()); + // Bootstrap cleanup (RFD 087): the user-global session → workspace + // records are owned by this layer, not `Workspace` — they exist before + // any workspace is selected and can reference workspaces this run never + // touched. The source-split rules live in + // `WorkspaceSessionStore::cleanup`. + cleanup_workspace_session_records(); + output.map_err(Into::into) } +/// Source-split cleanup of the user-global session → active-workspace store. +/// +/// A workspace ID counts as live while any registered checkout of it still +/// resolves to a workspace with that ID; expanding an ID also prunes its dead +/// registry entries opportunistically (RFD 087). +fn cleanup_workspace_session_records() { + let Ok(data_dir) = user_data_dir() else { + return; + }; + + let workspaces_dir = data_dir.join(USER_WORKSPACES_DIR); + WorkspaceSessionStore::at_user_data_dir(&data_dir).cleanup(&|id| { + !roots::resolve_live_roots(&workspaces_dir, id, DEFAULT_STORAGE_DIR).is_empty() + }); +} + /// Drain background tasks at end of run, with interrupt-aware cancellation. /// /// While [`TaskHandler::sync`] runs, prints a `⏱ Finishing background tasks… @@ -734,9 +795,14 @@ pub(crate) fn resolve_config( /// [`ConfigPipeline`]. /// No `--cfg` args or per-conversation config. /// +/// `cwd` is the bootstrap-resolved invocation directory for the `.jp.toml` +/// chain ([`bootstrap::ExecutionContext::config_cwd`]): the launch cwd, or the +/// workspace root when JP operates on a workspace other than the launch cwd's +/// own (RFD 087). +/// /// See: -fn load_base_partial(fs: Option<&FsStorageBackend>) -> Result { - let partials = load_partial_configs_from_files(fs, absolute_utf8(".").ok())?; +fn load_base_partial(fs: Option<&FsStorageBackend>, cwd: Utf8PathBuf) -> Result { + let partials = load_partial_configs_from_files(fs, Some(cwd))?; let partial = load_partials_with_inheritance(partials)?; load_envs(partial).map_err(|error| Error::CliConfig(error.to_string())) @@ -794,46 +860,19 @@ fn load_partial_configs_from_files( Ok(partials) } -/// Find the workspace for the current directory. +/// Construct the workspace at the given, bootstrap-selected checkout root. +/// +/// Root selection lives in [`bootstrap::resolve`]; this only builds the storage +/// backend and [`Workspace`] on top of it. /// /// When `persist` is `false` (`--no-persist`), the persist backend is swapped /// to [`NullPersistBackend`] and the lock backend to [`NullLockBackend`] so /// that ephemeral queries never write to disk and never block on lock /// contention. fn load_workspace( - workspace: Option<&WorkspaceIdOrPath>, + root: &Utf8Path, persist: bool, ) -> Result<(Workspace, Option>)> { - let cwd = match workspace { - None => absolute_utf8(".")?, - Some(WorkspaceIdOrPath::Path(path)) => path.clone(), - - // TODO: Centralize this in a new `UserStorage` struct. - Some(WorkspaceIdOrPath::Id(id)) => user_data_dir()? - .join("workspace") - .read_dir()? - .map(|dir| dir.ok().map(|dir| dir.path().clone())) - .find_map(|path| { - path.filter(|dir| { - dir.file_name() - .and_then(|v| v.to_str()) - .is_some_and(|v| v.ends_with(&id.to_string())) - }) - }) - .ok_or(jp_workspace::Error::MissingStorage)? - .join("storage") - .canonicalize()? - .try_into() - .map_err(FromPathBufError::into_io_error)?, - }; - trace!(cwd = %cwd, "Finding workspace."); - - let root = Workspace::find_root(cwd, DEFAULT_STORAGE_DIR).ok_or(cmd::Error::from(format!( - "Could not locate workspace. Use `{}` to create a new workspace.", - "jp init".bold().yellow() - )))?; - trace!(root = %root, "Found workspace root."); - let storage = root.join(DEFAULT_STORAGE_DIR); trace!(storage = %storage, "Initializing workspace storage."); @@ -847,16 +886,27 @@ fn load_workspace( let fs = FsStorageBackend::new(&storage).map_err(jp_workspace::Error::from)?; - let user_root = user_data_dir()?.join("workspace"); - // The workspace directory name slugs a freshly created silo so users can - // recognize it; an existing silo is reused by ID regardless of its slug. + let workspaces_dir = user_data_dir()?.join("workspace"); + // The workspace directory name slugs a freshly created user-workspace + // directory so users can recognize it; an existing one is reused by ID + // regardless of its slug. let slug = root.file_name(); let fs = fs - .with_user_storage(&user_root, slug, id.to_string()) + .with_user_storage(&workspaces_dir, slug, id.to_string()) .map_err(jp_workspace::Error::from)?; + // Register this checkout in the workspace's roots registry so `-w ` + // can target it from anywhere, folding in any pre-registry `storage` + // symlink first (see RFD 087). + if let Some(dir) = fs.user_storage_path() { + roots::migrate_legacy_symlink(dir, &id, DEFAULT_STORAGE_DIR); + if let Err(error) = roots::upsert_root(dir, root) { + warn!(%error, "Failed to record the checkout in the workspace roots registry."); + } + } + let fs = Arc::new(fs); - let mut workspace = Workspace::new_with_id(root, id).with_backend(fs.clone()); + let mut workspace = Workspace::new_with_id(root.to_owned(), id).with_backend(fs.clone()); if !persist { workspace = workspace .with_persist(Arc::new(NullPersistBackend)) diff --git a/crates/jp_cli/src/lib_tests.rs b/crates/jp_cli/src/lib_tests.rs index 7a5f96266..d060e3a48 100644 --- a/crates/jp_cli/src/lib_tests.rs +++ b/crates/jp_cli/src/lib_tests.rs @@ -581,3 +581,7 @@ fn resolve_config_consumes_default_id() { config.conversation.default_id, ); } + +// Workspace-root selection by ID moved into the bootstrap step (RFD 087 +// phase 2/3); its behavior is covered by `bootstrap_tests` and +// `cmd::workspace::target` tests. diff --git a/crates/jp_cli/src/shared/search_tests.rs b/crates/jp_cli/src/shared/search_tests.rs index c68f196ce..c0272bb44 100644 --- a/crates/jp_cli/src/shared/search_tests.rs +++ b/crates/jp_cli/src/shared/search_tests.rs @@ -30,6 +30,7 @@ fn setup_ctx_with_conversations( let workspace = Workspace::new(tmp.path()); let (printer, _, _) = Printer::memory(OutputFormat::TextPretty); let mut ctx = Ctx::new( + crate::bootstrap::ExecutionContext::for_workspace(&workspace), workspace, None, Runtime::new().unwrap(), diff --git a/crates/jp_inquire/src/lib.rs b/crates/jp_inquire/src/lib.rs index 21838f04f..3483ad389 100644 --- a/crates/jp_inquire/src/lib.rs +++ b/crates/jp_inquire/src/lib.rs @@ -1,3 +1,5 @@ +//! Inline terminal prompt widgets (reply and select) for JP. + mod inline_reply; mod inline_select; pub mod prompt; diff --git a/crates/jp_mcp/src/client.rs b/crates/jp_mcp/src/client.rs index d6ce8dd34..0c83aee4c 100644 --- a/crates/jp_mcp/src/client.rs +++ b/crates/jp_mcp/src/client.rs @@ -1,7 +1,7 @@ use std::{ collections::{HashMap, HashSet, VecDeque}, env, - path::Path, + path::{Path, PathBuf}, process::Stdio, sync::{Arc, Mutex}, time::Duration, @@ -52,6 +52,11 @@ pub struct Client { /// Running MCP services. services: Arc>>>, + + /// Working directory for spawned stdio servers. + /// + /// `None` inherits the process cwd. + child_cwd: Option, } impl std::fmt::Debug for Client { @@ -59,6 +64,7 @@ impl std::fmt::Debug for Client { f.debug_struct("Client") .field("servers", &self.servers) .field("services", &self.services.blocking_read().keys()) + .field("child_cwd", &self.child_cwd) .finish() } } @@ -75,9 +81,21 @@ impl Client { Self { services: Arc::new(RwLock::new(HashMap::new())), servers: Arc::new(RwLock::new(servers)), + child_cwd: None, } } + /// Set the working directory spawned stdio servers inherit. + /// + /// `None` (the default) inherits the JP process cwd. + /// The bootstrap sets this to the selected workspace root for from-anywhere + /// runs (RFD 087). + #[must_use] + pub fn with_child_cwd(mut self, cwd: Option) -> Self { + self.child_cwd = cwd; + self + } + /// Look up a tool definition on a specific MCP server. /// /// The server must be configured (i.e. present in the [`Client`]'s server @@ -96,7 +114,7 @@ impl Client { client.peer().list_all_tools().await? } else { drop(running); - match Self::try_create_client(server_id, server).await? { + match Self::try_create_client(server_id, server, self.child_cwd.as_deref()).await? { SpawnOutcome::Started(client) => client.list_all_tools().await?, SpawnOutcome::OptionalFailed => return Err(Error::UnknownTool(id.to_string())), } @@ -194,13 +212,14 @@ impl Client { joins.spawn({ let servers = self.servers.clone(); let clients = self.services.clone(); + let child_cwd = self.child_cwd.clone(); async move { let servers = servers.read().await; let server = servers .get(&server_id) .ok_or(Error::UnknownServer(server_id.clone()))?; - match Self::try_create_client(&server_id, server).await? { + match Self::try_create_client(&server_id, server, child_cwd.as_deref()).await? { SpawnOutcome::Started(client) => { clients.write().await.insert(server_id.clone(), client); } @@ -235,8 +254,9 @@ impl Client { async fn try_create_client( id: &McpServerId, config: &McpProviderConfig, + child_cwd: Option<&Path>, ) -> Result { - match Self::create_client(id, config).await { + match Self::create_client(id, config, child_cwd).await { Ok(client) => Ok(SpawnOutcome::Started(client)), Err(error) if config.optional() => { warn!( @@ -255,6 +275,7 @@ impl Client { async fn create_client( id: &McpServerId, config: &McpProviderConfig, + child_cwd: Option<&Path>, ) -> Result> { match config { McpProviderConfig::Stdio(config) => { @@ -290,6 +311,13 @@ impl Client { let mut cmd = Command::new(&config.command); cmd.args(&config.arguments); + // The root-as-working-directory invariant (RFD 087): when JP + // operates on a workspace other than the launch cwd's own, + // servers run as if launched from the selected workspace root. + if let Some(cwd) = child_cwd { + cmd.current_dir(cwd); + } + // Put the MCP server in its own process group so terminal // signals (Ctrl+C / SIGINT) don't kill it. JP manages the // server lifecycle through the MCP protocol and diff --git a/crates/jp_mcp/src/lib.rs b/crates/jp_mcp/src/lib.rs index f5f21ef40..b770919fc 100644 --- a/crates/jp_mcp/src/lib.rs +++ b/crates/jp_mcp/src/lib.rs @@ -1,3 +1,5 @@ +//! MCP (Model Context Protocol) client integration for JP. + mod client; pub mod error; pub mod id; diff --git a/crates/jp_storage/src/backend/fs.rs b/crates/jp_storage/src/backend/fs.rs index 206792072..0d7d1719a 100644 --- a/crates/jp_storage/src/backend/fs.rs +++ b/crates/jp_storage/src/backend/fs.rs @@ -48,14 +48,14 @@ impl FsStorageBackend { /// Configure user-local storage for workspace `id` under `root`. /// - /// The silo is located by ID suffix, so every worktree and clone of a - /// workspace shares one directory. - /// A new silo is named `-` (or bare `` when `slug` is absent + /// The user-workspace directory is located by ID suffix, so every worktree + /// and clone of a workspace shares one directory. + /// A new one is named `-` (or bare `` when `slug` is absent /// or empty); `slug` only ever names a new directory and never renames an /// existing one. - /// Runs a one-time migration on first setup that merges sibling silos and - /// imports the workspace's conversations so a durable user-local copy - /// exists. + /// Runs a one-time migration on first setup that merges sibling + /// user-workspace directories and imports the workspace's conversations so + /// a durable user-local copy exists. pub fn with_user_storage( self, root: &Utf8Path, diff --git a/crates/jp_storage/src/error.rs b/crates/jp_storage/src/error.rs index 007ec986c..667fcb468 100644 --- a/crates/jp_storage/src/error.rs +++ b/crates/jp_storage/src/error.rs @@ -8,9 +8,6 @@ pub enum Error { #[error("Path is not a directory: {0}")] NotDir(Utf8PathBuf), - #[error("Path is not a symlink: {0}")] - NotSymlink(Utf8PathBuf), - #[error("conversation error")] Conversation(#[from] jp_conversation::Error), diff --git a/crates/jp_storage/src/lib.rs b/crates/jp_storage/src/lib.rs index 9c93ba78b..2a4c82a64 100644 --- a/crates/jp_storage/src/lib.rs +++ b/crates/jp_storage/src/lib.rs @@ -1,3 +1,5 @@ +//! Storage backends and on-disk formats for JP workspace data. + pub mod backend; pub mod error; pub mod lock; @@ -65,17 +67,18 @@ impl Storage { /// Configure user-local storage for workspace `id` under `root`. /// - /// The silo is located by ID suffix, so every worktree and clone of a - /// workspace shares the single directory that already exists. + /// The user-workspace directory is located by ID suffix, so every worktree + /// and clone of a workspace shares the single directory that already + /// exists. /// When none does, a new `-` directory is created: `slug` /// (typically the workspace directory name) is cosmetic, only ever names a - /// *new* silo, is never validated, and an absent or empty slug yields a - /// bare `` directory. + /// *new* directory, is never validated, and an absent or empty slug yields + /// a bare `` directory. /// /// Before wiring up the directory, a one-time migration runs: any sibling - /// silos for this workspace are merged in, and on first setup the - /// workspace's conversations are imported so a durable user-local copy - /// exists. + /// user-workspace directories for this workspace are merged in, and on + /// first setup the workspace's conversations are imported so a durable + /// user-local copy exists. pub fn with_user_storage( mut self, root: &Utf8Path, @@ -83,7 +86,7 @@ impl Storage { id: impl Into, ) -> Result { let id: String = id.into(); - let (path, first_run) = resolve_user_dir(root, slug, &id); + let (path, first_run) = resolve_user_workspace_dir(root, slug, &id); migrate_user_storage(root, &id, &path, &self.root, first_run)?; @@ -96,33 +99,6 @@ impl Storage { trace!(path = %path, "Created user storage directory."); } - // Point the `storage` symlink back at the current workspace root, - // repairing a link inherited from another worktree during migration. - let link = path.join("storage"); - if link.is_symlink() - && fs::read_link(&link).is_ok_and(|target| target.as_path() != self.root.as_std_path()) - { - trace!(link = %link, "Re-pointing user storage symlink to current workspace."); - remove_storage_symlink(&link)?; - } - if link.exists() { - if !link.is_symlink() { - return Err(Error::NotSymlink(link)); - } - } else { - #[cfg(unix)] - std::os::unix::fs::symlink(&self.root, &link)?; - #[cfg(windows)] - std::os::windows::fs::symlink_dir(&self.root, &link)?; - #[cfg(not(any(unix, windows)))] - { - tracing::error!( - "Unsupported platform, cannot create symlink. Disabling user storage." - ); - return Ok(self); - } - } - self.user = Some(path); Ok(self) } @@ -581,39 +557,29 @@ impl Storage { } } -/// Remove a `storage` symlink without following it. +/// Resolve the user-workspace directory for workspace `id`. /// -/// On Windows a directory symlink is a reparse-point directory and must be -/// removed with `remove_dir`; `remove_file` returns "Access is denied". -/// On Unix `remove_file` unlinks the symlink itself. -fn remove_storage_symlink(link: &Utf8Path) -> io::Result<()> { - #[cfg(windows)] - { - fs::remove_dir(link) - } - #[cfg(not(windows))] - { - fs::remove_file(link) - } -} - -/// Resolve the user-local silo directory for workspace `id`. -/// -/// Silos are located by ID suffix (`` or `-`), never by exact -/// name, so every worktree and clone of a workspace shares the one silo that -/// already exists regardless of the directory it was cloned into. -/// The returned flag is `true` when no silo exists yet and one must be created. +/// The directory is located by ID suffix (`` or `-`), never by +/// exact name, so every worktree and clone of a workspace shares the one +/// directory that already exists regardless of the checkout it was cloned into. +/// The returned flag is `true` when none exists yet and one must be created. /// -/// A new silo is named `-` for human recognition; an absent or empty -/// `slug` yields a bare `` directory. -/// The slug only ever names a *new* silo: an existing one is reused as-is and -/// never renamed. +/// A new directory is named `-` for human recognition; an absent or +/// empty `slug` yields a bare `` name. +/// The slug only ever names a *new* directory: an existing one is reused as-is +/// and never renamed. /// -/// When several silos already exist (legacy per-worktree directories), the one -/// whose name matches `-` wins; otherwise the most recently modified -/// silo does. -fn resolve_user_dir(root: &Utf8Path, slug: Option<&str>, id: &str) -> (Utf8PathBuf, bool) { - if let Some(dir) = choose_canonical_user_dir(&matching_user_dirs(root, id), slug, id) { +/// When several already exist (legacy per-worktree directories), the one whose +/// name matches `-` wins; otherwise the most recently modified one +/// does. +fn resolve_user_workspace_dir( + root: &Utf8Path, + slug: Option<&str>, + id: &str, +) -> (Utf8PathBuf, bool) { + if let Some(dir) = + choose_canonical_user_workspace_dir(&matching_user_workspace_dirs(root, id), slug, id) + { return (dir, false); } @@ -624,8 +590,15 @@ fn resolve_user_dir(root: &Utf8Path, slug: Option<&str>, id: &str) -> (Utf8PathB (root.join(name), true) } -/// List the user-local silo directories whose name resolves to workspace `id`. -fn matching_user_dirs(root: &Utf8Path, id: &str) -> Vec { +/// List the user-workspace directories whose name resolves to workspace `id`. +/// +/// Directories are matched by ID suffix (`` or `-`) — the same +/// rule [`FsStorageBackend::with_user_storage`] uses to locate one — so +/// callers resolving a workspace ID without a `Storage` at hand (e.g. `-w +/// `) agree with it on which directory belongs to the workspace. +/// +/// [`FsStorageBackend::with_user_storage`]: backend::FsStorageBackend::with_user_storage +pub fn matching_user_workspace_dirs(root: &Utf8Path, id: &str) -> Vec { if !root.is_dir() { return vec![]; } @@ -641,13 +614,13 @@ fn matching_user_dirs(root: &Utf8Path, id: &str) -> Vec { .collect() } -/// Pick the canonical silo among existing matches, or `None` when there are -/// none. +/// Pick the canonical user-workspace directory among existing matches, or +/// `None` when there are none. /// /// An exact `-` match wins so a returning workspace keeps the -/// directory it recognizes; otherwise the most recently modified silo does, +/// directory it recognizes; otherwise the most recently modified one does, /// breaking mtime ties by name for determinism. -fn choose_canonical_user_dir( +fn choose_canonical_user_workspace_dir( dirs: &[Utf8PathBuf], slug: Option<&str>, id: &str, @@ -665,10 +638,10 @@ fn choose_canonical_user_dir( .map(|(_, dir)| dir.clone()) } -/// Migrate user-local storage into the chosen silo. +/// Migrate user-local storage into the chosen user-workspace directory. /// -/// Merges any sibling silos for this workspace into `target` (kept as-is, never -/// renamed). +/// Merges any sibling directories for this workspace into `target` (kept as-is, +/// never renamed). /// On the first run for a workspace it also imports the workspace's /// conversations so they gain a durable user-local copy. /// Later runs skip the import, leaving conversations committed by other @@ -682,7 +655,7 @@ fn migrate_user_storage( workspace_root: &Utf8Path, first_run: bool, ) -> Result<()> { - merge_sibling_user_dirs(user_root, id, target)?; + merge_sibling_user_workspace_dirs(user_root, id, target)?; if first_run { adopt_conversations(workspace_root, target, false)?; @@ -691,16 +664,21 @@ fn migrate_user_storage( Ok(()) } -/// Collapse every other silo for workspace `id` into `target`. +/// Collapse every other user-workspace directory for workspace `id` into +/// `target`. /// -/// Sibling silos are matched by ID suffix, so legacy per-worktree directories +/// Siblings are matched by ID suffix, so legacy per-worktree directories /// (`-`) and bare `` directories alike are folded in, /// conversation-by-conversation (the most recently modified copy wins on /// conflict). /// `target` itself is skipped and never renamed; once it has absorbed a /// sibling's conversations and residual entries, the empty sibling is removed. -fn merge_sibling_user_dirs(user_root: &Utf8Path, id: &str, target: &Utf8Path) -> Result<()> { - for dir in matching_user_dirs(user_root, id) { +fn merge_sibling_user_workspace_dirs( + user_root: &Utf8Path, + id: &str, + target: &Utf8Path, +) -> Result<()> { + for dir in matching_user_workspace_dirs(user_root, id) { if dir == *target { continue; } @@ -708,9 +686,10 @@ fn merge_sibling_user_dirs(user_root: &Utf8Path, id: &str, target: &Utf8Path) -> trace!(sibling = %dir, target = %target, "Merging sibling user storage directory."); adopt_conversations(&dir, target, true)?; - // Move any remaining entries (e.g. `sessions`) the target lacks. The - // `storage` symlink is recreated by `with_user_storage`, conversations - // are handled above, and anything still here is dropped with the dir. + // Move any remaining entries (e.g. `sessions`, `roots`) the target + // lacks. Conversations are handled above; a legacy `storage` symlink + // is dropped with the dir (the roots registry replaces it, and a live + // checkout re-registers itself on its next run). let residual: Vec<(String, Utf8PathBuf)> = dir_entries(&dir) .filter_map(|entry| { let name = entry.file_name().to_owned(); diff --git a/crates/jp_storage/src/lib_tests.rs b/crates/jp_storage/src/lib_tests.rs index 375f73e68..f57094dc5 100644 --- a/crates/jp_storage/src/lib_tests.rs +++ b/crates/jp_storage/src/lib_tests.rs @@ -373,11 +373,9 @@ fn with_user_storage_uses_workspace_id_path() { assert!(user_dir.is_dir(), "user storage lives at /"); assert_eq!(storage.user_storage_path(), Some(user_dir.as_path())); - let link = user_dir.join("storage"); - assert!(link.is_symlink(), "user dir holds a `storage` symlink"); - assert_eq!( - fs::read_link(&link).unwrap().as_path(), - workspace.as_std_path() + assert!( + !user_dir.join("storage").is_symlink(), + "no legacy `storage` symlink is created" ); } @@ -397,7 +395,7 @@ fn reuses_existing_slugged_user_dir_in_place() { #[cfg(unix)] std::os::unix::fs::symlink(tmp.path().join("old-workspace"), existing.join("storage")).unwrap(); - // No slug given: the silo is still located by ID suffix and reused as-is, + // No slug given: the directory is still located by ID suffix and reused as-is, // never renamed to a bare `` directory. let storage = Storage::new(workspace.clone()) .unwrap() @@ -431,8 +429,8 @@ fn reuses_existing_slugged_user_dir_in_place() { assert!(link.is_symlink()); assert_eq!( fs::read_link(&link).unwrap().as_path(), - workspace.as_std_path(), - "symlink re-pointed at the current workspace" + tmp.path().join("old-workspace").as_std_path(), + "the legacy symlink is left untouched" ); } } @@ -443,7 +441,7 @@ fn merges_sibling_user_dirs_keeping_newest_conversation() { let workspace = tmp.path().join("workspace"); let user_root = tmp.path().join("user"); - // The same conversation lives in two silos; the `feature-abc` copy is newer + // The same conversation lives in two user-workspace directories; the `feature-abc` copy is newer // and must win the merge. let id = ConversationId::try_from_deciseconds_str("17636257526").unwrap(); let old = write_conv_dir( @@ -459,7 +457,7 @@ fn merges_sibling_user_dirs_keeping_newest_conversation() { ); set_mtime(&new.join(EVENTS_FILE), 2_000); - // The slug selects `feature-abc` as the surviving silo; `main-abc` is + // The slug selects `feature-abc` as the surviving directory; `main-abc` is // folded in and removed. let storage = Storage::new(workspace) .unwrap() @@ -493,12 +491,15 @@ fn creates_slug_prefixed_dir_for_new_workspace() { .unwrap(); let dir = user_root.join("my-project-abc"); - assert!(dir.is_dir(), "a new silo is named -"); + assert!( + dir.is_dir(), + "a new user-workspace directory is named -" + ); assert_eq!(storage.user_storage_path(), Some(dir.as_path())); } #[test] -fn second_clone_reuses_silo_despite_different_slug() { +fn second_clone_reuses_user_workspace_dir_despite_different_slug() { let tmp = tempdir().unwrap(); let user_root = tmp.path().join("user"); @@ -506,11 +507,11 @@ fn second_clone_reuses_silo_despite_different_slug() { .unwrap() .with_user_storage(&user_root, Some("clone-a"), "abc") .unwrap(); - let silo = user_root.join("clone-a-abc"); - assert_eq!(first.user_storage_path(), Some(silo.as_path())); + let dir = user_root.join("clone-a-abc"); + assert_eq!(first.user_storage_path(), Some(dir.as_path())); // A second clone of the same workspace, in a directory with a different - // name, reuses the silo created by the first clone rather than minting a + // name, reuses the directory created by the first clone rather than minting a // `clone-b-abc` of its own. let second = Storage::new(tmp.path().join("clone-b")) .unwrap() @@ -518,14 +519,14 @@ fn second_clone_reuses_silo_despite_different_slug() { .unwrap(); assert_eq!( second.user_storage_path(), - Some(silo.as_path()), - "the existing silo is reused, not renamed" + Some(dir.as_path()), + "the existing directory is reused, not renamed" ); assert!(!user_root.join("clone-b-abc").exists()); } #[test] -fn picks_most_recently_modified_silo_without_slug_match() { +fn picks_most_recently_modified_dir_without_slug_match() { let tmp = tempdir().unwrap(); let workspace = tmp.path().join("workspace"); let user_root = tmp.path().join("user"); @@ -540,7 +541,7 @@ fn picks_most_recently_modified_silo_without_slug_match() { fs::write(feature.join("marker"), "b").unwrap(); set_mtime(&feature.join("marker"), 2_000); - // The slug matches neither silo, so the most recently modified one wins. + // The slug matches neither directory, so the most recently modified one wins. let storage = Storage::new(workspace) .unwrap() .with_user_storage(&user_root, Some("unrelated"), "abc") diff --git a/crates/jp_workspace/Cargo.toml b/crates/jp_workspace/Cargo.toml index df9a10576..6b55f133e 100644 --- a/crates/jp_workspace/Cargo.toml +++ b/crates/jp_workspace/Cargo.toml @@ -17,7 +17,7 @@ jp_config = { workspace = true } jp_conversation = { workspace = true } jp_storage = { workspace = true } -camino = { workspace = true } +camino = { workspace = true, features = ["serde1"] } camino-tempfile = { workspace = true } chrono = { workspace = true } directories = { workspace = true } diff --git a/crates/jp_workspace/src/lib.rs b/crates/jp_workspace/src/lib.rs index 05800061d..2358540a9 100644 --- a/crates/jp_workspace/src/lib.rs +++ b/crates/jp_workspace/src/lib.rs @@ -3,14 +3,21 @@ //! This crate provides data models and storage operations for the JP workspace, //! a CLI tool for managing LLM-assisted code conversations with fine-grained //! control over context and behavior. +//! +//! Session identity, the per-session active-workspace store, and the roots +//! registry (RFD 087) live in [`session`], [`session_store`], and [`roots`]. +//! The session store is user-global (above any checkout); the roots registry +//! maps a workspace ID to its live checkouts on disk. mod conversation_lock; mod error; mod handle; mod id; +pub mod roots; mod sanitize; pub mod session; pub(crate) mod session_mapping; +pub mod session_store; mod state; use std::{ diff --git a/crates/jp_workspace/src/roots.rs b/crates/jp_workspace/src/roots.rs new file mode 100644 index 000000000..69ba427df --- /dev/null +++ b/crates/jp_workspace/src/roots.rs @@ -0,0 +1,354 @@ +//! Roots registry: the checkouts on disk that belong to a workspace ID. +//! +//! One workspace ID can resolve to several checkouts (for example git worktrees +//! of the same repository). +//! Each checkout registers itself in the workspace's user-workspace directory +//! as its own file: +//! +//! ```text +//! /roots/.json +//! ``` +//! +//! `` is a stable hash of the checkout's canonical path, so each +//! checkout owns exactly one file and concurrent runs never contend on shared +//! state. +//! Liveness is derived, never stored: a recorded root is live when it still +//! holds a workspace whose ID matches the directory's. +//! Dead entries are pruned opportunistically whenever the registry is read. +//! +//! [`resolve_live_roots`] expands a workspace ID to its live checkouts; +//! [`upsert_root`] registers the checkout a command runs against. +//! +//! See: `docs/rfd/087-session-scoped-active-workspace.md` + +use std::{cmp::Reverse, collections::HashSet, fs, io}; + +use camino::{Utf8DirEntry, Utf8Path, Utf8PathBuf}; +use chrono::{DateTime, Duration, Utc}; +use jp_storage::{ + matching_user_workspace_dirs, + value::{read_json, write_json}, +}; +use serde::{Deserialize, Serialize}; +use sha2::{Digest, Sha256}; +use tracing::{trace, warn}; + +use crate::{Id, Workspace, error::Result}; + +/// Directory inside a user-workspace directory holding the per-root registry +/// files. +const ROOTS_DIR: &str = "roots"; + +/// Legacy single-checkout back-pointer, superseded by the roots registry. +const LEGACY_STORAGE_LINK: &str = "storage"; + +/// How recent a recorded `last_used` can be for [`upsert_root`] to skip +/// rewriting the entry, in minutes. +/// +/// Recency only feeds display ordering and `latest` targeting, where sub-minute +/// precision carries no meaning, so a fresh entry is left untouched rather than +/// rewritten on every run. +/// Skipping the rewrite keeps repeated `jp` runs from churning the user-global +/// data directory: an external file watcher restarting a long-running `jp` +/// process on data-directory changes would otherwise re-trigger itself on every +/// restart, forever. +const REFRESH_GRANULARITY_MINUTES: i64 = 5; + +/// A registered checkout of a workspace. +/// +/// The on-disk shape of a single `roots/.json` file. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct RootEntry { + /// Canonical path of the checkout root. + pub path: Utf8PathBuf, + + /// When a JP command last ran against this checkout. + pub last_used: DateTime, +} + +/// Record `root` as a checkout in the user-workspace directory at +/// `user_workspace_dir`. +/// +/// The path is canonicalized, then the checkout's own registry file is upserted +/// with a fresh `last_used` timestamp. +/// An existing entry still within the refresh granularity (see +/// `REFRESH_GRANULARITY_MINUTES`) is left untouched, so repeated runs against +/// the same checkout don't rewrite the file each time. +/// No other checkout's file is read or written, so concurrent runs from +/// different checkouts never contend. +pub fn upsert_root(user_workspace_dir: &Utf8Path, root: &Utf8Path) -> Result<()> { + let path = root.canonicalize_utf8()?; + let file = user_workspace_dir + .join(ROOTS_DIR) + .join(format!("{}.json", root_key(&path))); + let now = Utc::now(); + + // Leave a fresh, matching entry untouched. A `last_used` in the future + // (clock rollback, tampered file) is not fresh: it is rewritten to `now` + // so a bogus timestamp cannot pin recency ordering indefinitely. + if let Ok(existing) = read_json::(&file) { + let age = now - existing.last_used; + if existing.path == path + && age >= Duration::zero() + && age < Duration::minutes(REFRESH_GRANULARITY_MINUTES) + { + trace!(root = %path, file = %file, "Workspace checkout already fresh in roots registry."); + return Ok(()); + } + } + + let entry = RootEntry { + path, + last_used: now, + }; + + write_json(&file, &entry)?; + trace!(root = %entry.path, file = %file, "Recorded workspace checkout in roots registry."); + Ok(()) +} + +/// Expand workspace `id` to its live checkout roots. +/// +/// Scans `workspaces_dir` (the per-user `workspace/` data directory) for the +/// workspace's user-workspace directories, folds any legacy `storage` symlink +/// into the registry, prunes dead entries, and returns the live roots most +/// recently used first. +/// +/// A root is live when it still holds a workspace whose loaded ID equals `id`; +/// a deleted checkout, or one re-initialized as a different workspace, is +/// pruned. +#[must_use] +pub fn resolve_live_roots(workspaces_dir: &Utf8Path, id: &Id, storage_dir: &str) -> Vec { + let mut roots = vec![]; + for dir in matching_user_workspace_dirs(workspaces_dir, id) { + migrate_legacy_symlink(&dir, id, storage_dir); + roots.extend(live_roots(&dir, id, storage_dir)); + } + + // Most recently used first. When the same checkout is recorded in more + // than one user-workspace directory (a legacy per-worktree state), keep + // the freshest entry. + roots.sort_by_key(|entry| Reverse(entry.last_used)); + let mut seen = HashSet::new(); + roots.retain(|entry| seen.insert(entry.path.clone())); + roots +} + +/// Fold a legacy `storage` symlink into the roots registry. +/// +/// Older versions kept one `storage` symlink per user-workspace directory, +/// pointing at the last-used checkout's storage directory. +/// A link whose target still resolves to a workspace with a matching ID seeds a +/// registry entry; a dead or mismatched target seeds nothing. +/// The link is removed either way, so the migration runs at most once per +/// directory. +/// +/// Best-effort: failures are logged, never fatal. +/// A checkout that fails to seed re-registers itself on the next run from +/// inside it. +pub fn migrate_legacy_symlink(user_workspace_dir: &Utf8Path, id: &Id, storage_dir: &str) { + let link = user_workspace_dir.join(LEGACY_STORAGE_LINK); + if !link.is_symlink() { + return; + } + + // Canonicalizing follows the link, so a deleted target fails here and + // falls through to removal without seeding. + let root = link + .canonicalize_utf8() + .ok() + .and_then(|target| Workspace::find_root(target, storage_dir)) + .filter(|root| is_live(root, id, storage_dir)); + + if let Some(root) = root { + if let Err(error) = upsert_root(user_workspace_dir, &root) { + // Keep the link so a later run can retry the seed. + warn!(%error, %root, "Failed to seed roots registry from legacy storage symlink."); + return; + } + trace!(%root, "Seeded roots registry from legacy storage symlink."); + } + + if let Err(error) = remove_symlink(&link) { + warn!(%error, link = %link, "Failed to remove legacy storage symlink."); + } +} + +/// Read a user-workspace directory's roots registry, pruning entries whose +/// checkout is gone. +/// +/// Returns the live entries; a dead or unreadable entry's file is deleted +/// (best-effort) so the registry self-cleans as it is read. +fn live_roots(user_workspace_dir: &Utf8Path, id: &Id, storage_dir: &str) -> Vec { + let mut live = vec![]; + + for file in registry_files(&user_workspace_dir.join(ROOTS_DIR)) { + match read_json::(&file) { + Ok(entry) if is_live(&entry.path, id, storage_dir) => live.push(entry), + Ok(entry) => { + trace!(root = %entry.path, file = %file, "Pruning dead workspace root entry."); + prune(&file); + } + Err(error) => { + warn!(%error, file = %file, "Pruning unreadable workspace root entry."); + prune(&file); + } + } + } + + live +} + +/// Whether `root` still holds a workspace whose loaded ID equals `id`. +/// +/// The check is deliberately direct — `/` must itself be +/// the workspace's storage directory — rather than a walk-up discovery, so a +/// deleted checkout nested inside another checkout of the same workspace does +/// not masquerade as live. +#[must_use] +pub fn is_live(root: &Utf8Path, id: &Id, storage_dir: &str) -> bool { + let storage = root.join(storage_dir); + storage.is_dir() && matches!(Id::load(&storage), Some(Ok(loaded)) if loaded == *id) +} + +/// A workspace known to the per-user `workspace/` data directory. +/// +/// The listing unit behind the `jp w` picker, `jp w ls`, and fuzzy targeting: +/// one entry per workspace ID, expanded to its live checkouts. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct KnownWorkspace { + /// The workspace ID. + pub id: Id, + + /// The cosmetic display name, from the user-workspace directory's + /// `-` name. + /// + /// May be absent (a bare `` name), is never renamed, and is not unique + /// across workspaces — display and search only, never resolution. + pub slug: Option, + + /// The live checkout roots, most recently used first. + /// + /// Empty when every recorded checkout is gone. + pub roots: Vec, +} + +/// List every workspace known to the per-user `workspace/` data directory. +/// +/// Scans `workspaces_dir` for user-workspace directories (`` or +/// `-`), deduplicates by ID (legacy layouts can hold several +/// directories for one workspace), and expands each ID to its live checkouts +/// via [`resolve_live_roots`]. +/// Workspaces are ordered by most recently used checkout, newest first; +/// workspaces with no live checkout sort last. +#[must_use] +pub fn known_workspaces(workspaces_dir: &Utf8Path, storage_dir: &str) -> Vec { + let Ok(entries) = workspaces_dir.read_dir_utf8() else { + return vec![]; + }; + + // Dedupe user-workspace directories by ID, preferring a named one's slug + // over a bare one. + let mut ids: Vec<(Id, Option)> = vec![]; + for entry in entries.filter_map(std::result::Result::ok) { + if !entry.path().is_dir() { + continue; + } + let Some((id, slug)) = user_workspace_id_and_slug(entry.file_name()) else { + continue; + }; + + match ids.iter_mut().find(|(known, _)| *known == id) { + Some((_, known_slug)) => { + if known_slug.is_none() { + *known_slug = slug; + } + } + None => ids.push((id, slug)), + } + } + + let mut workspaces: Vec = ids + .into_iter() + .map(|(id, slug)| { + let roots = resolve_live_roots(workspaces_dir, &id, storage_dir); + KnownWorkspace { id, slug, roots } + }) + .collect(); + + // Most recently used first; rootless workspaces last, then by ID for a + // stable order. + workspaces.sort_by(|a, b| { + let recency = |w: &KnownWorkspace| w.roots.first().map(|entry| entry.last_used); + recency(b) + .cmp(&recency(a)) + .then_with(|| (*a.id).cmp(&*b.id)) + }); + workspaces +} + +/// Parse a user-workspace directory name into its workspace ID and optional +/// slug. +/// +/// Names are `` or `-`, with the ID always the suffix — the same +/// rule [`matching_user_workspace_dirs`] applies when locating the directory. +/// Returns `None` for names that don't end in a well-formed ID. +fn user_workspace_id_and_slug(name: &str) -> Option<(Id, Option)> { + if let Ok(id) = name.parse::() { + return Some((id, None)); + } + + let (slug, id) = name.rsplit_once('-')?; + let id = id.parse::().ok()?; + (!slug.is_empty()).then(|| (id, Some(slug.to_owned()))) +} + +/// The registry files in a user-workspace directory's `roots/` subdirectory. +fn registry_files(dir: &Utf8Path) -> Vec { + let Ok(entries) = dir.read_dir_utf8() else { + return vec![]; + }; + + entries + .filter_map(std::result::Result::ok) + .map(Utf8DirEntry::into_path) + .filter(|path| path.extension() == Some("json") && path.is_file()) + .collect() +} + +/// Filesystem-safe registry key for a checkout root. +/// +/// A truncated SHA-256 of the canonical path: stable across runs, and +/// collision-resistant for distinct paths, so each checkout owns exactly one +/// registry file. +fn root_key(path: &Utf8Path) -> String { + let digest = format!("{:x}", Sha256::digest(path.as_str().as_bytes())); + digest[..16].to_owned() +} + +/// Delete a registry file, logging (not propagating) failure. +fn prune(file: &Utf8Path) { + if let Err(error) = fs::remove_file(file) { + warn!(%error, file = %file, "Failed to prune workspace root entry."); + } +} + +/// Remove a symlink without following it. +/// +/// On Windows a directory symlink is a reparse-point directory and must be +/// removed with `remove_dir`; `remove_file` returns "Access is denied". +/// On Unix `remove_file` unlinks the symlink itself. +fn remove_symlink(link: &Utf8Path) -> io::Result<()> { + #[cfg(windows)] + { + fs::remove_dir(link) + } + #[cfg(not(windows))] + { + fs::remove_file(link) + } +} + +#[cfg(test)] +#[path = "roots_tests.rs"] +mod tests; diff --git a/crates/jp_workspace/src/roots_tests.rs b/crates/jp_workspace/src/roots_tests.rs new file mode 100644 index 000000000..9c6e0b3f1 --- /dev/null +++ b/crates/jp_workspace/src/roots_tests.rs @@ -0,0 +1,429 @@ +use std::{fs, str::FromStr as _}; + +use camino::Utf8Path; +use camino_tempfile::tempdir; +use datetime_literal::datetime; +use test_log::test; + +use super::*; + +const STORAGE_DIR: &str = ".jp"; + +fn wid(s: &str) -> Id { + Id::from_str(s).unwrap() +} + +/// Create a checkout at `path` whose storage directory holds `id`. +fn write_checkout(path: &Utf8Path, id: &Id) { + let storage = path.join(STORAGE_DIR); + fs::create_dir_all(&storage).unwrap(); + id.store(&storage).unwrap(); +} + +/// Write a registry entry with a fixed `last_used`, bypassing the fresh +/// timestamp `upsert_root` records. +fn write_entry(dir: &Utf8Path, root: &Utf8Path, last_used: DateTime) { + let path = root.canonicalize_utf8().unwrap(); + let file = dir + .join(ROOTS_DIR) + .join(format!("{}.json", root_key(&path))); + write_json(&file, &RootEntry { path, last_used }).unwrap(); +} + +#[test] +fn upsert_writes_one_stable_file_per_checkout() { + let tmp = tempdir().unwrap(); + let dir = tmp.path().join("dir"); + let id = wid("ws123"); + let root = tmp.path().join("checkout"); + write_checkout(&root, &id); + + upsert_root(&dir, &root).unwrap(); + upsert_root(&dir, &root).unwrap(); + + let files = registry_files(&dir.join(ROOTS_DIR)); + assert_eq!(files.len(), 1, "repeated upserts reuse the checkout's file"); + + let entry: RootEntry = read_json(&files[0]).unwrap(); + assert_eq!(entry.path, root.canonicalize_utf8().unwrap()); +} + +#[test] +fn upsert_leaves_a_fresh_entry_untouched() { + let tmp = tempdir().unwrap(); + let dir = tmp.path().join("dir"); + let id = wid("ws123"); + let root = tmp.path().join("checkout"); + write_checkout(&root, &id); + + let recent = Utc::now() - Duration::minutes(1); + write_entry(&dir, &root, recent); + + upsert_root(&dir, &root).unwrap(); + + let files = registry_files(&dir.join(ROOTS_DIR)); + let entry: RootEntry = read_json(&files[0]).unwrap(); + assert_eq!( + entry.last_used, recent, + "an entry fresher than the refresh granularity is not rewritten" + ); +} + +#[test] +fn upsert_refreshes_a_stale_entry() { + let tmp = tempdir().unwrap(); + let dir = tmp.path().join("dir"); + let id = wid("ws123"); + let root = tmp.path().join("checkout"); + write_checkout(&root, &id); + + let stale = Utc::now() - Duration::hours(2); + write_entry(&dir, &root, stale); + + upsert_root(&dir, &root).unwrap(); + + let files = registry_files(&dir.join(ROOTS_DIR)); + let entry: RootEntry = read_json(&files[0]).unwrap(); + assert!( + entry.last_used > stale, + "a stale entry gets a fresh timestamp" + ); +} + +#[test] +fn upsert_heals_a_future_timestamp() { + let tmp = tempdir().unwrap(); + let dir = tmp.path().join("dir"); + let id = wid("ws123"); + let root = tmp.path().join("checkout"); + write_checkout(&root, &id); + + // A timestamp ahead of the clock would otherwise pin recency ordering + // until the clock catches up; the upsert rewrites it to now. + let future = Utc::now() + Duration::days(1); + write_entry(&dir, &root, future); + + upsert_root(&dir, &root).unwrap(); + + let files = registry_files(&dir.join(ROOTS_DIR)); + let entry: RootEntry = read_json(&files[0]).unwrap(); + assert!( + entry.last_used < future, + "a future-stamped entry is rewritten to the present" + ); +} + +#[cfg(unix)] +#[test] +fn upsert_records_canonical_path_for_aliased_checkout() { + let tmp = tempdir().unwrap(); + let dir = tmp.path().join("dir"); + let id = wid("ws123"); + let root = tmp.path().join("checkout"); + write_checkout(&root, &id); + + let alias = tmp.path().join("alias"); + std::os::unix::fs::symlink(&root, &alias).unwrap(); + + upsert_root(&dir, &root).unwrap(); + upsert_root(&dir, &alias).unwrap(); + + assert_eq!( + registry_files(&dir.join(ROOTS_DIR)).len(), + 1, + "an aliased path resolves to the same registry file" + ); +} + +#[test] +fn resolve_returns_live_roots_and_prunes_dead_ones() { + let tmp = tempdir().unwrap(); + let workspaces_dir = tmp.path().join("workspace"); + let id = wid("ws123"); + let dir = workspaces_dir.join("proj-ws123"); + + // One live checkout, one deleted, and one re-initialized under a + // different workspace ID. + let live = tmp.path().join("live"); + write_checkout(&live, &id); + upsert_root(&dir, &live).unwrap(); + + let deleted = tmp.path().join("deleted"); + write_checkout(&deleted, &id); + upsert_root(&dir, &deleted).unwrap(); + fs::remove_dir_all(&deleted).unwrap(); + + let mismatched = tmp.path().join("mismatched"); + write_checkout(&mismatched, &wid("zz999")); + upsert_root(&dir, &mismatched).unwrap(); + + let roots = resolve_live_roots(&workspaces_dir, &id, STORAGE_DIR); + + assert_eq!(roots.len(), 1); + assert_eq!(roots[0].path, live.canonicalize_utf8().unwrap()); + assert_eq!( + registry_files(&dir.join(ROOTS_DIR)).len(), + 1, + "dead and mismatched entries are pruned from disk" + ); +} + +#[test] +fn resolve_orders_roots_most_recently_used_first() { + let tmp = tempdir().unwrap(); + let workspaces_dir = tmp.path().join("workspace"); + let id = wid("ws123"); + let dir = workspaces_dir.join("ws123"); + + let older = tmp.path().join("older"); + write_checkout(&older, &id); + let newer = tmp.path().join("newer"); + write_checkout(&newer, &id); + + write_entry(&dir, &older, datetime!(2026-01-01 10:00:00 Z)); + write_entry(&dir, &newer, datetime!(2026-06-01 10:00:00 Z)); + + let roots = resolve_live_roots(&workspaces_dir, &id, STORAGE_DIR); + let paths: Vec<_> = roots.into_iter().map(|entry| entry.path).collect(); + assert_eq!(paths, vec![ + newer.canonicalize_utf8().unwrap(), + older.canonicalize_utf8().unwrap(), + ]); +} + +#[test] +fn resolve_merges_roots_across_legacy_sibling_dirs() { + let tmp = tempdir().unwrap(); + let workspaces_dir = tmp.path().join("workspace"); + let id = wid("ws123"); + + let a = tmp.path().join("checkout-a"); + write_checkout(&a, &id); + upsert_root(&workspaces_dir.join("main-ws123"), &a).unwrap(); + + let b = tmp.path().join("checkout-b"); + write_checkout(&b, &id); + upsert_root(&workspaces_dir.join("feature-ws123"), &b).unwrap(); + + assert_eq!( + resolve_live_roots(&workspaces_dir, &id, STORAGE_DIR).len(), + 2 + ); +} + +#[test] +fn resolve_prunes_unreadable_registry_files() { + let tmp = tempdir().unwrap(); + let workspaces_dir = tmp.path().join("workspace"); + let id = wid("ws123"); + let roots_dir = workspaces_dir.join("ws123").join(ROOTS_DIR); + + fs::create_dir_all(&roots_dir).unwrap(); + fs::write(roots_dir.join("bogus.json"), "not json").unwrap(); + + assert!(resolve_live_roots(&workspaces_dir, &id, STORAGE_DIR).is_empty()); + assert!( + !roots_dir.join("bogus.json").exists(), + "corrupt entry removed" + ); +} + +#[test] +fn deleted_root_nested_in_same_id_workspace_is_not_live() { + // A registry entry for `/worktree` whose directory is gone must + // not count as live just because `` is a checkout of the same + // workspace: liveness is a direct check, not a walk-up discovery. + let tmp = tempdir().unwrap(); + let workspaces_dir = tmp.path().join("workspace"); + let id = wid("ws123"); + let dir = workspaces_dir.join("ws123"); + + let parent = tmp.path().join("parent"); + write_checkout(&parent, &id); + let nested = parent.join("worktree"); + write_checkout(&nested, &id); + upsert_root(&dir, &nested).unwrap(); + fs::remove_dir_all(&nested).unwrap(); + + assert!(resolve_live_roots(&workspaces_dir, &id, STORAGE_DIR).is_empty()); +} + +#[cfg(unix)] +mod legacy_symlink { + use test_log::test; + + use super::*; + + #[test] + fn seeds_registry_from_live_target_and_removes_link() { + let tmp = tempdir().unwrap(); + let workspaces_dir = tmp.path().join("workspace"); + let id = wid("ws123"); + let dir = workspaces_dir.join("proj-ws123"); + fs::create_dir_all(&dir).unwrap(); + + let checkout = tmp.path().join("checkout"); + write_checkout(&checkout, &id); + + // The legacy link points at the checkout's storage directory, not at + // the checkout root itself. + std::os::unix::fs::symlink(checkout.join(STORAGE_DIR), dir.join("storage")).unwrap(); + + let roots = resolve_live_roots(&workspaces_dir, &id, STORAGE_DIR); + + assert_eq!(roots.len(), 1); + assert_eq!(roots[0].path, checkout.canonicalize_utf8().unwrap()); + assert!( + !dir.join("storage").is_symlink(), + "legacy link removed after seeding" + ); + } + + #[test] + fn drops_link_with_dead_target_without_seeding() { + let tmp = tempdir().unwrap(); + let workspaces_dir = tmp.path().join("workspace"); + let id = wid("ws123"); + let dir = workspaces_dir.join("proj-ws123"); + fs::create_dir_all(&dir).unwrap(); + + std::os::unix::fs::symlink(tmp.path().join("gone"), dir.join("storage")).unwrap(); + + assert!(resolve_live_roots(&workspaces_dir, &id, STORAGE_DIR).is_empty()); + assert!(!dir.join("storage").is_symlink(), "dead link removed"); + assert!(registry_files(&dir.join(ROOTS_DIR)).is_empty()); + } + + #[test] + fn drops_link_with_mismatched_workspace_id_without_seeding() { + let tmp = tempdir().unwrap(); + let workspaces_dir = tmp.path().join("workspace"); + let id = wid("ws123"); + let dir = workspaces_dir.join("proj-ws123"); + fs::create_dir_all(&dir).unwrap(); + + // The link target is a live workspace, but not *this* workspace. + let other = tmp.path().join("other"); + write_checkout(&other, &wid("zz999")); + std::os::unix::fs::symlink(other.join(STORAGE_DIR), dir.join("storage")).unwrap(); + + assert!(resolve_live_roots(&workspaces_dir, &id, STORAGE_DIR).is_empty()); + assert!(!dir.join("storage").is_symlink(), "mismatched link removed"); + assert!(registry_files(&dir.join(ROOTS_DIR)).is_empty()); + } +} + +#[test] +fn user_workspace_dir_names_parse_to_id_and_optional_slug() { + assert_eq!( + user_workspace_id_and_slug("ws123"), + Some((wid("ws123"), None)) + ); + assert_eq!( + user_workspace_id_and_slug("proj-ws123"), + Some((wid("ws123"), Some("proj".to_owned()))) + ); + + // A multi-segment slug keeps everything before the final `-`. + assert_eq!( + user_workspace_id_and_slug("my-app-ws123"), + Some((wid("ws123"), Some("my-app".to_owned()))) + ); + + // Not valid names: bad ID length, bad characters, empty slug. + assert_eq!(user_workspace_id_and_slug("notanid"), None); + assert_eq!(user_workspace_id_and_slug("proj-WS123"), None); + assert_eq!(user_workspace_id_and_slug("-ws123"), None); + assert_eq!(user_workspace_id_and_slug(""), None); +} + +#[test] +fn known_workspaces_dedupes_dirs_and_prefers_named_slug() { + let tmp = tempdir().unwrap(); + let workspaces_dir = tmp.path().join("workspace"); + let id = wid("ws123"); + + // A legacy layout: one bare and one named directory for the same ID, + // each holding a live checkout. + let first = tmp.path().join("first"); + write_checkout(&first, &id); + upsert_root(&workspaces_dir.join("ws123"), &first).unwrap(); + + let second = tmp.path().join("second"); + write_checkout(&second, &id); + upsert_root(&workspaces_dir.join("proj-ws123"), &second).unwrap(); + + let known = known_workspaces(&workspaces_dir, STORAGE_DIR); + + assert_eq!( + known.len(), + 1, + "directories for one ID collapse to one workspace" + ); + assert_eq!(known[0].id, id); + assert_eq!(known[0].slug.as_deref(), Some("proj")); + assert_eq!( + known[0].roots.len(), + 2, + "roots union across both directories" + ); +} + +#[test] +fn known_workspaces_orders_by_recency_rootless_last() { + let tmp = tempdir().unwrap(); + let workspaces_dir = tmp.path().join("workspace"); + + let recent = tmp.path().join("recent"); + write_checkout(&recent, &wid("aaaaa")); + write_entry( + &workspaces_dir.join("aaaaa"), + &recent, + datetime!(2026-06-01 10:00:00 Z), + ); + + let stale = tmp.path().join("stale"); + write_checkout(&stale, &wid("bbbbb")); + write_entry( + &workspaces_dir.join("bbbbb"), + &stale, + datetime!(2026-01-01 10:00:00 Z), + ); + + // Two rootless directories, to exercise the stable ID tie-break. + fs::create_dir_all(workspaces_dir.join("ddddd")).unwrap(); + fs::create_dir_all(workspaces_dir.join("ccccc")).unwrap(); + + let known = known_workspaces(&workspaces_dir, STORAGE_DIR); + let ids: Vec = known.iter().map(|w| w.id.to_string()).collect(); + + assert_eq!(ids, ["aaaaa", "bbbbb", "ccccc", "ddddd"]); + assert!(known[2].roots.is_empty()); + assert!(known[3].roots.is_empty()); +} + +#[test] +fn known_workspaces_skips_non_workspace_entries() { + let tmp = tempdir().unwrap(); + let workspaces_dir = tmp.path().join("workspace"); + + let checkout = tmp.path().join("checkout"); + write_checkout(&checkout, &wid("ws123")); + upsert_root(&workspaces_dir.join("proj-ws123"), &checkout).unwrap(); + + // Not user-workspace directories: a directory whose name is no ID, and a + // plain file whose name would otherwise qualify. + fs::create_dir_all(workspaces_dir.join("not-a-workspace")).unwrap(); + fs::write(workspaces_dir.join("zzzzz"), b"").unwrap(); + + let known = known_workspaces(&workspaces_dir, STORAGE_DIR); + + assert_eq!(known.len(), 1); + assert_eq!(known[0].id, wid("ws123")); +} + +#[test] +fn known_workspaces_missing_dir_is_empty() { + let tmp = tempdir().unwrap(); + assert!(known_workspaces(&tmp.path().join("absent"), STORAGE_DIR).is_empty()); +} diff --git a/crates/jp_workspace/src/session_mapping.rs b/crates/jp_workspace/src/session_mapping.rs index 5f14e0573..b11507989 100644 --- a/crates/jp_workspace/src/session_mapping.rs +++ b/crates/jp_workspace/src/session_mapping.rs @@ -481,7 +481,7 @@ fn mapping_from_value(mut value: Value, fallback_id: &str) -> Option Liveness { +pub(crate) fn is_session_process_liveness(id: &SessionId, source: &SessionSource) -> Liveness { match source { SessionSource::Getsid => id.as_pid().map_or(Liveness::Unknown, pid_liveness), SessionSource::Hwnd => id.as_hwnd().map_or(Liveness::Unknown, hwnd_liveness), diff --git a/crates/jp_workspace/src/session_store.rs b/crates/jp_workspace/src/session_store.rs new file mode 100644 index 000000000..c06313d67 --- /dev/null +++ b/crates/jp_workspace/src/session_store.rs @@ -0,0 +1,322 @@ +//! User-global session → active-workspace store (RFD 087). +//! +//! Maps a terminal session to the workspace it drives, *above* any +//! user-workspace directory: +//! +//! ```text +//! /sessions/.json +//! ``` +//! +//! The store mirrors the per-workspace session-to-conversation mapping (RFD +//! 020, [`session_mapping`]): a most-recent-first `history` of selections +//! (active = `history[0]`, previous = `history[1]`), the session identity for +//! stale detection, and a session-level `sticky` flag (the precedence ladder's +//! persisted `A` choice). +//! Filenames reuse [`Session::storage_key`], so an automatic `getsid` / `Hwnd` +//! session can never alias an `Env` session sharing the same numeric value. +//! +//! Each entry records the workspace ID *and* the resolved checkout root: +//! distinct checkouts of one workspace are distinct history entries, and the ID +//! is what makes recovery possible after a recorded root is deleted. +//! +//! See: `docs/rfd/087-session-scoped-active-workspace.md` +//! +//! [`session_mapping`]: crate::session_mapping + +use std::{fs, str::FromStr as _}; + +use camino::{Utf8DirEntry, Utf8Path, Utf8PathBuf}; +use chrono::{DateTime, Utc}; +use jp_storage::value::{read_json, write_json}; +use serde::{Deserialize, Serialize}; +use tracing::{debug, trace, warn}; + +use crate::{ + Id, + error::Result, + session::{Session, SessionId, SessionSource}, + session_mapping::{Liveness, is_session_process_liveness}, +}; + +/// Directory under the user data directory holding the per-session records. +pub const SESSIONS_DIR: &str = "sessions"; + +/// A single selected workspace in a session's history. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct WorkspaceSelection { + /// The selected workspace ID. + /// + /// Stored alongside the root so recovery can expand the ID through the + /// roots registry once the recorded root is gone — at that point the ID + /// can no longer be read from `/.jp/.id`. + pub workspace_id: String, + + /// The concrete checkout root the selection resolved to. + pub root: Utf8PathBuf, + + /// When this selection was recorded. + pub selected_at: DateTime, +} + +impl WorkspaceSelection { + /// The entry's workspace ID, when it parses as one. + /// + /// A tampered or corrupted record can hold anything; callers treat an + /// unparseable ID as a dead entry. + #[must_use] + pub fn id(&self) -> Option { + Id::from_str(&self.workspace_id).ok() + } +} + +/// The on-disk shape of one session's record. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct WorkspaceSessionMapping { + /// Most-recent-first history of selected workspaces. + /// + /// The active workspace is `history[0]`; the previously active one — the + /// `session` / `s` target — is `history[1]`. + pub history: Vec, + + /// Whether the session keeps using its active workspace even when the cwd + /// resolves to a different one. + /// + /// The persisted `A` choice from the precedence ladder; interactive-only + /// state, cleared by `jp w use cwd`. + #[serde(default)] + pub sticky: bool, + + /// The session identity value, for stale detection. + pub id: SessionId, + + /// How the session identity was resolved; drives the cleanup rule split. + pub source: SessionSource, +} + +/// The user-global store of session → active-workspace records. +/// +/// Owned by the `jp_cli` bootstrap step, not by [`Workspace`]: records exist +/// before any workspace is selected and reference workspace IDs the current run +/// never touches. +/// +/// [`Workspace`]: crate::Workspace +#[derive(Debug, Clone)] +pub struct WorkspaceSessionStore { + /// The `sessions/` directory holding one JSON file per session. + dir: Utf8PathBuf, +} + +impl WorkspaceSessionStore { + /// A store rooted at the given `sessions/` directory. + pub fn new(dir: impl Into) -> Self { + Self { dir: dir.into() } + } + + /// A store at its standard location under the user data directory. + #[must_use] + pub fn at_user_data_dir(data_dir: &Utf8Path) -> Self { + Self::new(data_dir.join(SESSIONS_DIR)) + } + + /// The record file for `session`. + fn path(&self, session: &Session) -> Utf8PathBuf { + self.dir.join(format!("{}.json", session.storage_key())) + } + + /// Load the record for `session`, if any. + /// + /// Only a record matching the full session identity is honored: the env + /// value in the filename is hashed, so a (however unlikely) collision or a + /// tampered store must not hand a foreign selection to this session. + #[must_use] + pub fn load(&self, session: &Session) -> Option { + let mapping: WorkspaceSessionMapping = read_json(&self.path(session)).ok()?; + (mapping.id == session.id && mapping.source == session.source).then_some(mapping) + } + + /// The session's active workspace selection (`history[0]`). + #[must_use] + pub fn active(&self, session: &Session) -> Option { + self.load(session)?.history.into_iter().next() + } + + /// The session's previously active selection (`history[1]`), the `session` + /// / `s` target. + #[must_use] + pub fn previous(&self, session: &Session) -> Option { + self.load(session)?.history.into_iter().nth(1) + } + + /// Record a workspace selection as the session's active workspace. + /// + /// The (workspace, checkout) pair moves to the front of the history; + /// distinct checkouts of the same workspace ID stay distinct entries (`s` + /// restores the exact previously active checkout, like `cd -`). + /// The session-level `sticky` flag is preserved. + pub fn record_selection( + &self, + session: &Session, + workspace_id: &Id, + root: &Utf8Path, + now: DateTime, + ) -> Result<()> { + let mut mapping = self + .load(session) + .unwrap_or_else(|| WorkspaceSessionMapping { + history: vec![], + sticky: false, + id: session.id.clone(), + source: session.source.clone(), + }); + + mapping + .history + .retain(|entry| !(entry.workspace_id == **workspace_id && entry.root == root)); + mapping.history.insert(0, WorkspaceSelection { + workspace_id: workspace_id.to_string(), + root: root.to_owned(), + selected_at: now, + }); + + write_json(&self.path(session), &mapping)?; + trace!( + workspace = %workspace_id, + root = %root, + "Recorded session-active workspace selection." + ); + Ok(()) + } + + /// Persist the session's `sticky` flag (the precedence ladder's `A` + /// choice). + /// + /// A sticky session keeps using its active workspace even when the cwd + /// resolves to a different one, until `jp w use cwd` clears the record. + /// Without a record there is no selection to pin, so the call is a no-op. + pub fn set_sticky(&self, session: &Session, sticky: bool) -> Result<()> { + let Some(mut mapping) = self.load(session) else { + debug!("No session-active workspace recorded; nothing to pin."); + return Ok(()); + }; + + if mapping.sticky == sticky { + return Ok(()); + } + + mapping.sticky = sticky; + write_json(&self.path(session), &mapping)?; + trace!(sticky, "Updated the session's sticky flag."); + Ok(()) + } + + /// Drop the session's record (`jp w use cwd`). + /// + /// Clearing returns the session to cwd resolution; the record — history + /// and sticky flag included — is removed. + pub fn clear(&self, session: &Session) -> Result<()> { + match fs::remove_file(self.path(session)) { + Ok(()) => Ok(()), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(error) => Err(error.into()), + } + } + + /// Source-split cleanup pass over every record (RFD 087). + /// + /// - **Getsid / Hwnd** (process liveness is checkable): the record is + /// removed only when the originating process is confirmed dead; a live + /// process keeps its record unconditionally. + /// - **Env** (liveness unknown): existence-based across the whole history. + /// An entry is pruned only when its workspace ID has no live root — not + /// merely when its recorded root died, which is what lets missing-root + /// recovery re-prompt among the ID's surviving checkouts. + /// The record is removed only when no entry references a workspace ID + /// with any live root. + /// + /// `workspace_has_live_root` answers "does this workspace ID have at least + /// one live checkout?"; the caller supplies it so the store stays agnostic + /// of where roots registries live. + pub fn cleanup(&self, workspace_has_live_root: &dyn Fn(&Id) -> bool) { + for file in record_files(&self.dir) { + cleanup_record(&file, workspace_has_live_root); + } + } +} + +/// Evaluate a single session record file against the cleanup rules. +fn cleanup_record(path: &Utf8Path, workspace_has_live_root: &dyn Fn(&Id) -> bool) { + let mapping = match read_json::(path) { + Ok(mapping) => mapping, + Err(error) => { + // Unlike the per-workspace store, this store has no legacy + // formats: a record it cannot read can never become readable. + warn!(%error, file = %path, "Pruning unreadable workspace session record."); + prune(path); + return; + } + }; + + match is_session_process_liveness(&mapping.id, &mapping.source) { + // A live process keeps its record unconditionally. + Liveness::Alive => {} + Liveness::Dead => { + debug!( + file = %path, + source = %mapping.source, + "Removing stale workspace session record (process dead)." + ); + prune(path); + } + Liveness::Unknown => { + let live: Vec<_> = mapping + .history + .iter() + .filter(|entry| entry.id().is_some_and(|id| workspace_has_live_root(&id))) + .cloned() + .collect(); + + if live.is_empty() { + debug!( + file = %path, + "Removing stale workspace session record (no live workspaces)." + ); + prune(path); + return; + } + + if live.len() < mapping.history.len() { + let updated = WorkspaceSessionMapping { + history: live, + ..mapping + }; + if let Err(error) = write_json(path, &updated) { + warn!(%error, file = %path, "Failed to rewrite workspace session record."); + } + } + } + } +} + +/// The record files in the store's directory. +fn record_files(dir: &Utf8Path) -> Vec { + let Ok(entries) = dir.read_dir_utf8() else { + return vec![]; + }; + + entries + .filter_map(std::result::Result::ok) + .map(Utf8DirEntry::into_path) + .filter(|path| path.extension() == Some("json") && path.is_file()) + .collect() +} + +/// Delete a record file, logging (not propagating) failure. +fn prune(file: &Utf8Path) { + if let Err(error) = fs::remove_file(file) { + warn!(%error, file = %file, "Failed to prune workspace session record."); + } +} + +#[cfg(test)] +#[path = "session_store_tests.rs"] +mod tests; diff --git a/crates/jp_workspace/src/session_store_tests.rs b/crates/jp_workspace/src/session_store_tests.rs new file mode 100644 index 000000000..d995a9626 --- /dev/null +++ b/crates/jp_workspace/src/session_store_tests.rs @@ -0,0 +1,358 @@ +use std::collections::HashSet; + +use camino_tempfile::tempdir; +use chrono::TimeZone as _; + +use super::*; +use crate::session::{Session, SessionId, SessionSource}; + +fn env_session(value: &str) -> Session { + Session { + id: SessionId::new(value).expect("non-empty"), + source: SessionSource::env("JP_SESSION"), + } +} + +fn id(s: &str) -> Id { + Id::from_str(s).expect("valid workspace ID") +} + +fn at(secs: i64) -> DateTime { + Utc.timestamp_opt(secs, 0) + .single() + .expect("valid timestamp") +} + +struct TestStore { + store: WorkspaceSessionStore, + // Held for its Drop. + _tmp: camino_tempfile::Utf8TempDir, +} + +fn store() -> TestStore { + let tmp = tempdir().expect("tempdir"); + let dir = tmp.path().join(SESSIONS_DIR); + TestStore { + store: WorkspaceSessionStore::new(dir), + _tmp: tmp, + } +} + +#[test] +fn record_and_load_round_trip() { + let t = store(); + let session = env_session("tab-1"); + + t.store + .record_selection( + &session, + &id("abc12"), + Utf8Path::new("/tmp/checkout"), + at(1_000), + ) + .unwrap(); + + let mapping = t.store.load(&session).expect("mapping stored"); + assert_eq!(mapping.history.len(), 1); + assert_eq!(mapping.history[0].workspace_id, "abc12"); + assert_eq!(mapping.history[0].root, Utf8Path::new("/tmp/checkout")); + assert_eq!(mapping.history[0].selected_at, at(1_000)); + assert!(!mapping.sticky); + assert_eq!(mapping.id, session.id); + assert_eq!(mapping.source, session.source); + + let active = t.store.active(&session).expect("active selection"); + assert_eq!(active.workspace_id, "abc12"); +} + +#[test] +fn set_sticky_persists_and_reselection_preserves_it() { + let t = store(); + let session = env_session("tab-1"); + + t.store + .record_selection(&session, &id("abc12"), Utf8Path::new("/a"), at(1_000)) + .unwrap(); + t.store.set_sticky(&session, true).unwrap(); + + assert!(t.store.load(&session).unwrap().sticky); + + // Selecting another workspace keeps the session-level pin. + t.store + .record_selection(&session, &id("def34"), Utf8Path::new("/b"), at(2_000)) + .unwrap(); + assert!(t.store.load(&session).unwrap().sticky); + + t.store.set_sticky(&session, false).unwrap(); + assert!(!t.store.load(&session).unwrap().sticky); +} + +#[test] +fn set_sticky_without_a_record_is_a_no_op() { + let t = store(); + let session = env_session("tab-1"); + + // Nothing to pin: no record is created. + t.store.set_sticky(&session, true).unwrap(); + + assert!(t.store.load(&session).is_none()); +} + +#[test] +fn history_is_most_recent_first_and_previous_is_second() { + let t = store(); + let session = env_session("tab-1"); + + t.store + .record_selection(&session, &id("abc12"), Utf8Path::new("/a"), at(1_000)) + .unwrap(); + t.store + .record_selection(&session, &id("def34"), Utf8Path::new("/b"), at(2_000)) + .unwrap(); + + assert_eq!(t.store.active(&session).unwrap().workspace_id, "def34"); + assert_eq!(t.store.previous(&session).unwrap().workspace_id, "abc12"); +} + +#[test] +fn reselecting_a_pair_moves_it_to_the_front_without_duplicating() { + let t = store(); + let session = env_session("tab-1"); + + t.store + .record_selection(&session, &id("abc12"), Utf8Path::new("/a"), at(1_000)) + .unwrap(); + t.store + .record_selection(&session, &id("def34"), Utf8Path::new("/b"), at(2_000)) + .unwrap(); + t.store + .record_selection(&session, &id("abc12"), Utf8Path::new("/a"), at(3_000)) + .unwrap(); + + let mapping = t.store.load(&session).unwrap(); + assert_eq!(mapping.history.len(), 2); + assert_eq!(mapping.history[0].workspace_id, "abc12"); + assert_eq!(mapping.history[0].selected_at, at(3_000)); + assert_eq!(mapping.history[1].workspace_id, "def34"); +} + +#[test] +fn distinct_checkouts_of_one_workspace_are_distinct_entries() { + let t = store(); + let session = env_session("tab-1"); + + t.store + .record_selection(&session, &id("abc12"), Utf8Path::new("/feature"), at(1_000)) + .unwrap(); + t.store + .record_selection(&session, &id("abc12"), Utf8Path::new("/main"), at(2_000)) + .unwrap(); + + let mapping = t.store.load(&session).unwrap(); + assert_eq!(mapping.history.len(), 2); + assert_eq!(mapping.history[0].root, Utf8Path::new("/main")); + assert_eq!(mapping.history[1].root, Utf8Path::new("/feature")); +} + +#[test] +fn sticky_flag_survives_reselection() { + let t = store(); + let session = env_session("tab-1"); + + t.store + .record_selection(&session, &id("abc12"), Utf8Path::new("/a"), at(1_000)) + .unwrap(); + + // Flip sticky by hand (the ladder's `A` choice lands in phase 4). + let mut mapping = t.store.load(&session).unwrap(); + mapping.sticky = true; + write_json(&t.store.path(&session), &mapping).unwrap(); + + t.store + .record_selection(&session, &id("def34"), Utf8Path::new("/b"), at(2_000)) + .unwrap(); + + assert!(t.store.load(&session).unwrap().sticky); +} + +#[test] +fn clear_removes_the_record_and_is_idempotent() { + let t = store(); + let session = env_session("tab-1"); + + t.store + .record_selection(&session, &id("abc12"), Utf8Path::new("/a"), at(1_000)) + .unwrap(); + t.store.clear(&session).unwrap(); + + assert!(t.store.load(&session).is_none()); + + // Clearing an absent record is not an error. + t.store.clear(&session).unwrap(); +} + +#[test] +fn foreign_record_at_the_session_key_is_not_honored() { + let t = store(); + let session = env_session("tab-1"); + + // A record whose stored identity differs from the reading session (hash + // collision or tampering) is ignored. + let foreign = WorkspaceSessionMapping { + history: vec![WorkspaceSelection { + workspace_id: "abc12".into(), + root: "/a".into(), + selected_at: at(1_000), + }], + sticky: false, + id: SessionId::new("other-tab").unwrap(), + source: SessionSource::env("JP_SESSION"), + }; + write_json(&t.store.path(&session), &foreign).unwrap(); + + assert!(t.store.load(&session).is_none()); +} + +#[test] +fn sessions_with_same_value_but_different_sources_do_not_collide() { + let t = store(); + let jp = env_session("42"); + let tmux = Session { + id: SessionId::new("42").unwrap(), + source: SessionSource::env("TMUX_PANE"), + }; + + t.store + .record_selection(&jp, &id("abc12"), Utf8Path::new("/a"), at(1_000)) + .unwrap(); + t.store + .record_selection(&tmux, &id("def34"), Utf8Path::new("/b"), at(2_000)) + .unwrap(); + + assert_eq!(t.store.active(&jp).unwrap().workspace_id, "abc12"); + assert_eq!(t.store.active(&tmux).unwrap().workspace_id, "def34"); +} + +#[test] +fn cleanup_prunes_env_entries_whose_workspace_has_no_live_root() { + let t = store(); + let session = env_session("tab-1"); + + t.store + .record_selection(&session, &id("dead1"), Utf8Path::new("/dead"), at(1_000)) + .unwrap(); + t.store + .record_selection(&session, &id("live1"), Utf8Path::new("/live"), at(2_000)) + .unwrap(); + + let live: HashSet = [id("live1")].into(); + t.store.cleanup(&|id| live.contains(id)); + + let mapping = t.store.load(&session).expect("record survives"); + assert_eq!(mapping.history.len(), 1); + assert_eq!(mapping.history[0].workspace_id, "live1"); +} + +#[test] +fn cleanup_keeps_entries_of_a_live_workspace_even_when_its_recorded_root_died() { + let t = store(); + let session = env_session("tab-1"); + + // The workspace ID still has *some* live checkout; whether this exact + // recorded root is alive is not this pass's concern (missing-root + // recovery re-prompts among the surviving checkouts). + t.store + .record_selection( + &session, + &id("live1"), + Utf8Path::new("/removed-worktree"), + at(1_000), + ) + .unwrap(); + + t.store.cleanup(&|_| true); + + assert_eq!(t.store.load(&session).unwrap().history.len(), 1); +} + +#[test] +fn cleanup_removes_env_record_with_no_live_workspace_at_all() { + let t = store(); + let session = env_session("tab-1"); + + t.store + .record_selection(&session, &id("dead1"), Utf8Path::new("/a"), at(1_000)) + .unwrap(); + t.store + .record_selection(&session, &id("dead2"), Utf8Path::new("/b"), at(2_000)) + .unwrap(); + + t.store.cleanup(&|_| false); + + assert!(t.store.load(&session).is_none()); +} + +#[test] +fn cleanup_keeps_record_of_a_live_getsid_process_unconditionally() { + let t = store(); + // Our own process is alive by definition. + #[expect(clippy::cast_possible_wrap, reason = "test PIDs fit in i32")] + let session = Session::getsid(std::process::id() as i32); + + t.store + .record_selection(&session, &id("dead1"), Utf8Path::new("/gone"), at(1_000)) + .unwrap(); + + // Even with no live workspace anywhere, a live process keeps its record. + t.store.cleanup(&|_| false); + + assert_eq!(t.store.load(&session).unwrap().history.len(), 1); +} + +#[test] +fn cleanup_prunes_unreadable_records() { + let t = store(); + let session = env_session("tab-1"); + + // Ensure the directory exists, then plant garbage. + t.store + .record_selection(&session, &id("live1"), Utf8Path::new("/a"), at(1_000)) + .unwrap(); + let garbage = t.store.dir.join("garbage.json"); + fs::write(&garbage, b"not json").unwrap(); + + t.store.cleanup(&|_| true); + + assert!(!garbage.exists()); + assert!(t.store.load(&session).is_some(), "healthy record untouched"); +} + +#[test] +fn cleanup_on_missing_directory_is_a_no_op() { + let t = store(); + // No record was ever written; the sessions/ directory does not exist. + t.store.cleanup(&|_| true); +} + +#[test] +fn on_disk_shape_matches_rfd_087() { + let t = store(); + let session = env_session("tab-1"); + + t.store + .record_selection( + &session, + &id("abc12"), + Utf8Path::new("/checkout"), + at(1_000), + ) + .unwrap(); + + let raw: serde_json::Value = read_json(&t.store.path(&session)).unwrap(); + let entry = &raw["history"][0]; + assert_eq!(entry["workspace_id"], "abc12"); + assert_eq!(entry["root"], "/checkout"); + assert!(entry["selected_at"].is_string()); + assert_eq!(raw["sticky"], false); + assert_eq!(raw["source"]["type"], "env"); +} diff --git a/docs/.vitepress/rfd-summaries.json b/docs/.vitepress/rfd-summaries.json index f9118a285..a19cbfb57 100644 --- a/docs/.vitepress/rfd-summaries.json +++ b/docs/.vitepress/rfd-summaries.json @@ -120,7 +120,7 @@ "summary": "JP's concise DSL syntax for defining JSON Schema objects via command-line flags with types, descriptions, and nested structures." }, "031-durable-conversation-storage-with-workspace-projection.md": { - "hash": "8df148ee3d3403dbb87b7496568bb193ad1631e53541845d2d52592e2c9bd4f2", + "hash": "d3f2f88acf21e1c1c906a30dee5c6197211edda33f4ade97b6e6620afc24486d", "summary": "Make user-local storage the durable source; workspace copies become optional projections for git visibility." }, "032-grizzly-semantic-search.md": { @@ -344,7 +344,7 @@ "summary": "Convention for multi-value CLI arguments: `-` reads line-oriented values from stdin, starting with conversation targeting." }, "087-session-scoped-active-workspace.md": { - "hash": "63795fd09a15895ff477f2a6210cdda7698f6cb7421534934b244749bf74e828", + "hash": "291f15477d02a4ff94f627877388d8b2706d8b1a87908ea6c483463c388c497d", "summary": "Session-scoped active workspace lets JP commands run from anywhere after selecting a workspace with `jp w use`." }, "088-unified-editor-service-and-inline-reply-widget.md": { diff --git a/docs/architecture/ubiquitous-language.md b/docs/architecture/ubiquitous-language.md index a22e89267..47e0479ce 100644 --- a/docs/architecture/ubiquitous-language.md +++ b/docs/architecture/ubiquitous-language.md @@ -35,6 +35,7 @@ In disagreements between code and docs, the code is authoritative. - [Thread](#thread) - [Tool Call](#tool-call) - [Turn](#turn) + - [User-Workspace Directory](#user-workspace-directory) - [Workspace](#workspace) - [Workspace Projection](#workspace-projection) @@ -207,6 +208,20 @@ Implemented as `Turn<'a>` in `jp_conversation::stream::turn_iter`. A single Conversation contains many Turns, separated by `TurnStart` events. +### User-Workspace Directory + +A workspace's per-user data directory, +`~/.local/share/jp/workspace/-/`: this user's durable state for one +workspace — conversations, session mappings, locks, the roots registry, and the +user-workspace config search root. +Named for the user-workspace config scope: the *this user × this workspace* +point in the user-global / workspace / user-workspace scope taxonomy. +Located by workspace-ID suffix, never by exact name; `` is cosmetic, +display-only, and never renamed. +Implemented by `FsStorageBackend::with_user_storage` in `jp_storage`; +directory-name parsing lives in `jp_workspace::roots`. +See [RFD-031] and [RFD-087]. + ### Workspace The top-level project unit, housing conversations, configuration, plugins, and @@ -229,4 +244,5 @@ See [RFD-031]. [RFD-001]: ../rfd/001-jp-rfd-process.md [RFD-031]: ../rfd/031-durable-conversation-storage-with-workspace-projection.md +[RFD-087]: ../rfd/087-session-scoped-active-workspace.md [`shlex::split`]: https://docs.rs/shlex diff --git a/docs/rfd/031-durable-conversation-storage-with-workspace-projection.md b/docs/rfd/031-durable-conversation-storage-with-workspace-projection.md index 0e0439936..f43e91dc1 100644 --- a/docs/rfd/031-durable-conversation-storage-with-workspace-projection.md +++ b/docs/rfd/031-durable-conversation-storage-with-workspace-projection.md @@ -101,8 +101,8 @@ storage; all conversations are workspace-only. Today, user-local storage is keyed by both the worktree directory name and workspace ID: `~/.local/share/jp/workspace/-/`, looked up by exact name. -This means each worktree gets its own user-local silo, and removing a worktree -orphans its silo. +This means each worktree gets its own user-workspace directory, and removing a +worktree orphans it. This RFD keys user-local storage on the workspace ID, while keeping a human-recognizable directory name: @@ -111,31 +111,32 @@ human-recognizable directory name: ~/.local/share/jp/workspace/-/conversations/ ``` -The silo is *located* by ID suffix, never by exact name: JP scans -`~/.local/share/jp/workspace/` for a directory named `` or ending in +The user-workspace directory is *located* by ID suffix, never by exact name: JP +scans `~/.local/share/jp/workspace/` for a directory named `` or ending in `-`. Because a workspace ID is a fixed-length `[0-9a-z]` string (it never contains `-`), the suffix match is unambiguous. -All worktrees and clones for the same repository therefore share the single silo -that already exists, regardless of the directory each was cloned into. +All worktrees and clones for the same repository therefore share the single +user-workspace directory that already exists, regardless of the directory each +was cloned into. This aligns with [RFD 020], which already places session mappings and locks under the same per-workspace directory. -`` names a *newly created* silo only, so the user can recognize it among -sibling directories. +`` names a *newly created* directory only, so the user can recognize it +among sibling directories. It is the workspace directory name (a clone into `~/code/my-project` yields `my-project-`), is never validated, and may be absent — an absent or empty slug yields a bare `` directory. -Once a silo exists it is **never renamed**: a later clone with a different -directory name reuses the existing silo as-is rather than re-slugging it. +Once the directory exists it is **never renamed**: a later clone with a +different directory name reuses it as-is rather than re-slugging it. The `with_user_storage` method on `Storage` takes the optional slug alongside the ID. The rename-on-mismatch logic is replaced by a one-time migration that folds any -sibling silos for the workspace into the chosen one. +sibling user-workspace directories for the workspace into the chosen one. When several already exist (legacy `-` per-worktree directories), the -silo matching the current slug wins, else the most recently modified; the winner -is never renamed and the rest are merged in. +directory matching the current slug wins, else the most recently modified; the +winner is never renamed and the rest are merged in. The same migration imports existing workspace conversations into the shared user-local store. @@ -577,8 +578,8 @@ adding durability. ### Migration of existing user-local directories -Collapsing sibling silos into one needs to handle the case where multiple -worktrees have already created separate user-local directories (e.g., +Collapsing sibling user-workspace directories into one needs to handle the case +where multiple worktrees have already created separate directories (e.g., `main-otvo8` and `feature-a-otvo8`). JP picks one survivor — the slug match if present, else the most recently modified — and merges the others into it without renaming the survivor or @@ -638,11 +639,11 @@ same conversation the existing single-root loader would surface duplicate IDs. ### Phase 1: Shared User-Local Storage -Locate the user-local silo by workspace-ID suffix and name a newly created one -`-` (slug optional; bare `` when absent). +Locate the user-workspace directory by workspace-ID suffix and name a newly +created one `-` (slug optional; bare `` when absent). Add migration logic to fold existing per-worktree user-local directories into -the chosen silo, preferring the slug match else the most recently modified, and -never renaming the survivor. +the chosen directory, preferring the slug match else the most recently modified, +and never renaming the survivor. Import existing workspace conversations (active and archive partitions) into user-local storage so pre-RFD workspace-only conversations become durable before dual-write is enabled. diff --git a/docs/rfd/087-session-scoped-active-workspace.md b/docs/rfd/087-session-scoped-active-workspace.md index 972efa35b..d4bbac602 100644 --- a/docs/rfd/087-session-scoped-active-workspace.md +++ b/docs/rfd/087-session-scoped-active-workspace.md @@ -244,6 +244,10 @@ work: checkout owns exactly one file and writes never contend. - Each run upserts only its own file, recording the canonical path and a `last_used` timestamp. + The rewrite is debounced: an entry refreshed within the last few minutes is + left untouched, since recency only feeds display ordering and `latest` + targeting — a per-run rewrite would churn the user data directory on every + invocation and re-trigger any external file watcher observing it. No file is read-modified-written by more than one checkout. - Liveness is **derived**, not stored: a root is live when JP workspace discovery from that path resolves a workspace whose loaded ID equals `` @@ -261,9 +265,9 @@ work: { "path": "/Users/jean/Projects/jp.git/my-feature", "last_used": "2026-06-01T18:25:00Z" } ``` -The roots registry is workspace-scoped, living under the workspace's user-local -silo directory (`-`, located by ID suffix per [RFD 031]; the `` -shorthand in the paths above stands for that silo). +The roots registry is workspace-scoped, living under the workspace's +user-workspace directory (`-`, located by ID suffix per [RFD 031]; the +`` shorthand in the paths above stands for that directory). The session store is user-global (under `sessions/`, mapping a session to its active workspace). These are deliberately separate, not one store doing two jobs. @@ -370,10 +374,10 @@ previous workspace re-resolved against its roots; multiple roots of the same ID are distinct history entries. The picker and fuzzy free-text match display each workspace by its **slug** — -the `` in the user-local silo directory `-` (see [RFD 031]), the -workspace directory name captured when the silo was first created. -The slug is cosmetic: it may be absent (a bare `` silo, in which case the -`` is shown), is never renamed, and is not unique across workspaces. +the `` in the user-workspace directory `-` (see [RFD 031]), the +workspace directory name captured when the directory was first created. +The slug is cosmetic: it may be absent (a bare `` directory, in which case +the `` is shown), is never renamed, and is not unique across workspaces. Fuzzy free-text matches over slug, path, and ID, but resolution is always by ID and concrete root — never by slug — so a shared or stale slug affects only display and search, never which workspace a command runs against. From f71ed2094552337e1f041ff7a1a6a7ca598b2b64 Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Thu, 9 Jul 2026 22:49:20 +0200 Subject: [PATCH 2/6] fixup! feat(cli, workspace): Add `jp workspace` and RFD 087 bootstrap Signed-off-by: Jean Mertz --- docs/.vitepress/rfd-summaries.json | 2 +- docs/rfd/087-session-scoped-active-workspace.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/.vitepress/rfd-summaries.json b/docs/.vitepress/rfd-summaries.json index a19cbfb57..2ea521cb9 100644 --- a/docs/.vitepress/rfd-summaries.json +++ b/docs/.vitepress/rfd-summaries.json @@ -344,7 +344,7 @@ "summary": "Convention for multi-value CLI arguments: `-` reads line-oriented values from stdin, starting with conversation targeting." }, "087-session-scoped-active-workspace.md": { - "hash": "291f15477d02a4ff94f627877388d8b2706d8b1a87908ea6c483463c388c497d", + "hash": "c822b6467f8089b497d1f73e6d968bd92db54fd7469721dffb6a417d3221617a", "summary": "Session-scoped active workspace lets JP commands run from anywhere after selecting a workspace with `jp w use`." }, "088-unified-editor-service-and-inline-reply-widget.md": { diff --git a/docs/rfd/087-session-scoped-active-workspace.md b/docs/rfd/087-session-scoped-active-workspace.md index d4bbac602..85fd35c68 100644 --- a/docs/rfd/087-session-scoped-active-workspace.md +++ b/docs/rfd/087-session-scoped-active-workspace.md @@ -1,6 +1,6 @@ # RFD 087: Session-Scoped Active Workspace -- **Status**: Accepted +- **Status**: Implemented - **Category**: Design - **Authors**: Jean Mertz - **Date**: 2026-06-01 From abf9d5f9b93bcd6c43974dcc587f9328fa5f226c Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Thu, 9 Jul 2026 22:53:18 +0200 Subject: [PATCH 3/6] fixup! feat(cli, workspace): Add `jp workspace` and RFD 087 bootstrap Signed-off-by: Jean Mertz --- crates/jp_workspace/src/session_store.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/crates/jp_workspace/src/session_store.rs b/crates/jp_workspace/src/session_store.rs index c06313d67..5147d6e59 100644 --- a/crates/jp_workspace/src/session_store.rs +++ b/crates/jp_workspace/src/session_store.rs @@ -8,9 +8,9 @@ //! ``` //! //! The store mirrors the per-workspace session-to-conversation mapping (RFD -//! 020, [`session_mapping`]): a most-recent-first `history` of selections -//! (active = `history[0]`, previous = `history[1]`), the session identity for -//! stale detection, and a session-level `sticky` flag (the precedence ladder's +//! 020, `session_mapping`): a most-recent-first `history` of selections (active +//! = `history[0]`, previous = `history[1]`), the session identity for stale +//! detection, and a session-level `sticky` flag (the precedence ladder's //! persisted `A` choice). //! Filenames reuse [`Session::storage_key`], so an automatic `getsid` / `Hwnd` //! session can never alias an `Env` session sharing the same numeric value. @@ -20,8 +20,6 @@ //! is what makes recovery possible after a recorded root is deleted. //! //! See: `docs/rfd/087-session-scoped-active-workspace.md` -//! -//! [`session_mapping`]: crate::session_mapping use std::{fs, str::FromStr as _}; From c25f2c3ee6c7c08956ab19bb7dae1db8237ede84 Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Fri, 10 Jul 2026 09:59:06 +0200 Subject: [PATCH 4/6] fixup! feat(cli, workspace): Add `jp workspace` and RFD 087 bootstrap Signed-off-by: Jean Mertz --- .../jp_cli/src/cmd/workspace/target_tests.rs | 18 ++++++++++--- crates/jp_cli/src/cmd/workspace/use_tests.rs | 16 +++++++++--- .../jp_workspace/src/session_store_tests.rs | 25 +++++++++++++++++++ docs/.vitepress/rfd-summaries.json | 2 +- .../087-session-scoped-active-workspace.md | 8 +++--- 5 files changed, 59 insertions(+), 10 deletions(-) diff --git a/crates/jp_cli/src/cmd/workspace/target_tests.rs b/crates/jp_cli/src/cmd/workspace/target_tests.rs index f8dd07dd5..9d4cef3ba 100644 --- a/crates/jp_cli/src/cmd/workspace/target_tests.rs +++ b/crates/jp_cli/src/cmd/workspace/target_tests.rs @@ -72,9 +72,21 @@ fn env_session() -> Session { } } -/// Render an error via `Debug`, which includes wrapped messages and sources. -fn message_of(error: &impl std::fmt::Debug) -> String { - format!("{error:?}") +/// Render an error with its full source chain, so assertions can match the +/// message of a wrapped `cmd::Error`. +/// +/// Uses `Display`, not `Debug`: `Debug` escapes every backslash in Windows +/// paths, which breaks `contains()` assertions on messages listing candidate +/// roots. +fn message_of(error: &crate::Error) -> String { + let mut message = error.to_string(); + let mut source = std::error::Error::source(error); + while let Some(inner) = source { + message.push_str(": "); + message.push_str(&inner.to_string()); + source = inner.source(); + } + message } #[test] diff --git a/crates/jp_cli/src/cmd/workspace/use_tests.rs b/crates/jp_cli/src/cmd/workspace/use_tests.rs index f5813d289..eb95406c2 100644 --- a/crates/jp_cli/src/cmd/workspace/use_tests.rs +++ b/crates/jp_cli/src/cmd/workspace/use_tests.rs @@ -53,9 +53,19 @@ fn stdout_of(printer: &Printer, buffer: &jp_printer::SharedBuffer) -> String { buffer.lock().clone() } -/// Render an error via `Debug`, which includes wrapped messages and sources. -fn message_of(error: &impl std::fmt::Debug) -> String { - format!("{error:?}") +/// Render an error with its full source chain via `Display`. +/// +/// `Debug` would escape every backslash in Windows paths, breaking `contains()` +/// assertions on messages that list filesystem roots. +fn message_of(error: &crate::cmd::Error) -> String { + let mut message = error.to_string(); + let mut source = std::error::Error::source(error); + while let Some(inner) = source { + message.push_str(": "); + message.push_str(&inner.to_string()); + source = inner.source(); + } + message } #[test] diff --git a/crates/jp_workspace/src/session_store_tests.rs b/crates/jp_workspace/src/session_store_tests.rs index d995a9626..060060ed3 100644 --- a/crates/jp_workspace/src/session_store_tests.rs +++ b/crates/jp_workspace/src/session_store_tests.rs @@ -292,6 +292,11 @@ fn cleanup_removes_env_record_with_no_live_workspace_at_all() { assert!(t.store.load(&session).is_none()); } +/// PID liveness is only checkable on unix — `pid_liveness` returns `Unknown` +/// everywhere else — so the "confirmed alive" guarantee this test pins is +/// unix-only. +/// The non-unix fallback is pinned below. +#[cfg(unix)] #[test] fn cleanup_keeps_record_of_a_live_getsid_process_unconditionally() { let t = store(); @@ -309,6 +314,26 @@ fn cleanup_keeps_record_of_a_live_getsid_process_unconditionally() { assert_eq!(t.store.load(&session).unwrap().history.len(), 1); } +/// Without a PID check (non-unix), a `getsid` record cannot be confirmed alive: +/// its liveness is `Unknown`, so it degrades to the existence rule — no live +/// workspace in the history prunes the record, even while the recording process +/// (this test) is still running. +#[cfg(not(unix))] +#[test] +fn cleanup_prunes_getsid_record_without_pid_check() { + let t = store(); + #[expect(clippy::cast_possible_wrap, reason = "test PIDs fit in i32")] + let session = Session::getsid(std::process::id() as i32); + + t.store + .record_selection(&session, &id("dead1"), Utf8Path::new("/gone"), at(1_000)) + .unwrap(); + + t.store.cleanup(&|_| false); + + assert!(t.store.load(&session).is_none()); +} + #[test] fn cleanup_prunes_unreadable_records() { let t = store(); diff --git a/docs/.vitepress/rfd-summaries.json b/docs/.vitepress/rfd-summaries.json index 2ea521cb9..64b212d6e 100644 --- a/docs/.vitepress/rfd-summaries.json +++ b/docs/.vitepress/rfd-summaries.json @@ -344,7 +344,7 @@ "summary": "Convention for multi-value CLI arguments: `-` reads line-oriented values from stdin, starting with conversation targeting." }, "087-session-scoped-active-workspace.md": { - "hash": "c822b6467f8089b497d1f73e6d968bd92db54fd7469721dffb6a417d3221617a", + "hash": "dd942be2ea632f10a708b9ae5c80f051273e4c1f16a17eb834106d099caa2a57", "summary": "Session-scoped active workspace lets JP commands run from anywhere after selecting a workspace with `jp w use`." }, "088-unified-editor-service-and-inline-reply-widget.md": { diff --git a/docs/rfd/087-session-scoped-active-workspace.md b/docs/rfd/087-session-scoped-active-workspace.md index 85fd35c68..208665fe9 100644 --- a/docs/rfd/087-session-scoped-active-workspace.md +++ b/docs/rfd/087-session-scoped-active-workspace.md @@ -323,9 +323,11 @@ Examples below use `jp w` for brevity. durable store and every live root's workspace `.jp/conversations/`, so it includes external (`ext`) conversations that live only in one checkout (see [RFD 031]). - `jp w show ` therefore loads the conversation index (index only, not event - contents) for each live root and deduplicates by ID — the one place `show` - does a multi-root read, chosen so the count is accurate rather than cheap. + `jp w show ` therefore loads the conversation index (index only, not + event contents) for one live root — which already merges the user-local + store — and scans each sibling root's conversation directory for the IDs + that live there alone, deduplicating by ID: the one place `show` does a + multi-root read, accurate without paying a full workspace load per root. When the target resolves to a single concrete root — a path, or an `` with one live root — the readout shows that root. When an `` has several live roots, `jp w show` lists every live root and From 667d5323504e59c8a4a11eb567cce1c96265c62d Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Fri, 10 Jul 2026 10:03:16 +0200 Subject: [PATCH 5/6] fixup! feat(cli, workspace): Add `jp workspace` and RFD 087 bootstrap Signed-off-by: Jean Mertz --- docs/.vitepress/rfd-summaries.json | 2 +- docs/rfd/087-session-scoped-active-workspace.md | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/.vitepress/rfd-summaries.json b/docs/.vitepress/rfd-summaries.json index 64b212d6e..a300ca300 100644 --- a/docs/.vitepress/rfd-summaries.json +++ b/docs/.vitepress/rfd-summaries.json @@ -344,7 +344,7 @@ "summary": "Convention for multi-value CLI arguments: `-` reads line-oriented values from stdin, starting with conversation targeting." }, "087-session-scoped-active-workspace.md": { - "hash": "dd942be2ea632f10a708b9ae5c80f051273e4c1f16a17eb834106d099caa2a57", + "hash": "fc48244a5cdff093c7538e827f064a25551353609384a104d2b513af110e3514", "summary": "Session-scoped active workspace lets JP commands run from anywhere after selecting a workspace with `jp w use`." }, "088-unified-editor-service-and-inline-reply-widget.md": { diff --git a/docs/rfd/087-session-scoped-active-workspace.md b/docs/rfd/087-session-scoped-active-workspace.md index 208665fe9..e7e7a983f 100644 --- a/docs/rfd/087-session-scoped-active-workspace.md +++ b/docs/rfd/087-session-scoped-active-workspace.md @@ -323,11 +323,11 @@ Examples below use `jp w` for brevity. durable store and every live root's workspace `.jp/conversations/`, so it includes external (`ext`) conversations that live only in one checkout (see [RFD 031]). - `jp w show ` therefore loads the conversation index (index only, not - event contents) for one live root — which already merges the user-local - store — and scans each sibling root's conversation directory for the IDs - that live there alone, deduplicating by ID: the one place `show` does a - multi-root read, accurate without paying a full workspace load per root. + `jp w show ` therefore loads the conversation index (index only, not event + contents) for one live root — which already merges the user-local store — + and scans each sibling root's conversation directory for the IDs that live + there alone, deduplicating by ID: the one place `show` does a multi-root read, + accurate without paying a full workspace load per root. When the target resolves to a single concrete root — a path, or an `` with one live root — the readout shows that root. When an `` has several live roots, `jp w show` lists every live root and From d64cbd3858441f01bdd568dea39f68653341f358 Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Sat, 11 Jul 2026 13:09:38 +0200 Subject: [PATCH 6/6] fixup! feat(cli, workspace): Add `jp workspace` and RFD 087 bootstrap Signed-off-by: Jean Mertz --- crates/jp_cli/src/cmd/attachment/ls.rs | 26 +- crates/jp_cli/src/cmd/conversation/ls.rs | 33 ++- .../jp_cli/src/cmd/conversation/ls_tests.rs | 62 +++++ crates/jp_cli/src/cmd/conversation/show.rs | 7 +- crates/jp_cli/src/cmd/workspace/ls.rs | 46 +++- crates/jp_cli/src/cmd/workspace/ls_tests.rs | 53 ++++ crates/jp_cli/src/cmd/workspace/show.rs | 113 +++++---- crates/jp_cli/src/cmd/workspace/show_tests.rs | 53 ++++ crates/jp_cli/src/format.rs | 1 + crates/jp_cli/src/format/conversation.rs | 38 +++ crates/jp_cli/src/format/workspace.rs | 227 ++++++++++++++++++ crates/jp_cli/src/output.rs | 38 ++- crates/jp_cli/src/output_tests.rs | 65 ++--- crates/jp_storage/src/load.rs | 13 + crates/jp_term/src/table.rs | 75 ------ crates/jp_term/src/table_tests.rs | 60 +---- 16 files changed, 666 insertions(+), 244 deletions(-) create mode 100644 crates/jp_cli/src/format/workspace.rs diff --git a/crates/jp_cli/src/cmd/attachment/ls.rs b/crates/jp_cli/src/cmd/attachment/ls.rs index d59ab25a6..9954201a6 100644 --- a/crates/jp_cli/src/cmd/attachment/ls.rs +++ b/crates/jp_cli/src/cmd/attachment/ls.rs @@ -1,6 +1,10 @@ use jp_term::table::DetailRow; -use crate::{cmd::Output, ctx::Ctx, output::print_details}; +use crate::{ + cmd::Output, + ctx::Ctx, + output::{print_details, print_json}, +}; #[derive(Debug, clap::Args)] pub(crate) struct Ls {} @@ -11,18 +15,32 @@ impl Ls { let uris = &ctx.config().conversation.attachments; if uris.is_empty() { - ctx.printer.println("No attachments in current context."); + // Machine-readable formats get an empty payload, not prose. + if ctx.printer.format().is_json() { + print_json(&ctx.printer, &serde_json::json!([])); + } else { + ctx.printer.println("No attachments in current context."); + } return Ok(()); } let title = Some("Attachments".to_owned()); + // The JSON payload is a plain array of attachment URLs. let mut rows = vec![]; + let mut urls = vec![]; for uri in uris { - rows.push(DetailRow::bare(uri.to_url()?)); + let url = uri.to_url()?; + urls.push(url.to_string()); + rows.push(DetailRow::bare(url)); } - print_details(&ctx.printer, title.as_deref(), rows); + print_details( + &ctx.printer, + title.as_deref(), + rows, + &serde_json::json!(urls), + ); Ok(()) } } diff --git a/crates/jp_cli/src/cmd/conversation/ls.rs b/crates/jp_cli/src/cmd/conversation/ls.rs index 57797fa80..a4451e313 100644 --- a/crates/jp_cli/src/cmd/conversation/ls.rs +++ b/crates/jp_cli/src/cmd/conversation/ls.rs @@ -93,6 +93,37 @@ struct Details { external: bool, } +/// The stable machine-readable payload: one object per listed conversation. +/// +/// Keys are a fixed contract, deliberately decoupled from the table's display +/// columns: column headers, markers, and layout can change freely, these keys +/// cannot change without breaking consumers. +/// Absent fields serialize as `null`; titles are never truncated here (only the +/// pretty table shaves them to fit the terminal); timestamps are RFC 3339 in +/// UTC. +fn payload(conversations: &[Details]) -> serde_json::Value { + let items: Vec<_> = conversations + .iter() + .map(|d| { + serde_json::json!({ + "id": d.id.to_string(), + "title": d.title, + "active": d.active, + "pinned_at": d.pinned_at, + "archived_at": d.archived_at, + "local": d.local, + "external": d.external, + "events": d.messages, + "created_at": d.id.timestamp(), + "last_event_at": d.last_event_at, + "expires_at": d.expires_at, + }) + }) + .collect(); + + serde_json::Value::Array(items) +} + impl Ls { pub(crate) fn conversation_load_request(&self) -> ConversationLoadRequest { ConversationLoadRequest::explicit_or_none(&self.target) @@ -223,7 +254,7 @@ impl Ls { let header = build_header_row(columns, marker); let rows = self.build_body(ctx, &conversations, columns, title_budget, hidden); let footer = rows.len() > 20; - print_table(&ctx.printer, header, rows, footer); + print_table(&ctx.printer, header, rows, footer, &payload(&conversations)); Ok(()) } diff --git a/crates/jp_cli/src/cmd/conversation/ls_tests.rs b/crates/jp_cli/src/cmd/conversation/ls_tests.rs index d2379a737..aa9fe6046 100644 --- a/crates/jp_cli/src/cmd/conversation/ls_tests.rs +++ b/crates/jp_cli/src/cmd/conversation/ls_tests.rs @@ -105,3 +105,65 @@ fn local_cell_marks_external_distinctly() { assert_eq!(strip_str(local_cell(true, false)), "Y"); assert_eq!(strip_str(local_cell(false, true)), "ext"); } + +#[test] +fn payload_keys_are_the_stable_contract() { + let created = DateTime::::UNIX_EPOCH + std::time::Duration::from_secs(1_000_000); + let last_event = created + chrono::Duration::hours(1); + let details = Details { + id: ConversationId::try_from(created).unwrap(), + active: true, + pinned_at: None, + archived_at: None, + title: Some("My title".into()), + messages: 4, + last_event_at: Some(last_event), + expires_at: None, + local: true, + external: false, + }; + + let json = payload(std::slice::from_ref(&details)); + let items = json.as_array().expect("payload is a JSON array"); + assert_eq!(items.len(), 1); + + // Fixed key set: display columns (headers, markers, truncation) must not + // leak into the machine payload. A key rename here is a breaking change. + let item = &items[0]; + let mut keys: Vec<_> = item + .as_object() + .expect("one object per conversation") + .keys() + .map(String::as_str) + .collect(); + keys.sort_unstable(); + assert_eq!(keys, vec![ + "active", + "archived_at", + "created_at", + "events", + "expires_at", + "external", + "id", + "last_event_at", + "local", + "pinned_at", + "title", + ]); + + assert_eq!(item["id"], serde_json::json!(details.id.to_string())); + assert_eq!(item["title"], serde_json::json!("My title")); + assert_eq!(item["active"], serde_json::json!(true)); + assert_eq!(item["events"], serde_json::json!(4)); + // Absent optionals serialize as null, not as omitted keys. + assert_eq!(item["pinned_at"], serde_json::Value::Null); + assert_eq!(item["expires_at"], serde_json::Value::Null); + // Timestamps are RFC 3339 UTC strings. + assert!( + item["last_event_at"] + .as_str() + .is_some_and(|v| v.contains('T') && v.ends_with('Z')), + "RFC 3339 UTC timestamp, got: {}", + item["last_event_at"] + ); +} diff --git a/crates/jp_cli/src/cmd/conversation/show.rs b/crates/jp_cli/src/cmd/conversation/show.rs index e90b0acee..ee5232a9e 100644 --- a/crates/jp_cli/src/cmd/conversation/show.rs +++ b/crates/jp_cli/src/cmd/conversation/show.rs @@ -56,7 +56,12 @@ impl Show { .with_compactions(compactions) .with_pretty_printing(ctx.printer.pretty_printing_enabled()); - print_details(&ctx.printer, details.title.as_deref(), details.rows()); + print_details( + &ctx.printer, + details.title.as_deref(), + details.rows(), + &details.json(), + ); } Ok(()) } diff --git a/crates/jp_cli/src/cmd/workspace/ls.rs b/crates/jp_cli/src/cmd/workspace/ls.rs index 0342e28da..3a237062d 100644 --- a/crates/jp_cli/src/cmd/workspace/ls.rs +++ b/crates/jp_cli/src/cmd/workspace/ls.rs @@ -6,7 +6,7 @@ use jp_workspace::roots; use crate::{ DEFAULT_STORAGE_DIR, cmd::{Output, workspace::target::TargetEnv}, - output::print_table, + output::{print_json, print_table}, }; /// List known workspaces and their checkouts. @@ -22,10 +22,16 @@ impl Ls { pub(crate) fn run(self, printer: &Printer, env: &TargetEnv<'_>) -> Output { let known = roots::known_workspaces(&env.workspaces_dir, DEFAULT_STORAGE_DIR); if known.is_empty() { - printer.println( - "No known workspaces. JP registers a workspace when a command runs from inside it." - .to_owned(), - ); + // Machine-readable formats get an empty payload, not prose. + if printer.format().is_json() { + print_json(printer, &serde_json::json!([])); + } else { + printer.println( + "No known workspaces. JP registers a workspace when a command runs from \ + inside it." + .to_owned(), + ); + } return Ok(()); } @@ -39,9 +45,31 @@ impl Ls { let header = Row::from(vec!["", "ID", "Name", "Checkout", "Last used"]); let mut rows = Vec::new(); + // The JSON payload is one self-contained object per workspace with its + // checkouts nested, while the display rows are a projection: + // continuation rows blank the repeated ID/name, placeholder text marks + // checkout-less workspaces, and timestamps are localized. + let mut payload = Vec::with_capacity(known.len()); + for workspace in known { let name = workspace.slug.as_deref().unwrap_or("").to_owned(); + payload.push(serde_json::json!({ + "id": workspace.id.to_string(), + "slug": workspace.slug, + "checkouts": workspace + .roots + .iter() + .map(|entry| { + serde_json::json!({ + "path": entry.path, + "active": is_active(&workspace.id, &entry.path), + "last_used": entry.last_used, + }) + }) + .collect::>(), + })); + if workspace.roots.is_empty() { rows.push(Row::from(vec![ String::new(), @@ -77,7 +105,13 @@ impl Ls { } } - print_table(printer, header, rows, false); + print_table( + printer, + header, + rows, + false, + &serde_json::Value::Array(payload), + ); Ok(()) } } diff --git a/crates/jp_cli/src/cmd/workspace/ls_tests.rs b/crates/jp_cli/src/cmd/workspace/ls_tests.rs index 20128bede..41a3ab466 100644 --- a/crates/jp_cli/src/cmd/workspace/ls_tests.rs +++ b/crates/jp_cli/src/cmd/workspace/ls_tests.rs @@ -121,3 +121,56 @@ fn marks_the_session_active_checkout() { .any(|line| line.contains('*') && line.contains(canonical.as_str())); assert!(marked, "no active marker in output: {stdout}"); } + +#[test] +fn json_format_nests_checkouts_per_workspace() { + let tmp = tempdir().unwrap(); + let root = make_workspace(tmp.path(), "proj", "ws123"); + let session = env_session(); + let env = env_at(tmp.path().to_owned(), tmp.path(), Some(&session)); + register(&env, "proj", "ws123", &root); + + let canonical = root.canonicalize_utf8().unwrap(); + env.store + .record_selection( + &session, + &Id::from_str("ws123").unwrap(), + &canonical, + Utc::now(), + ) + .unwrap(); + + let (printer, out, _err) = Printer::memory(OutputFormat::Json); + Ls {}.run(&printer, &env).unwrap(); + + let stdout = stdout_of(&printer, &out); + let json: serde_json::Value = serde_json::from_str(stdout.trim()).unwrap(); + let workspaces = json.as_array().expect("list output is a JSON array"); + assert_eq!(workspaces.len(), 1, "one object per workspace: {stdout}"); + + // One self-contained object per workspace: identity at the top, + // checkouts nested — not one row-shaped object per checkout, and no + // display-column keys. + let workspace = &workspaces[0]; + assert_eq!(workspace["id"], serde_json::json!("ws123")); + assert_eq!(workspace["slug"], serde_json::json!("proj")); + + let checkouts = workspace["checkouts"] + .as_array() + .expect("checkouts is a JSON array"); + assert_eq!(checkouts.len(), 1, "one entry per live checkout: {stdout}"); + + let checkout = &checkouts[0]; + assert_eq!(checkout["path"], serde_json::json!(canonical.as_str())); + assert_eq!( + checkout["active"], + serde_json::json!(true), + "session-active checkout flags `active`: {stdout}" + ); + assert!( + checkout["last_used"] + .as_str() + .is_some_and(|v| !v.is_empty()), + "last-used timestamp should be present: {stdout}" + ); +} diff --git a/crates/jp_cli/src/cmd/workspace/show.rs b/crates/jp_cli/src/cmd/workspace/show.rs index a7b94e078..66c7767f4 100644 --- a/crates/jp_cli/src/cmd/workspace/show.rs +++ b/crates/jp_cli/src/cmd/workspace/show.rs @@ -1,6 +1,6 @@ use std::collections::BTreeSet; -use camino::{Utf8Path, Utf8PathBuf}; +use camino::Utf8Path; use crossterm::style::Stylize as _; use jp_conversation::ConversationId; use jp_printer::Printer; @@ -13,6 +13,8 @@ use crate::{ Output, workspace::target::{self, ResolvedTarget, TargetEnv, WorkspaceTarget}, }, + format::workspace::{DetailsFmt, checkout_detail_item}, + output::print_details, }; /// Show a workspace: identity, checkouts, and how it resolves. @@ -36,8 +38,8 @@ pub(crate) struct Show { struct Subject { id: Id, slug: Option, - /// Live checkout roots, most recently used first. - roots: Vec, + /// Live checkouts, most recently used first. + roots: Vec, /// How the subject was resolved, for the readout. resolved: &'static str, } @@ -212,10 +214,7 @@ fn subject_for(env: &TargetEnv<'_>, id: Id, resolved: &'static str) -> Subject { .into_iter() .find(|workspace| workspace.id == id) .and_then(|workspace| workspace.slug); - let roots = roots::resolve_live_roots(&env.workspaces_dir, &id, DEFAULT_STORAGE_DIR) - .into_iter() - .map(|entry| entry.path) - .collect(); + let roots = roots::resolve_live_roots(&env.workspaces_dir, &id, DEFAULT_STORAGE_DIR); Subject { id, @@ -239,12 +238,7 @@ fn render( cwd_root: Option<&Utf8Path>, persist: bool, ) { - let name = match &subject.slug { - Some(slug) => format!("{} ({})", slug.clone().bold(), subject.id), - None => subject.id.to_string().bold().to_string(), - }; - printer.println(format!("Workspace: {name}")); - printer.println(format!("Resolved from: {}", subject.resolved)); + let pretty = printer.pretty_printing_enabled(); // Sticky is session-level state about the *active* workspace, so it only // renders when the subject is the active one. @@ -253,40 +247,31 @@ fn render( && let Some(session) = env.session && let Some(mapping) = env.store.load(session) { - printer.println(format!( - "Sticky: {}", - if mapping.sticky { "yes" } else { "no" }, - )); - mapping.sticky + Some(mapping.sticky) } else { - false + None }; - if subject.roots.is_empty() { - printer.println("Checkouts: (no live checkouts)".to_owned()); - } else { - printer.println("Checkouts:".to_owned()); - for root in &subject.roots { - let marker = if active.is_some_and(|entry| entry.root == *root) { - "*" - } else { - " " - }; - printer.println(format!(" {marker} {root}")); - } - } + let checkouts = subject + .roots + .iter() + .map(|entry| { + let is_active = active.is_some_and(|selection| selection.root == entry.path); + checkout_detail_item(&entry.path, entry.last_used, is_active, pretty) + }) + .collect(); - // The one multi-root read: union the conversation index across the - // user-local durable store and every live checkout, deduplicated by ID — - // accurate rather than cheap (RFD 087). - if let Some(stats) = conversation_stats(env, &subject.roots, persist) { - printer.println(format!("Conversations: {}", stats.count)); + let stats = conversation_stats(env, &subject.roots, persist); - if let Some((id, title)) = stats.active { - let title = title.map(|t| format!(": {t}")).unwrap_or_default(); - printer.println(format!("Active conversation: {id}{title}")); - } - } + let details = DetailsFmt::new(subject.id.clone(), subject.resolved) + .with_slug(subject.slug.as_deref()) + .with_sticky(sticky) + .with_checkouts(checkouts) + .with_conversations(stats.as_ref().map(|stats| stats.count)) + .with_active_conversation(stats.and_then(|stats| stats.active)) + .with_pretty_printing(pretty); + + print_details(printer, details.title(), details.rows(), &details.json()); // The cwd-vs-active tension, surfaced instead of silently resolved (RFD // 087's precedence ladder): a sticky session keeps the active workspace; @@ -295,7 +280,7 @@ fn render( && let Some(cwd_root) = cwd_root && active.is_some_and(|entry| entry.root != cwd_root) { - let note = if sticky { + let note = if sticky.unwrap_or(false) { format!( "Note: the current directory resolves to `{cwd_root}`, but this session is sticky \ to its active workspace, which takes precedence for commands run here." @@ -316,21 +301,33 @@ struct ConversationStats { active: Option<(ConversationId, Option)>, } -/// Load each live root's conversation index (index only, not event contents) -/// and deduplicate by conversation ID. +/// Union the conversation IDs across the user-local durable store and every +/// live checkout, deduplicated by ID — accurate *and* cheap (RFD 087). +/// +/// Only the first loadable root pays a full workspace load: that load already +/// merges the user-local durable store, so every sibling checkout can only add +/// conversations that live in its own projection alone. +/// Those are picked up with a bare directory scan per sibling — no workspace +/// construction, no user-local re-merge, and no roots-registry writes, keeping +/// `show` read-only for the checkouts it merely reports on. /// /// Roots that fail to load are skipped with a warning; `None` when no root /// produced an index. fn conversation_stats( env: &TargetEnv<'_>, - roots: &[Utf8PathBuf], + roots: &[roots::RootEntry], persist: bool, ) -> Option { let mut ids: BTreeSet = BTreeSet::new(); let mut active = None; - let mut loaded_any = false; + let mut remaining = roots.iter(); - for root in roots { + // One full load: user-local union, plus the session's active + // conversation. The session → conversation mapping is per workspace ID, + // so any loaded checkout answers it. + let mut loaded_any = false; + for entry in remaining.by_ref() { + let root = &entry.path; let (mut workspace, _backend) = match crate::load_workspace(root, persist) { Ok(loaded) => loaded, Err(error) => { @@ -343,10 +340,7 @@ fn conversation_stats( ids.extend(workspace.conversations().map(|(id, _)| *id)); loaded_any = true; - // The session → conversation mapping is per workspace ID, so any - // loaded checkout answers it; take the first. - if active.is_none() - && let Some(session) = env.session + if let Some(session) = env.session && let Some(id) = workspace.session_active_conversation(session) { let title = workspace @@ -356,9 +350,22 @@ fn conversation_stats( .and_then(|metadata| metadata.title.clone()); active = Some((id, title)); } + + break; + } + + if !loaded_any { + return None; + } + + // The sibling checkouts: checkout-only conversations via directory scan. + for entry in remaining { + ids.extend(jp_storage::load::projected_conversation_ids( + &entry.path.join(DEFAULT_STORAGE_DIR), + )); } - loaded_any.then_some(ConversationStats { + Some(ConversationStats { count: ids.len(), active, }) diff --git a/crates/jp_cli/src/cmd/workspace/show_tests.rs b/crates/jp_cli/src/cmd/workspace/show_tests.rs index 909e4f8e4..952769d87 100644 --- a/crates/jp_cli/src/cmd/workspace/show_tests.rs +++ b/crates/jp_cli/src/cmd/workspace/show_tests.rs @@ -3,6 +3,7 @@ use std::str::FromStr as _; use camino::{Utf8Path, Utf8PathBuf}; use camino_tempfile::tempdir; use chrono::Utc; +use jp_conversation::ConversationId; use jp_printer::{OutputFormat, Printer}; use jp_workspace::{ session::{Session, SessionId, SessionSource}, @@ -114,6 +115,58 @@ fn ambiguous_fuzzy_match_errors_with_candidates() { ); } +#[test] +fn conversation_count_unions_sibling_checkout_scans() { + let tmp = tempdir().unwrap(); + + // Two live checkouts of the same workspace ID. The most recently used one + // pays the full workspace load; its sibling is only directory-scanned. + let first = make_workspace(tmp.path(), "first", "ws123"); + let second = make_workspace(tmp.path(), "second", "ws123"); + + let env = env_at(tmp.path().to_owned(), tmp.path(), None); + let user_dir = env.workspaces_dir.join("proj-ws123"); + let id = Id::from_str("ws123").unwrap(); + // `first` is upserted last, so it is the most recently used checkout and + // takes the full load; `second` stays a scanned sibling. A full load + // would sanitize the bare (event-less) directory below away — the scan + // must not, since it merely lists projected IDs. + jp_workspace::roots::upsert_root(&user_dir, &second).unwrap(); + jp_workspace::roots::upsert_root(&user_dir, &first).unwrap(); + + // A checkout-only conversation in the *second* root: present only as a + // projection directory, invisible to the first root's full load. + let conversation = ConversationId::from_str("jp-c17636257521").unwrap(); + std::fs::create_dir_all( + second + .join(crate::DEFAULT_STORAGE_DIR) + .join("conversations") + .join(conversation.to_dirname(None)) + .as_std_path(), + ) + .unwrap(); + + let (printer, out, _err) = Printer::memory(OutputFormat::Json); + Show { + target: Some(WorkspaceTarget::Id(id)), + } + .run(&printer, &env, false) + .unwrap(); + + let stdout = stdout_of(&printer, &out); + let json: serde_json::Value = serde_json::from_str(stdout.trim()).unwrap(); + assert_eq!( + json["conversations"], + serde_json::json!(1), + "checkout-only conversation should be counted: {stdout}" + ); + assert_eq!( + json["checkouts"].as_array().map(Vec::len), + Some(2), + "both live checkouts should be listed: {stdout}" + ); +} + #[test] fn session_active_subject_notes_cwd_precedence() { let tmp = tempdir().unwrap(); diff --git a/crates/jp_cli/src/format.rs b/crates/jp_cli/src/format.rs index bbb9d464d..ef036affd 100644 --- a/crates/jp_cli/src/format.rs +++ b/crates/jp_cli/src/format.rs @@ -1,5 +1,6 @@ pub(crate) mod conversation; pub(crate) mod datetime; +pub(crate) mod workspace; use jp_config::types::color::Color; use jp_conversation::{Compaction, ToolCallPolicy}; diff --git a/crates/jp_cli/src/format/conversation.rs b/crates/jp_cli/src/format/conversation.rs index ec285ca57..26b87f1c9 100644 --- a/crates/jp_cli/src/format/conversation.rs +++ b/crates/jp_cli/src/format/conversation.rs @@ -4,6 +4,7 @@ use chrono::{DateTime, Utc}; use crossterm::style::Stylize as _; use jp_conversation::ConversationId; use jp_term::table::{DetailItem, DetailRow, details}; +use serde_json::json; use super::datetime::DateTimeFmt; @@ -159,6 +160,43 @@ impl DetailsFmt { self.title.as_deref() } + /// The stable machine-readable payload for `jp c show`. + /// + /// Keys are a fixed contract, deliberately decoupled from the display + /// labels in [`Self::rows`]: labels can be reworded freely, these keys + /// cannot change without breaking consumers. + /// Fields the display hides stay present here — counts as `0`, unqueried + /// flags as `null` — so consumers can rely on key presence. + /// Timestamps are RFC 3339 in UTC; the display's derived forms ("Currently + /// Active", "On Deactivation") are projections of `active`, + /// `last_activated_at`, and `expires_at`. + #[must_use] + pub fn json(&self) -> serde_json::Value { + json!({ + "id": self.id.to_string(), + "title": self.title, + "assistant": self.assistant_name, + "active": self.active_conversation.map(|active| active == self.id), + "pinned": self.pinned, + "local": self.local, + "events": self.message_count, + "turns": self.turn_count, + "last_message_at": self.last_message_at, + "last_activated_at": self.last_activated_at, + "expires_at": self.expires_at, + "attachments": self + .attachments + .iter() + .map(|item| item.json.clone()) + .collect::>(), + "compactions": self + .compactions + .iter() + .map(|item| item.json.clone()) + .collect::>(), + }) + } + /// Return rows for a table displaying the conversation details. #[must_use] pub fn rows(&self) -> Vec { diff --git a/crates/jp_cli/src/format/workspace.rs b/crates/jp_cli/src/format/workspace.rs new file mode 100644 index 000000000..466ecbcbb --- /dev/null +++ b/crates/jp_cli/src/format/workspace.rs @@ -0,0 +1,227 @@ +use std::fmt; + +use camino::Utf8Path; +use chrono::{DateTime, Utc}; +use crossterm::style::Stylize as _; +use jp_conversation::ConversationId; +use jp_term::table::{DetailItem, DetailRow, details}; +use jp_workspace::Id; +use serde_json::json; + +/// Details view for `jp w show`, mirroring the conversation [`DetailsFmt`]. +/// +/// [`DetailsFmt`]: super::conversation::DetailsFmt +pub struct DetailsFmt { + /// The ID of the workspace. + pub id: Id, + + /// The user-workspace directory slug; cosmetic display name (RFD 031). + pub slug: Option, + + /// How the subject was resolved, for the readout. + pub resolved: &'static str, + + /// Session-level sticky state. + /// `None` (subject is not the session-active workspace) hides the row. + pub sticky: Option, + + /// Live checkouts, most recently used first. + pub checkouts: Vec, + + /// Union conversation count across the user-local durable store and every + /// live checkout. + /// `None` (no checkout could be loaded) hides the row. + pub conversations: Option, + + /// The session's active conversation in this workspace, with its title. + pub active_conversation: Option<(ConversationId, Option)>, + + /// Pretty-print the output. + pub pretty: bool, +} + +impl DetailsFmt { + #[must_use] + pub fn new(id: Id, resolved: &'static str) -> Self { + Self { + id, + slug: None, + resolved, + sticky: None, + checkouts: vec![], + conversations: None, + active_conversation: None, + pretty: true, + } + } + + #[must_use] + pub fn with_slug(mut self, slug: Option>) -> Self { + self.slug = slug.map(Into::into); + self + } + + #[must_use] + pub fn with_sticky(self, sticky: Option) -> Self { + Self { sticky, ..self } + } + + #[must_use] + pub fn with_checkouts(mut self, checkouts: Vec) -> Self { + self.checkouts = checkouts; + self + } + + #[must_use] + pub fn with_conversations(self, conversations: Option) -> Self { + Self { + conversations, + ..self + } + } + + #[must_use] + pub fn with_active_conversation( + self, + active_conversation: Option<(ConversationId, Option)>, + ) -> Self { + Self { + active_conversation, + ..self + } + } + + /// Use color in the output. + #[must_use] + pub fn with_pretty_printing(self, pretty: bool) -> Self { + Self { pretty, ..self } + } + + /// Return the title of the workspace: its cosmetic slug, when known. + #[must_use] + pub fn title(&self) -> Option<&str> { + self.slug.as_deref() + } + + /// The stable machine-readable payload for `jp w show`. + /// + /// Keys are a fixed contract, deliberately decoupled from the display + /// labels in [`Self::rows`]: labels can be reworded freely, these keys + /// cannot change without breaking consumers. + /// Rows the display hides serialize as `null` instead of disappearing, so + /// consumers can rely on key presence: `sticky` is `null` when the subject + /// is not the session-active workspace, and `conversations` is `null` when + /// no checkout could be loaded. + /// Timestamps are RFC 3339 in UTC. + #[must_use] + pub fn json(&self) -> serde_json::Value { + json!({ + "id": self.id.to_string(), + "slug": self.slug, + "resolved_from": self.resolved, + "sticky": self.sticky, + "checkouts": self + .checkouts + .iter() + .map(|item| item.json.clone()) + .collect::>(), + "conversations": self.conversations, + "active_conversation": self.active_conversation.as_ref().map(|(id, title)| { + json!({ + "id": id.to_string(), + "title": title, + }) + }), + }) + } + + /// Return rows for a table displaying the workspace details. + #[must_use] + pub fn rows(&self) -> Vec { + let mut rows = vec![]; + + rows.push(self.scalar("ID", self.id.to_string())); + rows.push(self.scalar("Resolved From", self.resolved.to_owned())); + + if let Some(sticky) = self.sticky { + let value = if sticky && self.pretty { + "Yes".bold().yellow().to_string() + } else if sticky { + "Yes".to_owned() + } else { + "No".to_owned() + }; + rows.push(self.scalar("Sticky", value)); + } + + if self.checkouts.is_empty() { + rows.push(self.scalar("Checkouts", "(no live checkouts)".to_owned())); + } else { + rows.push(DetailRow::list( + self.styled_label("Checkouts"), + self.checkouts.clone(), + )); + } + + if let Some(count) = self.conversations { + rows.push(self.scalar("Conversations", count.to_string())); + } + + if let Some((id, title)) = &self.active_conversation { + let value = match title { + Some(title) => format!("{id}: {title}"), + None => id.to_string(), + }; + rows.push(self.scalar("Active Conversation", value)); + } + + rows + } + + /// Bold the label when pretty-printing is enabled. + fn styled_label(&self, label: &str) -> String { + if self.pretty { + label.bold().to_string() + } else { + label.to_owned() + } + } + + fn scalar(&self, label: &str, value: String) -> DetailRow { + DetailRow::scalar(self.styled_label(label), value) + } +} + +impl fmt::Display for DetailsFmt { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", details(self.title(), self.rows())) + } +} + +/// Build a list item for a live checkout root. +/// +/// The terminal text is the path, marked `(active)` when the session's active +/// selection points at this checkout. +/// The JSON form carries the canonical `path`, the `active` flag, and the roots +/// registry's `last_used` timestamp. +pub(crate) fn checkout_detail_item( + path: &Utf8Path, + last_used: DateTime, + active: bool, + pretty: bool, +) -> DetailItem { + let text = match (active, pretty) { + (true, true) => format!("{path} {}", "(active)".green().bold()), + (true, false) => format!("{path} (active)"), + (false, _) => path.to_string(), + }; + + DetailItem::new( + text, + json!({ + "path": path, + "active": active, + "last_used": last_used, + }), + ) +} diff --git a/crates/jp_cli/src/output.rs b/crates/jp_cli/src/output.rs index a3c62ab9b..8cb2897a5 100644 --- a/crates/jp_cli/src/output.rs +++ b/crates/jp_cli/src/output.rs @@ -3,31 +3,29 @@ //! These functions dispatch table/details/value rendering based on the //! printer's [`OutputFormat`], so commands don't need to branch on the format //! themselves. +//! +//! Text formats render the display rows; JSON formats emit the explicit `json` +//! payload the command supplies. +//! The two are deliberately decoupled: display labels, ordering, and layout can +//! change freely, while the JSON payload is a stable machine contract with +//! `snake_case` keys — it is never derived from display text. use comfy_table::Row; use jp_printer::{OutputFormat, Printer}; -use jp_term::table::{ - DetailRow, details, details_json, details_markdown, list, list_json, list_markdown, -}; +use jp_term::table::{DetailRow, details, details_markdown, list, list_markdown}; use serde_json::{Value, to_string, to_string_pretty}; /// Print a list table (header + rows) in the format dictated by the printer. /// /// - `TextPretty` → unicode box-drawing table /// - `Text` → pipe-delimited markdown table -/// - `Json` / `JsonPretty` → JSON array of objects -pub fn print_table(printer: &Printer, header: Row, rows: Vec, footer: bool) { +/// - `Json` / `JsonPretty` → the explicit `json` payload +pub fn print_table(printer: &Printer, header: Row, rows: Vec, footer: bool, json: &Value) { let output = match printer.format() { OutputFormat::TextPretty => list(header, rows, footer), OutputFormat::Text => list_markdown(header, rows), - OutputFormat::Json => { - let json = list_json(header, rows); - to_string(&json).unwrap_or_else(|_| json.to_string()) - } - OutputFormat::JsonPretty => { - let json = list_json(header, rows); - to_string_pretty(&json).unwrap_or_else(|_| json.to_string()) - } + OutputFormat::Json => to_string(json).unwrap_or_else(|_| json.to_string()), + OutputFormat::JsonPretty => to_string_pretty(json).unwrap_or_else(|_| json.to_string()), }; // Use println_raw: JSON variants already contain valid JSON, text variants @@ -39,19 +37,13 @@ pub fn print_table(printer: &Printer, header: Row, rows: Vec, footer: bool) /// /// - `TextPretty` → borderless aligned table with optional title /// - `Text` → pipe-delimited markdown table with optional title -/// - `Json` / `JsonPretty` → JSON object -pub fn print_details(printer: &Printer, title: Option<&str>, rows: Vec) { +/// - `Json` / `JsonPretty` → the explicit `json` payload +pub fn print_details(printer: &Printer, title: Option<&str>, rows: Vec, json: &Value) { let output = match printer.format() { OutputFormat::TextPretty => details(title, rows), OutputFormat::Text => details_markdown(title, rows), - OutputFormat::Json => { - let json = details_json(title, rows); - to_string(&json).unwrap_or_else(|_| json.to_string()) - } - OutputFormat::JsonPretty => { - let json = details_json(title, rows); - to_string_pretty(&json).unwrap_or_else(|_| json.to_string()) - } + OutputFormat::Json => to_string(json).unwrap_or_else(|_| json.to_string()), + OutputFormat::JsonPretty => to_string_pretty(json).unwrap_or_else(|_| json.to_string()), }; printer.println_raw(&output); diff --git a/crates/jp_cli/src/output_tests.rs b/crates/jp_cli/src/output_tests.rs index e10f91b57..6f7fd9306 100644 --- a/crates/jp_cli/src/output_tests.rs +++ b/crates/jp_cli/src/output_tests.rs @@ -23,7 +23,7 @@ fn table_text_pretty_renders_unicode_box() { let header = row(&["Name", "Age"]); let rows = vec![row(&["Alice", "30"]), row(&["Bob", "25"])]; - print_table(&printer, header, rows, false); + print_table(&printer, header, rows, false, &json!([])); let output = flush_stdout(&printer, &out); // Unicode box-drawing uses these characters @@ -38,7 +38,7 @@ fn table_text_renders_markdown_pipes() { let header = row(&["Name", "Age"]); let rows = vec![row(&["Alice", "30"])]; - print_table(&printer, header, rows, false); + print_table(&printer, header, rows, false, &json!([])); let output = flush_stdout(&printer, &out); assert!(output.contains("| Name"), "expected markdown table header"); @@ -47,40 +47,47 @@ fn table_text_renders_markdown_pipes() { } #[test] -fn table_json_returns_compact_array() { +fn table_json_emits_payload_not_rows() { let (printer, out, _) = Printer::memory(OutputFormat::Json); + // Display rows and JSON payload deliberately disagree: the payload is + // the contract, the rows are presentation. let header = row(&["Name", "Age"]); - let rows = vec![row(&["Alice", "30"])]; + let rows = vec![row(&["Alice (30)", ""])]; + let payload = json!([{"name": "Alice", "age": 30}]); - print_table(&printer, header, rows, false); + print_table(&printer, header, rows, false, &payload); let output = flush_stdout(&printer, &out); let parsed: serde_json::Value = serde_json::from_str(output.trim()).unwrap(); - assert_eq!(parsed, json!([{"Name": "Alice", "Age": "30"}])); + assert_eq!(parsed, payload); + assert!( + !output.contains("Alice (30)"), + "display text must not leak into JSON" + ); // Compact: no interior newlines assert!(!output.trim().contains('\n'), "expected single-line JSON"); } #[test] -fn table_json_pretty_returns_indented_array() { +fn table_json_pretty_returns_indented_payload() { let (printer, out, _) = Printer::memory(OutputFormat::JsonPretty); let header = row(&["Id"]); let rows = vec![row(&["abc"])]; - print_table(&printer, header, rows, false); + print_table(&printer, header, rows, false, &json!([{"id": "abc"}])); let output = flush_stdout(&printer, &out); let parsed: serde_json::Value = serde_json::from_str(output.trim()).unwrap(); - assert_eq!(parsed, json!([{"Id": "abc"}])); + assert_eq!(parsed, json!([{"id": "abc"}])); assert!(output.contains("\n "), "expected indented JSON"); } #[test] -fn table_empty_rows() { +fn table_empty_payload() { let (printer, out, _) = Printer::memory(OutputFormat::Json); let header = row(&["X"]); - print_table(&printer, header, vec![], false); + print_table(&printer, header, vec![], false, &json!([])); let output = flush_stdout(&printer, &out); let parsed: serde_json::Value = serde_json::from_str(output.trim()).unwrap(); @@ -92,7 +99,7 @@ fn details_text_pretty_with_title() { let (printer, out, _) = Printer::memory(OutputFormat::TextPretty); let rows = vec![DetailRow::scalar("Key", "Value")]; - print_details(&printer, Some("My Title"), rows); + print_details(&printer, Some("My Title"), rows, &json!({})); let output = flush_stdout(&printer, &out); assert!(output.contains("My Title")); @@ -105,7 +112,7 @@ fn details_text_renders_markdown() { let (printer, out, _) = Printer::memory(OutputFormat::Text); let rows = vec![DetailRow::scalar("color", "red")]; - print_details(&printer, None, rows); + print_details(&printer, None, rows, &json!({})); let output = flush_stdout(&printer, &out); assert!(output.contains('|'), "expected pipe-delimited output"); @@ -114,20 +121,22 @@ fn details_text_renders_markdown() { } #[test] -fn details_json_compact() { +fn details_json_emits_payload_not_rows() { let (printer, out, _) = Printer::memory(OutputFormat::Json); - let rows = vec![ - DetailRow::scalar("name", "jp"), - DetailRow::scalar("version", "1.0"), - ]; + // Display rows and JSON payload deliberately disagree: display labels are + // Title Case prose, payload keys are a snake_case contract. + let rows = vec![DetailRow::scalar("Version (semver)", "v1.0")]; + let payload = json!({"name": "jp", "version": "1.0"}); - print_details(&printer, Some("info"), rows); + print_details(&printer, Some("info"), rows, &payload); let output = flush_stdout(&printer, &out); let parsed: serde_json::Value = serde_json::from_str(output.trim()).unwrap(); - assert_eq!(parsed["title"], "info"); - assert_eq!(parsed["details"]["name"], "jp"); - assert_eq!(parsed["details"]["version"], "1.0"); + assert_eq!(parsed, payload); + assert!( + !output.contains("Version (semver)"), + "display labels must not leak into JSON" + ); assert!(!output.trim().contains('\n'), "expected compact JSON"); } @@ -136,24 +145,26 @@ fn details_json_pretty_is_indented() { let (printer, out, _) = Printer::memory(OutputFormat::JsonPretty); let rows = vec![DetailRow::scalar("k", "v")]; - print_details(&printer, None, rows); + print_details(&printer, None, rows, &json!({"k": "v"})); let output = flush_stdout(&printer, &out); let parsed: serde_json::Value = serde_json::from_str(output.trim()).unwrap(); - assert_eq!(parsed["details"]["k"], "v"); + assert_eq!(parsed["k"], "v"); assert!(output.contains("\n "), "expected indented JSON"); } #[test] -fn details_no_title_json() { +fn details_title_stays_out_of_json() { let (printer, out, _) = Printer::memory(OutputFormat::Json); let rows = vec![DetailRow::scalar("a", "b")]; - print_details(&printer, None, rows); + print_details(&printer, Some("Decorative Title"), rows, &json!({"a": "b"})); let output = flush_stdout(&printer, &out); + // The title is a display concern; the payload alone defines the JSON. let parsed: serde_json::Value = serde_json::from_str(output.trim()).unwrap(); - assert!(parsed["title"].is_null()); + assert_eq!(parsed, json!({"a": "b"})); + assert!(!output.contains("Decorative Title")); } #[test] diff --git a/crates/jp_storage/src/load.rs b/crates/jp_storage/src/load.rs index de19a85e3..53b4c6661 100644 --- a/crates/jp_storage/src/load.rs +++ b/crates/jp_storage/src/load.rs @@ -148,6 +148,19 @@ impl Storage { } } +/// The conversation IDs projected in a checkout's storage directory. +/// +/// A bare directory scan of the active (non-archived) partition: no workspace +/// construction, no user-local merge, no registry writes. +/// `jp w show` uses this to union checkout-only conversations across a +/// workspace's sibling roots without paying a full workspace load per root — +/// one loaded checkout already contributes every user-local conversation, so +/// the siblings can only add conversations that live in their projection alone. +#[must_use] +pub fn projected_conversation_ids(storage_dir: &Utf8Path) -> Vec { + scan_conversation_ids(&storage_dir.join(CONVERSATIONS_DIR)) +} + /// Scan a single conversations directory for IDs. /// /// Dot-prefixed entries (`.trash/`, leftover `.tmp`/`.staging-`/`.old-` dirs diff --git a/crates/jp_term/src/table.rs b/crates/jp_term/src/table.rs index 3f0e70b97..2b30ee24b 100644 --- a/crates/jp_term/src/table.rs +++ b/crates/jp_term/src/table.rs @@ -173,33 +173,6 @@ pub fn list_markdown(header: Row, rows: Vec) -> String { out } -/// Render a list table as a JSON array of objects. -/// -/// Each row becomes an object keyed by the header cell content. -#[must_use] -#[expect(clippy::needless_pass_by_value)] -pub fn list_json(header: Row, rows: Vec) -> serde_json::Value { - let headers: Vec = header - .cell_iter() - .map(|c| strip_ansi_escapes::strip_str(c.content())) - .collect(); - - let items: Vec = rows - .iter() - .map(|row| { - let mut obj = serde_json::Map::new(); - for (idx, cell) in row.cell_iter().enumerate() { - let key = headers.get(idx).cloned().unwrap_or_else(|| idx.to_string()); - let val = strip_ansi_escapes::strip_str(cell.content()); - obj.insert(key, val.into()); - } - serde_json::Value::Object(obj) - }) - .collect(); - - serde_json::Value::Array(items) -} - /// Render a key-value details table with no borders. #[must_use] pub fn details(title: Option<&str>, rows: Vec) -> String { @@ -310,54 +283,6 @@ fn md_row(label: Option<&str>, value: &str) -> Row { r } -/// Render key-value details as JSON. -/// -/// A scalar value becomes a string; a list value becomes a JSON array. -#[must_use] -pub fn details_json(title: Option<&str>, rows: Vec) -> serde_json::Value { - let mut details = serde_json::Map::new(); - for DetailRow { label, value } in rows { - match label { - Some(label) => { - details.insert(strip(&label), detail_json_value(value)); - } - // A label-less row has no key of its own; emit each value as a key - // with an empty value, matching the label-less column rendering - // used by listing commands. - None => match value { - DetailValue::Scalar(s) => { - details.insert(strip(&s), String::new().into()); - } - DetailValue::List(items) => { - for item in items { - details.insert(strip(&item.text), String::new().into()); - } - } - }, - } - } - - serde_json::json!({ - "title": title, - "details": details, - }) -} - -fn detail_json_value(value: DetailValue) -> serde_json::Value { - match value { - DetailValue::Scalar(s) => strip(&s).into(), - DetailValue::List(items) => items - .into_iter() - .map(|item| item.json) - .collect::>() - .into(), - } -} - -fn strip(s: &str) -> String { - strip_ansi_escapes::strip_str(s) -} - /// Find the maximum column count across all rows. fn max_columns(rows: &[&Row]) -> usize { rows.iter() diff --git a/crates/jp_term/src/table_tests.rs b/crates/jp_term/src/table_tests.rs index 5979a1758..544ef73ba 100644 --- a/crates/jp_term/src/table_tests.rs +++ b/crates/jp_term/src/table_tests.rs @@ -99,70 +99,22 @@ fn markdown_details_list_expands_to_one_row_per_item() { assert!(lines[1].contains("b://y"), "got: {output}"); } -#[test] -fn json_details_list_of_plain_items_is_string_array() { - let json = details_json(None, vec![DetailRow::list("Attachments", vec![ - DetailItem::plain("a://x"), - DetailItem::plain("b://y"), - ])]); - - assert_eq!( - json["details"]["Attachments"], - serde_json::json!(["a://x", "b://y"]) - ); -} - #[test] fn list_item_text_and_json_forms_can_differ() { let item = DetailItem::new( "cmd (Desc): cmd://x", serde_json::json!({ "scheme": "cmd", "url": "cmd://x" }), ); - let rows = vec![DetailRow::list("Attachments", vec![item])]; - // Pretty uses the text form. + // Pretty uses the text form; the structured form rides along for callers + // assembling machine-readable payloads. + let rows = vec![DetailRow::list("Attachments", vec![item.clone()])]; assert!( - details(None, rows.clone()).contains("- cmd (Desc): cmd://x"), + details(None, rows).contains("- cmd (Desc): cmd://x"), "text form should drive the pretty view" ); - - // JSON uses the structured form. - let json = details_json(None, rows); - assert_eq!(json["details"]["Attachments"][0]["scheme"], "cmd"); - assert_eq!(json["details"]["Attachments"][0]["url"], "cmd://x"); -} - -#[test] -fn json_details_bare_row_uses_value_as_key() { - let json = details_json(None, vec![DetailRow::bare("a://x")]); - assert_eq!(json["details"]["a://x"], ""); -} - -#[test] -fn json_list() { - let json = list_json(header(), rows()); - let arr = json.as_array().unwrap(); - assert_eq!(arr.len(), 2); - assert_eq!(arr[0]["Name"], "Alice"); - assert_eq!(arr[0]["Age"], "30"); - assert_eq!(arr[1]["Name"], "Bob"); - assert_eq!(arr[1]["Age"], "7"); -} - -#[test] -fn json_details() { - let json = details_json(Some("title"), vec![DetailRow::scalar("ID", "jp-c123")]); - assert_eq!(json["title"], "title"); - assert_eq!(json["details"]["ID"], "jp-c123"); -} - -#[test] -fn json_details_strips_ansi() { - let json = details_json(None, vec![DetailRow::scalar( - "\x1b[1mKey\x1b[0m", - "\x1b[32mVal\x1b[0m", - )]); - assert_eq!(json["details"]["Key"], "Val"); + assert_eq!(item.json["scheme"], "cmd"); + assert_eq!(item.json["url"], "cmd://x"); } #[test]