Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions crates/sandbox-core/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}."),
Expand All @@ -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,
Expand Down
47 changes: 31 additions & 16 deletions crates/sandbox-core/src/jj.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,19 @@ pub async fn jj_resolve_revision(dir: &Path, rev: &str) -> Result<String, Sandbo
.await
}

/// Create a jj workspace at `dest` based on `revision`, set and edit the bookmark.
/// Detects the repo's configured user and configures the workspace identity.
/// Create a jj workspace at `dest` based on `starting_revision`.
///
/// When `resume_revision` is present, it is checked out directly instead of
/// resolving the sandbox bookmark. This lets a suspended sandbox resume its
/// actual working-copy change after the ephemeral workspace and visible
/// bookmark have been removed. Detects the repo's configured user and
/// configures the workspace identity.
pub async fn jj_git_clone(
remote: &str,
dest: &Path,
bookmark_name: &str,
revision: &str,
starting_revision: &str,
resume_revision: Option<&str>,
) -> Result<(), SandboxError> {
let (name, email) = jj_configured_user(Path::new(remote))
.await
Expand All @@ -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",
Expand All @@ -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())
Expand All @@ -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<String, SandboxError> {
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 `@`).
Expand Down Expand Up @@ -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(
Expand All @@ -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,
Expand Down
12 changes: 7 additions & 5 deletions crates/sandbox-core/src/sandbox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Option<String>, SandboxError>;

/// Squash the changes from `from_bookmark` into the sandbox, including
/// any push needed to make the result visible at the sandbox's bookmark.
Expand Down Expand Up @@ -176,14 +177,15 @@ pub trait SandboxBackend: Send + Sync {
) -> Result<SpawnedCommand, SandboxError>;

/// 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<Option<String>, SandboxError>;

/// Clean up the sandbox temp dir.
async fn cleanup_sandbox(&self, sandbox_dir: &Path) -> Result<(), SandboxError>;
Expand Down
27 changes: 24 additions & 3 deletions crates/sandbox-core/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,8 @@ async fn handle_clone_repo<B: SandboxBackend, M: MetadataStore, C: CallbackClien
remote_uri: remote_uri.clone(),
bookmark: bookmark.clone(),
mode: init.mode,
resume_revision: None,
retention_ref: None,
sandbox_path: None,
write_orig_granted: false,
write_path_grants: Default::default(),
Expand Down Expand Up @@ -548,6 +550,8 @@ async fn handle_open_sandbox_direct<B: SandboxBackend, M: MetadataStore, C: Call
remote_uri: remote_uri.clone(),
bookmark: format!("sandbox-{}", invocation.group_id),
mode: SandboxMode::Direct,
resume_revision: None,
retention_ref: None,
sandbox_path: None,
write_orig_granted: false,
write_path_grants: Default::default(),
Expand Down Expand Up @@ -582,6 +586,21 @@ fn append_co_author_trailer(description: &str) -> String {
format!("{description}\n\nCo-authored-by: {DEFAULT_SANDBOX_NAME} <{DEFAULT_SANDBOX_EMAIL}>")
}

async fn record_resume_revision<M: MetadataStore>(
metadata: &M,
group_id: &str,
resume_revision: Option<String>,
) -> 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)`.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
73 changes: 69 additions & 4 deletions crates/sandbox-core/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -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<String>,
/// 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<String>,
/// Path to the created sandbox workspace.
#[serde(default)]
pub sandbox_path: Option<String>,
Expand Down Expand Up @@ -187,3 +209,46 @@ pub struct GrepArgs {
#[serde(rename = "caseSensitive")]
pub case_sensitive: Option<bool>,
}

#[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");
}
}
10 changes: 6 additions & 4 deletions crates/sandbox-local/src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,18 +447,20 @@ impl SandboxBackend for LocalBackend {
sandbox_dir: &Path,
group_id: &str,
description: Option<&str>,
) -> Result<(), SandboxError> {
) -> Result<Option<String>, 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!(
Expand Down
Loading
Loading