diff --git a/crates/jp_cli/src/bootstrap.rs b/crates/jp_cli/src/bootstrap.rs index 23d85fa53..22d026a1f 100644 --- a/crates/jp_cli/src/bootstrap.rs +++ b/crates/jp_cli/src/bootstrap.rs @@ -46,11 +46,18 @@ pub(crate) enum WorkspaceRequirement { /// plugin child cwd, path parsing — simply do not run. None, - /// Resolve and validate a target root, without loading the conversation + /// The workspace is the command's *subject* rather than its context. + /// + /// `jp w show ` reports on a workspace instead of running inside one, + /// and `jp w use ` records a selection for later runs to resolve. + /// Both resolve their own [`WorkspaceTarget`] against the pre-workspace + /// [`TargetEnv`], from outside every workspace and possibly to no workspace + /// at all, so selecting a root on their behalf would answer a question they + /// have not asked. + Subject, + + /// Resolve a root, construct the [`Workspace`], and load the conversation /// index. - Resolve, - - /// Resolve, construct the [`Workspace`], and load the conversation index. Load, } @@ -74,8 +81,7 @@ pub(crate) enum RootSource { /// w use`). SessionActive, - /// The interactive picker fallback, recorded as the session's new active - /// workspace. + /// The interactive picker fallback, resolving this run only. Picker, } @@ -191,10 +197,14 @@ impl ExecutionContext { /// 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. +/// 6. Else the picker, resolving this run only. /// /// Non-interactive runs ignore the session layer entirely (steps 2–3 and /// 5–6), so scripts never depend on hidden per-session state. +/// +/// Resolution may *repair* the session's recorded selection when its checkout +/// is gone, but never creates one: attaching a workspace to a session is `jp w +/// use`'s job, or the `C` / `A` answers to the conflict prompt. pub(crate) fn resolve( target: Option<&WorkspaceTarget>, session: Option<&Session>, @@ -261,7 +271,7 @@ fn source_for(target: &WorkspaceTarget) -> RootSource { WorkspaceTarget::Session | WorkspaceTarget::SessionPicker | WorkspaceTarget::Picker - | WorkspaceTarget::Latest + | WorkspaceTarget::Recent | WorkspaceTarget::Fuzzy(_) => RootSource::CliSelector, WorkspaceTarget::Cwd | WorkspaceTarget::Help => { unreachable!("resolved before source mapping") @@ -311,7 +321,7 @@ fn ladder(env: &TargetEnv<'_>, session: &Session) -> Result<(Utf8PathBuf, RootSo } // Step 6: the picker. - (None, None) => picker(env, session), + (None, None) => picker(env), } } @@ -531,22 +541,15 @@ fn apply_conflict_choice( /// 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)> { +/// The choice resolves this run only. +/// Taking no session identity is the point: a fallback the user was never asked +/// to keep cannot quietly become the answer to every later run from the same +/// terminal. +fn picker(env: &TargetEnv<'_>) -> 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)) } diff --git a/crates/jp_cli/src/cmd.rs b/crates/jp_cli/src/cmd.rs index cfd105653..0af2f7ee7 100644 --- a/crates/jp_cli/src/cmd.rs +++ b/crates/jp_cli/src/cmd.rs @@ -114,12 +114,12 @@ impl Commands { /// 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. + /// declaration drives the startup dispatch, so a command reaches exactly + /// the pre-workspace work it asked for. pub(crate) fn workspace_requirement(&self) -> WorkspaceRequirement { match self { Commands::Init(_) => WorkspaceRequirement::None, - Commands::Workspace(args) => args.workspace_requirement(), + Commands::Workspace(_) => WorkspaceRequirement::Subject, Commands::Query(_) | Commands::Config(_) | Commands::Conversation(_) diff --git a/crates/jp_cli/src/cmd/conversation/use_.rs b/crates/jp_cli/src/cmd/conversation/use_.rs index babc17229..80b5637be 100644 --- a/crates/jp_cli/src/cmd/conversation/use_.rs +++ b/crates/jp_cli/src/cmd/conversation/use_.rs @@ -19,8 +19,10 @@ use crate::{ /// Set the active conversation. /// -/// Without flags, `jp c use [ID]` activates the given conversation (or opens a -/// picker when no target is provided). +/// `jp c use ` activates the given conversation. +/// Bare `jp c use` returns to the session's previously active conversation, the +/// way `cd -` returns to the previous directory, and opens a picker only when +/// the session has no previous conversation to return to. /// `--grep` and `--created-since`/`--created-before` restrict the picker's /// candidate set; when the combined filter leaves a single conversation, it is /// activated directly without prompting. diff --git a/crates/jp_cli/src/cmd/workspace.rs b/crates/jp_cli/src/cmd/workspace.rs index 0e255a580..f8d54bdff 100644 --- a/crates/jp_cli/src/cmd/workspace.rs +++ b/crates/jp_cli/src/cmd/workspace.rs @@ -19,10 +19,7 @@ use jp_printer::Printer; use jp_workspace::session::Session; use target::{TargetEnv, WorkspaceTarget}; -use crate::{ - bootstrap::WorkspaceRequirement, - cmd::{self, Output}, -}; +use crate::cmd::{self, Output}; /// Manage workspaces. #[derive(Debug, clap::Args)] @@ -72,19 +69,6 @@ impl Workspace { 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, - } - } } impl Commands { diff --git a/crates/jp_cli/src/cmd/workspace/show.rs b/crates/jp_cli/src/cmd/workspace/show.rs index fdb91b617..742825dd2 100644 --- a/crates/jp_cli/src/cmd/workspace/show.rs +++ b/crates/jp_cli/src/cmd/workspace/show.rs @@ -176,7 +176,7 @@ impl Show { Ok(Some(subject_for(env, id, "session history"))) } - WorkspaceTarget::Latest => Ok(roots::known_workspaces( + WorkspaceTarget::Recent => Ok(roots::known_workspaces( &env.workspaces_dir, DEFAULT_STORAGE_DIR, ) diff --git a/crates/jp_cli/src/cmd/workspace/target.rs b/crates/jp_cli/src/cmd/workspace/target.rs index cc4483d5e..13031fe82 100644 --- a/crates/jp_cli/src/cmd/workspace/target.rs +++ b/crates/jp_cli/src/cmd/workspace/target.rs @@ -12,7 +12,7 @@ //! | `?` | 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 | +//! | `r`, `recent` | 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 | @@ -64,9 +64,9 @@ pub(crate) enum WorkspaceTarget { /// `s` / `session` — the session's previously active workspace. Session, - /// `l` / `latest` — the live root with the newest `last_used` across the + /// `r` / `recent` — the live root with the newest `last_used` across the /// roots registry (global recency, distinct from `s`). - Latest, + Recent, /// `cwd` / `.` — the cwd-derived workspace. /// @@ -98,7 +98,7 @@ impl FromStr for WorkspaceTarget { "?" => Self::Picker, "?s" | "?session" => Self::SessionPicker, "s" | "session" => Self::Session, - "l" | "latest" => Self::Latest, + "r" | "recent" => Self::Recent, "cwd" | "." => Self::Cwd, "-" => Self::Stdin, "help" => Self::Help, @@ -119,7 +119,7 @@ pub(crate) fn help() -> String { ? 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 + r, recent 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 @@ -314,7 +314,7 @@ pub(crate) fn resolve(target: &WorkspaceTarget, env: &TargetEnv<'_>) -> Result latest_root(env) + WorkspaceTarget::Recent => recent_root(env) .ok_or(no_known_workspaces().into()) .map(ResolvedTarget::Root), @@ -407,7 +407,7 @@ pub(crate) fn pick_known_workspace( } /// The live root with the newest `last_used` across every known workspace. -fn latest_root(env: &TargetEnv<'_>) -> Option { +fn recent_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) diff --git a/crates/jp_cli/src/cmd/workspace/target_tests.rs b/crates/jp_cli/src/cmd/workspace/target_tests.rs index 9d4cef3ba..9a12058ab 100644 --- a/crates/jp_cli/src/cmd/workspace/target_tests.rs +++ b/crates/jp_cli/src/cmd/workspace/target_tests.rs @@ -112,12 +112,12 @@ fn keywords_parse() { WorkspaceTarget::Session )); assert!(matches!( - WorkspaceTarget::from_str("l").unwrap(), - WorkspaceTarget::Latest + WorkspaceTarget::from_str("r").unwrap(), + WorkspaceTarget::Recent )); assert!(matches!( - WorkspaceTarget::from_str("latest").unwrap(), - WorkspaceTarget::Latest + WorkspaceTarget::from_str("recent").unwrap(), + WorkspaceTarget::Recent )); assert!(matches!( WorkspaceTarget::from_str("cwd").unwrap(), @@ -301,7 +301,7 @@ fn session_target_errors_without_a_previous_entry() { } #[test] -fn latest_resolves_the_newest_live_checkout() { +fn recent_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"); @@ -309,7 +309,7 @@ fn latest_resolves_the_newest_live_checkout() { 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 { + let ResolvedTarget::Root(selected) = resolve(&WorkspaceTarget::Recent, &env).unwrap() else { panic!("expected a resolved root"); }; assert_eq!(selected.root, b); @@ -317,11 +317,11 @@ fn latest_resolves_the_newest_live_checkout() { } #[test] -fn latest_errors_with_an_empty_registry() { +fn recent_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(); + let error = resolve(&WorkspaceTarget::Recent, &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 index f3c5179bd..84f10c6d4 100644 --- a/crates/jp_cli/src/cmd/workspace/use_.rs +++ b/crates/jp_cli/src/cmd/workspace/use_.rs @@ -1,7 +1,7 @@ use chrono::Utc; use crossterm::style::Stylize as _; use jp_printer::Printer; -use tracing::warn; +use tracing::{debug, warn}; use crate::cmd::{ Output, @@ -12,8 +12,11 @@ use crate::cmd::{ /// /// 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. +/// Bare `jp w use` returns to the session's previously active workspace, the +/// way `cd -` returns to the previous directory, and opens the picker only when +/// the session has no previous workspace to return to. +/// `jp w use ?` always picks; `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 @@ -24,7 +27,8 @@ pub(crate) struct Use { /// See `jp w use help` for the grammar. /// /// Also settable with the global `--workspace` flag, but not both at once. - /// Defaults to the picker (`?`). + /// Defaults to the previously active workspace (`s`), or the picker (`?`) + /// when the session has none. pub(super) target: Option, /// Keep using this workspace even from inside another one. @@ -42,9 +46,7 @@ pub(crate) struct Use { 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) { + if matches!(self.target, Some(WorkspaceTarget::Help)) { printer.println(target::help()); return Ok(()); } @@ -72,7 +74,9 @@ impl Use { // `cwd` drops the record the sticky flag would live on, so the two // together ask for a selection that is both absent and permanent. - if self.always && matches!(target, WorkspaceTarget::Cwd) { + // Only an explicit target can be `cwd`; the bare default below never + // resolves to it. + if self.always && matches!(self.target, Some(WorkspaceTarget::Cwd)) { return Err(format!( "`{}` clears the session-active workspace, so there is nothing for `{}` to keep \ active.", @@ -87,7 +91,7 @@ impl Use { let previous = mapping.and_then(|mapping| mapping.history.into_iter().next()); let suffix = sticky_suffix(self.always, was_sticky); - match target::resolve(&target, env)? { + match resolve_target(self.target, env)? { ResolvedTarget::Help => unreachable!("handled before resolution"), // Clearing is just selecting the cwd-derived workspace: the @@ -173,6 +177,27 @@ impl Use { } } +/// Resolve the workspace this invocation selects. +/// +/// A bare invocation returns to the workspace the session came from, like `cd +/// -`, and falls back to the picker when there is no live one to return to. +fn resolve_target( + target: Option, + env: &TargetEnv<'_>, +) -> crate::error::Result { + if let Some(target) = target { + return target::resolve(&target, env); + } + + match target::resolve(&WorkspaceTarget::Session, env) { + Ok(resolved) => Ok(resolved), + Err(error) => { + debug!(%error, "No previous workspace to return to; picking instead."); + target::resolve(&WorkspaceTarget::Picker, env) + } + } +} + /// The trailing clause naming what the invocation did to the session's sticky /// flag, empty when it leaves the flag off. fn sticky_suffix(always: bool, was_sticky: bool) -> &'static str { diff --git a/crates/jp_cli/src/cmd/workspace/use_tests.rs b/crates/jp_cli/src/cmd/workspace/use_tests.rs index 47252209e..ff606bdb3 100644 --- a/crates/jp_cli/src/cmd/workspace/use_tests.rs +++ b/crates/jp_cli/src/cmd/workspace/use_tests.rs @@ -364,6 +364,55 @@ fn use_cwd_clears_the_selection() { assert!(env.store.load(&session).is_none()); } +// Bare `jp w use` is the `cd -` of workspaces, mirroring bare `jp c use`: it +// returns to where the session came from rather than asking again. +#[test] +fn bare_use_returns_to_the_previous_workspace() { + let tmp = tempdir().unwrap(); + let first = make_workspace(tmp.path(), "first", "ws123"); + let second = make_workspace(tmp.path(), "second", "ws456"); + 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(first.clone())), + always: false, + } + .run(&printer, &env) + .unwrap(); + + let (printer, _out, _err) = Printer::memory(OutputFormat::Text); + Use { + target: Some(WorkspaceTarget::Path(second)), + always: false, + } + .run(&printer, &env) + .unwrap(); + + // `first` is now `history[1]`, so a bare `use` switches back to it without + // a prompt. Both selections registered their checkouts, so falling through + // to the picker would have two rows to offer and would try to prompt, + // failing without a terminal — the assertions below cannot pass that way. + let (printer, out, _err) = Printer::memory(OutputFormat::Text); + Use { + target: None, + always: false, + } + .run(&printer, &env) + .unwrap(); + + let active = env.store.active(&session).expect("active entry"); + assert_eq!(active.workspace_id, "ws123"); + assert_eq!(active.root, first); + + let stdout = stdout_of(&printer, &out); + assert!( + stdout.contains("Switched the session-active workspace"), + "unexpected output: {stdout}" + ); +} + #[test] fn a_path_without_a_workspace_id_is_rejected() { let tmp = tempdir().unwrap(); diff --git a/crates/jp_cli/src/lib.rs b/crates/jp_cli/src/lib.rs index 7651649df..5009e7422 100644 --- a/crates/jp_cli/src/lib.rs +++ b/crates/jp_cli/src/lib.rs @@ -577,48 +577,58 @@ fn build_printer(globals: &Globals, format: OutputFormat) -> Printer { fn run_inner(cli: Cli, format: OutputFormat) -> Result<()> { let printer = build_printer(&cli.globals, format); - // `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(); - - // The global `--workspace` flag names the workspace `use` and `show` - // act on here, rather than the one the run operates from. - let output = args - .run( - &printer, - session.as_ref(), - cli.globals.persist, - cli.globals.workspace.as_ref(), - cli.globals.no_interactive, - ) - .map_err(Into::into); + // The per-command workspace bootstrap requirement (RFD 087) drives the + // startup dispatch: each arm performs exactly the pre-workspace work its + // commands declared, so no command can reach a path another arm owns. + let requirement = cli.command.workspace_requirement(); + match requirement { + // Nothing to resolve: `jp init` creates the workspace another command + // would need selected, so the consumers that assume a root — config + // loading, MCP and plugin child cwd, path parsing — do not run. + WorkspaceRequirement::None => { + let Commands::Init(args) = cli.command else { + unreachable!("`None` is declared by `jp init` alone"); + }; - // `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 args + .run(&printer, cli.globals.no_interactive) + .map_err(Into::into); + } - return output; - } + // `jp w` takes a workspace as its subject: it resolves its own target + // against the pre-workspace `TargetEnv` — from outside every + // workspace, possibly to no workspace at all — so it needs a session + // identity but never a selected root, and never a `Ctx`. + WorkspaceRequirement::Subject => { + let Commands::Workspace(args) = cli.command else { + unreachable!("`Subject` is declared by `jp workspace` alone"); + }; - // 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"); - }; + trace!("Resolving session identity."); + let session = session::resolve(); + + // The global `--workspace` flag names the workspace `use` and + // `show` act on here, rather than the one the run operates from. + let output = args + .run( + &printer, + session.as_ref(), + cli.globals.persist, + cli.globals.workspace.as_ref(), + cli.globals.no_interactive, + ) + .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; + } - return args - .run(&printer, cli.globals.no_interactive) - .map_err(Into::into); + // Resolves a root below, then constructs the workspace and loads the + // conversation index. + WorkspaceRequirement::Load => {} } // The pre-workspace bootstrap (RFD 087): session identity and the @@ -650,26 +660,22 @@ fn run_inner(cli: Cli, format: OutputFormat) -> Result<()> { let (mut workspace, fs_backend) = load_workspace(&exec.root, cli.globals.persist, LoadIntent::Run)?; - // `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" - ); - } + 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(); } + // Populate the conversation index. This does NOT load the contents of + // individual conversations, this is done lazily as needed. + workspace.load_conversation_index(); + // `--no-cfg` is shorthand for a leading `--cfg=NONE`, applied to config // resolution only. `Globals.config` stays as the user typed it: commands // re-consume the raw `--cfg` args (e.g. `config set` persists them), and @@ -1207,7 +1213,7 @@ pub(crate) enum LoadIntent { /// Reuses an existing user-workspace directory read-only and writes /// nothing: no directory creation, no migration, no import, no registry /// entry, no ID write. - /// Inspecting a workspace therefore cannot change which checkout `latest` + /// Inspecting a workspace therefore cannot change which checkout `recent` /// resolves to, nor mint user-local state for a workspace the user never /// ran a command in. Inspect, diff --git a/crates/jp_workspace/src/roots.rs b/crates/jp_workspace/src/roots.rs index 69ba427df..ef7f72260 100644 --- a/crates/jp_workspace/src/roots.rs +++ b/crates/jp_workspace/src/roots.rs @@ -45,7 +45,7 @@ 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 +/// Recency only feeds display ordering and `recent` 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 diff --git a/docs/.vitepress/rfd-summaries.json b/docs/.vitepress/rfd-summaries.json index a5c556f04..4dd73b082 100644 --- a/docs/.vitepress/rfd-summaries.json +++ b/docs/.vitepress/rfd-summaries.json @@ -148,7 +148,7 @@ "summary": "Introduces `await` built-in tool to synchronize on parallel stateful tool handles with `any`/`all` completion modes." }, "039-conversation-trees.md": { - "hash": "a40e2fab5d72dfa57e3b4a00affd43e49047dce03286fa3e7239fc46209ed5c2", + "hash": "aede63689185e2491d065558cbdf601555af5d5a0dcc4241f1b335e54695b74e", "summary": "Add parent-child conversation relationships via metadata field, enabling fork lineage and hierarchical organization without nested directories." }, "040-hidden-conversations-and-tool-context.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": "ce808d79fe9ffe5cf10f8bf87bdfe1a62ef9e6c09bc439f110a5e51bf63e613d", + "hash": "9dc586dc628538bf0c6002027ef8eed36865dad6ca197df72235554643fef9f0", "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": { @@ -422,5 +422,9 @@ "105-mid-conversation-operator-directives.md": { "hash": "0ac17903569c11f2c6e09ccfc1a349aa708c2762fe815d1cf195a56fbd240726", "summary": "Mid-conversation system and tool changes anchored in message stream preserve Anthropic's prompt cache via positional directives." + }, + "109-hierarchy-targets-and-explicit-selection-clearing.md": { + "hash": "cdd25fa1134995e0a7bb479a396a398adf628a6cc363b12f53716400a66367a5", + "summary": "Hierarchy targets `.`, `..`, `../..`, `/` navigate workspace/conversation nesting; `--clear` flag replaces `cwd` for clearing selections." } } diff --git a/docs/rfd/039-conversation-trees.md b/docs/rfd/039-conversation-trees.md index f38757935..8593cd3b6 100644 --- a/docs/rfd/039-conversation-trees.md +++ b/docs/rfd/039-conversation-trees.md @@ -119,6 +119,25 @@ with the conversation through git — team members who pull a conversation also get its parent reference. Reparenting a conversation is an edit to a single file. +### Depth is unbounded + +Trees nest to any depth: a child conversation can itself have children. +A `parent_id` chain of arbitrary length is a supported shape, not an accident of +the storage format, and every operation defined below is written against it. +`--promote` reparents children onto the *grandparent* rather than onto the root, +`--tree` indents each level under its own parent, and `--root=` renders a +subtree from any node rather than only from a top-level conversation. + +A sub-agent delegating to a further sub-agent is the motivating case: the +hierarchy that produces is three levels deep before any user has organized +anything by hand. + +Only two limits apply. +A conversation has exactly one parent (see [Non-Goals](#non-goals)), so the +structure is a tree rather than a graph, and a `parent_id` cycle is invalid — +the tree index treats a conversation reachable from itself as a root and warns, +the same way it treats a missing parent. + ### Tree index On workspace load, JP builds an in-memory tree index from the `parent_id` fields @@ -323,6 +342,42 @@ pub fn has_children(&self, id: &ConversationId) -> bool; These methods read from the in-memory tree index, not from disk. +### Path-shaped targeting + +Once conversations form a hierarchy, the natural way to name a position in it is +the vocabulary users already have for hierarchies. +The intended grammar, as an extension of `ConversationTarget`: + +| Target | Meaning | +| ------- | ------------------------------------------------ | +| `.` | the session's active conversation (exists today) | +| `..` | the parent of the active conversation | +| `../..` | its grandparent, and so on for any depth | +| `/` | the root of the active conversation's tree | + +The segments compose against whatever the target resolves to, so `..` applied to +a root conversation is an error naming the conversation that has no parent, +rather than silently resolving to the root itself. + +The grammar is deliberately shared with workspace targeting ([RFD 087]), where +the same segments walk the workspace-nesting hierarchy instead: `jp -w .` is the +current workspace whichever subdirectory you stand in, while `jp c show ..` is +the active conversation's parent. +One navigation vocabulary, two hierarchies — which is what makes `.` worth +keeping as a conversation target rather than a lone borrowed character. + +The segments resolve against the hierarchy, never against directories: `.`, +`..`, `../..`, and `/` are keywords, and a filesystem path is spelled with a +named segment (`./foo`, `../foo`, `/foo/bar`). +That distinction is what lets `..` mean "one level up the hierarchy" rather than +"one directory up", which are different questions on the workspace axis and only +the first of which has an answer on the conversation axis. + +This RFD does not implement the grammar; it establishes the tree the grammar +addresses. +`..` and `/` need `parent_of` and a walk to the root, both of which the tree +index provides. + ### Interaction with RFD 020 (Parallel Conversations) [RFD 020] introduces conversation locks and per-session conversation tracking. @@ -456,6 +511,10 @@ self-contained. - **Cross-tree references.** A conversation can only be a child of one parent. Having a conversation appear in multiple trees is not supported. +- **The path-shaped targeting grammar itself.** [Path-shaped + targeting](#path-shaped-targeting) records the intended spelling so the tree + is designed to support it, but `..`, `../..`, and `/` are a separate change. + - **Tree-level config overrides.** Config inheritance flows from parent to child at creation time (via [RFD 038]). There is no mechanism to change a parent's config and have it propagate to @@ -595,3 +654,4 @@ Depends on Phase 1. [RFD 046]: 046-nested-workspace-projection.md [RFD 050]: 050-scripting-ergonomics-for-conversation-management.md [RFD 051]: 051-sub-agent-workflows.md +[RFD 087]: 087-session-scoped-active-workspace.md diff --git a/docs/rfd/087-session-scoped-active-workspace.md b/docs/rfd/087-session-scoped-active-workspace.md index ffa2617cd..e4dda67ef 100644 --- a/docs/rfd/087-session-scoped-active-workspace.md +++ b/docs/rfd/087-session-scoped-active-workspace.md @@ -6,6 +6,7 @@ - **Date**: 2026-06-01 - **Extends**: [RFD 020] - **Tracking Issue**: [\#793] +- **Extended by**: [RFD 109] ## Summary @@ -138,23 +139,32 @@ keeps the launch-cwd / root / child-cwd distinction from collapsing again. Not every command needs a workspace selected, so each command declares its requirement — the workspace-level analog of today's per-command `conversation_load_request` (`jp_cli::cmd`). -The bootstrap step reads this declaration and only runs the resolution ladder -(steps 5–6 above) when the command asks for it: +The declaration drives the startup dispatch, so a command reaches exactly the +pre-workspace work it asked for: - **none** — no workspace is bootstrapped. - `jp w ls` reads the user-global registries only; `jp w use cwd` just clears - the session record; `jp init` is unchanged. -- **resolve** — resolve and validate a target root to record a selection, - without loading the conversation index. - `jp w use ?` and `jp w use ` need the root, not the conversation data. -- **load** — resolve, construct `Workspace`, and load the conversation index. - `jp q`, `jp w show`, and most existing commands. + `jp init` creates the workspace another command would need selected. +- **subject** — the workspace is the command's *subject* rather than its + context. + `jp w` resolves its own target against the pre-workspace environment, from + outside every workspace and possibly to no workspace at all, so the bootstrap + selects no root on its behalf. + `jp w show ` reports *on* a workspace instead of running *inside* one; `jp + w use ` records a selection for later runs to resolve; `jp w ls` reads the + user-global registries only. +- **load** — run the resolution ladder, construct `Workspace`, and load the + conversation index. + `jp q` and every other existing command. + +A fourth *resolve* mode — a validated root without the conversation index — +was specified for `jp w use` and has no client: a subject command resolves its +own target, and nothing else wants a root it will not read conversations from. +It is omitted rather than kept for a hypothetical caller. The bootstrap handoff therefore has a *no workspace selected* form. -For `none` commands — and for `resolve` / `load` commands that legitimately -resolve to no workspace, such as `jp w show` from outside any workspace with -nothing active — the downstream consumers that assume a root (config loading, -MCP / plugin child cwd, path parsing) simply do not run. +For `none` and `subject` commands — including `jp w show` from outside any +workspace with nothing active — the downstream consumers that assume a root +(config loading, MCP / plugin child cwd, path parsing) simply do not run. This makes "absence of a selected workspace" a first-class bootstrap outcome rather than something each command has to fake. @@ -245,7 +255,7 @@ work: - 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` + left untouched, since recency only feeds display ordering and `recent` 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. @@ -352,7 +362,7 @@ Examples below use `jp w` for brevity. read-only and script-friendly. Read-only is literal: inspecting a workspace creates no user-workspace directory, runs no migration or import, writes no registry entry, and leaves - recency untouched, so `jp w show` can never change what `l` / `latest` + recency untouched, so `jp w show` can never change what `r` / `recent` resolves to. - `jp -w `: a per-command workspace override using the same targeting grammar. @@ -441,6 +451,13 @@ Interactive ladder, in order: 5. Else use the session-active workspace when live. 6. Else picker. +The picker at step 6 resolves the run only; it does not record a selection. +Resolution may **repair** a recorded selection whose checkout is gone, but never +creates one — attaching a workspace to a session is `jp w use`'s job, or the +`C` / `A` answers below. +A session that never selected one is asked again on the next run, mirroring the +conversation picker, which likewise leaves the session mapping untouched. + The conflict prompt fires on any difference (different workspace ID *or* a different checkout of the same ID): @@ -467,8 +484,8 @@ This RFD does not pin that signal to a specific mechanism: [RFD 049] is the eventual canonical definition (controlling-terminal availability rather than stdout being a TTY), and RFD 087 inherits whatever the shared signal resolves to as it evolves. -Non-interactively, a workspace-consuming command (bootstrap `load` or `resolve`) -runs from inside a workspace or with an explicit `-w`, and errors otherwise. +Non-interactively, a workspace-consuming command (bootstrap `load`) runs from +inside a workspace or with an explicit `-w`, and errors otherwise. The explicit `-w` accepts only concrete targets — a workspace ``, a path, `cwd` / `.` (resolve from the invocation directory), or `-` (read an ID from stdin). @@ -677,7 +694,7 @@ can be merged independently of the session layer. Move session resolution ahead of workspace construction and add the `jp_cli` bootstrap step that selects the root. Establish the root-as-working-directory invariant for from-anywhere runs. -Add the per-command workspace bootstrap requirement (none / resolve / load), the +Add the per-command workspace bootstrap requirement (none / subject / load), the analog of `conversation_load_request`. Depends on: Phase 1. @@ -713,4 +730,5 @@ Depends on: Phase 3. [RFD 031]: 031-durable-conversation-storage-with-workspace-projection.md [RFD 049]: 049-non-interactive-mode-and-detached-prompt-policy.md [RFD 065]: 065-typed-resource-model-for-attachments.md +[RFD 109]: 109-hierarchy-targets-and-explicit-selection-clearing.md [\#793]: https://github.com/dcdpr/jp/issues/793 diff --git a/docs/rfd/109-hierarchy-targets-and-explicit-selection-clearing.md b/docs/rfd/109-hierarchy-targets-and-explicit-selection-clearing.md new file mode 100644 index 000000000..bab1a630b --- /dev/null +++ b/docs/rfd/109-hierarchy-targets-and-explicit-selection-clearing.md @@ -0,0 +1,313 @@ +# RFD 109: Hierarchy Targets and Explicit Selection Clearing + +- **Status**: Discussion +- **Category**: Design +- **Authors**: Jean Mertz +- **Date**: 2026-09-09 +- **Extends**: [RFD 087] + +## Summary + +`.`, `..`, `../..`, and `/` become hierarchy targets that walk the workspace +nesting chain, and `--clear` replaces `jp w use cwd` as the way to drop a +session's selection. +Together these free `cwd` to mean one thing in every position, and give +workspaces and conversations one navigation vocabulary over two hierarchies. + +## Motivation + +Two defects in the current targeting grammar share a cause. + +**`cwd` means two different things depending on the subcommand.** As a +`--workspace` target it resolves the workspace you are standing in; as a `jp w +use` target it drops the session record entirely. +[RFD 087] justified the overload on the grounds that "clearing is just selecting +the cwd-derived workspace", but the two diverge the moment you `cd`: a recorded +cwd-selection follows the session and fires the conflict prompt, a cleared +record does not. + +**`.` has an unresolved collision.** [RFD 087]'s grammar table lists `cwd`, `.` +for workspaces, and the `[!WARNING]` immediately below it argues that `.` should +be dropped because `ConversationTarget` spells the session's active conversation +`.`. +Neither the table nor the warning has won: the code implements the table, and +the RFD contradicts itself in print. + +The warning frames the collision as two opposite meanings for one character. +That reading leads to dropping `.` from one grammar, which is the wrong +conclusion, because it misidentifies what `.` is. +`.` is not a workspace keyword that happens to clash with a conversation +keyword. +It is the first segment of a *hierarchy navigation vocabulary* that applies to +both axes — the vocabulary whose remaining segments are `..`, `../..`, and `/`. +Workspaces nest, and [RFD 039] gives conversations a parent-child tree. +Both hierarchies want the same words. + +Do nothing and three things persist: a keyword that means "select" or "clear" +depending on where it appears, a published RFD arguing with its own +specification, and no way to say "the workspace above this one" or "the +conversation this one was forked from" without looking up an ID by hand. + +There is also a plain capability gap. +`jp c use` has no way to clear the session's active conversation — no flag, no +keyword. +The only way to a clean slate is a new terminal. + +## Design + +### Hierarchy targets + +Four targets are added to both grammars. +A target composed only of `.`, `..`, and `/` is a hierarchy target; anything +containing a named segment (`./foo`, `../foo`, `/foo/bar`) stays a filesystem +path. + +| Target | Workspace axis | Conversation axis | +| ------- | --------------------------------- | --------------------------------- | +| `.` | the current workspace | the active conversation | +| `..` | the parent workspace | the conversation's parent | +| `../..` | its grandparent, and so on | its grandparent, and so on | +| `/` | the outermost enclosing workspace | the root of the conversation tree | + +```sh +$ jp -w .. c ls # the workspace containing this one +$ jp c show .. # the conversation this one was forked from +$ jp -w / config show # the outermost workspace in the nesting chain +``` + +The segments walk the *hierarchy*, never the directory tree. +`jp -w .` is the workspace you are in whichever subdirectory you stand in, which +is already how `.` behaves today — `Workspace::find_root` walks up until it +finds `.jp`. +Making `..` mean "one level up the workspace hierarchy" rather than "one +directory up" removes an inconsistency rather than introducing one. + +Hierarchy targets resolve from the launch cwd, not from the session's active +workspace, consistent with every other explicit `--workspace` target bypassing +the session layer. + +### Resolving a parent workspace + +Workspace parenthood is derived, not stored. +Given the current root: + +1. Read the current workspace ID. +2. Move up one directory and resolve a root from there. +3. If that root carries the *same* ID, repeat from step 2. +4. The first root carrying a *different* ID is the parent workspace. +5. Reaching the filesystem root without finding one means there is no parent. + +The ID comparison is what makes this correct. +Two checkouts of the same workspace can nest — a git worktree inside its own +repository is the common case, and `roots.rs` already models it. +Stopping at the first root found above the current one would return a sibling +checkout of the same workspace and call it a parent. + +`/` runs the same walk to exhaustion and takes the last root found. +`../..` applies the walk twice. +No new state is stored anywhere: parenthood is a function of directory +containment and ID inequality, computed from `Workspace::find_root` and the ID +file, both of which exist. + +Two failures are distinguished, because they need different guidance: + +- Not in a workspace at all — points at `jp init` or `--workspace `. +- In a workspace that has no enclosing one — says the current workspace is + outermost. + +When nothing nests, `/` resolves to the current workspace and `..` errors. +That is the common case, and the help text says so rather than implying the +segments always have somewhere to go. + +Hierarchy targets are deterministic, read no session state, and never prompt, so +they join ``, a path, and `-` in the set of targets [RFD 087] permits +non-interactively. +Scripts gain hierarchy navigation. + +### `cwd` becomes an ordinary selection + +`jp w use cwd` selects the workspace you are standing in and records it, exactly +as `jp w use ` does, it no longer clears anything. +`jp -w cwd` is unchanged. + +`cwd` and `.` therefore agree in every position, which is the property that made +`.` inconsistent under the old meaning: two near-identical spellings, one +selecting and one clearing. + +### `--clear` + +Both `jp w use --clear` and `jp c use --clear` drop the session record for their +axis. +Clearing is not a target, so it is a flag rather than a word in the grammar. +Putting the absence of a selection into the slot that names selections is what +produced the `cwd` overload. + +Clearing drops the **whole record**, not just the active entry: + +```sh +$ jp w use --clear +Cleared the session's active workspace: ~/Projects/jp + +$ jp c use --clear +Cleared the session's active conversation: jp-c17866928997 +``` + +The alternative — marking "nothing active" while retaining history — needs a +new state in two persisted formats (`WorkspaceSessionMapping` and [RFD 020]'s +`SessionMapping`) that every reader of either must then handle, to save one +picker invocation. +The cost lands permanently on every consumer; the benefit is already available +as `jp w use ?`. + +Two consequences follow and are intended: + +- **`s` and `?s` go dark together.** Dropping the record takes the history with + it, so after `--clear` there is no previous workspace to return to and no + session history to pick from. + That is what a clean slate means. +- **The two axes fall back to different things.** A cleared workspace selection + returns to cwd resolution, an ambient default that always exists. + A cleared conversation selection has no ambient equivalent, so the next + command asks to pick a conversation or create a new one. + +`--clear` with any target is rejected: the invocation would be asking to both +select and not select. + +### `.` stays in both grammars + +[RFD 087]'s warning is resolved by keeping `.` on both axes. +It is the `.` of a navigation vocabulary shared by two hierarchies. +Dropping it from either would leave that grammar with `..` and `/` and no way to +name the position they are relative to. + +### Bare `use` is unaffected + +Bare `jp w use` and bare `jp c use` return to the session's previously active +selection, falling back to a picker. +This RFD does not change that: `--clear` is a distinct request, and `cwd` / `.` +name the current position rather than the previous one. + +## Drawbacks + +**Two user-facing grammar changes at once.** `jp w use cwd` changes meaning +rather than erroring, which is the worst kind of breaking change — a script +that used it to clear will silently record a selection instead. +This is the strongest argument against the proposal and the reason `jp w use +cwd` should error for one release rather than switching meaning quietly. + +**`..` and `/` are inert for most users.** Nested workspaces are uncommon. +Most people will never have a parent workspace, so two thirds of the new grammar +resolves to "no parent workspace" or to the workspace they are already in. + +**Parenthood-by-containment is a definition, not a discovery.** Deriving it from +directory nesting plus ID inequality is cheap and needs no stored state, but it +is a choice. +A user who deliberately nests two unrelated projects gets a parent relationship +they did not ask for. + +## Alternatives + +**Drop `.` from the workspace grammar, as [RFD 087] suggests.** Rejected: it +resolves the collision by conceding the character, which only works while `.` is +the whole vocabulary. +Once `..` and `/` exist, the grammar that lost `.` has no way to name the +position the other segments are relative to. + +**Drop `.` from both grammars and let it mean only a path.** Rejected for the +same reason, and it costs a breaking change to conversation targeting to buy +nothing the position-type separation does not already provide. + +**Keep `cwd` as the clearing target and leave `.` out.** Rejected: it preserves +the overload that caused the problem, and leaves `jp c use` with no way to clear +at all. + +**A `none` or `NONE` keyword instead of `--clear`.** [RFD 038] established +uppercase `NONE` as this project's reset spelling for `--cfg`. +Rejected here because that grammar takes values that are usually paths, where a +keyword needs visual separation, while target grammars already let lowercase +keywords shadow paths. +More importantly, it repeats the original mistake: a word in the target slot +that does not name a target. + +**Path-based `..`.** Rejected: resolving `..` as a directory and then finding a +workspace from there returns the workspace you are already in whenever you stand +in a subdirectory, since `find_root` walks up. +It is a no-op spelling wearing the appearance of navigation. + +## Non-Goals + +- **Conversation-axis `..`, `../..`, and `/`.** The grammar is defined here for + both axes, but the conversation half needs the parent-child tree. + [RFD 039] specifies it and this RFD implements only the workspace half plus + the existing conversation `.`. + +- **A workspace picker that creates a workspace.** After `--clear`, a run from + outside every workspace reaches the picker, and offering "create one here" + there is an `init`-flow change and belongs with that work. + +- **Declared workspace parenthood.** Parenthood is derived from containment. + A config field or manifest declaring an unrelated workspace as a parent is a + larger design question about what a project is. + +- **Multi-target hierarchy segments.** `+..` and similar have no meaning; every + hierarchy target names exactly one thing. + +## Risks and Open Questions + +**Clearing and the end-of-run cleanup.** The cleanup pass prunes session records +whose sources are dead. +Dropping a record explicitly and having it pruned implicitly must not race +within one run. + +## Implementation Plan + +### Phase 1: `--clear` on both axes + +Add `--clear` to `jp w use` and `jp c use`, dropping the whole session record +for their axis. +Reject `--clear` alongside a target. +Add clearing to the conversation session mapping, which has no such operation +today. + +Independently mergeable. +Leaves `jp w use cwd` clearing as well, so nothing breaks yet. + +### Phase 2: `cwd` deprecation window + +`jp w use cwd` errors, naming `--clear` to clear and `jp w use .` to select. +`jp -w cwd` is untouched. + +Depends on Phase 1, so the replacement exists before the error points at it. + +### Phase 3: Workspace hierarchy targets + +Implement the parent walk (ID inequality) and add `.`, `..`, `../..`, and `/` to +`WorkspaceTarget`, with the only-dot-segments parse rule and the two distinct +error messages. +Add them to the non-interactive target set. + +Depends on Phase 2 for `.` on the `use` axis to be unambiguous. +`jp -w` hierarchy targets do not depend on it and could land earlier if the +phases are split further. + +### Phase 4: `cwd` as a selection + +`jp w use cwd` selects and records the current workspace, matching `jp w use .`. + +Depends on Phase 2 having shipped in a release. + +## References + +- [RFD 020] — session-to-conversation mappings, the record `jp c use --clear` + drops. +- [RFD 038] — the `NONE` reset keyword, the nearest precedent for spelling + "reset" in a value position. +- [RFD 039] — the conversation tree, and the conversation-axis half of the + hierarchy grammar. +- [RFD 087] — the workspace targeting grammar, the `cwd` overload, and the `.` + warning this RFD resolves. + +[RFD 020]: 020-parallel-conversations.md +[RFD 038]: 038-config-reset-keywords.md +[RFD 039]: 039-conversation-trees.md +[RFD 087]: 087-session-scoped-active-workspace.md diff --git a/docs/ticket/01ghsz6-classify-workspace-load-side-effects.md b/docs/ticket/01ghsz6-classify-workspace-load-side-effects.md index 458518f41..4c472b3df 100644 --- a/docs/ticket/01ghsz6-classify-workspace-load-side-effects.md +++ b/docs/ticket/01ghsz6-classify-workspace-load-side-effects.md @@ -22,7 +22,7 @@ None of these go through the persist backend, so `--no-persist` does not suppress them. RFD 087 introduced `LoadIntent::{Run, Inspect}` to stop `jp w show` from -reordering `l` / `latest` recency merely by reporting on a workspace. +reordering `r` / `recent` recency merely by reporting on a workspace. That fixed one command by declaring it writes nothing at all. It did not answer the general question, which is what this ticket is for: