Resolve durable final patch blobs for Files Changed

This commit is contained in:
Scott Werner 2026-10-01 14:42:33 -04:00
parent e7b4859038
commit 82156442a8
3 changed files with 326 additions and 34 deletions

View file

@ -326,6 +326,25 @@ describe("RunFiles rendering", () => {
expect(lastCall.mountId).not.toBe(firstMountId);
});
test("renders the saved final patch after the sandbox is gone", () => {
const patch = "diff --git a/docs/live.md b/docs/live.md\n--- a/docs/live.md\n+++ b/docs/live.md\n@@ -1 +1,2 @@\n original\n+saved change\n";
currentFilesPayload = makePatchPayload(patch);
currentFilesPayload.meta = {
...currentFilesPayload.meta,
source: "final_patch",
degraded: true,
degraded_reason: "sandbox_gone",
};
const renderer = renderRunFiles();
expect(renderer.root.findAllByProps({ "data-run-file-row": "true" })).toHaveLength(1);
expect(patchDiffCalls).toHaveLength(1);
expect(patchDiffCalls[0].patch).toBe(patch);
expect(multiFileDiffCalls).toHaveLength(0);
expect(treeText(renderer.toJSON())).not.toContain("Diff unavailable");
});
test("refreshing from a populated diff to an empty diff shows a no-changes toast", async () => {
currentFilesPayload = makePayload(1);
const renderer = renderRunFiles("/runs/run_1/files?scope=all");

View file

@ -35,8 +35,8 @@ use fabro_api::types::{
RunFilesMetaToSha,
};
use fabro_redact::SecretRedactor;
use fabro_types::RunId;
use fabro_util::shell;
use fabro_types::{RunId, blob_ref};
use fabro_util::{error, shell};
use fabro_workflow::sandbox_git::{
DiffError, DiffNumstat, RawDiffEntry, SubmoduleChange, SymlinkChange, list_changed_files_raw,
list_diff_numstat, stream_blob_metadata, stream_blobs,
@ -250,9 +250,9 @@ where
/// 4. On garbage-collected base commits for aggregate scopes, fall through to a
/// degraded response built from the terminal conclusion diff.
///
/// All logging emits a single `tracing::info!` with an allowlisted field
/// set enforced by [`RunFilesMetrics::emit`] — no paths, contents, or raw
/// git stderr.
/// Success metrics emit a single `tracing::info!` with an allowlisted field
/// set enforced by [`RunFilesMetrics::emit`]. Logs exclude paths, contents,
/// and raw git stderr.
pub async fn list_run_files(
_auth: RequiredUser,
State(state): State<Arc<AppState>>,
@ -538,20 +538,29 @@ async fn materialize_sandbox_path(
let sandbox = match reconnect_run_sandbox(state, run_id, &projection).await {
Ok(sandbox) => sandbox,
Err(err) if sandbox_read_error_should_fallback(&err) => {
return Ok(build_fallback_response(
return load_fallback_response(
state,
&projection,
RunFilesMetaDegradedReason::SandboxGone,
run_id,
start,
));
)
.await;
}
Err(err) => return Err(err),
};
let materialized = match scope {
ListRunFilesScope::Committed => {
materialize_committed_sandbox_path(&sandbox, &projection, &base_sha, run_id, start)
.await
materialize_committed_sandbox_path(
state,
&sandbox,
&projection,
&base_sha,
run_id,
start,
)
.await
}
ListRunFilesScope::Uncommitted => {
materialize_working_tree_sandbox_path(
@ -577,12 +586,16 @@ async fn materialize_sandbox_path(
match materialized {
Ok(body) => Ok(body),
Err(err) if sandbox_read_error_should_fallback(&err) => Ok(build_fallback_response(
&projection,
RunFilesMetaDegradedReason::SandboxGone,
run_id,
start,
)),
Err(err) if sandbox_read_error_should_fallback(&err) => {
load_fallback_response(
state,
&projection,
RunFilesMetaDegradedReason::SandboxGone,
run_id,
start,
)
.await
}
Err(err) => Err(err),
}
}
@ -595,6 +608,7 @@ fn sandbox_read_error_should_fallback(err: &ApiError) -> bool {
}
async fn materialize_committed_sandbox_path(
state: &AppState,
sandbox: &SandboxCheckout,
projection: &fabro_store::RunProjection,
base_sha: &str,
@ -605,7 +619,7 @@ async fn materialize_committed_sandbox_path(
let (to_sha, to_sha_committed_at) = resolve_head_sha_and_time(sandbox).await?;
materialize_committed_range_sandbox_path(
sandbox,
Some(projection),
Some((state, projection)),
base_sha,
&to_sha,
to_sha_committed_at,
@ -618,7 +632,7 @@ async fn materialize_committed_sandbox_path(
async fn materialize_committed_range_sandbox_path(
sandbox: &SandboxCheckout,
fallback_projection: Option<&fabro_store::RunProjection>,
fallback: Option<(&AppState, &fabro_store::RunProjection)>,
base_sha: &str,
to_sha: &str,
to_sha_committed_at: Option<chrono::DateTime<chrono::Utc>>,
@ -649,13 +663,15 @@ async fn materialize_committed_range_sandbox_path(
let raw_entries = match raw_res {
Ok(v) => v,
Err(DiffError::Permanent { .. }) => {
if let Some(projection) = fallback_projection {
return Ok(build_fallback_response(
if let Some((state, projection)) = fallback {
return load_fallback_response(
state,
projection,
RunFilesMetaDegradedReason::SandboxGone,
run_id,
start,
));
)
.await;
}
return Err(ApiError::bad_request("Invalid git diff range."));
}
@ -800,24 +816,76 @@ fn sandbox_git_error(op: &str, error: &sandbox_driver::Error) -> ApiError {
transient_503(op, &display_for_log(error, &SecretRedactor))
}
/// Build the degraded response from the stored terminal diff patch.
/// When `conclusion.diff.patch` is `None`, returns the empty envelope (UI maps
/// this to R4(c)). Keeps the same `FileDiff[]` shape as live responses, but
/// leaves contents unavailable because the server only has a unified patch.
fn build_fallback_response(
/// Resolve the terminal patch before parsing it: Petri's projection carries
/// a blob reference, while older projections may carry inline patch text.
async fn load_fallback_response(
state: &AppState,
projection: &fabro_store::RunProjection,
reason: RunFilesMetaDegradedReason,
run_id: &RunId,
start: Instant,
) -> PaginatedRunFileList {
) -> ListRunFilesResult {
let Some(patch) = projection
.conclusion
.as_ref()
.and_then(|conclusion| conclusion.diff.patch.as_deref())
else {
return empty_envelope(RunFilesMetaSource::FinalPatch, RunFilesMetaScope::Committed);
return Ok(empty_envelope(
RunFilesMetaSource::FinalPatch,
RunFilesMetaScope::Committed,
));
};
let bytes;
let patch = if let Some(hash) = blob_ref::parse_blob_ref(patch) {
bytes = state
.store_ref()
.blobs()
.read(&hash)
.await
.map_err(|error| {
tracing::error!(
%run_id,
error = %error::collect_chain(&error).join(": "),
"Failed to read final patch blob"
);
ApiError::new(
StatusCode::INTERNAL_SERVER_ERROR,
"The saved final patch could not be read.",
)
})?
.ok_or_else(|| {
tracing::error!(%run_id, "Final patch blob is missing");
ApiError::new(
StatusCode::INTERNAL_SERVER_ERROR,
"The saved final patch is missing.",
)
})?;
std::str::from_utf8(&bytes).map_err(|error| {
tracing::error!(%run_id, %error, "Final patch blob is not UTF-8");
ApiError::new(
StatusCode::INTERNAL_SERVER_ERROR,
"The saved final patch is not valid UTF-8.",
)
})?
} else {
patch
};
Ok(build_fallback_response(
projection, patch, reason, run_id, start,
))
}
/// Build the degraded response from resolved patch text. Full file contents
/// remain unavailable because the server only has a unified patch.
fn build_fallback_response(
projection: &fabro_store::RunProjection,
patch: &str,
reason: RunFilesMetaDegradedReason,
run_id: &RunId,
start: Instant,
) -> PaginatedRunFileList {
let entries: Vec<String> = split_patch_sections(patch)
.into_iter()
.map(|section| section.text.to_string())
@ -2382,6 +2450,7 @@ index 1111111..2222222 160000
fn fallback_response_json(patch: &str) -> serde_json::Value {
serde_json::to_value(build_fallback_response(
&fallback_projection(patch),
patch,
RunFilesMetaDegradedReason::SandboxGone,
&RunId::new(),
Instant::now(),
@ -2389,6 +2458,58 @@ index 1111111..2222222 160000
.expect("fallback response should serialize")
}
#[tokio::test]
async fn fallback_loader_preserves_inline_and_absent_patches() {
let state = crate::test_support::test_app_state();
let patch = simple_patch("README.md");
let mut projection = fallback_projection(&patch);
let response = load_fallback_response(
&state,
&projection,
RunFilesMetaDegradedReason::SandboxGone,
&projection.spec.run_id,
Instant::now(),
)
.await
.unwrap();
assert_eq!(response.data.len(), 1);
assert_eq!(
response.data[0].unified_patch.as_deref(),
Some(patch.as_str())
);
projection.conclusion.as_mut().unwrap().diff.patch = None;
let response = load_fallback_response(
&state,
&projection,
RunFilesMetaDegradedReason::SandboxGone,
&projection.spec.run_id,
Instant::now(),
)
.await
.unwrap();
assert!(response.data.is_empty());
assert_eq!(response.meta.total_changed, 0);
}
#[tokio::test]
async fn fallback_loader_rejects_non_utf8_patch_bytes() {
let state = crate::test_support::test_app_state();
let hash = state.store_ref().blobs().write(&[0xff]).await.unwrap();
let projection = fallback_projection(&blob_ref::format_blob_ref(&hash));
let error = load_fallback_response(
&state,
&projection,
RunFilesMetaDegradedReason::SandboxGone,
&projection.spec.run_id,
Instant::now(),
)
.await
.unwrap_err();
assert_eq!(error.status(), StatusCode::INTERNAL_SERVER_ERROR);
assert_eq!(error.detail(), "The saved final patch is not valid UTF-8.");
}
fn sandbox_patch_response_json(entries: &[String]) -> serde_json::Value {
serde_json::to_value(build_patch_backed_response(
entries,

View file

@ -1,18 +1,21 @@
//! HTTP-level integration tests for `GET /api/v1/runs/{id}/files`.
//!
//! These tests exercise the handler's request-plumbing branches —
//! authentication extractor, route matching, query validation, demo-mode
//! branching, and the empty-envelope / not-found responses — without
//! requiring a reconnected sandbox. The sandbox happy path is covered by
//! unit tests on the sandbox-git helpers and by `stitch_file_diff` tests
//! in `run_files.rs`.
//! Request plumbing, empty/not-found responses, and recovery from a durable
//! final patch after deleting an isolated local run's sandbox. Sandbox-git
//! helpers and file assembly also have unit coverage in `run_files.rs`.
use std::sync::Arc;
use axum::body::Body;
use axum::http::{Request, StatusCode};
use fabro_server::test_support::{TestAppStateBuilder, test_store_bundle};
use fabro_store::{BlobStore, test_support as store_test_support};
use fabro_types::blob_ref;
use tower::ServiceExt;
use crate::helpers::{
MINIMAL_DOT, api, minimal_intent_json, response_json, response_status, test_app_state,
MINIMAL_DOT, api, create_and_start_run_from_intent, minimal_intent_json, response_json,
response_status, run_json, test_app_state, test_app_with_scheduler, wait_for_run_status,
};
fn files_url(run_id: &str) -> String {
@ -27,6 +30,155 @@ fn files_url_with_scope(run_id: &str, scope: &str) -> String {
format!("{}?scope={scope}", files_url(run_id))
}
async fn get_json(app: &axum::Router, url: &str, status: StatusCode) -> serde_json::Value {
let response = app
.clone()
.oneshot(
Request::builder()
.uri(url)
.body(Body::empty())
.expect("GET request should build"),
)
.await
.expect("test router should respond");
response_json(response, status, format!("GET {url}")).await
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn final_patch_survives_sandbox_removal_and_reports_unreadable_blobs() {
// Execute a real command-only run in an isolated local checkout. Keeping
// the blob pool lets us simulate storage damage after verifying recovery.
let workspace = tempfile::tempdir().unwrap();
tokio::fs::write(workspace.path().join("README.md"), "original\n")
.await
.unwrap();
{
let repo = git2::Repository::init(workspace.path()).unwrap();
let mut index = repo.index().unwrap();
index.add_path(std::path::Path::new("README.md")).unwrap();
index.write().unwrap();
let tree_id = index.write_tree().unwrap();
let tree = repo.find_tree(tree_id).unwrap();
let signature = git2::Signature::now("Fabro Test", "fabro@example.com").unwrap();
repo.commit(Some("HEAD"), &signature, &signature, "initial", &tree, &[])
.unwrap();
}
let pool = store_test_support::in_memory_pool_with(&[fabro_db::BLOBS_MIGRATION_SQL]);
let blobs = Arc::new(BlobStore::new(pool.clone()));
let store = Arc::new(store_test_support::test_database_with_blobs(Arc::clone(
&blobs,
)));
let (_, artifacts) = test_store_bundle();
let state = TestAppStateBuilder::new()
.in_process_execution()
.store_bundle(store, artifacts)
.build();
let app = test_app_with_scheduler(Arc::clone(&state));
let workflow = r#"digraph Changes {
start [shape=Mdiamond]
edit [shape=parallelogram, script="printf 'updated\\n' >> README.md; mkdir -p artifacts; printf 'saved\\n' > artifacts/report.txt; printf '\\000\\001' > artifacts/data.bin"]
exit [shape=Msquare]
start -> edit -> exit
}"#;
let mut intent = minimal_intent_json(&app, workflow, workspace.path()).await;
intent["title"] = serde_json::json!("Saved final patch");
let run_id = create_and_start_run_from_intent(&app, intent).await;
let status = wait_for_run_status(&app, &run_id, &["succeeded", "failed"]).await;
assert_eq!(status, "succeeded", "{}", run_json(&app, &run_id).await);
state
.test_petri_projector()
.settle(run_id.parse().unwrap())
.await;
let projection = get_json(&app, &api(&format!("/runs/{run_id}/state")), StatusCode::OK).await;
let reference = projection["conclusion"]["diff"]["patch"].as_str().unwrap();
let hash = blob_ref::parse_blob_ref(reference).expect("the final patch is blob-backed");
let patch = blobs
.read(&hash)
.await
.unwrap()
.expect("durable patch bytes");
let live = get_json(&app, &files_url(&run_id), StatusCode::OK).await;
assert_eq!(live["meta"]["source"], "sandbox");
assert_eq!(live["meta"]["total_changed"], 3);
let sandbox_dir = projection["sandbox"]["instance"]["runtime"]["working_directory"]
.as_str()
.unwrap();
assert_ne!(std::path::Path::new(sandbox_dir), workspace.path());
tokio::fs::remove_dir_all(sandbox_dir).await.unwrap();
assert!(!tokio::fs::try_exists(sandbox_dir).await.unwrap());
let saved = get_json(&app, &files_url(&run_id), StatusCode::OK).await;
assert_eq!(saved["meta"]["source"], "final_patch");
assert_eq!(saved["meta"]["degraded"], true);
assert_eq!(saved["meta"]["degraded_reason"], "sandbox_gone");
assert_eq!(saved["meta"]["scope"], "committed");
assert_eq!(saved["meta"]["total_changed"], 3);
assert_eq!(
saved["meta"]["stats"],
serde_json::json!({"additions": 2, "deletions": 0})
);
let files = saved["data"].as_array().unwrap();
assert_eq!(files.len(), 3);
let file = |name: &str| {
files
.iter()
.find(|f| f["new_file"]["name"] == name)
.unwrap()
};
let readme = file("README.md");
assert_eq!(readme["change_kind"], "modified");
assert!(readme["old_file"]["contents"].is_null());
assert!(readme["new_file"]["contents"].is_null());
let readme_patch = readme["unified_patch"].as_str().unwrap();
assert!(readme_patch.contains(" original\n+updated\n"));
let report = file("artifacts/report.txt");
assert_eq!(report["change_kind"], "added");
assert!(
report["unified_patch"]
.as_str()
.unwrap()
.contains("+saved\n")
);
assert_eq!(file("artifacts/data.bin")["binary"], true);
assert!(file("artifacts/data.bin")["unified_patch"].is_null());
assert_eq!(blobs.read(&hash).await.unwrap().unwrap(), patch);
// A saved reference whose bytes are gone must not look like an empty diff.
sqlx::query("DELETE FROM blobs WHERE hash = ?")
.bind(hash.to_string())
.execute(&pool)
.await
.unwrap();
let missing = get_json(&app, &files_url(&run_id), StatusCode::INTERNAL_SERVER_ERROR).await;
assert_eq!(
missing["errors"][0]["detail"],
"The saved final patch is missing."
);
sqlx::query("INSERT INTO blobs (hash, data) VALUES (?, ?)")
.bind(hash.to_string())
.bind(b"corrupt patch".as_slice())
.execute(&pool)
.await
.unwrap();
let corrupt = get_json(&app, &files_url(&run_id), StatusCode::INTERNAL_SERVER_ERROR).await;
assert_eq!(
corrupt["errors"][0]["detail"],
"The saved final patch could not be read."
);
// Repair is visible on the next request; failed materializations aren't cached.
sqlx::query("UPDATE blobs SET data = ? WHERE hash = ?")
.bind(patch.as_ref())
.bind(hash.to_string())
.execute(&pool)
.await
.unwrap();
assert_eq!(
get_json(&app, &files_url(&run_id), StatusCode::OK).await,
saved
);
}
#[tokio::test]
async fn invalid_run_id_returns_400() {
let app = fabro_server::test_support::build_test_router(test_app_state());