Resolve the saved final patch when creating a pull request afterwards

Creating a pull request for a finished run read the projection's final
patch field directly. Petri stores that patch in the blob table and the
field holds a blob reference, so the description model was handed the
reference string instead of the diff, and the empty-diff check could
never fire.

Move the reference resolution out of Files Changed into a shared
final_patch::load helper and use it for both readers. Pull request input
extraction now reads the real patch, judges emptiness by its text, and
reports a missing or unreadable blob instead of describing a placeholder.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

lib/apps/fabro-server/src/final_patch.rs

@@ -0,0 +1,119 @@
+//! A run's final patch as text.
+//!
+//! Petri keeps the final patch in the blob table, and the projection's
+//! `conclusion.diff.patch` carries a `blob://sha256/<hex>` reference to it;
+//! older projections carry the patch text inline. Server readers resolve the
+//! patch here so none of them mistakes the reference for patch text.
+
+use std::borrow::Cow;
+
+use axum::http::StatusCode;
+use fabro_store::RunProjection;
+use fabro_types::blob_ref;
+use fabro_util::error;
+
+use crate::error::ApiError;
+use crate::server::AppState;
+
+/// The run's final patch, or `None` when the run recorded none. A reference
+/// whose bytes are missing, fail the blob store's integrity check, or are not
+/// UTF-8 is an error, never an empty patch.
+pub(crate) async fn load<'a>(
+    state: &AppState,
+    projection: &'a RunProjection,
+) -> Result<Option<Cow<'a, str>>, ApiError> {
+    let Some(patch) = projection
+        .conclusion
+        .as_ref()
+        .and_then(|conclusion| conclusion.diff.patch.as_deref())
+    else {
+        return Ok(None);
+    };
+    let Some(hash) = blob_ref::parse_blob_ref(patch.trim()) else {
+        return Ok(Some(Cow::Borrowed(patch)));
+    };
+
+    let run_id = &projection.spec.run_id;
+    let 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.",
+            )
+        })?;
+    let text = String::from_utf8(bytes.into()).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.",
+        )
+    })?;
+    Ok(Some(Cow::Owned(text)))
+}
+
+#[cfg(test)]
+mod tests {
+    use axum::http::StatusCode;
+    use fabro_types::blob_ref;
+
+    use super::load;
+    use crate::test_support::{test_app_state, test_concluded_run_projection};
+
+    #[tokio::test]
+    async fn inline_and_absent_patches_pass_through() {
+        let state = test_app_state();
+        let projection = test_concluded_run_projection(Some("diff --git a/x b/x\n"));
+        let patch = load(&state, &projection).await.unwrap();
+        assert_eq!(patch.as_deref(), Some("diff --git a/x b/x\n"));
+
+        let projection = test_concluded_run_projection(None);
+        assert!(load(&state, &projection).await.unwrap().is_none());
+    }
+
+    #[tokio::test]
+    async fn blob_reference_resolves_to_its_bytes() {
+        let state = test_app_state();
+        let text = "diff --git a/x b/x\n+saved\n";
+        let hash = state
+            .store_ref()
+            .blobs()
+            .write(text.as_bytes())
+            .await
+            .unwrap();
+        let projection = test_concluded_run_projection(Some(&blob_ref::format_blob_ref(&hash)));
+        let patch = load(&state, &projection).await.unwrap();
+        assert_eq!(patch.as_deref(), Some(text));
+    }
+
+    #[tokio::test]
+    async fn missing_and_non_utf8_blobs_are_errors() {
+        let state = test_app_state();
+        let absent = fabro_types::BlobHash::new(b"never written");
+        let projection = test_concluded_run_projection(Some(&blob_ref::format_blob_ref(&absent)));
+        let error = load(&state, &projection).await.unwrap_err();
+        assert_eq!(error.status(), StatusCode::INTERNAL_SERVER_ERROR);
+        assert_eq!(error.detail(), "The saved final patch is missing.");
+
+        let hash = state.store_ref().blobs().write(&[0xff]).await.unwrap();
+        let projection = test_concluded_run_projection(Some(&blob_ref::format_blob_ref(&hash)));
+        let error = load(&state, &projection).await.unwrap_err();
+        assert_eq!(error.status(), StatusCode::INTERNAL_SERVER_ERROR);
+        assert_eq!(error.detail(), "The saved final patch is not valid UTF-8.");
+    }
+}

lib/apps/fabro-server/src/lib.rs

@@ -25,6 +25,7 @@ pub mod csp;
 mod demo;
 pub mod diagnostics;
 pub mod error;
+mod final_patch;
 mod git_checkout;
 pub mod github_webhooks;
 pub mod install;

lib/apps/fabro-server/src/run_files.rs

@@ -35,8 +35,8 @@ use fabro_api::types::{
     RunFilesMetaToSha,
 };
 use fabro_redact::SecretRedactor;
-use fabro_types::{RunId, blob_ref};
-use fabro_util::{error, shell};
+use fabro_types::RunId;
+use fabro_util::shell;
 use fabro_workflow::sandbox_git::{
     DiffError, DiffNumstat, RawDiffEntry, SubmoduleChange, SymlinkChange, list_changed_files_raw,
     list_diff_numstat, stream_blob_metadata, stream_blobs,
@@ -53,8 +53,8 @@ use tokio::sync::{Mutex, watch};
 use crate::error::ApiError;
 use crate::principal_middleware::RequiredUser;
 use crate::run_files_security::{RunFilesMetrics, is_sensitive};
-use crate::sandbox_access;
 use crate::server::{AppState, parse_run_id_path};
+use crate::{final_patch, sandbox_access};
 
 /// Per-file cap: 256 KiB OR 20k lines (whichever comes first).
 pub(crate) const PER_FILE_BYTES_CAP: u64 = 256 * 1024;
@@ -816,8 +816,8 @@ fn sandbox_git_error(op: &str, error: &sandbox_driver::Error) -> ApiError {
     transient_503(op, &display_for_log(error, &SecretRedactor))
 }
 
-/// Resolve the terminal patch before parsing it: Petri's projection carries
-/// a blob reference, while older projections may carry inline patch text.
+/// The degraded response from the run's final patch, resolved through
+/// [`final_patch::load`] so a blob reference is read, not parsed as a patch.
 async fn load_fallback_response(
     state: &AppState,
     projection: &fabro_store::RunProjection,
@@ -825,55 +825,14 @@ async fn load_fallback_response(
     run_id: &RunId,
     start: Instant,
 ) -> ListRunFilesResult {
-    let Some(patch) = projection
-        .conclusion
-        .as_ref()
-        .and_then(|conclusion| conclusion.diff.patch.as_deref())
-    else {
+    let Some(patch) = final_patch::load(state, projection).await? else {
         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,
+        projection, &patch, reason, run_id, start,
     ))
 }
 
@@ -1805,12 +1764,13 @@ fn count_flags(data: &[FileDiff]) -> (u64, u64, u64, u64) {
 mod tests {
     use std::sync::atomic::{AtomicUsize, Ordering};
 
-    use fabro_types::{PetriAdmission, RunId, test_support};
+    use fabro_types::RunId;
     use pebble_coding_agent::sandbox_driver::test_support::{MockSandbox, exec_result};
     use sandbox_driver::ExecResult;
     use tokio::time::{Duration, sleep};
 
     use super::*;
+    use crate::test_support::{test_app_state, test_concluded_run_projection};
 
     /// The mock as the Run Files endpoints hold a sandbox.
     fn checkout(mock: &MockSandbox) -> SandboxCheckout {
@@ -2407,49 +2367,9 @@ index 1111111..2222222 160000
         }
     }
 
-    fn fallback_projection(patch: &str) -> fabro_store::RunProjection {
-        let mut projection = fabro_store::RunProjection::new(
-            "Test run".to_string(),
-            fabro_types::RunSpec {
-                run_id:              fabro_types::fixtures::RUN_1,
-                settings:            fabro_types::WorkflowSettings::default(),
-                graph:               fabro_types::RunGraph::new("test"),
-                graph_source:        None,
-                workflow_slug:       None,
-                workflow_version_id: None,
-                target:              None,
-                automation:          None,
-                source_directory:    None,
-                labels:              HashMap::default(),
-                provenance:          test_support::test_run_provenance(),
-                definition_blob:     None,
-                spec_blob:           None,
-                git:                 None,
-                fork_source_ref:     None,
-                admission:           PetriAdmission::default(),
-            },
-            chrono::Utc::now(),
-        );
-        projection.conclusion = Some(fabro_types::Conclusion {
-            timestamp:            chrono::Utc::now(),
-            status:               fabro_types::StageOutcome::Succeeded,
-            timing:               fabro_types::RunTiming::wall_only(1),
-            failure:              None,
-            final_git_commit_sha: None,
-            stages:               Vec::new(),
-            usage:                None,
-            total_retries:        0,
-            diff:                 fabro_types::RunDiff {
-                patch:   Some(patch.to_string()),
-                summary: None,
-            },
-        });
-        projection
-    }
-
     fn fallback_response_json(patch: &str) -> serde_json::Value {
         serde_json::to_value(build_fallback_response(
-            &fallback_projection(patch),
+            &test_concluded_run_projection(Some(patch)),
             patch,
             RunFilesMetaDegradedReason::SandboxGone,
             &RunId::new(),
@@ -2460,9 +2380,9 @@ index 1111111..2222222 160000
 
     #[tokio::test]
     async fn fallback_loader_preserves_inline_and_absent_patches() {
-        let state = crate::test_support::test_app_state();
+        let state = test_app_state();
         let patch = simple_patch("README.md");
-        let mut projection = fallback_projection(&patch);
+        let mut projection = test_concluded_run_projection(Some(&patch));
         let response = load_fallback_response(
             &state,
             &projection,
@@ -2492,24 +2412,6 @@ index 1111111..2222222 160000
         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,

lib/apps/fabro-server/src/server/handler/pull_requests.rs

@@ -1,3 +1,4 @@
+use std::borrow::Cow;
 use std::sync::Arc;
 use std::time::Duration;
 
@@ -12,6 +13,7 @@ use super::super::{
     PullRequestLink, RequireRunScoped, Response, Router, RunId, State, StatusCode, get, post,
     run_records, warn,
 };
+use crate::final_patch;
 
 pub(super) fn routes() -> Router<Arc<AppState>> {
     Router::new()
@@ -189,13 +191,16 @@ pub(in crate::server) struct RunPrInputs<'a> {
     pub(in crate::server) base_branch:       &'a str,
     pub(in crate::server) run_branch:        &'a str,
     pub(in crate::server) final_git_sha:     &'a str,
-    pub(in crate::server) diff:              &'a str,
+    pub(in crate::server) diff:              Cow<'a, str>,
     pub(in crate::server) conclusion:        &'a fabro_types::Conclusion,
     pub(in crate::server) normalized_origin: String,
 }
 
 impl<'a> RunPrInputs<'a> {
-    pub(in crate::server) fn extract(
+    /// What a pull request for the run needs, with its final patch resolved
+    /// from the blob table so the description is written from the real diff.
+    pub(in crate::server) async fn extract(
+        state: &AppState,
         run_state: &'a fabro_store::RunProjection,
         force: bool,
     ) -> Result<Self, ApiError> {
@@ -228,10 +233,8 @@ impl<'a> RunPrInputs<'a> {
                     "missing_run_branch",
                 )
             })?;
-        let diff = run_state
-            .conclusion
-            .as_ref()
-            .and_then(|conclusion| conclusion.diff.patch.as_deref())
+        let diff = final_patch::load(state, run_state)
+            .await?
             .filter(|d| !d.trim().is_empty())
             .ok_or_else(|| {
                 ApiError::with_code(
@@ -333,7 +336,7 @@ async fn create_run_pull_request(
     {
         return accepted_pull_request_creation_response(&id, creation.clone());
     }
-    if let Err(err) = RunPrInputs::extract(&run_state, body.force) {
+    if let Err(err) = RunPrInputs::extract(&state, &run_state, body.force).await {
         return err.into_response();
     }
     if let Err(err) = load_server_github_credentials(state.as_ref()).await {
@@ -605,3 +608,75 @@ async fn close_run_pull_request(
         Err(err) => ApiError::new(StatusCode::BAD_GATEWAY, err.to_string()).into_response(),
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use axum::http::StatusCode;
+    use fabro_types::{DirtyStatus, GitContext, StartRecord, blob_ref};
+
+    use super::RunPrInputs;
+    use crate::test_support::{test_app_state, test_concluded_run_projection};
+
+    /// A finished run with everything a pull request needs and `patch` as
+    /// its final diff.
+    fn pull_request_ready_projection(patch: &str) -> fabro_store::RunProjection {
+        let mut projection = test_concluded_run_projection(Some(patch));
+        projection.spec.git = Some(GitContext {
+            origin_url: "https://github.com/acme/widgets.git".to_string(),
+            branch:     "main".to_string(),
+            sha:        None,
+            dirty:      DirtyStatus::Clean,
+        });
+        projection.start = Some(StartRecord {
+            start_time: chrono::Utc::now(),
+            run_branch: Some("fabro/run/test".to_string()),
+            base_sha:   None,
+        });
+        projection.conclusion.as_mut().unwrap().final_git_commit_sha = Some("abc123".to_string());
+        projection
+    }
+
+    #[tokio::test]
+    async fn extract_reads_a_blob_backed_final_patch() {
+        let state = test_app_state();
+        let text = "diff --git a/src/lib.rs b/src/lib.rs\n+fn x() {}\n";
+        let hash = state
+            .store_ref()
+            .blobs()
+            .write(text.as_bytes())
+            .await
+            .unwrap();
+        let projection = pull_request_ready_projection(&blob_ref::format_blob_ref(&hash));
+
+        let inputs = RunPrInputs::extract(&state, &projection, false)
+            .await
+            .unwrap();
+        assert_eq!(inputs.diff, text);
+    }
+
+    #[tokio::test]
+    async fn extract_judges_emptiness_by_the_resolved_patch() {
+        let state = test_app_state();
+        let hash = state.store_ref().blobs().write(b"\n").await.unwrap();
+        let projection = pull_request_ready_projection(&blob_ref::format_blob_ref(&hash));
+
+        let Err(error) = RunPrInputs::extract(&state, &projection, false).await else {
+            panic!("a whitespace-only saved patch should be empty");
+        };
+        assert_eq!(error.status(), StatusCode::BAD_REQUEST);
+        assert_eq!(error.code(), Some("empty_diff"));
+    }
+
+    #[tokio::test]
+    async fn extract_reports_a_missing_patch_blob() {
+        let state = test_app_state();
+        let absent = fabro_types::BlobHash::new(b"never written");
+        let projection = pull_request_ready_projection(&blob_ref::format_blob_ref(&absent));
+
+        let Err(error) = RunPrInputs::extract(&state, &projection, false).await else {
+            panic!("a missing patch blob should fail extraction");
+        };
+        assert_eq!(error.status(), StatusCode::INTERNAL_SERVER_ERROR);
+        assert_eq!(error.detail(), "The saved final patch is missing.");
+    }
+}

lib/apps/fabro-server/src/server/pull_request_supervisor.rs

@@ -161,7 +161,7 @@ async fn attempt_pull_request_creation(
     run_state: &fabro_store::RunProjection,
     creation: &PullRequestCreation,
 ) -> anyhow::Result<Result<(), String>> {
-    let inputs = match RunPrInputs::extract(run_state, creation.force) {
+    let inputs = match RunPrInputs::extract(state, run_state, creation.force).await {
         Ok(inputs) => inputs,
         Err(err) => return Ok(Err(err.detail().to_string())),
     };
@@ -181,7 +181,7 @@ async fn attempt_pull_request_creation(
         head_branch: inputs.run_branch,
         expected_head_sha: inputs.final_git_sha,
         goal: inputs.goal,
-        diff: inputs.diff,
+        diff: &inputs.diff,
         model: &creation.model,
         draft: true,
         auto_merge: None,

lib/apps/fabro-server/src/test_support.rs

@@ -381,6 +381,54 @@ pub fn test_app_state() -> Arc<AppState> {
     ready_test_app_state_builder().build()
 }
 
+/// A succeeded run's projection whose conclusion carries `patch` as its final
+/// diff: patch text, a blob reference, or nothing.
+#[cfg(test)]
+pub(crate) fn test_concluded_run_projection(patch: Option<&str>) -> fabro_store::RunProjection {
+    use fabro_types::{
+        Conclusion, PetriAdmission, RunDiff, RunGraph, RunSpec, RunTiming, StageOutcome,
+        WorkflowSettings, fixtures,
+    };
+
+    let mut projection = fabro_store::RunProjection::new(
+        "Test run".to_string(),
+        RunSpec {
+            run_id:              fixtures::RUN_1,
+            settings:            WorkflowSettings::default(),
+            graph:               RunGraph::new("test"),
+            graph_source:        None,
+            workflow_slug:       None,
+            workflow_version_id: None,
+            target:              None,
+            automation:          None,
+            source_directory:    None,
+            labels:              HashMap::default(),
+            provenance:          fabro_types::test_support::test_run_provenance(),
+            definition_blob:     None,
+            spec_blob:           None,
+            git:                 None,
+            fork_source_ref:     None,
+            admission:           PetriAdmission::default(),
+        },
+        chrono::Utc::now(),
+    );
+    projection.conclusion = Some(Conclusion {
+        timestamp:            chrono::Utc::now(),
+        status:               StageOutcome::Succeeded,
+        timing:               RunTiming::wall_only(1),
+        failure:              None,
+        final_git_commit_sha: None,
+        stages:               Vec::new(),
+        usage:                None,
+        total_retries:        0,
+        diff:                 RunDiff {
+            patch:   patch.map(str::to_string),
+            summary: None,
+        },
+    });
+    projection
+}
+
 pub fn test_app_state_in_process() -> Arc<AppState> {
     ready_test_app_state_builder()
         .in_process_execution()