From 2eee184001631693f09f3e52fe500e41ffda8407 Mon Sep 17 00:00:00 2001 From: Yuto Nishida Date: Tue, 11 Aug 2026 08:01:08 -0700 Subject: [PATCH] [silverwood] Split workstream creation into register + checkout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `silverwood new basic` gains a required `--checkout-extent full|skip`. `full` is the existing behavior (register, then provision the checkout before returning). `skip` registers the workstream and returns immediately, leaving the checkout unprovisioned in a new `CheckoutState::InitializedWithoutCheckout` state — distinct from `pending` (actively provisioning). A new `silverwood workstream basic checkout` subcommand provisions a skipped workstream: it validates the kind is basic and that the workstream is still awaiting checkout (else `Error::NotAwaitingCheckout`), then runs the same pending -> ready/failed provisioning. In core, `Forest::create_workstream` is split so precheck (validation) runs for both extents while provisioning is gated on the extent, with a shared `provision_checkout` tail reused by `checkout_workstream`. Because validation still happens at register time, a `skip` create rejects a bad seed synchronously while deferring the slow clone/copy — the accept boundary papyrus needs. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../crates/silverwood-cli/src/main.rs | 99 +++++++++++- .../crates/silverwood-cli/tests/cli.rs | 97 +++++++++++- .../crates/silverwood-cli/tests/common/mod.rs | 2 + .../crates/silverwood-cli/tests/e2e.rs | 66 ++++++++ .../crates/silverwood-core/src/error.rs | 10 ++ .../crates/silverwood-core/src/forest.rs | 144 +++++++++++++----- .../crates/silverwood-core/src/lib.rs | 2 +- .../crates/silverwood-core/src/workstream.rs | 58 ++++++- .../silverwood-core/tests/common/mod.rs | 10 +- .../silverwood-core/tests/provisioning.rs | 76 ++++++++- 10 files changed, 504 insertions(+), 60 deletions(-) diff --git a/experimental/silverwood/crates/silverwood-cli/src/main.rs b/experimental/silverwood/crates/silverwood-cli/src/main.rs index 6436021..54bfaa8 100644 --- a/experimental/silverwood/crates/silverwood-cli/src/main.rs +++ b/experimental/silverwood/crates/silverwood-cli/src/main.rs @@ -11,11 +11,11 @@ use std::path::PathBuf; use std::process::ExitCode; use std::str::FromStr; -use clap::{Parser, Subcommand}; +use clap::{Parser, Subcommand, ValueEnum}; use silverwood_core::{ - agent_shell_plan, base_shell_plan, AbsolutePath, AgentKind, AgentSession, CheckoutState, - DoctorReport, Forest, HttpsGitUrl, LocationWithinForest, NewCheckoutMode, NewKind, - NewWorkstream, SpawnSeed, UpgradeReport, Workstream, WorkstreamId, DOC_SCHEMA_VERSION, + agent_shell_plan, base_shell_plan, AbsolutePath, AgentKind, AgentSession, CheckoutExtent, + CheckoutState, DoctorReport, Forest, HttpsGitUrl, LocationWithinForest, NewCheckoutMode, + NewKind, NewWorkstream, SpawnSeed, UpgradeReport, Workstream, WorkstreamId, DOC_SCHEMA_VERSION, }; /// Frontend-agnostic backend for the code you work on and the agent sessions @@ -40,7 +40,8 @@ enum Command { /// Open the forest (creating it if absent) and print its identity. Info, - /// Create a workstream, provisioning its checkout. + /// Create a workstream. Whether its checkout is provisioned now or deferred is + /// chosen per kind (basic: `--checkout-extent full|skip`). New { /// Human-friendly name (accepted at any `new` subcommand level). #[arg(long, global = true)] @@ -86,6 +87,15 @@ enum Command { name: String, }, + /// Per-kind workstream management operations. The `` subcommand (e.g. + /// `basic`) is validated against the workstream's actual kind. + Workstream { + /// The workstream id (from `silverwood ls`). + id: String, + #[command(subcommand)] + kind: WorkstreamCommand, + }, + /// Namespaced key-value state (frontend-owned). #[command(subcommand)] Kv(KvCommand), @@ -254,11 +264,54 @@ enum SessionCreate { enum NewVariant { /// A basic workstream, materialized by a checkout mode. Basic { + /// Whether to provision the checkout now (`full`) or register the workstream and + /// defer provisioning to `silverwood workstream basic checkout` (`skip`). + #[arg(long, value_enum)] + checkout_extent: CheckoutExtentArg, #[command(subcommand)] mode: NewModeArg, }, } +/// CLI spelling of [`CheckoutExtent`] for `new basic --checkout-extent`. +#[derive(Clone, Copy, ValueEnum)] +enum CheckoutExtentArg { + /// Provision the checkout now, before the command returns. + Full, + /// Register only; provision later via `workstream basic checkout`. + Skip, +} + +impl From for CheckoutExtent { + fn from(arg: CheckoutExtentArg) -> Self { + match arg { + CheckoutExtentArg::Full => CheckoutExtent::Full, + CheckoutExtentArg::Skip => CheckoutExtent::Skip, + } + } +} + +/// Per-kind management operations for `silverwood workstream …`. The kind +/// subcommand is validated against the workstream's actual kind at dispatch. Only +/// `basic` today (`WorkstreamKind` is `#[non_exhaustive]`); the kebab name matches the +/// stored `kind` tag. +#[derive(Subcommand)] +enum WorkstreamCommand { + /// Operations on a basic workstream. + Basic { + #[command(subcommand)] + op: BasicOp, + }, +} + +/// Operations on a basic workstream (`silverwood workstream basic `). +#[derive(Subcommand)] +enum BasicOp { + /// Provision the checkout of a workstream created with `--checkout-extent skip`. + /// Fails if the workstream has already been checked out (or is mid-provision). + Checkout, +} + /// A `basic` workstream's checkout mode — the second `new` subcommand level. Each variant /// carries that mode's creation seed as positionals; the kebab name matches the stored /// `checkout_mode` tag. @@ -465,7 +518,9 @@ fn run(cli: Cli) -> CliResult { let id = parse_id(&id)?; forest.archive(id)?; let ws = forest.get(id)?; - emit(json, &workstream_json(&ws), || println!("archived {}", ws.id)); + emit(json, &workstream_json(&ws), || { + println!("archived {}", ws.id) + }); Ok(()) } @@ -473,7 +528,9 @@ fn run(cli: Cli) -> CliResult { let id = parse_id(&id)?; forest.remove(id, force)?; let ws = forest.get(id)?; - emit(json, &workstream_json(&ws), || println!("deleted {}", ws.id)); + emit(json, &workstream_json(&ws), || { + println!("deleted {}", ws.id) + }); Ok(()) } @@ -485,6 +542,8 @@ fn run(cli: Cli) -> CliResult { Ok(()) } + Command::Workstream { id, kind } => run_workstream(&forest, json, &id, kind), + Command::Kv(cmd) => run_kv(&forest, json, cmd), Command::Session(cmd) => run_session(&forest, json, cmd), @@ -507,17 +566,41 @@ fn run(cli: Cli) -> CliResult { fn run_new(forest: &Forest, json: bool, name: Option, variant: NewVariant) -> CliResult { let name = name.ok_or("`--name ` is required")?; - let NewVariant::Basic { mode } = variant; + let NewVariant::Basic { + mode, + checkout_extent, + } = variant; let ws = forest.create_workstream(NewWorkstream { name, kind: NewKind::Basic { mode: mode.into_new_mode()?, + checkout_extent: checkout_extent.into(), }, })?; emit(json, &workstream_json(&ws), || print_workstream(&ws)); Ok(()) } +fn run_workstream(forest: &Forest, json: bool, id: &str, kind: WorkstreamCommand) -> CliResult { + let id = parse_id(id)?; + let WorkstreamCommand::Basic { op } = kind; + + // The `basic` subcommand asserts the workstream's kind; reject a mismatch so the + // per-kind operation only runs on the kind it was written for. + let actual = forest.get(id)?.body.kind.tag(); + if actual != "basic" { + return Err(format!("workstream {id} is not a basic workstream (it is {actual})").into()); + } + + match op { + BasicOp::Checkout => { + let ws = forest.checkout_workstream(id)?; + emit(json, &workstream_json(&ws), || print_workstream(&ws)); + Ok(()) + } + } +} + fn run_kv(forest: &Forest, json: bool, cmd: KvCommand) -> CliResult { match cmd { KvCommand::Set { diff --git a/experimental/silverwood/crates/silverwood-cli/tests/cli.rs b/experimental/silverwood/crates/silverwood-cli/tests/cli.rs index 0a8f73d..1adabe9 100644 --- a/experimental/silverwood/crates/silverwood-cli/tests/cli.rs +++ b/experimental/silverwood/crates/silverwood-cli/tests/cli.rs @@ -4,6 +4,8 @@ mod common; +use std::path::Path; + use common::{fails, forest, json, run}; #[test] @@ -66,6 +68,8 @@ fn new_with_non_https_source_is_rejected_without_cloning() { &[ "new", "basic", + "--checkout-extent", + "full", "jj-colocated", "git@github.com:a/b.git", "--name", @@ -77,6 +81,8 @@ fn new_with_non_https_source_is_rejected_without_cloning() { &[ "new", "basic", + "--checkout-extent", + "full", "jj-colocated", "http://github.com/a/b.git", "--name", @@ -97,15 +103,29 @@ fn new_subcommand_structure_mirrors_the_data_model() { fails(&dir, &["new"]); fails(&dir, &["new", "basic"]); + // `basic` demands its `--checkout-extent` (full|skip); omitting it is a clap error. + let stderr = fails(&dir, &["new", "basic", "jj-colocated", "--name", "x"]); + assert!(stderr.contains("checkout-extent"), "got: {stderr}"); + // A mode leaf demands its seed: the `` positional. - let stderr = fails(&dir, &["new", "basic", "jj-colocated"]); + let stderr = fails( + &dir, + &["new", "basic", "--checkout-extent", "full", "jj-colocated"], + ); assert!(stderr.contains("SOURCE_HTTPS_URL"), "got: {stderr}"); // `--name` is required (enforced in the handler); omitting it fails before the // source is ever validated. let stderr = fails( &dir, - &["new", "basic", "jj-colocated", "http://github.com/a/b.git"], + &[ + "new", + "basic", + "--checkout-extent", + "full", + "jj-colocated", + "http://github.com/a/b.git", + ], ); assert!(stderr.contains("--name"), "got: {stderr}"); @@ -118,6 +138,8 @@ fn new_subcommand_structure_mirrors_the_data_model() { "--name", "x", "basic", + "--checkout-extent", + "full", "jj-colocated", "http://github.com/a/b.git", ], @@ -125,25 +147,48 @@ fn new_subcommand_structure_mirrors_the_data_model() { assert!(stderr.contains("scheme must be https"), "got: {stderr}"); // The apfs-cow mode leaf demands its own seed: an `` positional... - let stderr = fails(&dir, &["new", "basic", "apfs-cow"]); + let stderr = fails( + &dir, + &["new", "basic", "--checkout-extent", "full", "apfs-cow"], + ); assert!(stderr.contains("ABSOLUTE_PATH"), "got: {stderr}"); // ...and it must be absolute — a relative path is rejected before any forest work. let stderr = fails( &dir, - &["new", "basic", "apfs-cow", "relative/path", "--name", "x"], + &[ + "new", + "basic", + "--checkout-extent", + "full", + "apfs-cow", + "relative/path", + "--name", + "x", + ], ); assert!(stderr.contains("absolute"), "got: {stderr}"); // The apfs-cow-direnv-unsafe leaf exists with the same seed: ``, // absolute-only (its extra `direnv allow` doesn't change the creation contract). - let stderr = fails(&dir, &["new", "basic", "apfs-cow-direnv-unsafe"]); + let stderr = fails( + &dir, + &[ + "new", + "basic", + "--checkout-extent", + "full", + "apfs-cow-direnv-unsafe", + ], + ); assert!(stderr.contains("ABSOLUTE_PATH"), "got: {stderr}"); let stderr = fails( &dir, &[ "new", "basic", + "--checkout-extent", + "full", "apfs-cow-direnv-unsafe", "relative/path", "--name", @@ -156,6 +201,48 @@ fn new_subcommand_structure_mirrors_the_data_model() { assert_eq!(json(&dir, &["--json", "ls"]), serde_json::json!([])); } +#[test] +fn new_skip_registers_without_provisioning() { + let dir = forest(); + + // `--checkout-extent skip` registers the workstream but does not clone, so this runs + // in the no-network sandbox: the checkout rests at `initialized-without-checkout` and + // nothing is materialized on disk. + let ws = json( + &dir, + &[ + "--json", + "new", + "basic", + "--checkout-extent", + "skip", + "jj-colocated", + "https://github.com/octocat/Hello-World.git", + "--name", + "deferred", + ], + ); + assert_eq!( + ws["overall_state"], + "active - basic.initialized-without-checkout" + ); + assert_eq!(ws["mode"]["state"], "initialized-without-checkout"); + + // No working copy was provisioned. + let location = ws["location"]["within"]["path"].as_str().unwrap(); + assert!( + !Path::new(location).exists(), + "skip must not materialize a checkout: {location}" + ); + + // `show` reflects the same deferred state. + let id = ws["id"].as_str().unwrap(); + assert_eq!( + json(&dir, &["--json", "show", id])["overall_state"], + "active - basic.initialized-without-checkout" + ); +} + #[test] fn show_rejects_bad_and_absent_ids() { let dir = forest(); diff --git a/experimental/silverwood/crates/silverwood-cli/tests/common/mod.rs b/experimental/silverwood/crates/silverwood-cli/tests/common/mod.rs index 0878f3e..310a174 100644 --- a/experimental/silverwood/crates/silverwood-cli/tests/common/mod.rs +++ b/experimental/silverwood/crates/silverwood-cli/tests/common/mod.rs @@ -90,6 +90,8 @@ pub fn create(dir: &TempDir, name: &str) -> String { "--json", "new", "basic", + "--checkout-extent", + "full", "jj-colocated", EXAMPLE_SOURCE, "--name", diff --git a/experimental/silverwood/crates/silverwood-cli/tests/e2e.rs b/experimental/silverwood/crates/silverwood-cli/tests/e2e.rs index ec8f37d..823793f 100644 --- a/experimental/silverwood/crates/silverwood-cli/tests/e2e.rs +++ b/experimental/silverwood/crates/silverwood-cli/tests/e2e.rs @@ -69,6 +69,8 @@ fn new_creates_a_ready_colocated_checkout() { "--json", "new", "basic", + "--checkout-extent", + "full", "jj-colocated", EXAMPLE_SOURCE, "--name", @@ -113,6 +115,56 @@ fn new_creates_a_ready_colocated_checkout() { assert_eq!(json(&dir, &["--json", "show", id]), listed[0]); } +/// `--checkout-extent skip` registers without cloning; `workstream basic checkout` +/// then provisions it, and a second checkout is rejected. The register step needs no +/// network, but the checkout step clones, so this is `#[ignore]`d like the rest. +#[test] +#[ignore = "network + jj; run via `cargo test -- --ignored`"] +fn skip_then_checkout_provisions_and_rejects_recheckout() { + let dir = forest(); + + // Register with the checkout deferred: no clone yet. + let ws = json( + &dir, + &[ + "--json", + "new", + "basic", + "--checkout-extent", + "skip", + "jj-colocated", + EXAMPLE_SOURCE, + "--name", + "deferred", + ], + ); + let id = ws["id"].as_str().unwrap().to_string(); + assert_eq!( + ws["overall_state"], + "active - basic.initialized-without-checkout" + ); + let location = ws["location"]["within"]["path"] + .as_str() + .unwrap() + .to_string(); + assert!( + !Path::new(&location).exists(), + "skip must not clone: {location}" + ); + + // Checking out clones now and flips to ready. + let checked_out = json(&dir, &["--json", "workstream", &id, "basic", "checkout"]); + assert_eq!(checked_out["overall_state"], "active - basic.ready"); + assert!( + Path::new(&location).join("README.md").is_file(), + "checkout must be provisioned on disk" + ); + + // A second checkout is rejected — it is no longer awaiting one. + let stderr = fails(&dir, &["workstream", &id, "basic", "checkout"]); + assert!(stderr.contains("not awaiting checkout"), "got: {stderr}"); +} + /// The apfs-cow mode copy-on-write clones a local directory into the checkout. No /// network, but it needs a real APFS volume (macOS temp dirs are APFS) and shells out /// to `/bin/cp -c`, so it is `#[ignore]`d like the rest and must run on macOS. The @@ -133,6 +185,8 @@ fn new_apfs_cow_creates_a_ready_checkout() { "--json", "new", "basic", + "--checkout-extent", + "full", "apfs-cow", src_path, "--name", @@ -179,6 +233,8 @@ fn new_apfs_cow_direnv_unsafe_creates_a_ready_checkout() { "--json", "new", "basic", + "--checkout-extent", + "full", "apfs-cow-direnv-unsafe", src_path, "--name", @@ -218,6 +274,8 @@ fn new_direnv_unsafe_mode_is_ready() { "--json", "new", "basic", + "--checkout-extent", + "full", "jj-colocated-direnv-unsafe", EXAMPLE_SOURCE, "--name", @@ -247,6 +305,8 @@ fn spawn_plan_reflects_checkout_mode() { "--json", "new", "basic", + "--checkout-extent", + "full", "jj-colocated", EXAMPLE_SOURCE, "--name", @@ -275,6 +335,8 @@ fn spawn_plan_reflects_checkout_mode() { "--json", "new", "basic", + "--checkout-extent", + "full", "jj-colocated-direnv-unsafe", EXAMPLE_SOURCE, "--name", @@ -476,6 +538,8 @@ fn archive_tombstones_but_keeps_checkout_and_persists() { "--json", "new", "basic", + "--checkout-extent", + "full", "jj-colocated", EXAMPLE_SOURCE, "--name", @@ -521,6 +585,8 @@ fn remove_deletes_checkout_but_keeps_deleted_tombstone() { "--json", "new", "basic", + "--checkout-extent", + "full", "jj-colocated", EXAMPLE_SOURCE, "--name", diff --git a/experimental/silverwood/crates/silverwood-core/src/error.rs b/experimental/silverwood/crates/silverwood-core/src/error.rs index 0b7721f..15a6c7a 100644 --- a/experimental/silverwood/crates/silverwood-core/src/error.rs +++ b/experimental/silverwood/crates/silverwood-core/src/error.rs @@ -45,6 +45,16 @@ pub enum Error { #[error("workstream not found: {0}")] NotFound(WorkstreamId), + /// A checkout was requested on a workstream that is not awaiting one — it was not + /// created with the checkout deferred (`--checkout-extent skip`), or has already + /// been checked out (or is mid-provision). Only an `initialized-without-checkout` + /// workstream can be checked out. + #[error("workstream {id} is not awaiting checkout (state: {state})")] + NotAwaitingCheckout { + id: WorkstreamId, + state: &'static str, + }, + /// A workstream was not deemed safe to remove (and `--force` was not given). #[error("workstream {0} is not safe to remove; pass --force to remove anyway")] UnsafeToRemove(WorkstreamId), diff --git a/experimental/silverwood/crates/silverwood-core/src/forest.rs b/experimental/silverwood/crates/silverwood-core/src/forest.rs index e266e99..fdded15 100644 --- a/experimental/silverwood/crates/silverwood-core/src/forest.rs +++ b/experimental/silverwood/crates/silverwood-core/src/forest.rs @@ -6,6 +6,7 @@ use std::path::{Path, PathBuf}; use time::format_description::well_known::Rfc3339; use time::OffsetDateTime; +use loro::LoroDoc; use serde::Serialize; use crate::apfs; @@ -17,9 +18,9 @@ use crate::id::{ForestId, WorkstreamId}; use crate::migrate; use crate::provider::{CheckoutProvider, JjColocated}; use crate::workstream::{ - AgentKind, CheckoutMode, CheckoutState, DoctorReport, Location, LocationWithinForest, - NewCheckoutMode, NewKind, NewWorkstream, SessionLock, Status, Workstream, WorkstreamBody, - WorkstreamKind, RESERVED_NS_PREFIX, + AgentKind, CheckoutExtent, CheckoutMode, CheckoutState, DoctorReport, Location, + LocationWithinForest, NewCheckoutMode, NewKind, NewWorkstream, SessionLock, Status, Workstream, + WorkstreamBody, WorkstreamKind, RESERVED_NS_PREFIX, }; /// The outcome for one document in a [`Forest::upgrade_all`] pass. @@ -109,50 +110,39 @@ impl Forest { &self.config } - /// Create a workstream, provisioning its checkout. + /// Create a workstream, provisioning its checkout unless deferred. /// - /// The document is written first with the checkout `Pending`, then the - /// checkout is provisioned, then the state is flipped to `Ready`/`Failed` - /// in place. A failed provision leaves a recoverable workstream (its - /// document persists with state `Failed`) and surfaces the error. + /// The document is written first (with the checkout `Pending` for a `Full` + /// create, or `InitializedWithoutCheckout` for a `Skip` create). A `Full` create + /// then provisions the checkout and flips the state to `Ready`/`Failed` in place; + /// a failed provision leaves a recoverable workstream (its document persists with + /// state `Failed`) and surfaces the error. A `Skip` create returns immediately with + /// the checkout unprovisioned, to be materialized later by [`Self::checkout_workstream`]. pub fn create_workstream(&self, new: NewWorkstream) -> Result { - let NewKind::Basic { mode: new_mode } = &new.kind; + let NewKind::Basic { + mode: new_mode, + checkout_extent, + } = &new.kind; let working_copies = self.root.join(WORKING_COPIES_DIR); // Mode-specific creation preconditions that must reject *before* anything is // persisted (a hard failure, unlike a provisioning error, which leaves a - // recoverable `Failed` document). Today only apfs-cow has one. + // recoverable `Failed` document). Today only apfs-cow has one. Runs for both + // extents, so a deferred (`Skip`) create still rejects a bad seed up front. precheck_new_mode(new_mode, &working_copies)?; let id = WorkstreamId::generate(); let dest = working_copies.join(id.to_string()); - // The stored mode starts `pending`; core flips it after provisioning. Each - // NewCheckoutMode maps to its matching pending CheckoutMode (dest/location and - // the provision call below are mode-independent). - let mode = match new_mode { - NewCheckoutMode::JjColocated { initial_source } => CheckoutMode::JjColocated { - initial_source: initial_source.as_str().to_string(), - state: CheckoutState::Pending, - }, - NewCheckoutMode::JjColocatedDirenvUnsafe { initial_source } => { - CheckoutMode::JjColocatedDirenvUnsafe { - initial_source: initial_source.as_str().to_string(), - state: CheckoutState::Pending, - } - } - NewCheckoutMode::ApfsCow { source_path } => CheckoutMode::ApfsCow { - initial_source: source_path.as_str().to_string(), - state: CheckoutState::Pending, - }, - NewCheckoutMode::ApfsCowDirenvUnsafe { source_path } => { - CheckoutMode::ApfsCowDirenvUnsafe { - initial_source: source_path.as_str().to_string(), - state: CheckoutState::Pending, - } - } + // The stored mode's initial state depends on the extent: `Full` starts `Pending` + // (core flips it after provisioning below); `Skip` rests at + // `InitializedWithoutCheckout`. dest/location are mode- and extent-independent. + let initial_state = match checkout_extent { + CheckoutExtent::Full => CheckoutState::Pending, + CheckoutExtent::Skip => CheckoutState::InitializedWithoutCheckout, }; + let mode = stored_mode(new_mode, initial_state); // The location records this forest as the single materialization site. let body = WorkstreamBody { @@ -171,19 +161,69 @@ impl Forest { kv: BTreeMap::new(), }; - // Persist the pending document, then provision, then record the outcome - // by mutating the same in-memory document in place. let doc = doc::build(self.peer_id(), &body)?; self.docs.save(id, &doc::snapshot(&doc)?)?; - let provisioned = self.provider.provision(new_mode, &dest); + match checkout_extent { + // Registered only; the caller provisions later via `checkout_workstream`. + CheckoutExtent::Skip => self.get(id), + // Provision now, recording the outcome in place. + CheckoutExtent::Full => self.provision_checkout(id, &doc, new_mode, &dest), + } + } + + /// Provision the checkout of a workstream that was created with the checkout + /// deferred (`InitializedWithoutCheckout`). + /// + /// Validates the workstream is basic and has not been checked out, then runs the + /// same `Pending` → `Ready`/`Failed` provisioning as a `Full` create. Errors + /// [`Error::NotAwaitingCheckout`] (leaving the document untouched) if it is already + /// checked out, mid-provision, or previously failed. + pub fn checkout_workstream(&self, id: WorkstreamId) -> Result { + let bytes = self.docs.load(id)?.ok_or(Error::NotFound(id))?; + let ws = doc::hydrate(id, &bytes)?; + + // Basic is the only kind, so this destructure is irrefutable in-crate; a future + // non-checkout kind would branch here. + let WorkstreamKind::Basic { mode, location } = &ws.body.kind; + if mode.state() != CheckoutState::InitializedWithoutCheckout { + return Err(Error::NotAwaitingCheckout { + id, + state: mode.state().as_str(), + }); + } + + let new_mode = mode.to_new_mode()?; + let LocationWithinForest::BasicForest { path } = &location.within; + let dest = PathBuf::from(path); + + let doc = doc::load(self.peer_id(), &bytes)?; + self.provision_checkout(id, &doc, &new_mode, &dest) + } + + /// Provision `new_mode` into `dest` for the already-persisted workstream `id`, + /// marking it `Pending` while the provider runs and recording `Ready`/`Failed` + /// afterward — mutating the same in-memory document in place so lineage is + /// preserved. Surfaces a provisioning error after persisting the recoverable + /// `Failed` state. Shared by a `Full` create and [`Self::checkout_workstream`]. + fn provision_checkout( + &self, + id: WorkstreamId, + doc: &LoroDoc, + new_mode: &NewCheckoutMode, + dest: &Path, + ) -> Result { + doc::set_state(doc, CheckoutState::Pending)?; + self.docs.save(id, &doc::snapshot(doc)?)?; + + let provisioned = self.provider.provision(new_mode, dest); let state = if provisioned.is_ok() { CheckoutState::Ready } else { CheckoutState::Failed }; - doc::set_state(&doc, state)?; - self.docs.save(id, &doc::snapshot(&doc)?)?; + doc::set_state(doc, state)?; + self.docs.save(id, &doc::snapshot(doc)?)?; provisioned?; self.get(id) @@ -487,6 +527,32 @@ fn ensure_dir(path: &Path) -> Result<()> { fs::create_dir_all(path).map_err(|e| Error::io(path, e)) } +/// Build the stored [`CheckoutMode`] for a new checkout from its creation-side seed, +/// stamping the given initial provisioning `state`. dest/location are handled by the +/// caller; only the mode's seed and state are set here. +fn stored_mode(new_mode: &NewCheckoutMode, state: CheckoutState) -> CheckoutMode { + match new_mode { + NewCheckoutMode::JjColocated { initial_source } => CheckoutMode::JjColocated { + initial_source: initial_source.as_str().to_string(), + state, + }, + NewCheckoutMode::JjColocatedDirenvUnsafe { initial_source } => { + CheckoutMode::JjColocatedDirenvUnsafe { + initial_source: initial_source.as_str().to_string(), + state, + } + } + NewCheckoutMode::ApfsCow { source_path } => CheckoutMode::ApfsCow { + initial_source: source_path.as_str().to_string(), + state, + }, + NewCheckoutMode::ApfsCowDirenvUnsafe { source_path } => CheckoutMode::ApfsCowDirenvUnsafe { + initial_source: source_path.as_str().to_string(), + state, + }, + } +} + /// Validate a mode's creation preconditions that must reject *before* any document /// is persisted. A no-op for modes without one. /// diff --git a/experimental/silverwood/crates/silverwood-core/src/lib.rs b/experimental/silverwood/crates/silverwood-core/src/lib.rs index 4c7b309..d6375b0 100644 --- a/experimental/silverwood/crates/silverwood-core/src/lib.rs +++ b/experimental/silverwood/crates/silverwood-core/src/lib.rs @@ -38,7 +38,7 @@ pub use provider::{CheckoutProvider, JjColocated}; pub use source::{AbsolutePath, HttpsGitUrl}; pub use spawn::{agent_shell_plan, base_shell_plan, ShellPlan, SpawnSeed}; pub use workstream::{ - AgentKind, AgentSession, CheckoutMode, CheckoutState, DoctorReport, Location, + AgentKind, AgentSession, CheckoutExtent, CheckoutMode, CheckoutState, DoctorReport, Location, LocationWithinForest, NewCheckoutMode, NewKind, NewWorkstream, SessionLock, Status, Workstream, WorkstreamBody, WorkstreamKind, }; diff --git a/experimental/silverwood/crates/silverwood-core/src/workstream.rs b/experimental/silverwood/crates/silverwood-core/src/workstream.rs index 0ea922f..df25e9f 100644 --- a/experimental/silverwood/crates/silverwood-core/src/workstream.rs +++ b/experimental/silverwood/crates/silverwood-core/src/workstream.rs @@ -2,6 +2,7 @@ use std::collections::BTreeMap; use serde::{Deserialize, Serialize}; +use crate::error::Result; use crate::id::{ForestId, WorkstreamId}; use crate::source::{AbsolutePath, HttpsGitUrl}; @@ -81,9 +82,14 @@ pub enum CheckoutMode { /// Provisioning state of a per-forest checkout. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] -#[serde(rename_all = "lowercase")] +#[serde(rename_all = "kebab-case")] pub enum CheckoutState { - /// The document exists; the working copy is not yet provisioned. + /// Registered with the checkout deliberately deferred (`new … --checkout-extent + /// skip`): the document exists but no working copy was ever provisioned. Distinct + /// from `Pending`; a workstream in this state can be provisioned later via + /// `Forest::checkout_workstream` (`silverwood workstream … checkout`). + InitializedWithoutCheckout, + /// The document exists and its working copy is actively being provisioned. Pending, /// The working copy has been provisioned successfully. Ready, @@ -171,11 +177,38 @@ impl CheckoutMode { | CheckoutMode::ApfsCowDirenvUnsafe { initial_source, .. } => initial_source, } } + + /// Reconstruct the creation-side [`NewCheckoutMode`] from this stored mode by + /// re-parsing its `initial_source` back into the typed seed. Used to re-drive the + /// provider when checking out a workstream that was created with the checkout + /// deferred (`InitializedWithoutCheckout`). The stored source was validated at + /// creation, so a parse failure here means the document is corrupt. + pub fn to_new_mode(&self) -> Result { + Ok(match self { + CheckoutMode::JjColocated { initial_source, .. } => NewCheckoutMode::JjColocated { + initial_source: HttpsGitUrl::parse(initial_source)?, + }, + CheckoutMode::JjColocatedDirenvUnsafe { initial_source, .. } => { + NewCheckoutMode::JjColocatedDirenvUnsafe { + initial_source: HttpsGitUrl::parse(initial_source)?, + } + } + CheckoutMode::ApfsCow { initial_source, .. } => NewCheckoutMode::ApfsCow { + source_path: AbsolutePath::parse(initial_source)?, + }, + CheckoutMode::ApfsCowDirenvUnsafe { initial_source, .. } => { + NewCheckoutMode::ApfsCowDirenvUnsafe { + source_path: AbsolutePath::parse(initial_source)?, + } + } + }) + } } impl CheckoutState { pub(crate) fn as_str(self) -> &'static str { match self { + CheckoutState::InitializedWithoutCheckout => "initialized-without-checkout", CheckoutState::Pending => "pending", CheckoutState::Ready => "ready", CheckoutState::Failed => "failed", @@ -412,9 +445,25 @@ pub enum NewKind { Basic { /// How to materialize the checkout (carries its seed; `state` is core-owned). mode: NewCheckoutMode, + /// Whether to provision the checkout now or defer it (see [`CheckoutExtent`]). + checkout_extent: CheckoutExtent, }, } +/// Whether creation should provision the checkout synchronously or only register the +/// workstream and defer provisioning. `Full` runs the (possibly slow) provider before +/// returning — the workstream comes back `Ready`/`Failed`. `Skip` returns immediately +/// with the checkout `InitializedWithoutCheckout`, to be provisioned later via +/// [`crate::Forest::checkout_workstream`]. Only meaningful for kinds that have a +/// checkout (today: `Basic`). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum CheckoutExtent { + /// Provision the checkout now, before returning. + Full, + /// Register only; leave the checkout unprovisioned (`InitializedWithoutCheckout`). + Skip, +} + /// Creation-side counterpart to [`CheckoutMode`]: the caller supplies the /// mode-specific seed, but never the provisioning `state` (core owns that /// lifecycle — it mints `pending` then flips to `ready`/`failed`). Mirrors the @@ -466,6 +515,7 @@ mod tests { serde_json::json!(Status::Deleted.as_str()) ); for state in [ + CheckoutState::InitializedWithoutCheckout, CheckoutState::Pending, CheckoutState::Ready, CheckoutState::Failed, @@ -565,6 +615,10 @@ mod tests { basic_body(Status::Deleted, CheckoutState::Failed).overall_state(), "deleted - basic.failed" ); + assert_eq!( + basic_body(Status::Active, CheckoutState::InitializedWithoutCheckout).overall_state(), + "active - basic.initialized-without-checkout" + ); // The kind segment alone is `.`. let body = basic_body(Status::Active, CheckoutState::Pending); diff --git a/experimental/silverwood/crates/silverwood-core/tests/common/mod.rs b/experimental/silverwood/crates/silverwood-core/tests/common/mod.rs index a724a07..a7c6c03 100644 --- a/experimental/silverwood/crates/silverwood-core/tests/common/mod.rs +++ b/experimental/silverwood/crates/silverwood-core/tests/common/mod.rs @@ -5,7 +5,7 @@ use std::path::{Path, PathBuf}; use silverwood_core::{ - CheckoutProvider, HttpsGitUrl, NewCheckoutMode, NewKind, NewWorkstream, Result, + CheckoutExtent, CheckoutProvider, HttpsGitUrl, NewCheckoutMode, NewKind, NewWorkstream, Result, }; /// A fresh, unique temp dir for an isolated forest. @@ -25,8 +25,13 @@ impl CheckoutProvider for FakeOk { } } -/// A `NewWorkstream` with a valid public HTTPS source. +/// A `NewWorkstream` with a valid public HTTPS source, provisioned in full. pub fn new_ws(name: &str) -> NewWorkstream { + new_ws_extent(name, CheckoutExtent::Full) +} + +/// A `NewWorkstream` with a valid public HTTPS source and the given checkout extent. +pub fn new_ws_extent(name: &str, checkout_extent: CheckoutExtent) -> NewWorkstream { NewWorkstream { name: name.into(), kind: NewKind::Basic { @@ -34,6 +39,7 @@ pub fn new_ws(name: &str) -> NewWorkstream { initial_source: HttpsGitUrl::parse("https://github.com/octocat/Hello-World.git") .unwrap(), }, + checkout_extent, }, } } diff --git a/experimental/silverwood/crates/silverwood-core/tests/provisioning.rs b/experimental/silverwood/crates/silverwood-core/tests/provisioning.rs index f9c8ba2..31be3f1 100644 --- a/experimental/silverwood/crates/silverwood-core/tests/provisioning.rs +++ b/experimental/silverwood/crates/silverwood-core/tests/provisioning.rs @@ -8,10 +8,10 @@ mod common; use std::path::Path; -use common::{new_ws, temp_forest, FakeOk}; +use common::{new_ws, new_ws_extent, temp_forest, FakeOk}; use silverwood_core::{ - CheckoutProvider, CheckoutState, Error, Forest, LocationWithinForest, NewCheckoutMode, Result, - Status, + CheckoutExtent, CheckoutProvider, CheckoutState, Error, Forest, LocationWithinForest, + NewCheckoutMode, Result, Status, }; /// A provider that always fails provisioning. @@ -106,6 +106,76 @@ fn failed_provision_is_recoverable() { let _ = std::fs::remove_dir_all(&dir); } +#[test] +fn skip_defers_provisioning_then_checkout_readies() { + let dir = temp_forest("skip-ready"); + let forest = Forest::open_with_provider(&dir, Box::new(FakeOk)).unwrap(); + + // A skip create registers the workstream without provisioning its checkout. + let ws = forest + .create_workstream(new_ws_extent("deferred", CheckoutExtent::Skip)) + .unwrap(); + assert_eq!( + ws.body.state(), + Some(CheckoutState::InitializedWithoutCheckout) + ); + let LocationWithinForest::BasicForest { path } = &ws.body.location().unwrap().within else { + panic!("expected a basic-forest location"); + }; + assert!( + !Path::new(path).exists(), + "skip must not provision the checkout (FakeOk would have created it)" + ); + + // Checking it out runs the provider and flips to Ready. + let ready = forest.checkout_workstream(ws.id).unwrap(); + assert_eq!(ready.body.state(), Some(CheckoutState::Ready)); + assert!( + Path::new(path).is_dir(), + "checkout must now be provisioned on disk" + ); + + // A second checkout is rejected — it is no longer awaiting one. + assert!(matches!( + forest.checkout_workstream(ws.id).unwrap_err(), + Error::NotAwaitingCheckout { .. } + )); + + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn skip_then_failed_checkout_records_failed() { + let dir = temp_forest("skip-fail"); + let forest = Forest::open_with_provider(&dir, Box::new(FakeFail)).unwrap(); + + // The skip create SUCCEEDS despite the always-failing provider — proof it never + // provisioned at creation time. + let ws = forest + .create_workstream(new_ws_extent("deferred", CheckoutExtent::Skip)) + .unwrap(); + assert_eq!( + ws.body.state(), + Some(CheckoutState::InitializedWithoutCheckout) + ); + + // Checking out runs the failing provider and records a recoverable Failed. + let err = forest.checkout_workstream(ws.id).unwrap_err(); + assert!(matches!(err, Error::Provision(_))); + assert_eq!( + forest.get(ws.id).unwrap().body.state(), + Some(CheckoutState::Failed) + ); + + // A Failed checkout is not re-checkout-able via this command. + assert!(matches!( + forest.checkout_workstream(ws.id).unwrap_err(), + Error::NotAwaitingCheckout { .. } + )); + + let _ = std::fs::remove_dir_all(&dir); +} + #[test] fn persists_across_reopen() { let dir = temp_forest("reopen"); -- 2.54.0