diff --git a/docs-internal/run-directory-keys.md b/docs-internal/run-directory-keys.md index a77029c3f..04c9e5eba 100644 --- a/docs-internal/run-directory-keys.md +++ b/docs-internal/run-directory-keys.md @@ -15,7 +15,6 @@ There is no `_init.json` anymore. Run existence in the database is determined by | File | Purpose | Source | |---|---|---| | `workflow_bundle.json` | Bundled workflow input used by `start` to restore `workflow_path` and bundled child workflows/files | Written during create from the resolved workflow bundle | -| `final.patch` | Final git diff for checkpointed runs | Local git state at finalize time, not a direct event payload | | `run.pid` | Legacy detached-run pid file from older runs | Legacy only; current flows do not rely on it | ## Local-Only Directories @@ -41,3 +40,4 @@ These names are still real, but they are no longer live scratch files by default ## Notes - Artifact binaries are no longer stored in the SlateDB keyspace. They live in `ArtifactStore`; the run scratch tree only contains local cached copies when a workflow stage writes them to disk. +- Final diffs for checkpointed runs are projected from the run store; they are no longer written as scratch files. diff --git a/docs/reference/cli.mdx b/docs/reference/cli.mdx index 3a1d02c2c..04974f067 100644 --- a/docs/reference/cli.mdx +++ b/docs/reference/cli.mdx @@ -408,7 +408,7 @@ fabro pr create --model claude-opus-4-6 | `` | Run ID or prefix (required) | | `--model ` | LLM model for generating the PR description | -The run must have completed successfully (or with partial success) and have a `final.patch` with changes. +The run must have completed successfully (or with partial success) and have a stored diff with changes. ### `fabro pr list` @@ -658,20 +658,17 @@ fabro repo deinit ## `fabro diff` -Show the diff from a workflow run. Displays the `final.patch` for completed runs, or connects to the sandbox for a live diff from in-progress runs. +Show the diff from a workflow run. Reads the stored diff for completed runs. ```bash fabro diff fabro diff --node work -fabro diff --stat ``` | Argument / Flag | Description | |---|---| | `` | Run ID or prefix (required) | | `--node ` | Show diff for a specific node instead of the full run | -| `--stat` | Show diffstat instead of full patch (live diffs only) | -| `--shortstat` | Show only files-changed/insertions/deletions summary (live diffs only) | Output is colorized when writing to a terminal. diff --git a/docs/reference/run-directory.mdx b/docs/reference/run-directory.mdx index a39c5885b..79d1f3f2c 100644 --- a/docs/reference/run-directory.mdx +++ b/docs/reference/run-directory.mdx @@ -20,7 +20,6 @@ The naming format is `YYYYMMDD-{run_id}`, where `run_id` is the ULID assigned to | File | Format | When written | Description | |---|---|---|---| | `workflow_bundle.json` | JSON | Run create | Bundled workflow input used to restart the run without re-reading the original workflow files. Includes the root workflow path plus bundled child workflow sources and inline files. | -| `final.patch` | Diff | Run end | Git diff from `base_sha` to final HEAD. Only present in git checkpoint mode. | | `run.pid` | Text | Legacy only | Legacy process ID file from older runs. Current detached launches use launcher records instead, and current attach/resume no longer read `run.pid`. | ## Local-only directories @@ -32,7 +31,7 @@ These paths are local runtime state and caches, not the canonical run record. - **`cache/artifacts/files/`** — Captured artifact files organized by node and retry, plus a `manifest.json` for each retry directory. - **`nodes/{manager_node}_{visit}/child/`** — Nested scratch directories for manager-loop child workflows. -Large durable values, event streams, checkpoints, conclusions, and retros are no longer projected into live scratch by default. Use `fabro logs`, `fabro inspect`, the API, or `fabro store dump` for those surfaces. +Large durable values, event streams, checkpoints, diffs, conclusions, and retros are no longer projected into live scratch by default. Use `fabro logs`, `fabro inspect`, the API, or `fabro store dump` for those surfaces. ## Reconstructed and export-only layouts @@ -59,7 +58,6 @@ fabro ps --filter workflow=my-workflow ├── 20260307-01JQXYZ123ABC456DEF789/ # One directory per run │ ├── workflow_bundle.json │ ├── run.pid # Legacy only; older runs may contain this -│ ├── final.patch │ ├── runtime/ │ │ └── blobs/ │ │ └── 01JT5Y3KJ0N5S9E1Y7YFBR2G4D.json diff --git a/lib/crates/fabro-cli/src/commands/pr/create.rs b/lib/crates/fabro-cli/src/commands/pr/create.rs index b4b8582b3..5ae23ebe6 100644 --- a/lib/crates/fabro-cli/src/commands/pr/create.rs +++ b/lib/crates/fabro-cli/src/commands/pr/create.rs @@ -51,7 +51,7 @@ pub(super) async fn create_command( .final_patch .context("Failed to load final patch from store — no diff available")?; if diff.trim().is_empty() { - bail!("final.patch is empty — nothing to create a PR for"); + bail!("Stored diff is empty — nothing to create a PR for"); } let cwd = std::env::current_dir().context("Failed to get current directory")?; diff --git a/lib/crates/fabro-cli/src/commands/run/diff.rs b/lib/crates/fabro-cli/src/commands/run/diff.rs index 6deb27834..3db803ffb 100644 --- a/lib/crates/fabro-cli/src/commands/run/diff.rs +++ b/lib/crates/fabro-cli/src/commands/run/diff.rs @@ -64,13 +64,13 @@ fn resolve_diff(state: &RunProjection, args: &DiffArgs) -> Result { .ok_or_else(|| anyhow::anyhow!("This run was not git-checkpointed; no diff available"))?; if let Some(patch) = state.final_patch.clone() { - debug!("Reading final.patch from store"); + debug!("Reading stored diff from run state"); return Ok(patch); } if state.conclusion.is_some() { bail!( - "Run completed but no final.patch exists — the run may not have produced any changes" + "Run completed but no stored diff exists — the run may not have produced any changes" ); } diff --git a/lib/crates/fabro-cli/tests/it/cmd/diff.rs b/lib/crates/fabro-cli/tests/it/cmd/diff.rs index 791ce66c8..af9d0ea82 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/diff.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/diff.rs @@ -43,7 +43,7 @@ fn diff_completed_run_without_changes_reports_no_patch() { exit_code: 1 ----- stdout ----- ----- stderr ----- - error: Run completed but no final.patch exists — the run may not have produced any changes + error: Run completed but no stored diff exists — the run may not have produced any changes "); } diff --git a/lib/crates/fabro-config/src/storage.rs b/lib/crates/fabro-config/src/storage.rs index 24e1f6b69..a4bf4736c 100644 --- a/lib/crates/fabro-config/src/storage.rs +++ b/lib/crates/fabro-config/src/storage.rs @@ -120,11 +120,6 @@ impl RunScratch { self.root.join("cache").join("artifacts") } - #[must_use] - pub fn blob_cache_dir(&self) -> PathBuf { - self.artifact_cache_dir().join("values") - } - #[must_use] pub fn artifact_files_dir(&self) -> PathBuf { self.artifact_cache_dir().join("files") @@ -137,15 +132,9 @@ impl RunScratch { .join(format!("retry_{attempt}")) } - #[must_use] - pub fn final_patch(&self) -> PathBuf { - self.root.join("final.patch") - } - pub fn create(&self) -> std::io::Result<()> { std::fs::create_dir_all(self.worktree_dir())?; std::fs::create_dir_all(self.runtime_dir())?; - std::fs::create_dir_all(self.blob_cache_dir())?; std::fs::create_dir_all(self.artifact_files_dir())?; Ok(()) } @@ -230,14 +219,6 @@ mod tests { scratch.artifact_cache_dir(), scratch.root().join("cache").join("artifacts") ); - assert_eq!( - scratch.blob_cache_dir(), - scratch - .root() - .join("cache") - .join("artifacts") - .join("values") - ); assert_eq!( scratch.artifact_files_dir(), scratch.root().join("cache").join("artifacts").join("files") @@ -252,14 +233,21 @@ mod tests { .join("plan") .join("retry_2") ); - assert_eq!(scratch.final_patch(), scratch.root().join("final.patch")); scratch.create().unwrap(); assert!(scratch.root().exists()); assert!(scratch.worktree_dir().exists()); assert!(scratch.runtime_dir().exists()); - assert!(scratch.blob_cache_dir().exists()); assert!(scratch.artifact_files_dir().exists()); + assert!( + !scratch + .root() + .join("cache") + .join("artifacts") + .join("values") + .exists() + ); + assert!(!scratch.root().join("final.patch").exists()); scratch.remove().unwrap(); assert!(!scratch.root().exists()); diff --git a/lib/crates/fabro-workflow/src/artifact.rs b/lib/crates/fabro-workflow/src/artifact.rs index d1df58ed1..724a34694 100644 --- a/lib/crates/fabro-workflow/src/artifact.rs +++ b/lib/crates/fabro-workflow/src/artifact.rs @@ -32,14 +32,11 @@ const ARTIFACT_POINTER_PREFIX: &str = "file://"; /// /// # Errors /// -/// Returns an error if blob persistence or cache materialization fails. +/// Returns an error if blob persistence fails. pub async fn offload_large_values( updates: &mut HashMap, run_store: &RunStoreHandle, - cache_dir: &Path, ) -> Result<()> { - let _ = cache_dir; - for value in updates.values_mut() { let bytes = serde_json::to_vec(&*value) .map_err(|e| FabroError::engine(format!("artifact serialize failed: {e}")))?; @@ -381,7 +378,6 @@ mod tests { #[tokio::test] async fn offload_replaces_large_values_with_blob_backed_pointer() { - let dir = tempfile::tempdir().unwrap(); let run_store = make_run_store("artifact-offload").await; let large_string = "x".repeat(BLOB_OFFLOAD_THRESHOLD + 1); @@ -391,7 +387,7 @@ mod tests { let mut updates = HashMap::new(); updates.insert("response.plan".to_string(), serde_json::json!(large_string)); - offload_large_values(&mut updates, &run_store.clone().into(), dir.path()) + offload_large_values(&mut updates, &run_store.clone().into()) .await .unwrap(); @@ -408,26 +404,20 @@ mod tests { .expect("blob should exist"); let blob_value: serde_json::Value = serde_json::from_slice(&blob).unwrap(); assert_eq!(blob_value, serde_json::json!(large_string)); - assert!( - std::fs::read_dir(dir.path()).unwrap().next().is_none(), - "offload should not materialize host cache files" - ); } #[tokio::test] async fn offload_leaves_small_values_untouched() { - let dir = tempfile::tempdir().unwrap(); let run_store = make_run_store("artifact-small").await; let small_value = serde_json::json!("hello world"); let mut updates = HashMap::new(); updates.insert("small_key".to_string(), small_value.clone()); - offload_large_values(&mut updates, &run_store.clone().into(), dir.path()) + offload_large_values(&mut updates, &run_store.clone().into()) .await .unwrap(); assert_eq!(updates.get("small_key").unwrap(), &small_value); - assert!(std::fs::read_dir(dir.path()).unwrap().next().is_none()); } #[test] diff --git a/lib/crates/fabro-workflow/src/lifecycle/artifact.rs b/lib/crates/fabro-workflow/src/lifecycle/artifact.rs index 1954d2aac..69c63685f 100644 --- a/lib/crates/fabro-workflow/src/lifecycle/artifact.rs +++ b/lib/crates/fabro-workflow/src/lifecycle/artifact.rs @@ -37,7 +37,6 @@ const ARTIFACT_UPLOAD_RETRY_DELAYS: [Duration; 3] = [ pub(crate) struct ArtifactLifecycle { pub sandbox: Arc, pub run_store: RunStoreHandle, - pub blob_cache_dir: PathBuf, pub emitter: Arc, pub artifacts_dir: PathBuf, pub artifact_globs: Vec, @@ -52,7 +51,6 @@ impl ArtifactLifecycle { pub(crate) fn new( sandbox: Arc, run_store: RunStoreHandle, - blob_cache_dir: PathBuf, emitter: Arc, artifacts_dir: PathBuf, artifact_globs: Vec, @@ -62,7 +60,6 @@ impl ArtifactLifecycle { Self { sandbox, run_store, - blob_cache_dir, emitter, artifacts_dir, artifact_globs, @@ -174,12 +171,8 @@ impl RunLifecycle for ArtifactLifecycle { let node_id = node.id(); // Offload large context_updates values to artifact store - if let Err(e) = offload_large_values( - &mut result.outcome.context_updates, - &self.run_store, - &self.blob_cache_dir, - ) - .await + if let Err(e) = + offload_large_values(&mut result.outcome.context_updates, &self.run_store).await { self.emitter.emit(&Event::RunNotice { level: RunNoticeLevel::Warn, diff --git a/lib/crates/fabro-workflow/src/lifecycle/git.rs b/lib/crates/fabro-workflow/src/lifecycle/git.rs index a3cf2eb9c..1b5ba6b4c 100644 --- a/lib/crates/fabro-workflow/src/lifecycle/git.rs +++ b/lib/crates/fabro-workflow/src/lifecycle/git.rs @@ -1,11 +1,8 @@ use std::collections::HashMap; -use std::path::PathBuf; use std::sync::{Arc, Mutex}; use async_trait::async_trait; -use fabro_config::RunScratch; use fabro_types::RunId; -use tokio::fs; use fabro_core::error::{CoreError, Result as CoreResult}; use fabro_core::graph::NodeSpec; @@ -67,7 +64,6 @@ pub(crate) struct GitCheckpointResult { pub(crate) struct GitLifecycle { pub sandbox: Arc, pub emitter: Arc, - pub run_dir: PathBuf, pub run_id: RunId, pub run_store: RunStoreHandle, pub run_options: Arc, @@ -301,7 +297,7 @@ impl RunLifecycle for GitLifecycle { } async fn on_run_end(&self, outcome: &Outcome, _state: &WfRunState) { - // Write final.patch on success + // Capture the final diff on success for event/store projection. if (outcome.status == StageStatus::Success || outcome.status == StageStatus::PartialSuccess) && self.run_options.git.is_some() { @@ -314,15 +310,6 @@ impl RunLifecycle for GitLifecycle { match git_diff(&*self.sandbox, &base_sha).await { Ok(patch) if !patch.is_empty() => { *self.final_patch.lock().unwrap() = Some(patch.clone()); - if let Err(err) = - fs::write(RunScratch::new(&self.run_dir).final_patch(), patch).await - { - self.emitter.emit(&Event::RunNotice { - level: RunNoticeLevel::Warn, - code: "final_patch_write_failed".to_string(), - message: format!("failed to write final.patch: {err}"), - }); - } } Ok(_) => { *self.final_patch.lock().unwrap() = None; diff --git a/lib/crates/fabro-workflow/src/lifecycle/mod.rs b/lib/crates/fabro-workflow/src/lifecycle/mod.rs index 6cb58dad2..5bb251148 100644 --- a/lib/crates/fabro-workflow/src/lifecycle/mod.rs +++ b/lib/crates/fabro-workflow/src/lifecycle/mod.rs @@ -153,7 +153,6 @@ impl WorkflowLifecycle { let git = GitLifecycle { sandbox: Arc::clone(sandbox), emitter: Arc::clone(emitter), - run_dir: run_dir.clone(), run_id: run_options.run_id, run_store: run_store.clone(), run_options: Arc::clone(run_options), @@ -166,7 +165,6 @@ impl WorkflowLifecycle { let artifact = ArtifactLifecycle::new( Arc::clone(sandbox), run_store.clone(), - run_scratch.blob_cache_dir(), Arc::clone(emitter), run_scratch.artifact_files_dir(), run_options.artifact_globs().to_vec(), diff --git a/lib/crates/fabro-workflow/tests/it/daytona_integration.rs b/lib/crates/fabro-workflow/tests/it/daytona_integration.rs index 2ade1715f..924445c2b 100644 --- a/lib/crates/fabro-workflow/tests/it/daytona_integration.rs +++ b/lib/crates/fabro-workflow/tests/it/daytona_integration.rs @@ -728,11 +728,12 @@ async fn daytona_git_checkpoint_remote_emits_events() { "checkpoint should have git_commit_sha" ); - // Assert final.patch exists and contains changes from the run + // Assert scratch final.patch is no longer written let final_patch = dir.path().join("final.patch"); - assert!(final_patch.exists(), "final.patch should exist in run_dir"); - let patch_content = std::fs::read_to_string(&final_patch).unwrap(); - assert!(!patch_content.is_empty(), "final.patch should not be empty"); + assert!( + !final_patch.exists(), + "final.patch should not be written to scratch" + ); env.cleanup().await.unwrap(); } @@ -1242,9 +1243,12 @@ async fn daytona_git_checkpoint_with_shadow_branch() { "sandbox commit should have Fabro-Run trailer, got:\n{commit_msg}" ); - // Assert final.patch exists + // Assert scratch final.patch is no longer written let final_patch = dir.path().join("final.patch"); - assert!(final_patch.exists(), "final.patch should exist in run_dir"); + assert!( + !final_patch.exists(), + "final.patch should not be written to scratch" + ); env.cleanup().await.unwrap(); } diff --git a/lib/crates/fabro-workflow/tests/it/integration.rs b/lib/crates/fabro-workflow/tests/it/integration.rs index 2b3708ae4..ef3b8af49 100644 --- a/lib/crates/fabro-workflow/tests/it/integration.rs +++ b/lib/crates/fabro-workflow/tests/it/integration.rs @@ -8667,7 +8667,10 @@ async fn large_context_values_are_offloaded_to_artifact_store() { assert!( !RunScratch::new(dir.path()) - .blob_cache_dir() + .root() + .join("cache") + .join("artifacts") + .join("values") .join(format!("{expected_blob_id}.json")) .exists(), "legacy host blob cache file should not exist" @@ -10384,13 +10387,11 @@ async fn git_checkpoint_host_emits_events_and_diff_patch() { "checkpoint should have git_commit_sha" ); - // 9. Assert final.patch exists and contains the changes + // 9. Assert scratch final.patch is no longer written let final_patch = run_dir.path().join("final.patch"); - assert!(final_patch.exists(), "final.patch should exist in run_dir"); - let patch_content = std::fs::read_to_string(&final_patch).unwrap(); assert!( - patch_content.contains("hello.txt"), - "final.patch should contain hello.txt changes" + !final_patch.exists(), + "final.patch should not be written to scratch" ); // Cleanup worktree @@ -10827,17 +10828,11 @@ async fn parallel_git_branching_host_e2e() { "parallel branch ref should still exist for debugging" ); - // 11. Verify final.patch contains the winner's changes + // 11. Verify scratch final.patch is no longer written let final_patch = run_dir.path().join("final.patch"); - assert!(final_patch.exists(), "final.patch should exist in run_dir"); - let patch_content = std::fs::read_to_string(&final_patch).unwrap(); assert!( - patch_content.contains(&format!("{best_id}.txt")), - "final.patch should contain winner's file" - ); - assert!( - !patch_content.contains(&format!("{loser_id}.txt")), - "final.patch should NOT contain loser's file" + !final_patch.exists(), + "final.patch should not be written to scratch" ); // 12. Verify events