~ololduck/Frogue: feat(patches): freeze diff snapshots on merge/close

~ololduck/Frogue

feat(patches): freeze diff snapshots on merge/close

9e4c8062026-10-25T10:56:19+01:00Ololduck

11 files changed, +1065, −57

frogue/src/activitypub/forge/builders.rs+1−1

@@ -77,7 +77,7 @@ pub async fn build_patch_document(
 
     // Get the diff between target and source branches
     let source_ref = format!("refs/patches/{}", patch.number);
−    let range = format!("{}...{}", patch.target_branch, source_ref);
+    let range = crate::patches::compute_diff_range(patch);
     let diff_files = repositories::git::compare_diff(bare_path, &range)
         .await
         .map_err(|e| format!("Failed to get diff: {}", e))?

frogue/src/entities/patch_proposals.rs+4−0

@@ -68,6 +68,10 @@ pub struct Model {
     /// present. This survives jj rewrites, so pushes of the same logical
     /// change route to the same patch proposal instead of creating a new one.
     pub change_id: Option<String>,
+    /// Frozen merge-base OID at merge/close time (for diff snapshots).
+    pub snapshot_base_oid: Option<String>,
+    /// Frozen source head OID at merge/close time (for diff snapshots).
+    pub snapshot_head_oid: Option<String>,
     pub created_at: DateTimeWithTimeZone,
     pub updated_at: DateTimeWithTimeZone,
     #[sea_orm(

frogue/src/git_hook.rs+2−2

@@ -498,7 +498,7 @@ async fn process_push_command(
 
         if cmd.new_oid == ZERO_OID {
             // Deleting the patch proposal
−            patches::store::close(db, pp.id)
+            patches::store::close(db, pp.id, None, None)
                 .await
                 .map_err(|e| format!("failed to close patch proposal: {}", e))?;
 

@@ -983,7 +983,7 @@ async fn emit_patch_open_activity(
         owner: owner.name.clone(),
         slug: repository.slug.clone(),
     };
−    pub const FROGUE_REPO_ROOT_VAR_KEY: &'static str = "FROGUE_REPO_ROOT";
+    pub const FROGUE_REPO_ROOT_VAR_KEY: &str = "FROGUE_REPO_ROOT";
     let bare_path = repo_path.bare_path(std::path::Path::new(
         &std::env::var(FROGUE_REPO_ROOT_VAR_KEY).unwrap_or_else(|_| DEFAULT_REPO_ROOT.to_string()),
     ));

frogue/src/patches/merge.rs+53−5

@@ -14,7 +14,12 @@ use crate::entities::patch_proposals::MergePolicy;
 #[derive(Debug)]
 pub enum MergeResult {
     /// Merge succeeded with the new target commit OID.
−    Success { new_target_oid: String },
+    /// Also includes the snapshot OIDs (base and head) for diff preservation.
+    Success {
+        new_target_oid: String,
+        snapshot_base_oid: String,
+        snapshot_head_oid: String,
+    },
     /// Merge failed due to conflicts.
     Conflict { reason: String },
     /// Merge refused by policy (e.g., fast-forward-only with divergence).

@@ -75,12 +80,28 @@ pub async fn merge(
 
     match policy {
         MergePolicy::FastForwardOnly => {
−            try_fast_forward(bare, &target_oid, &source_oid, target_branch).await
+            try_fast_forward(
+                bare,
+                &target_oid,
+                &source_oid,
+                target_branch,
+                &merge_base,
+                &source_oid,
+            )
+            .await
         }
         MergePolicy::PreferFastForward => {
             // Try FF first, fall back to merge commit
             if is_ancestor(bare, &target_oid, &source_oid).await? {
−                try_fast_forward(bare, &target_oid, &source_oid, target_branch).await
+                try_fast_forward(
+                    bare,
+                    &target_oid,
+                    &source_oid,
+                    target_branch,
+                    &merge_base,
+                    &source_oid,
+                )
+                .await
             } else {
                 create_merge_commit(bare, &merge_base, &target_oid, &source_oid, target_branch)
                     .await

@@ -91,7 +112,16 @@ pub async fn merge(
         }
         MergePolicy::RebaseThenFastForward => {
             // Rebase source onto target, then FF
−            try_rebase_and_ff(bare, &merge_base, &target_oid, &source_oid, target_branch).await
+            try_rebase_and_ff(
+                bare,
+                &merge_base,
+                &target_oid,
+                &source_oid,
+                target_branch,
+                &merge_base,
+                &source_oid,
+            )
+            .await
         }
     }
 }

@@ -102,6 +132,8 @@ async fn try_fast_forward(
     target_oid: &str,
     source_oid: &str,
     target_branch: &str,
+    snapshot_base_oid: &str,
+    snapshot_head_oid: &str,
 ) -> std::io::Result<MergeResult> {
     // Check if target is an ancestor of source (can FF)
     if !is_ancestor(bare, target_oid, source_oid).await? {

@@ -126,6 +158,8 @@ async fn try_fast_forward(
 
     Ok(MergeResult::Success {
         new_target_oid: source_oid.to_string(),
+        snapshot_base_oid: snapshot_base_oid.to_string(),
+        snapshot_head_oid: snapshot_head_oid.to_string(),
     })
 }
 

@@ -194,6 +228,8 @@ async fn create_merge_commit(
 
     Ok(MergeResult::Success {
         new_target_oid: commit_oid,
+        snapshot_base_oid: merge_base.to_string(),
+        snapshot_head_oid: source_oid.to_string(),
     })
 }
 

@@ -204,6 +240,8 @@ async fn try_rebase_and_ff(
     target_oid: &str,
     source_oid: &str,
     target_branch: &str,
+    snapshot_base_oid: &str,
+    snapshot_head_oid: &str,
 ) -> std::io::Result<MergeResult> {
     // Try to rebase: replay commits from merge_base..source onto target
     // For simplicity, we'll use git format-patch and git am

@@ -219,7 +257,15 @@ async fn try_rebase_and_ff(
 
     if commits.is_empty() {
         // No commits to rebase, just FF
−        return try_fast_forward(bare, target_oid, source_oid, target_branch).await;
+        return try_fast_forward(
+            bare,
+            target_oid,
+            source_oid,
+            target_branch,
+            snapshot_base_oid,
+            snapshot_head_oid,
+        )
+        .await;
     }
 
     // For each commit, create a patch and apply it

@@ -271,6 +317,8 @@ async fn try_rebase_and_ff(
 
     Ok(MergeResult::Success {
         new_target_oid: current_base,
+        snapshot_base_oid: snapshot_base_oid.to_string(),
+        snapshot_head_oid: snapshot_head_oid.to_string(),
     })
 }
 

frogue/src/patches/mod.rs+12−0

@@ -9,6 +9,7 @@ pub mod change_id;
 pub mod merge;
 pub mod store;
 
+use crate::entities::patch_proposals::Model as PatchProposal;
 use crate::entities::patch_proposals::{MergePolicy, Status};
 use crate::entities::repositories::MergePolicy as RepoMergePolicy;
 

@@ -21,6 +22,17 @@ pub const PATCHES_REF: &str = "refs/patches";
 /// The namespace under which individual patch proposals live.
 pub const PATCHES_NAMESPACE: &str = "refs/patches/";
 
+/// Computes the diff range for a patch proposal.
+/// If the patch has snapshot OIDs (merged or closed), uses those OIDs.
+/// Otherwise, uses the live branch references.
+pub fn compute_diff_range(patch: &PatchProposal) -> String {
+    if let (Some(base_oid), Some(head_oid)) = (&patch.snapshot_base_oid, &patch.snapshot_head_oid) {
+        format!("{}...{}", base_oid, head_oid)
+    } else {
+        format!("{}...{}", patch.target_branch, patch.source_ref)
+    }
+}
+
 /// The summary a patch proposal may be saved with: the text with the space
 /// around it taken off, or the reason it cannot be one.
 pub fn checked_summary(summary: &str) -> Result<&str, String> {

frogue/src/patches/store.rs+79−1

@@ -283,8 +283,62 @@ pub async fn update_with_resolve_url(
     pp.update(db).await
 }
 
+/// Updates a patch proposal with snapshot OIDs for diff preservation.
+#[allow(clippy::too_many_arguments)]
+pub async fn update_with_merge_snapshot(
+    db: &DatabaseConnection,
+    patch_id: i64,
+    summary: Option<&str>,
+    description: Option<&str>,
+    target_branch: Option<&str>,
+    status: Option<Status>,
+    merge_policy: Option<MergePolicy>,
+    snapshot_base_oid: Option<&str>,
+    snapshot_head_oid: Option<&str>,
+    resolve_activity_url: Option<String>,
+) -> Result<patch_proposals::Model, DbErr> {
+    let mut pp = patch_proposals::Entity::find_by_id(patch_id)
+        .one(db)
+        .await?
+        .ok_or(DbErr::RecordNotFound(format!(
+            "patch proposal {patch_id} not found"
+        )))?
+        .into_active_model();
+
+    if let Some(s) = summary {
+        pp.summary = Set(s.to_string());
+    }
+    if let Some(d) = description {
+        pp.description = Set(Some(d.to_string()));
+    }
+    if let Some(t) = target_branch {
+        pp.target_branch = Set(t.to_string());
+    }
+    if let Some(s) = status {
+        pp.status = Set(s);
+    }
+    if let Some(m) = merge_policy {
+        pp.merge_policy = Set(m);
+    }
+    if let Some(base) = snapshot_base_oid {
+        pp.snapshot_base_oid = Set(Some(base.to_string()));
+    }
+    if let Some(head) = snapshot_head_oid {
+        pp.snapshot_head_oid = Set(Some(head.to_string()));
+    }
+    pp.resolve_activity_url = Set(resolve_activity_url);
+
+    pp.update(db).await
+}
+
 /// Closes a patch proposal by setting its status to Closed.
−pub async fn close(db: &DatabaseConnection, patch_id: i64) -> Result<(), DbErr> {
+/// If snapshot OIDs are provided, they are stored to preserve the diff state at close time.
+pub async fn close(
+    db: &DatabaseConnection,
+    patch_id: i64,
+    snapshot_base_oid: Option<&str>,
+    snapshot_head_oid: Option<&str>,
+) -> Result<(), DbErr> {
     let pp = patch_proposals::Entity::find_by_id(patch_id)
         .one(db)
         .await?

@@ -295,6 +349,30 @@ pub async fn close(db: &DatabaseConnection, patch_id: i64) -> Result<(), DbErr>
 
     let mut pp = pp;
     pp.status = Set(Status::Closed);
+    if let Some(base) = snapshot_base_oid {
+        pp.snapshot_base_oid = Set(Some(base.to_string()));
+    }
+    if let Some(head) = snapshot_head_oid {
+        pp.snapshot_head_oid = Set(Some(head.to_string()));
+    }
+    pp.update(db).await?;
+
+    Ok(())
+}
+
+/// Clears the snapshot OIDs on a patch proposal (used when reopening).
+pub async fn clear_snapshot_oids(db: &DatabaseConnection, patch_id: i64) -> Result<(), DbErr> {
+    let pp = patch_proposals::Entity::find_by_id(patch_id)
+        .one(db)
+        .await?
+        .ok_or(DbErr::RecordNotFound(format!(
+            "patch proposal {patch_id} not found"
+        )))?
+        .into_active_model();
+
+    let mut pp = pp;
+    pp.snapshot_base_oid = Set(None);
+    pp.snapshot_head_oid = Set(None);
     pp.update(db).await?;
 
     Ok(())

frogue/src/views/patches.rs+8−0

@@ -145,6 +145,8 @@ pub struct PatchDetail<'a> {
     pub commits: Vec<CommitSummary>,
     /// The files changed in this patch.
     pub files: Vec<crate::repositories::git::DiffFile>,
+    /// If the diff cannot be shown, the reason (e.g. merged before snapshots existed).
+    pub diff_unavailable_reason: Option<String>,
     /// The comments on this patch, in chronological order.
     pub comments: Vec<super::comments::CommentView<'a>>,
     /// Whether the visitor may comment on this patch.

@@ -241,6 +243,12 @@ impl Render for PatchBody<'_> {
                     }
                 }
 
+                @if let Some(reason) = &detail.diff_unavailable_reason {
+                    section class="patch-diff-unavailable" {
+                        p { (reason) }
+                    }
+                }
+
                 @if detail.may_change && detail.is_owner && (detail.status == "draft" || detail.status == "open") {
                     section class="patch-actions" {
                         form method="post" action=(format!("{base}/patches/{}/merge", detail.number)) class="inline-form" {

frogue/src/webui/repositories/patches.rs+198−48

@@ -345,24 +345,39 @@ pub async fn get_patch(
     }
     .bare_path(&repo_root);
 
−    // Fetch commits and diff for the patch
−    let range = format!("{}...{}", patch.target_branch, patch.source_ref);
−    let commits = match git::history(&bare, &range).await {
−        Ok(Some(commits)) => commits,
−        Ok(None) => Vec::new(),
−        Err(e) => {
−            tracing::error!(%e, "failed to fetch patch commits");
−            Vec::new()
−        }
+    // Check if the diff is unavailable (merged/closed before snapshots were implemented)
+    let diff_unavailable_reason = if matches!(patch.status, Status::Merged | Status::Closed)
+        && patch.snapshot_base_oid.is_none()
+        && patch.snapshot_head_oid.is_none()
+    {
+        Some("Diff not available: this patch was merged/closed before diff snapshots were implemented".to_string())
+    } else {
+        None
     };
 
−    let files = match git::compare_diff(&bare, &range).await {
−        Ok(Some(files)) => files,
−        Ok(None) => Vec::new(),
−        Err(e) => {
−            tracing::error!(%e, "failed to fetch patch diff");
−            Vec::new()
−        }
+    // Fetch commits and diff for the patch
+    let (commits, files) = if diff_unavailable_reason.is_some() {
+        (Vec::new(), Vec::new())
+    } else {
+        let range = patches::compute_diff_range(&patch);
+        let commits = match git::history(&bare, &range).await {
+            Ok(Some(commits)) => commits,
+            Ok(None) => Vec::new(),
+            Err(e) => {
+                tracing::error!(%e, "failed to fetch patch commits");
+                Vec::new()
+            }
+        };
+
+        let files = match git::compare_diff(&bare, &range).await {
+            Ok(Some(files)) => files,
+            Ok(None) => Vec::new(),
+            Err(e) => {
+                tracing::error!(%e, "failed to fetch patch diff");
+                Vec::new()
+            }
+        };
+        (commits, files)
     };
 
     // Convert commits to views format

@@ -456,6 +471,7 @@ pub async fn get_patch(
             is_owner,
             commits: commit_summaries,
             files,
+            diff_unavailable_reason,
             comments,
             may_comment,
             comment_error: None,

@@ -704,22 +720,125 @@ pub async fn post_edit_patch(
             old_resolve_url.clone()
         };
 
−    if let Err(error) = store::update_with_resolve_url(
−        &db,
−        patch.id,
−        Some(&summary),
−        description,
−        Some(target_branch),
−        Some(status),
−        merge_policy,
−        new_resolve_url.clone(),
−    )
−    .await
−    {
+    // Compute snapshot OIDs if closing the patch
+    let snapshot_oids =
+        if old_status != status && status == crate::entities::patch_proposals::Status::Closed {
+            // Compute merge-base and head of source ref for the snapshot
+            let source_ref_full = if patch.source_ref.starts_with("refs/") {
+                patch.source_ref.clone()
+            } else {
+                format!("refs/{}", patch.source_ref)
+            };
+
+            let target_head = match crate::repositories::git::resolves_to_commit(
+                &bare_path,
+                &format!("refs/heads/{}", patch.target_branch),
+            )
+            .await
+            {
+                Ok(true) => Some(format!("refs/heads/{}", patch.target_branch)),
+                _ => None,
+            };
+
+            let source_head =
+                match crate::repositories::git::resolves_to_commit(&bare_path, &source_ref_full)
+                    .await
+                {
+                    Ok(true) => Some(source_ref_full.clone()),
+                    _ => None,
+                };
+
+            if let (Some(target), Some(source)) = (target_head, source_head) {
+                // Compute merge-base using git command
+                let output = tokio::process::Command::new("git")
+                    .arg("-C")
+                    .arg(&bare_path)
+                    .arg("merge-base")
+                    .arg(&target)
+                    .arg(&source)
+                    .output()
+                    .await;
+
+                match output {
+                    Ok(out) if out.status.success() => {
+                        let merge_base = String::from_utf8_lossy(&out.stdout).trim().to_string();
+
+                        // Get the actual OID for source
+                        let source_oid_output = tokio::process::Command::new("git")
+                            .arg("-C")
+                            .arg(&bare_path)
+                            .arg("rev-parse")
+                            .arg(&source_ref_full)
+                            .output()
+                            .await;
+
+                        if let Ok(out) = source_oid_output {
+                            if out.status.success() {
+                                let source_oid =
+                                    String::from_utf8_lossy(&out.stdout).trim().to_string();
+                                Some((merge_base, source_oid))
+                            } else {
+                                None
+                            }
+                        } else {
+                            None
+                        }
+                    }
+                    _ => None,
+                }
+            } else {
+                None
+            }
+        } else {
+            None
+        };
+
+    // Update the patch with appropriate function based on whether we're closing
+    let update_result = if let Some((base_oid, head_oid)) = &snapshot_oids {
+        // Closing with snapshot - use update_with_merge_snapshot
+        store::update_with_merge_snapshot(
+            &db,
+            patch.id,
+            Some(&summary),
+            description,
+            Some(target_branch),
+            Some(status),
+            merge_policy,
+            Some(base_oid),
+            Some(head_oid),
+            new_resolve_url.clone(),
+        )
+        .await
+    } else {
+        // Regular update - use update_with_resolve_url
+        store::update_with_resolve_url(
+            &db,
+            patch.id,
+            Some(&summary),
+            description,
+            Some(target_branch),
+            Some(status),
+            merge_policy,
+            new_resolve_url.clone(),
+        )
+        .await
+    };
+
+    if let Err(error) = update_result {
         error!(%error, owner = %owner.name, slug = %repository.slug, number, "editing a patch failed");
         return HttpResponse::InternalServerError().finish();
     }
 
+    // If reopening, clear the snapshot OIDs to go back to live diff
+    if old_status != status
+        && old_status == crate::entities::patch_proposals::Status::Closed
+        && status == crate::entities::patch_proposals::Status::Open
+        && let Err(error) = store::clear_snapshot_oids(&db, patch.id).await
+    {
+        error!(%error, owner = %owner.name, slug = %repository.slug, number, "clearing snapshot OIDs on reopen failed");
+        return HttpResponse::InternalServerError().finish();
+    }
+
     // Emit federation activities
     let state_guard = state_read(&data).await;
     let federation_enabled = crate::activitypub::enabled(&state_guard);

@@ -1063,10 +1182,25 @@ pub async fn post_merge_patch(
     };
 
     match merge_result {
−        merge::MergeResult::Success { new_target_oid } => {
−            // Update the patch status to merged
−            if let Err(error) =
−                store::update(&db, patch.id, None, None, None, Some(Status::Merged), None).await
+        merge::MergeResult::Success {
+            new_target_oid,
+            snapshot_base_oid,
+            snapshot_head_oid,
+        } => {
+            // Update the patch status to merged and store the snapshot OIDs
+            if let Err(error) = store::update_with_merge_snapshot(
+                &db,
+                patch.id,
+                None,
+                None,
+                None,
+                Some(Status::Merged),
+                None,
+                Some(&snapshot_base_oid),
+                Some(&snapshot_head_oid),
+                None,
+            )
+            .await
             {
                 error!(%error, owner = %owner.name, slug = %repository.slug, number, "updating patch status to merged failed");
                 return HttpResponse::InternalServerError().finish();

@@ -1829,24 +1963,39 @@ async fn patch_with_comment_error(
     }
     .bare_path(&repo_root);
 
−    // Fetch commits and diff for the patch
−    let range = format!("{}...{}", patch.target_branch, patch.source_ref);
−    let commits = match crate::repositories::git::history(&bare, &range).await {
−        Ok(Some(commits)) => commits,
−        Ok(None) => Vec::new(),
−        Err(e) => {
−            tracing::error!(%e, "failed to fetch patch commits");
−            Vec::new()
−        }
+    // Check if the diff is unavailable (merged/closed before snapshots were implemented)
+    let diff_unavailable_reason = if matches!(patch.status, Status::Merged | Status::Closed)
+        && patch.snapshot_base_oid.is_none()
+        && patch.snapshot_head_oid.is_none()
+    {
+        Some("Diff not available: this patch was merged/closed before diff snapshots were implemented".to_string())
+    } else {
+        None
     };
 
−    let files = match crate::repositories::git::compare_diff(&bare, &range).await {
−        Ok(Some(files)) => files,
−        Ok(None) => Vec::new(),
−        Err(e) => {
−            tracing::error!(%e, "failed to fetch patch diff");
−            Vec::new()
−        }
+    // Fetch commits and diff for the patch
+    let (commits, files) = if diff_unavailable_reason.is_some() {
+        (Vec::new(), Vec::new())
+    } else {
+        let range = patches::compute_diff_range(patch);
+        let commits = match crate::repositories::git::history(&bare, &range).await {
+            Ok(Some(commits)) => commits,
+            Ok(None) => Vec::new(),
+            Err(e) => {
+                tracing::error!(%e, "failed to fetch patch commits");
+                Vec::new()
+            }
+        };
+
+        let files = match crate::repositories::git::compare_diff(&bare, &range).await {
+            Ok(Some(files)) => files,
+            Ok(None) => Vec::new(),
+            Err(e) => {
+                tracing::error!(%e, "failed to fetch patch diff");
+                Vec::new()
+            }
+        };
+        (commits, files)
     };
 
     // Convert commits to views format

@@ -1938,6 +2087,7 @@ async fn patch_with_comment_error(
             is_owner,
             commits: commit_summaries,
             files,
+            diff_unavailable_reason,
             comments,
             may_comment,
             comment_error: error,

frogue/tests/diff_snapshot.rs+673−0

@@ -0,0 +1,673 @@
+//! Integration tests for diff snapshot preservation on merge/close.
+//!
+//! When a patch proposal is merged or closed, we need to freeze the diff state
+//! so that viewing the patch later still shows what was actually merged/closed,
+//! even if the branches are deleted or move forward.
+
+mod support;
+
+use std::fs;
+use std::path::{Path, PathBuf};
+use std::process::{Command, Output};
+use std::sync::atomic::{AtomicU64, Ordering};
+
+use support::{Client, Server, free_port};
+
+const PASSWORD: &str = "correct horse battery staple";
+
+static COUNTER: AtomicU64 = AtomicU64::new(0);
+
+fn spawn() -> (Server, u16) {
+    let port = free_port();
+    let mut server = Server::spawn(&format!("127.0.0.1:{port}"));
+    server.wait_for_connection(port);
+    let ssh_port = server.ssh_port();
+    server.wait_for_connection(ssh_port);
+    (server, port)
+}
+
+struct Scratch {
+    dir: PathBuf,
+}
+
+impl Scratch {
+    fn new(label: &str) -> Self {
+        let count = COUNTER.fetch_add(1, Ordering::Relaxed);
+        let dir =
+            std::env::temp_dir().join(format!("frogue-{label}-{}-{count}", std::process::id()));
+        fs::create_dir_all(&dir).expect("the scratch directory should be creatable");
+        Scratch { dir }
+    }
+
+    fn path(&self, name: &str) -> PathBuf {
+        self.dir.join(name)
+    }
+}
+
+impl Drop for Scratch {
+    fn drop(&mut self) {
+        let _ = fs::remove_dir_all(&self.dir);
+    }
+}
+
+fn keygen(scratch: &Scratch, name: &str) -> PathBuf {
+    let key = scratch.path(name);
+    let output = Command::new("ssh-keygen")
+        .arg("-t")
+        .arg("ed25519")
+        .arg("-f")
+        .arg(&key)
+        .arg("-N")
+        .arg("")
+        .arg("-C")
+        .arg(name)
+        .output()
+        .expect("ssh-keygen should run");
+    assert!(output.status.success(), "ssh-keygen failed");
+
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::PermissionsExt as _;
+        fs::set_permissions(&key, fs::Permissions::from_mode(0o600))
+            .expect("the private key should be readable by its owner alone");
+    }
+
+    key
+}
+
+fn register_and_login(client: &mut Client, name: &str, email: &str) {
+    let response = client.post_form(
+        "/register",
+        &[("name", name), ("email", email), ("password", PASSWORD)],
+    );
+    assert_eq!(response.status, 303, "registering should log in");
+}
+
+fn add_key(client: &mut Client, owner: &str, public_key: &Path) {
+    let key = fs::read_to_string(public_key).expect("the public key should be readable");
+    let response = client.post_form(&format!("/~{owner}/ssh-keys"), &[("key", key.trim())]);
+    assert_eq!(response.status, 303, "adding a key should redirect");
+}
+
+fn create_repository(client: &mut Client, owner: &str, slug: &str, visibility: &str) {
+    let response = client.post_form(
+        &format!("/~{owner}/repositories"),
+        &[
+            ("slug", slug),
+            ("description", "a repository from the diff snapshot tests"),
+            ("visibility", visibility),
+        ],
+    );
+    assert_eq!(
+        response.status, 303,
+        "creating a repository should redirect"
+    );
+}
+
+fn ssh_command(key: &Path, port: u16) -> String {
+    format!(
+        "ssh -i {} -o IdentitiesOnly=yes -o BatchMode=yes \
+         -o PreferredAuthentications=publickey \
+         -o StrictHostKeyChecking=accept-new \
+         -o UserKnownHostsFile=/dev/null -p {port}",
+        key.display()
+    )
+}
+
+fn git(dir: &Path, key: &Path, port: u16, args: &[&str]) -> Output {
+    Command::new("git")
+        .current_dir(dir)
+        .arg("-c")
+        .arg("user.name=Test")
+        .arg("-c")
+        .arg("user.email=test@example.org")
+        .args(args)
+        .env("GIT_SSH_COMMAND", ssh_command(key, port))
+        .env("GIT_TERMINAL_PROMPT", "0")
+        .env("GIT_CONFIG_GLOBAL", "/dev/null")
+        .env("GIT_CONFIG_SYSTEM", "/dev/null")
+        .output()
+        .expect("git should run")
+}
+
+fn assert_ok(output: &Output, what: &str) {
+    assert!(
+        output.status.success(),
+        "{what} should succeed, but git exited with {:?}\nstdout:\n{}\nstderr:\n{}",
+        output.status.code(),
+        String::from_utf8_lossy(&output.stdout),
+        String::from_utf8_lossy(&output.stderr),
+    );
+}
+
+fn url(port: u16, owner: &str, slug: &str) -> String {
+    format!("ssh://git@127.0.0.1:{port}/{owner}/{slug}.git")
+}
+
+fn tooling_present() -> bool {
+    fn have(binary: &str, arg: &str) -> bool {
+        Command::new(binary).arg(arg).output().is_ok()
+    }
+    have("git", "--version") && have("ssh", "-V") && have("ssh-keygen", "--version")
+}
+
+macro_rules! skip_without_tooling {
+    () => {
+        if !tooling_present() {
+            eprintln!("skipping: this test needs `git`, `ssh` and `ssh-keygen` on PATH");
+            return;
+        }
+    };
+}
+
+#[test]
+fn merged_patch_diff_uses_snapshot() {
+    skip_without_tooling!();
+
+    let (server, port) = spawn();
+    let ssh_port = server.ssh_port();
+    let scratch = Scratch::new("snapshot-merge");
+
+    let alice_key = keygen(&scratch, "alice_key");
+    let bob_key = keygen(&scratch, "bob_key");
+
+    let mut alice = Client::new(port);
+    register_and_login(&mut alice, "alice", "alice@example.org");
+    add_key(&mut alice, "alice", &scratch.path("alice_key.pub"));
+    create_repository(&mut alice, "alice", "test-repo", "public");
+
+    // Alice initializes the repository with a main branch
+    let alice_clone = scratch.path("alice-clone");
+    assert_ok(
+        &git(
+            &scratch.dir,
+            &alice_key,
+            ssh_port,
+            &[
+                "clone",
+                &url(ssh_port, "alice", "test-repo"),
+                alice_clone.to_str().unwrap(),
+            ],
+        ),
+        "alice cloning",
+    );
+    fs::write(alice_clone.join("README.md"), "# Notes\n").expect("writing README");
+    assert_ok(
+        &git(&alice_clone, &alice_key, ssh_port, &["add", "README.md"]),
+        "staging",
+    );
+    assert_ok(
+        &git(
+            &alice_clone,
+            &alice_key,
+            ssh_port,
+            &["commit", "-m", "Initial commit"],
+        ),
+        "committing",
+    );
+    assert_ok(
+        &git(
+            &alice_clone,
+            &alice_key,
+            ssh_port,
+            &["push", "origin", "main"],
+        ),
+        "alice pushing main",
+    );
+
+    // Bob clones and creates a feature branch with a patch
+    let mut bob = Client::new(port);
+    register_and_login(&mut bob, "bob", "bob@example.org");
+    add_key(&mut bob, "bob", &scratch.path("bob_key.pub"));
+
+    let bob_clone = scratch.path("bob-clone");
+    assert_ok(
+        &git(
+            &scratch.dir,
+            &bob_key,
+            ssh_port,
+            &[
+                "clone",
+                &url(ssh_port, "alice", "test-repo"),
+                bob_clone.to_str().unwrap(),
+            ],
+        ),
+        "bob cloning",
+    );
+    fs::write(bob_clone.join("feature.txt"), "new feature content\n").expect("writing feature");
+    assert_ok(
+        &git(&bob_clone, &bob_key, ssh_port, &["add", "feature.txt"]),
+        "bob staging",
+    );
+    assert_ok(
+        &git(
+            &bob_clone,
+            &bob_key,
+            ssh_port,
+            &["commit", "-m", "Add feature"],
+        ),
+        "bob committing",
+    );
+
+    // Bob pushes to refs/patches
+    let output = git(
+        &bob_clone,
+        &bob_key,
+        ssh_port,
+        &["push", "origin", "HEAD:refs/patches"],
+    );
+    assert_ok(&output, "bob pushing to refs/patches");
+
+    // Verify the patch was created
+    let patch_page = alice.get("/~alice/test-repo/patches/1");
+    assert_eq!(patch_page.status, 200);
+    assert!(patch_page.body.contains("Add feature"));
+
+    // Alice merges the patch
+    let merge_response = alice.post_form("/~alice/test-repo/patches/1/merge", &[]);
+    assert_eq!(merge_response.status, 303);
+
+    // Verify the patch is now merged
+    let patch_page = alice.get("/~alice/test-repo/patches/1");
+    assert_eq!(patch_page.status, 200);
+    assert!(patch_page.body.contains("merged"));
+
+    // The diff should still show the changes, even after merge
+    assert!(patch_page.body.contains("feature.txt"));
+    assert!(patch_page.body.contains("new feature content"));
+}
+
+#[test]
+fn closed_patch_diff_uses_snapshot() {
+    skip_without_tooling!();
+
+    let (server, port) = spawn();
+    let ssh_port = server.ssh_port();
+    let scratch = Scratch::new("snapshot-close");
+
+    let alice_key = keygen(&scratch, "alice_key");
+    let bob_key = keygen(&scratch, "bob_key");
+
+    let mut alice = Client::new(port);
+    register_and_login(&mut alice, "alice", "alice@example.org");
+    add_key(&mut alice, "alice", &scratch.path("alice_key.pub"));
+    create_repository(&mut alice, "alice", "test-repo", "public");
+
+    // Alice initializes the repository
+    let alice_clone = scratch.path("alice-clone");
+    assert_ok(
+        &git(
+            &scratch.dir,
+            &alice_key,
+            ssh_port,
+            &[
+                "clone",
+                &url(ssh_port, "alice", "test-repo"),
+                alice_clone.to_str().unwrap(),
+            ],
+        ),
+        "alice cloning",
+    );
+    fs::write(alice_clone.join("README.md"), "# Notes\n").expect("writing README");
+    assert_ok(
+        &git(&alice_clone, &alice_key, ssh_port, &["add", "README.md"]),
+        "staging",
+    );
+    assert_ok(
+        &git(
+            &alice_clone,
+            &alice_key,
+            ssh_port,
+            &["commit", "-m", "Initial commit"],
+        ),
+        "committing",
+    );
+    assert_ok(
+        &git(
+            &alice_clone,
+            &alice_key,
+            ssh_port,
+            &["push", "origin", "main"],
+        ),
+        "alice pushing main",
+    );
+
+    // Bob creates a patch
+    let mut bob = Client::new(port);
+    register_and_login(&mut bob, "bob", "bob@example.org");
+    add_key(&mut bob, "bob", &scratch.path("bob_key.pub"));
+
+    let bob_clone = scratch.path("bob-clone");
+    assert_ok(
+        &git(
+            &scratch.dir,
+            &bob_key,
+            ssh_port,
+            &[
+                "clone",
+                &url(ssh_port, "alice", "test-repo"),
+                bob_clone.to_str().unwrap(),
+            ],
+        ),
+        "bob cloning",
+    );
+    fs::write(bob_clone.join("feature.txt"), "rejected content\n").expect("writing feature");
+    assert_ok(
+        &git(&bob_clone, &bob_key, ssh_port, &["add", "feature.txt"]),
+        "bob staging",
+    );
+    assert_ok(
+        &git(
+            &bob_clone,
+            &bob_key,
+            ssh_port,
+            &["commit", "-m", "Add rejected feature"],
+        ),
+        "bob committing",
+    );
+
+    // Bob pushes to refs/patches
+    let output = git(
+        &bob_clone,
+        &bob_key,
+        ssh_port,
+        &["push", "origin", "HEAD:refs/patches"],
+    );
+    assert_ok(&output, "bob pushing to refs/patches");
+
+    // Bob closes his own patch
+    let close_response = bob.post_form(
+        "/~alice/test-repo/patches/1",
+        &[
+            ("summary", "Add rejected feature"),
+            ("description", ""),
+            ("target_branch", "main"),
+            ("status", "closed"),
+        ],
+    );
+    assert_eq!(close_response.status, 303);
+
+    // Verify the patch is now closed
+    let patch_page = alice.get("/~alice/test-repo/patches/1");
+    assert_eq!(patch_page.status, 200);
+    assert!(patch_page.body.contains("closed"));
+
+    // The diff should still show the changes
+    assert!(patch_page.body.contains("feature.txt"));
+    assert!(patch_page.body.contains("rejected content"));
+}
+
+#[test]
+fn reopened_patch_clears_snapshot() {
+    skip_without_tooling!();
+
+    let (server, port) = spawn();
+    let ssh_port = server.ssh_port();
+    let scratch = Scratch::new("snapshot-reopen");
+
+    let alice_key = keygen(&scratch, "alice_key");
+    let bob_key = keygen(&scratch, "bob_key");
+
+    let mut alice = Client::new(port);
+    register_and_login(&mut alice, "alice", "alice@example.org");
+    add_key(&mut alice, "alice", &scratch.path("alice_key.pub"));
+    create_repository(&mut alice, "alice", "test-repo", "public");
+
+    // Alice initializes the repository
+    let alice_clone = scratch.path("alice-clone");
+    assert_ok(
+        &git(
+            &scratch.dir,
+            &alice_key,
+            ssh_port,
+            &[
+                "clone",
+                &url(ssh_port, "alice", "test-repo"),
+                alice_clone.to_str().unwrap(),
+            ],
+        ),
+        "alice cloning",
+    );
+    fs::write(alice_clone.join("README.md"), "# Notes\n").expect("writing README");
+    assert_ok(
+        &git(&alice_clone, &alice_key, ssh_port, &["add", "README.md"]),
+        "staging",
+    );
+    assert_ok(
+        &git(
+            &alice_clone,
+            &alice_key,
+            ssh_port,
+            &["commit", "-m", "Initial commit"],
+        ),
+        "committing",
+    );
+    assert_ok(
+        &git(
+            &alice_clone,
+            &alice_key,
+            ssh_port,
+            &["push", "origin", "main"],
+        ),
+        "alice pushing main",
+    );
+
+    // Bob creates a patch
+    let mut bob = Client::new(port);
+    register_and_login(&mut bob, "bob", "bob@example.org");
+    add_key(&mut bob, "bob", &scratch.path("bob_key.pub"));
+
+    let bob_clone = scratch.path("bob-clone");
+    assert_ok(
+        &git(
+            &scratch.dir,
+            &bob_key,
+            ssh_port,
+            &[
+                "clone",
+                &url(ssh_port, "alice", "test-repo"),
+                bob_clone.to_str().unwrap(),
+            ],
+        ),
+        "bob cloning",
+    );
+    fs::write(bob_clone.join("feature.txt"), "initial content\n").expect("writing feature");
+    assert_ok(
+        &git(&bob_clone, &bob_key, ssh_port, &["add", "feature.txt"]),
+        "bob staging",
+    );
+    assert_ok(
+        &git(
+            &bob_clone,
+            &bob_key,
+            ssh_port,
+            &["commit", "-m", "Add feature"],
+        ),
+        "bob committing",
+    );
+
+    // Bob pushes to refs/patches
+    let output = git(
+        &bob_clone,
+        &bob_key,
+        ssh_port,
+        &["push", "origin", "HEAD:refs/patches"],
+    );
+    assert_ok(&output, "bob pushing to refs/patches");
+
+    // Bob closes the patch
+    let close_response = bob.post_form(
+        "/~alice/test-repo/patches/1",
+        &[
+            ("summary", "Add feature"),
+            ("description", ""),
+            ("target_branch", "main"),
+            ("status", "closed"),
+        ],
+    );
+    assert_eq!(close_response.status, 303);
+
+    // Verify it's closed
+    let patch_page = alice.get("/~alice/test-repo/patches/1");
+    assert!(patch_page.body.contains("closed"));
+
+    // Bob reopens the patch
+    let reopen_response = bob.post_form(
+        "/~alice/test-repo/patches/1",
+        &[
+            ("summary", "Add feature"),
+            ("description", ""),
+            ("target_branch", "main"),
+            ("status", "open"),
+        ],
+    );
+    assert_eq!(reopen_response.status, 303);
+
+    // Verify it's open again
+    let patch_page = alice.get("/~alice/test-repo/patches/1");
+    assert!(patch_page.body.contains("open"));
+
+    // The diff should still show the changes (live diff, not snapshot)
+    assert!(patch_page.body.contains("feature.txt"));
+    assert!(patch_page.body.contains("initial content"));
+}
+
+#[test]
+fn snapshot_captures_exact_state() {
+    skip_without_tooling!();
+
+    let (server, port) = spawn();
+    let ssh_port = server.ssh_port();
+    let scratch = Scratch::new("snapshot-exact");
+
+    let alice_key = keygen(&scratch, "alice_key");
+    let bob_key = keygen(&scratch, "bob_key");
+
+    let mut alice = Client::new(port);
+    register_and_login(&mut alice, "alice", "alice@example.org");
+    add_key(&mut alice, "alice", &scratch.path("alice_key.pub"));
+    create_repository(&mut alice, "alice", "test-repo", "public");
+
+    // Alice initializes the repository
+    let alice_clone = scratch.path("alice-clone");
+    assert_ok(
+        &git(
+            &scratch.dir,
+            &alice_key,
+            ssh_port,
+            &[
+                "clone",
+                &url(ssh_port, "alice", "test-repo"),
+                alice_clone.to_str().unwrap(),
+            ],
+        ),
+        "alice cloning",
+    );
+    fs::write(alice_clone.join("README.md"), "# Notes\n").expect("writing README");
+    assert_ok(
+        &git(&alice_clone, &alice_key, ssh_port, &["add", "README.md"]),
+        "staging",
+    );
+    assert_ok(
+        &git(
+            &alice_clone,
+            &alice_key,
+            ssh_port,
+            &["commit", "-m", "Initial commit"],
+        ),
+        "committing",
+    );
+    assert_ok(
+        &git(
+            &alice_clone,
+            &alice_key,
+            ssh_port,
+            &["push", "origin", "main"],
+        ),
+        "alice pushing main",
+    );
+
+    // Bob creates a patch with initial content
+    let mut bob = Client::new(port);
+    register_and_login(&mut bob, "bob", "bob@example.org");
+    add_key(&mut bob, "bob", &scratch.path("bob_key.pub"));
+
+    let bob_clone = scratch.path("bob-clone");
+    assert_ok(
+        &git(
+            &scratch.dir,
+            &bob_key,
+            ssh_port,
+            &[
+                "clone",
+                &url(ssh_port, "alice", "test-repo"),
+                bob_clone.to_str().unwrap(),
+            ],
+        ),
+        "bob cloning",
+    );
+    fs::write(bob_clone.join("feature.txt"), "original content\n").expect("writing feature");
+    assert_ok(
+        &git(&bob_clone, &bob_key, ssh_port, &["add", "feature.txt"]),
+        "bob staging",
+    );
+    assert_ok(
+        &git(
+            &bob_clone,
+            &bob_key,
+            ssh_port,
+            &["commit", "-m", "Add feature"],
+        ),
+        "bob committing",
+    );
+
+    // Bob pushes to refs/patches
+    let output = git(
+        &bob_clone,
+        &bob_key,
+        ssh_port,
+        &["push", "origin", "HEAD:refs/patches"],
+    );
+    assert_ok(&output, "bob pushing to refs/patches");
+
+    // Alice merges the patch
+    let merge_response = alice.post_form("/~alice/test-repo/patches/1/merge", &[]);
+    assert_eq!(merge_response.status, 303);
+
+    // Verify the merged patch shows "original content"
+    let patch_page = alice.get("/~alice/test-repo/patches/1");
+    assert_eq!(patch_page.status, 200);
+    assert!(patch_page.body.contains("original content"));
+
+    // Bob pushes more changes to the patch branch after merge
+    fs::write(bob_clone.join("feature.txt"), "updated content\n").expect("writing updated feature");
+    assert_ok(
+        &git(&bob_clone, &bob_key, ssh_port, &["add", "feature.txt"]),
+        "bob staging updated",
+    );
+    assert_ok(
+        &git(
+            &bob_clone,
+            &bob_key,
+            ssh_port,
+            &["commit", "-m", "Update feature"],
+        ),
+        "bob committing updated",
+    );
+    assert_ok(
+        &git(
+            &bob_clone,
+            &bob_key,
+            ssh_port,
+            &["push", "origin", "HEAD:refs/patches"],
+        ),
+        "bob pushing updated to refs/patches",
+    );
+
+    // The patch page should still show the original merged content, not the new changes
+    let patch_page = alice.get("/~alice/test-repo/patches/1");
+    assert_eq!(patch_page.status, 200);
+    assert!(patch_page.body.contains("original content"));
+    // The snapshot should not contain the post-merge changes
+    assert!(!patch_page.body.contains("updated content"));
+}

migration/src/lib.rs+2−0

@@ -13,6 +13,7 @@ mod m20261022_000010_add_state_to_tickets;
 mod m20261023_000011_create_ap_actors;
 mod m20261023_000012_create_ap_remote_actors;
 mod m20261025_000013_create_ap_notes;
+mod m20261025_000025_add_snapshot_oids_to_patch_proposals;
 mod m20261027_000014_create_ap_deliveries;
 mod m20261029_000015_create_ap_reactions;
 mod m20261030_000016_create_invites_table;

@@ -55,6 +56,7 @@ impl MigratorTrait for Migrator {
             Box::new(m20261103_000022_federate_comments::Migration),
             Box::new(m20261103_000023_create_ap_remote_tickets::Migration),
             Box::new(m20261103_000024_create_ap_remote_actor_tombstones::Migration),
+            Box::new(m20261025_000025_add_snapshot_oids_to_patch_proposals::Migration),
         ]
     }
 }

migration/src/m20261025_000025_add_snapshot_oids_to_patch_proposals.rs+33−0

@@ -0,0 +1,33 @@
+use sea_orm_migration::prelude::*;
+
+#[derive(DeriveMigrationName)]
+pub struct Migration;
+
+#[async_trait::async_trait]
+impl MigrationTrait for Migration {
+    async fn up(&self, manager: &SchemaManager) -> Result<(), DbErr> {
+        manager
+            .get_connection()
+            .execute_unprepared(
+                r#"
+ALTER TABLE patch_proposals ADD COLUMN snapshot_base_oid TEXT;
+ALTER TABLE patch_proposals ADD COLUMN snapshot_head_oid TEXT;
+"#,
+            )
+            .await?;
+        Ok(())
+    }
+
+    async fn down(&self, manager: &SchemaManager) -> Result<(), DbErr> {
+        manager
+            .get_connection()
+            .execute_unprepared(
+                r#"
+ALTER TABLE patch_proposals DROP COLUMN snapshot_base_oid;
+ALTER TABLE patch_proposals DROP COLUMN snapshot_head_oid;
+"#,
+            )
+            .await?;
+        Ok(())
+    }
+}