Remove SandboxRecordExt trait (save/load were unused in production)

The trait just wrapped serde_json + std::fs. Tests using it were testing
serialization, not sandbox behavior — deleted those and simplified the
daytona cp test to pass the record directly to reconnect.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-04-03 14:58:18 -07:00
parent 01b1ce45e0
commit a19878e590
No known key found for this signature in database
4 changed files with 9 additions and 97 deletions

View file

@ -39,7 +39,7 @@ pub use local::LocalSandbox;
#[cfg(feature = "docker")]
pub use docker::{DockerSandbox, DockerSandboxOptions};
pub use sandbox_record::{SandboxRecord, SandboxRecordExt};
pub use sandbox_record::SandboxRecord;
#[cfg(feature = "daytona")]
pub use daytona::detect_clone_params;

View file

@ -1,26 +1 @@
use std::path::Path;
pub use fabro_types::sandbox_record::SandboxRecord;
pub trait SandboxRecordExt {
fn save(&self, path: &Path) -> anyhow::Result<()>;
fn load(path: &Path) -> anyhow::Result<Self>
where
Self: Sized;
}
impl SandboxRecordExt for SandboxRecord {
fn save(&self, path: &Path) -> anyhow::Result<()> {
let json = serde_json::to_string_pretty(self)
.map_err(|e| anyhow::anyhow!("sandbox_record serialize failed: {e}"))?;
std::fs::write(path, json)?;
Ok(())
}
fn load(path: &Path) -> anyhow::Result<Self> {
let data = std::fs::read_to_string(path)
.map_err(|e| anyhow::anyhow!("failed to read {}: {e}", path.display()))?;
serde_json::from_str(&data)
.map_err(|e| anyhow::anyhow!("sandbox_record deserialize failed: {e}"))
}
}

View file

@ -6,8 +6,8 @@
#![allow(clippy::ignore_without_reason)]
use fabro_sandbox::SandboxRecord;
use fabro_sandbox::reconnect::reconnect;
use fabro_sandbox::{SandboxRecord, SandboxRecordExt};
// ---------------------------------------------------------------------------
// Local sandbox
@ -111,25 +111,6 @@ async fn local_cp_creates_parent_dirs() {
assert_eq!(std::fs::read(&local_dst).unwrap(), content);
}
#[tokio::test]
async fn local_cp_record_save_load_round_trip() {
let sandbox_dir = tempfile::tempdir().unwrap();
let scratch = tempfile::tempdir().unwrap();
let record = local_record(sandbox_dir.path());
let path = scratch.path().join("sandbox.json");
record.save(&path).unwrap();
let loaded = SandboxRecord::load(&path).unwrap();
assert_eq!(loaded.provider, "local");
assert_eq!(loaded.working_directory, record.working_directory);
assert!(loaded.identifier.is_none());
// Reconnect from the loaded record still works
let sandbox = reconnect(&loaded).await.expect("reconnect from loaded");
assert_eq!(sandbox.working_directory(), record.working_directory);
}
// ---------------------------------------------------------------------------
// Docker sandbox
// ---------------------------------------------------------------------------
@ -265,39 +246,3 @@ async fn docker_cp_custom_mount_point() {
assert_eq!(std::fs::read(&local_dst).unwrap(), content);
}
#[tokio::test]
#[ignore] // requires Docker daemon
async fn docker_cp_record_save_load_round_trip() {
let host_dir = tempfile::tempdir().unwrap();
let scratch = tempfile::tempdir().unwrap();
let record = docker_record(host_dir.path(), "/workspace");
let path = scratch.path().join("sandbox.json");
record.save(&path).unwrap();
let loaded = SandboxRecord::load(&path).unwrap();
assert_eq!(loaded.provider, "docker");
assert_eq!(
loaded.host_working_directory.as_deref(),
Some(host_dir.path().to_str().unwrap())
);
assert_eq!(loaded.container_mount_point.as_deref(), Some("/workspace"));
// Reconnect from the loaded record still works
let sandbox = reconnect(&loaded).await.expect("reconnect from loaded");
let content = b"from loaded record\n";
let local_src = scratch.path().join("loaded.txt");
std::fs::write(&local_src, content).unwrap();
sandbox
.upload_file_from_local(&local_src, "loaded.txt")
.await
.expect("upload via loaded record");
assert_eq!(
std::fs::read(host_dir.path().join("loaded.txt")).unwrap(),
content
);
}

View file

@ -20,7 +20,6 @@ use std::sync::Arc;
use fabro_agent::Sandbox;
use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node};
use fabro_llm::provider::Provider;
use fabro_sandbox::SandboxRecordExt;
use fabro_sandbox::daytona::{DaytonaConfig, DaytonaSandbox, DaytonaSnapshotConfig};
use fabro_store::RuntimeState;
use fabro_types::{RunId, Settings};
@ -1699,7 +1698,7 @@ async fn daytona_toolbox_idle_diagnostic() {
/// E2E test for `fabro cp` against a live Daytona sandbox.
///
/// Creates a sandbox, saves a SandboxRecord, reconnects via `cp::reconnect`,
/// Creates a sandbox, reconnects via `cp::reconnect`,
/// uploads a file, downloads it back, and verifies the round-trip.
#[fabro_macros::e2e_test(live("DAYTONA_API_KEY"), live("GITHUB_APP_PRIVATE_KEY"))]
async fn daytona_cp_upload_download_round_trip() {
@ -1725,18 +1724,11 @@ async fn daytona_cp_upload_download_round_trip() {
container_mount_point: None,
};
// 3. Save to temp dir and reload (verify serialization round-trip)
// 3. Reconnect via the real cp::reconnect path
let tmp = tempfile::tempdir().unwrap();
let record_path = tmp.path().join("sandbox.json");
record.save(&record_path).unwrap();
let loaded = SandboxRecord::load(&record_path).unwrap();
assert_eq!(loaded.provider, "daytona");
assert_eq!(loaded.identifier.as_deref(), Some(sandbox_name.as_str()));
let reconnected = reconnect(&record).await.expect("reconnect should succeed");
// 4. Reconnect via the real cp::reconnect path
let reconnected = reconnect(&loaded).await.expect("reconnect should succeed");
// 5. Upload: write a local file, then upload it to the sandbox
// 4. Upload: write a local file, then upload it to the sandbox
let upload_content = b"hello from fabro cp e2e test\n";
let local_upload = tmp.path().join("upload.txt");
std::fs::write(&local_upload, upload_content).unwrap();
@ -1746,7 +1738,7 @@ async fn daytona_cp_upload_download_round_trip() {
.await
.expect("upload_file_from_local should succeed");
// 6. Verify the file exists in the sandbox via the original connection
// 5. Verify the file exists in the sandbox via the original connection
assert!(
env.file_exists("cp_test_upload.txt").await.unwrap(),
"uploaded file should exist in the sandbox"
@ -1760,7 +1752,7 @@ async fn daytona_cp_upload_download_round_trip() {
"expected uploaded content in sandbox, got: {remote_content}"
);
// 7. Download: retrieve the file back to local via the reconnected sandbox
// 6. Download: retrieve the file back to local via the reconnected sandbox
let local_download = tmp.path().join("download.txt");
reconnected
.download_file_to_local("cp_test_upload.txt", &local_download)
@ -1770,7 +1762,7 @@ async fn daytona_cp_upload_download_round_trip() {
let downloaded = std::fs::read(&local_download).unwrap();
assert_eq!(downloaded, upload_content);
// 8. Upload a binary file to test non-UTF-8 content
// 7. Upload a binary file to test non-UTF-8 content
let binary_content: Vec<u8> = (0..=255).collect();
let local_binary = tmp.path().join("binary.bin");
std::fs::write(&local_binary, &binary_content).unwrap();