From 0ee7ec4102c85996fe077336160167381d41b3a7 Mon Sep 17 00:00:00 2001 From: Mingwei Samuel Date: Fri, 24 Jul 2026 21:21:19 +0000 Subject: [PATCH] feat(sandbox): resume JJ sandboxes at persisted WIP revisions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Persist the absolute JJ working-copy change ID after successful sandbox pushes and use it to initialize a replacement workspace after suspension, rather than recreating from immutable starting provenance or depending on the sandbox bookmark. The existing `starting_revision` remains the fallback for legacy state without a resume revision. Add an end-to-end local JJ restart test: it moves the actual temporary workspace with `jj split`, deletes that workspace and the visible bookmark, gracefully shuts down the RAP server, then starts a fresh server and verifies it restores both the WIP file and exact change ID. Add shutdown-capable test-server support so the test proves restoration is not served from an in-memory backend cache. Update provider/backend push interfaces to return an optional durable WIP revision; Git retains its current behavior and does not yet record one. Co-authored-by: Infinity 🤖 PR: #84 --- crates/sandbox-core/src/git.rs | 4 +- crates/sandbox-core/src/jj.rs | 47 ++++-- crates/sandbox-core/src/sandbox.rs | 12 +- crates/sandbox-core/src/server.rs | 27 ++- crates/sandbox-core/src/types.rs | 73 ++++++++- crates/sandbox-local/src/backend.rs | 10 +- crates/sandbox-local/src/providers.rs | 23 +-- crates/sandbox-local/tests/common.rs | 51 ++++++ .../sandbox-local/tests/describe_changes.rs | 7 +- crates/sandbox-local/tests/jj_resume.rs | 155 ++++++++++++++++++ crates/sandbox-remote/src/backend.rs | 16 +- crates/sandbox-remote/src/metadata.rs | 38 ++++- 12 files changed, 407 insertions(+), 56 deletions(-) create mode 100644 crates/sandbox-local/tests/jj_resume.rs diff --git a/crates/sandbox-core/src/git.rs b/crates/sandbox-core/src/git.rs index ebc9a98e..8c8b4d64 100644 --- a/crates/sandbox-core/src/git.rs +++ b/crates/sandbox-core/src/git.rs @@ -172,7 +172,7 @@ pub async fn detect_mode( .to_owned(), )); }; - let base_revision = output.trim().to_owned(); + let starting_revision = output.trim().to_owned(); let message = match base_bookmark { Some(rev) => format!("Repository initialized on top of {rev}."), @@ -182,7 +182,7 @@ pub async fn detect_mode( // compares it against a canonicalized requested path. let repo_root = repo_path.canonicalize().map_err(SandboxError::Io)?; Ok(Some(ModeInit { - mode: SandboxMode::Git { base_revision }, + mode: SandboxMode::Git { starting_revision }, repo_root, message, precreate: false, diff --git a/crates/sandbox-core/src/jj.rs b/crates/sandbox-core/src/jj.rs index ce07ab11..0049b4ce 100644 --- a/crates/sandbox-core/src/jj.rs +++ b/crates/sandbox-core/src/jj.rs @@ -51,13 +51,19 @@ pub async fn jj_resolve_revision(dir: &Path, rev: &str) -> Result, ) -> Result<(), SandboxError> { let (name, email) = jj_configured_user(Path::new(remote)) .await @@ -75,6 +81,10 @@ pub async fn jj_git_clone( let mut cmd = jj_command( Path::new(remote), &[ + "--config", + &format!("user.name={name}"), + "--config", + &format!("user.email={email}"), "workspace", "add", "-r", @@ -83,10 +93,6 @@ pub async fn jj_git_clone( .expect("bug: sandbox dest path is not valid UTF-8"), ], ); - cmd.arg("--config") - .arg(format!("user.name={name}")) - .arg("--config") - .arg(format!("user.email={email}")); let output = cmd .stdout(Stdio::piped()) .stderr(Stdio::piped()) @@ -109,20 +115,29 @@ pub async fn jj_git_clone( ) .await?; - // Edit (checkout) the bookmark, creating it if it doesn't yet exist. - tracing::info!(bookmark_name, "Edit bookmark"); - if run_jj(dest, &["edit", bookmark_name]).await.is_err() { - tracing::info!(bookmark_name, revision, "Bookmark not found, creating new"); - run_jj(dest, &["new", revision]).await?; + // A persisted change ID identifies the previous WIP directly. Do not use + // the bookmark to resume: it is intentionally allowed to disappear when + // the workspace is suspended. + if let Some(resume_revision) = resume_revision { + run_jj(dest, &["edit", resume_revision]).await?; + } else if run_jj(dest, &["edit", bookmark_name]).await.is_err() { + tracing::info!( + bookmark_name, + starting_revision, + "Bookmark not found, creating new" + ); + run_jj(dest, &["new", starting_revision]).await?; run_jj(dest, &["bookmark", "set", bookmark_name]).await?; } Ok(()) } /// Push the current working copy to the remote. -pub async fn jj_push_working_copy(dir: &Path, bookmark_name: &str) -> Result<(), SandboxError> { +pub async fn jj_push_working_copy(dir: &Path, bookmark_name: &str) -> Result { run_jj(dir, &["bookmark", "set", bookmark_name, "-B"]).await?; - Ok(()) + jj_resolve_revision(dir, "@").await.map_err(|error| { + SandboxError::JujutsuError(format!("failed to resolve sandbox WIP: {error}")) + }) } /// Check if a bookmark has been moved externally (i.e. no longer points to `@`). @@ -231,7 +246,7 @@ pub async fn detect_mode( } else { "@" }; - let Ok(base_revision) = + let Ok(starting_revision) = jj_resolve_revision(repo_path, base_bookmark.unwrap_or(default_rev)).await else { return Err(SandboxError::Other( @@ -250,7 +265,7 @@ pub async fn detect_mode( // compares it against a canonicalized requested path. let repo_root = repo_path.canonicalize().map_err(SandboxError::Io)?; Ok(Some(ModeInit { - mode: SandboxMode::Jj { base_revision }, + mode: SandboxMode::Jj { starting_revision }, repo_root, message, precreate: false, diff --git a/crates/sandbox-core/src/sandbox.rs b/crates/sandbox-core/src/sandbox.rs index 3eefb144..183306b7 100644 --- a/crates/sandbox-core/src/sandbox.rs +++ b/crates/sandbox-core/src/sandbox.rs @@ -109,13 +109,14 @@ pub trait ModeProvider: Send + Sync { None } - /// Push the sandbox working copy back to its source. + /// Push the sandbox working copy back to its source. Returns the provider's + /// durable WIP revision when one can be recorded for later resumption. async fn push_sandbox( &self, sandbox_dir: &Path, state: &RepoState, description: Option<&str>, - ) -> Result<(), SandboxError>; + ) -> Result, SandboxError>; /// Squash the changes from `from_bookmark` into the sandbox, including /// any push needed to make the result visible at the sandbox's bookmark. @@ -176,14 +177,15 @@ pub trait SandboxBackend: Send + Sync { ) -> Result; /// Push the updated working copy from the sandbox back to the remote. - /// `description` is an optional commit message; when `None` the backend - /// uses a default like `sandbox-{group_id}`. + /// + /// Returns the provider's durable WIP revision when one can be recorded + /// for resuming a later sandbox instance. async fn push_sandbox( &self, sandbox_dir: &Path, group_id: &str, description: Option<&str>, - ) -> Result<(), SandboxError>; + ) -> Result, SandboxError>; /// Clean up the sandbox temp dir. async fn cleanup_sandbox(&self, sandbox_dir: &Path) -> Result<(), SandboxError>; diff --git a/crates/sandbox-core/src/server.rs b/crates/sandbox-core/src/server.rs index ba6bd92f..9b92107d 100644 --- a/crates/sandbox-core/src/server.rs +++ b/crates/sandbox-core/src/server.rs @@ -490,6 +490,8 @@ async fn handle_clone_repo String { format!("{description}\n\nCo-authored-by: {DEFAULT_SANDBOX_NAME} <{DEFAULT_SANDBOX_EMAIL}>") } +async fn record_resume_revision( + metadata: &M, + group_id: &str, + resume_revision: Option, +) -> Result<(), SandboxError> { + let Some(resume_revision) = resume_revision else { + return Ok(()); + }; + let Some(mut repo_state) = metadata.get(group_id).await? else { + return Ok(()); + }; + repo_state.resume_revision = Some(resume_revision); + metadata.put(&repo_state).await +} + /// Run an action inside a sandbox: create → action → push → cleanup. /// /// The closure receives the sandbox directory path and returns `(text, display_as)`. @@ -633,10 +652,11 @@ where }; if modifies { - state + let resume_revision = state .backend .push_sandbox(&sandbox_dir, group_id, description_ref) .await?; + record_resume_revision(&state.metadata, group_id, resume_revision).await?; } if modifies { @@ -1061,10 +1081,11 @@ async fn handle_execute_command_streaming_inner< // Process finished within 5 seconds — return a normal tool_result. state.in_flight.lock().await.remove(&invocation.id); let text = format_exec_output(&stdout_buf, &stderr_buf, code); - let _ = state + let resume_revision = state .backend .push_sandbox(&sandbox_dir, &invocation.group_id, None) - .await; + .await?; + record_resume_revision(&state.metadata, &invocation.group_id, resume_revision).await?; send_tool_result(&state.callback_client, invocation, &text, None, false).await; push_diff_view(state, invocation, &sandbox_dir).await; if let Err(e) = state.backend.cleanup_sandbox(&sandbox_dir).await { diff --git a/crates/sandbox-core/src/types.rs b/crates/sandbox-core/src/types.rs index bfd50688..abbbd971 100644 --- a/crates/sandbox-core/src/types.rs +++ b/crates/sandbox-core/src/types.rs @@ -7,13 +7,21 @@ use serde::{Deserialize, Serialize}; pub enum SandboxMode { /// Jujutsu workspace (the default for repos with `.jj`). Jj { - /// Absolute jj change_id the workspace is based on. - base_revision: String, + /// Immutable absolute jj change ID from which the sandbox was created. + /// + /// This is creation provenance only. Resume logic must use + /// [`RepoState::resume_revision`], not this value. + #[serde(alias = "base_revision")] + starting_revision: String, }, /// Plain git worktree. Git { - /// Absolute git commit hash the worktree is based on. - base_revision: String, + /// Immutable absolute git commit hash from which the sandbox was created. + /// + /// This is creation provenance only. Resume logic must use + /// [`RepoState::resume_revision`], not this value. + #[serde(alias = "base_revision")] + starting_revision: String, }, /// Externally-provided custom mode, keyed by a string id with opaque JSON data. /// @@ -85,6 +93,20 @@ pub struct RepoState { pub bookmark: String, /// The version-control mode and its associated data. pub mode: SandboxMode, + /// The current durable sandbox WIP revision. + /// + /// In Jujutsu mode this will eventually be the WIP change ID; in Git mode + /// it is the WIP commit ID. It is intentionally distinct from the + /// mode-specific `starting_revision` so a suspended sandbox can resume + /// where its history was last moved instead of being recreated at its + /// original base. + #[serde(default)] + pub resume_revision: Option, + /// Provider-managed retention reference that keeps `resume_revision` + /// reachable when the visible sandbox bookmark or branch is removed during + /// suspension. Its storage and format are mode-specific. + #[serde(default)] + pub retention_ref: Option, /// Path to the created sandbox workspace. #[serde(default)] pub sandbox_path: Option, @@ -187,3 +209,46 @@ pub struct GrepArgs { #[serde(rename = "caseSensitive")] pub case_sensitive: Option, } + +#[cfg(test)] +mod tests { + use super::{RepoState, SandboxMode}; + + #[test] + fn legacy_base_revision_state_deserializes_as_starting_revision() { + let state: RepoState = serde_json::from_value(serde_json::json!({ + "group_id": "legacy-thread", + "remote_uri": "/repo", + "bookmark": "sandbox-legacy-thread", + "mode": { "Jj": { "base_revision": "qzvkmq" } + } + })) + .expect("legacy state should deserialize"); + + assert!(matches!( + state.mode, + SandboxMode::Jj { starting_revision } if starting_revision == "qzvkmq" + )); + assert_eq!(state.resume_revision, None); + assert_eq!(state.retention_ref, None); + } + + #[test] + fn new_history_state_round_trips() { + let state: RepoState = serde_json::from_value(serde_json::json!({ + "group_id": "thread", + "remote_uri": "/repo", + "bookmark": "sandbox-thread", + "mode": { "Git": { "starting_revision": "abc123" } }, + "resume_revision": "def456", + "retention_ref": "refs/infinity/sandboxes/thread" + })) + .expect("new state should deserialize"); + + let json = serde_json::to_value(&state).expect("state should serialize"); + assert_eq!(json["mode"]["Git"]["starting_revision"], "abc123"); + assert!(json["mode"]["Git"].get("base_revision").is_none()); + assert_eq!(json["resume_revision"], "def456"); + assert_eq!(json["retention_ref"], "refs/infinity/sandboxes/thread"); + } +} diff --git a/crates/sandbox-local/src/backend.rs b/crates/sandbox-local/src/backend.rs index 82734a5f..4c39210d 100644 --- a/crates/sandbox-local/src/backend.rs +++ b/crates/sandbox-local/src/backend.rs @@ -447,18 +447,20 @@ impl SandboxBackend for LocalBackend { sandbox_dir: &Path, group_id: &str, description: Option<&str>, - ) -> Result<(), SandboxError> { + ) -> Result, SandboxError> { let state = { let cache = self.cache.lock().unwrap_or_else(|e| e.into_inner()); cache.get(group_id).map(|e| e.state.clone()) }; match state { Some(state) => match &state.mode { - SandboxMode::Direct => Ok(()), + SandboxMode::Direct => Ok(None), mode => { - self.require_provider(mode)? + let resume_revision = self + .require_provider(mode)? .push_sandbox(sandbox_dir, &state, description) - .await + .await?; + Ok(resume_revision) } }, None => Err(SandboxError::Other(format!( diff --git a/crates/sandbox-local/src/providers.rs b/crates/sandbox-local/src/providers.rs index a7bb9bcc..5f82015c 100644 --- a/crates/sandbox-local/src/providers.rs +++ b/crates/sandbox-local/src/providers.rs @@ -80,7 +80,7 @@ impl ModeProvider for LocalJjProvider { } async fn create_sandbox(&self, state: &RepoState) -> Result { - let SandboxMode::Jj { base_revision } = &state.mode else { + let SandboxMode::Jj { starting_revision } = &state.mode else { return Err(SandboxError::Other( "bug: jj provider invoked with a non-jj mode".to_owned(), )); @@ -91,7 +91,8 @@ impl ModeProvider for LocalJjProvider { &state.remote_uri, &sandbox_dir, &state.bookmark, - base_revision, + starting_revision, + state.resume_revision.as_deref(), ) .await?; Ok(sandbox_dir) @@ -128,8 +129,8 @@ impl ModeProvider for LocalJjProvider { sandbox_dir: &Path, state: &RepoState, _description: Option<&str>, - ) -> Result<(), SandboxError> { - jj::jj_push_working_copy(sandbox_dir, &state.bookmark).await?; + ) -> Result, SandboxError> { + let wip_revision = jj::jj_push_working_copy(sandbox_dir, &state.bookmark).await?; // Keep colocated git refs in sync so git commands via // write-orig see the latest jj state. let orig = PathBuf::from(&state.remote_uri); @@ -138,7 +139,7 @@ impl ModeProvider for LocalJjProvider { { tracing::warn!(error = %e, "jj git export failed"); } - Ok(()) + Ok(Some(wip_revision)) } async fn squash( @@ -148,7 +149,8 @@ impl ModeProvider for LocalJjProvider { from_bookmark: &str, ) -> Result<(), SandboxError> { jj::squash_from(sandbox_dir, from_bookmark).await?; - self.push_sandbox(sandbox_dir, state, None).await + self.push_sandbox(sandbox_dir, state, None).await?; + Ok(()) } async fn diff_files( @@ -215,7 +217,7 @@ impl ModeProvider for LocalGitProvider { } async fn create_sandbox(&self, state: &RepoState) -> Result { - let SandboxMode::Git { base_revision } = &state.mode else { + let SandboxMode::Git { starting_revision } = &state.mode else { return Err(SandboxError::Other( "bug: git provider invoked with a non-git mode".to_owned(), )); @@ -226,7 +228,7 @@ impl ModeProvider for LocalGitProvider { &PathBuf::from(&state.remote_uri), &sandbox_dir, &state.bookmark, - Some(base_revision), + Some(starting_revision), ) .await?; Ok(sandbox_dir) @@ -237,8 +239,9 @@ impl ModeProvider for LocalGitProvider { sandbox_dir: &Path, _state: &RepoState, description: Option<&str>, - ) -> Result<(), SandboxError> { - git::git_amend_all(sandbox_dir, description).await + ) -> Result, SandboxError> { + git::git_amend_all(sandbox_dir, description).await?; + Ok(None) } async fn squash( diff --git a/crates/sandbox-local/tests/common.rs b/crates/sandbox-local/tests/common.rs index 08005e30..7ec9e0f5 100644 --- a/crates/sandbox-local/tests/common.rs +++ b/crates/sandbox-local/tests/common.rs @@ -23,6 +23,19 @@ pub async fn start_test_server(metadata_dir: &Path) -> String { start_test_server_sandboxed(metadata_dir, false).await } +/// Start a RAP server that the caller can shut down and await. Restart tests +/// use this to ensure the original backend (and its sandbox cache) is dropped +/// before state is restored by a new server. +pub async fn start_test_server_with_shutdown( + metadata_dir: &Path, +) -> ( + String, + tokio::sync::oneshot::Sender<()>, + tokio::task::JoinHandle<()>, +) { + start_test_server_with_shutdown_sandboxed(metadata_dir, false).await +} + /// Start the RAP server with platform sandboxing (bwrap/sandbox-exec) enabled. pub async fn start_test_server_sandboxed(metadata_dir: &Path, sandbox_enabled: bool) -> String { std::fs::create_dir_all(metadata_dir).expect("create metadata dir"); @@ -47,6 +60,44 @@ pub async fn start_test_server_sandboxed(metadata_dir: &Path, sandbox_enabled: b format!("http://127.0.0.1:{port}") } +async fn start_test_server_with_shutdown_sandboxed( + metadata_dir: &Path, + sandbox_enabled: bool, +) -> ( + String, + tokio::sync::oneshot::Sender<()>, + tokio::task::JoinHandle<()>, +) { + std::fs::create_dir_all(metadata_dir).expect("create metadata dir"); + + unsafe { + std::env::set_var( + "XDG_CONFIG_HOME", + std::env::temp_dir().join("xdg-config-home"), + ); + } + + let backend = LocalBackend::new(sandbox_enabled); + let metadata = FileMetadataStore::new(metadata_dir.to_path_buf()); + let (app, _tracker) = build_router(backend, metadata, PlainCallbackClient::new(), false, None); + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind test server"); + let port = listener.local_addr().expect("get local addr").port(); + let (shutdown_tx, shutdown_rx) = tokio::sync::oneshot::channel(); + let server = tokio::spawn(async move { + axum::serve(listener, app) + .with_graceful_shutdown(async { + let _ = shutdown_rx.await; + }) + .await + .expect("serve test server") + }); + + (format!("http://127.0.0.1:{port}"), shutdown_tx, server) +} + /// POST a RapInvocation and wait for the callback result text. pub async fn invoke( server_url: &str, diff --git a/crates/sandbox-local/tests/describe_changes.rs b/crates/sandbox-local/tests/describe_changes.rs index f0b38359..f822311e 100644 --- a/crates/sandbox-local/tests/describe_changes.rs +++ b/crates/sandbox-local/tests/describe_changes.rs @@ -100,7 +100,12 @@ async fn describe_and_read_log(repo: &Path) -> String { String::from_utf8_lossy(&output.stderr) ); - redact_jj_log(&String::from_utf8(output.stdout).expect("jj log output as utf8")) + let log = redact_jj_log(&String::from_utf8(output.stdout).expect("jj log output as utf8")); + assert!( + !log.contains("Committer: (no name set)"), + "sandbox fallback identity must be applied before workspace creation:\n{log}" + ); + log } #[tokio::test] diff --git a/crates/sandbox-local/tests/jj_resume.rs b/crates/sandbox-local/tests/jj_resume.rs new file mode 100644 index 00000000..f9e97a3c --- /dev/null +++ b/crates/sandbox-local/tests/jj_resume.rs @@ -0,0 +1,155 @@ +//! A suspended JJ sandbox must resume its actual working-copy change, not the +//! sandbox bookmark or its original starting revision. + +mod common; + +use std::fs; +use std::path::Path; +use std::process::Command; + +use common::{invoke, jj_init_with_file, start_test_server_with_shutdown}; +use rap_client::callback_server::start_callback_channel; +use sandbox_core::types::RepoState; + +fn jj_output(dir: &Path, args: &[&str]) -> String { + let output = Command::new("jj") + .args(args) + .current_dir(dir) + .output() + .expect("run jj"); + assert!( + output.status.success(), + "jj {} failed: {}", + args.join(" "), + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8(output.stdout) + .expect("jj output is UTF-8") + .trim() + .to_owned() +} + +/// Simulates server suspension by forgetting and deleting the actual `.tmp…` +/// workspace and deleting the visible bookmark. The second server must restore +/// the WIP change recorded in file metadata, even though neither of those +/// workspace/bookmark identities remains. +#[tokio::test] +async fn jj_sandbox_resumes_moved_working_copy_after_server_restart() { + let _ = tracing_subscriber::fmt::try_init(); + + let repo = jj_init_with_file("README.md", "base\n"); + let metadata_dir = tempfile::tempdir().expect("create metadata dir"); + let group_id = "resume-moved-wip"; + let (server_url, shutdown, server) = start_test_server_with_shutdown(metadata_dir.path()).await; + let (callback_url, mut rx) = start_callback_channel() + .await + .expect("start callback channel"); + + let clone = invoke( + &server_url, + &callback_url, + group_id, + "clone_repo", + serde_json::json!({ "repo": repo.path() }), + &mut rx, + None, + ) + .await; + assert!(clone.contains("Jujutsu workspaces"), "got: {clone}"); + + // `jj split` writes a file and moves @ to a new empty WIP change. It is a + // sandbox-local mutation, so no write-orig permission is required. + let split = invoke( + &server_url, + &callback_url, + group_id, + "execute_command", + serde_json::json!({ + "command": "printf 'resumed content\\n' > resumed.txt && jj split -m split-wip resumed.txt" + }), + &mut rx, + None, + ) + .await; + assert!(split.contains("exit code: 0"), "got: {split}"); + + let metadata_path = metadata_dir.path().join(format!("{group_id}.json")); + let persisted: RepoState = + serde_json::from_str(&fs::read_to_string(&metadata_path).expect("read persisted metadata")) + .expect("parse persisted metadata"); + let resume_revision = persisted + .resume_revision + .expect("successful sandbox mutation must persist its WIP change ID"); + assert_ne!( + resume_revision, + match persisted.mode { + sandbox_core::types::SandboxMode::Jj { starting_revision } => starting_revision, + _ => unreachable!("test repo must use JJ mode"), + }, + "split should have moved the working copy away from its starting revision" + ); + + // `sandbox_path` is deliberately not persisted yet, so identify the one + // temp workspace created beneath this repo. Forget it, delete it, and drop + // the visible bookmark: resume must rely only on `resume_revision`. + let sandboxes_dir = repo.path().join(".infinity/.sandboxes"); + let sandbox_path = fs::read_dir(&sandboxes_dir) + .expect("list sandboxes") + .map(|entry| entry.expect("read sandbox entry").path()) + .find(|path| path.is_dir()) + .expect("sandbox workspace exists"); + jj_output( + &sandbox_path, + &["--ignore-working-copy", "workspace", "forget"], + ); + fs::remove_dir_all(&sandbox_path).expect("remove suspended workspace directory"); + jj_output(repo.path(), &["bookmark", "delete", &persisted.bookmark]); + + // Stop and await server one before restoring. This drops its LocalBackend + // and proves server two cannot reuse an in-memory cached workspace. + shutdown.send(()).expect("shut down initial server"); + server.await.expect("initial server exits cleanly"); + + // A fresh backend has no cache. Reading the file materializes a new .tmp + // workspace, which must check out the persisted WIP change and its files. + let (resumed_server, resumed_shutdown, resumed_server_task) = + start_test_server_with_shutdown(metadata_dir.path()).await; + let resumed = invoke( + &resumed_server, + &callback_url, + group_id, + "read_file", + serde_json::json!({ "path": "resumed.txt" }), + &mut rx, + None, + ) + .await; + assert!(resumed.contains("resumed content"), "got: {resumed}"); + + let resumed_sandbox = fs::read_dir(&sandboxes_dir) + .expect("list resumed sandboxes") + .map(|entry| entry.expect("read resumed sandbox entry").path()) + .find(|path| path.is_dir()) + .expect("resumed sandbox workspace exists"); + assert_ne!(resumed_sandbox, sandbox_path, "must create a new workspace"); + assert_eq!( + jj_output( + &resumed_sandbox, + &[ + "log", + "--no-graph", + "-r", + "@", + "-T", + "change_id ++ '/' ++ change_offset", + ], + ), + resume_revision, + "the new workspace must resume at the moved WIP change ID" + ); + + resumed_shutdown.send(()).expect("shut down resumed server"); + resumed_server_task + .await + .expect("resumed server exits cleanly"); +} diff --git a/crates/sandbox-remote/src/backend.rs b/crates/sandbox-remote/src/backend.rs index fe3a0223..06d9c85c 100644 --- a/crates/sandbox-remote/src/backend.rs +++ b/crates/sandbox-remote/src/backend.rs @@ -116,8 +116,8 @@ impl SandboxBackend for EfsBackend { /// Create a temp dir and jj git clone from the EFS bare repo. /// If we have a previous bookmark, fetch and create a new working copy on top of it. async fn create_sandbox(&self, state: &RepoState) -> Result { - let base_revision = match &state.mode { - SandboxMode::Jj { base_revision } => base_revision.as_str(), + let starting_revision = match &state.mode { + SandboxMode::Jj { starting_revision } => starting_revision.as_str(), _ => { return Err(SandboxError::Other( "EFS backend only supports Jj mode".to_owned(), @@ -132,7 +132,8 @@ impl SandboxBackend for EfsBackend { &state.remote_uri, &sandbox_dir, &state.bookmark, - base_revision, + starting_revision, + state.resume_revision.as_deref(), ) .await?; @@ -167,9 +168,11 @@ impl SandboxBackend for EfsBackend { sandbox_dir: &Path, group_id: &str, _description: Option<&str>, - ) -> Result<(), SandboxError> { + ) -> Result, SandboxError> { let bookmark = format!("sandbox-{group_id}"); - jj::jj_push_working_copy(sandbox_dir, &bookmark).await + Ok(Some( + jj::jj_push_working_copy(sandbox_dir, &bookmark).await?, + )) } /// Remove the temp sandbox directory. @@ -212,7 +215,8 @@ impl SandboxBackend for EfsBackend { from_bookmark: &str, ) -> Result<(), SandboxError> { jj::squash_from(sandbox_dir, from_bookmark).await?; - jj::jj_push_working_copy(sandbox_dir, &state.bookmark).await + jj::jj_push_working_copy(sandbox_dir, &state.bookmark).await?; + Ok(()) } async fn diff_files( diff --git a/crates/sandbox-remote/src/metadata.rs b/crates/sandbox-remote/src/metadata.rs index ff6d99dc..e653f797 100644 --- a/crates/sandbox-remote/src/metadata.rs +++ b/crates/sandbox-remote/src/metadata.rs @@ -45,17 +45,30 @@ impl MetadataStore for DynamoMetadataStore { .cloned() .unwrap_or_else(|| format!("sandbox-{group_id}")); - let base_revision = item - .get("base_revision") + let starting_revision = item + .get("starting_revision") + .or_else(|| item.get("base_revision")) .and_then(|v| v.as_s().ok()) .cloned() .unwrap_or_default(); + let resume_revision = item + .get("resume_revision") + .and_then(|v| v.as_s().ok()) + .cloned(); + + let retention_ref = item + .get("retention_ref") + .and_then(|v| v.as_s().ok()) + .cloned(); + Ok(Some(RepoState { group_id: group_id.to_owned(), remote_uri, bookmark, - mode: SandboxMode::Jj { base_revision }, + mode: SandboxMode::Jj { starting_revision }, + resume_revision, + retention_ref, sandbox_path: None, write_orig_granted: false, write_path_grants: Default::default(), @@ -72,8 +85,23 @@ impl MetadataStore for DynamoMetadataStore { .item("remote_uri", AttributeValue::S(state.remote_uri.clone())) .item("bookmark", AttributeValue::S(state.bookmark.clone())); - if let SandboxMode::Jj { ref base_revision } = state.mode { - req = req.item("base_revision", AttributeValue::S(base_revision.clone())); + if let SandboxMode::Jj { + ref starting_revision, + } = state.mode + { + req = req.item( + "starting_revision", + AttributeValue::S(starting_revision.clone()), + ); + } + if let Some(resume_revision) = &state.resume_revision { + req = req.item( + "resume_revision", + AttributeValue::S(resume_revision.clone()), + ); + } + if let Some(retention_ref) = &state.retention_ref { + req = req.item("retention_ref", AttributeValue::S(retention_ref.clone())); } req.send()