michal/tit

Browse tree · Show commit · Download archive

Diff

50dd33344d49 → 46226121ee30

@@ -1102,6 +1102,7 @@
  "gix-ignore",
  "gix-index",
  "gix-lock",
+ "gix-merge",
  "gix-object",
  "gix-odb",
  "gix-pack",
@@ -1464,6 +1465,32 @@
 ]
 
 [[package]]
+name = "gix-merge"
+version = "0.17.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "7543e1eceb25fbbd1a29459c794148d8bafc1c77555eb386617a2d99b5371971"
+dependencies = [
+ "bstr",
+ "gix-command",
+ "gix-diff",
+ "gix-filter",
+ "gix-fs",
+ "gix-hash",
+ "gix-imara-diff",
+ "gix-index",
+ "gix-object",
+ "gix-path",
+ "gix-quote",
+ "gix-revision",
+ "gix-revwalk",
+ "gix-tempfile",
+ "gix-trace",
+ "gix-worktree",
+ "nonempty",
+ "thiserror",
+]
+
+[[package]]
 name = "gix-object"
 version = "0.61.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -1637,13 +1664,16 @@
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "0b47c88884dd3c1a19a39da19d10211fcdea2809aadc86869b6e824a1774340f"
 dependencies = [
+ "bitflags 2.13.1",
  "bstr",
  "gix-commitgraph",
  "gix-date",
  "gix-error",
  "gix-hash",
+ "gix-hashtable",
  "gix-object",
  "gix-revwalk",
+ "gix-trace",
  "nonempty",
 ]
 

@@ -15,7 +15,7 @@
 askama = { version = "0.16", default-features = false, features = ["derive", "std"] }
 axum = { version = "0.8", default-features = false, features = ["http1", "original-uri", "query", "tokio"] }
 clap = { version = "4.6", default-features = false, features = ["derive", "error-context", "help", "std", "usage"] }
-gix = { version = "0.84", default-features = false, features = ["blame", "parallel", "sha1", "sha256"] }
+gix = { version = "0.84", default-features = false, features = ["blame", "merge", "parallel", "revision", "sha1", "sha256"] }
 gix-pack = { version = "0.71", default-features = false, features = ["generate", "sha1", "sha256", "streaming-input"] }
 httpdate = "1.0.3"
 jiff = { version = "0.2.34", default-features = false, features = ["std"] }

@@ -390,3 +390,17 @@
 after a ref change, the Web forms, and historical schema migration. Read the
 [pull-request ref architectural decision record](docs/adr/0022-pull-request-refs.md)
 for the record, ref, permission, and recovery contracts.
+
+## Milestone 5.2 gate
+
+Run the pull-request comparison gate:
+
+```text
+./scripts/check-m5-2
+```
+
+This command tests SHA-1 and SHA-256 merge bases, commit ranges, changed paths,
+diffs, immutable revision selection, mergeability states, unrelated histories,
+work limits, and Web output. Read the
+[pull-request comparison architectural decision record](docs/adr/0023-pull-request-comparison.md)
+for the computation, limit, and cache contracts.

@@ -1,0 +1,85 @@
+# Architectural decision record 0023: Pull-request comparison
+
+Status: Accepted
+
+Date: 2026-07-22
+
+## Context
+
+A pull-request revision stores immutable base and head commit IDs. A comparison
+must use these IDs so a later branch update cannot change an old review. The
+repository can contain large or damaged object graphs. Mergeability analysis
+must not change the repository or depend on a Git executable.
+
+## Decision
+
+Compute each comparison from the selected pull-request revision and the bare
+repository. Do not store a comparison cache. Git objects and the immutable
+revision record can reproduce all comparison values.
+
+Walk the base and head histories in breadth-first order. Compute the best Git
+merge base after the bounded walks finish. The commit range contains each head
+ancestor that is not a base ancestor. Compute the changed paths and unified
+diff from the merge-base tree to the head tree. If the histories are unrelated,
+use the empty tree for the diff and report that the histories are unrelated.
+
+Classify mergeability as follows:
+
+- `already merged` means that the head is an ancestor of the base.
+- `fast-forward` means that the base is an ancestor of the head.
+- `clean merge` means that an in-memory three-way merge has no unresolved
+  conflict.
+- `conflicts` means that the three-way merge has an unresolved conflict.
+- `unrelated histories` means that Git cannot find a merge base.
+
+Use `gix` for merge-base selection and the worktree-free three-way merge. Enable
+an in-memory object overlay before the merge. Thus, temporary merge objects do
+not enter the bare object database. Rename detection checks a maximum of
+1,000,000 fuzzy candidate pairs. Stop the tree merge after its first unresolved
+conflict.
+
+## Work and output limits
+
+One comparison has one 30-second deadline. It stops after a combined total of
+10,000 visited base and head commits. A commit object can contain a maximum of
+1 MiB. All returned commit messages can contain a maximum total of 64 MiB.
+
+A path can contain a maximum of 4,096 bytes. A tree object can contain a
+maximum of 16 MiB. A flattened tree can contain a maximum of 100,000 entries.
+A blob can contain a maximum of 16 MiB. The diff limit is 64 MiB and includes
+the old content, new content, and generated hunks. These limits apply before
+the Web template creates output. Apply the diff limits to each side before a
+divergent three-way merge starts.
+
+The comparison API also accepts a cancellation signal. A caller can stop work
+between bounded object operations.
+
+## Failure and threat cases
+
+Reject a missing object, a non-commit object, damaged object data, and an
+invalid revision number. Apply the current repository read policy before object
+access. Do not resolve the stored commit IDs through branch names.
+
+An unrelated history is a valid result. Show all files in its head tree as
+changes from the empty tree. Do not attempt an unrelated-history merge.
+
+The merge implementation can read repository attributes. A pushed attribute
+cannot define a process by itself because custom merge drivers require trusted
+repository configuration. The service does not run Git or OpenSSH.
+
+## Evidence
+
+The pull-request integration test uses SHA-1 and SHA-256 repositories. It
+checks the merge base, commit range, changed paths, diff, immutable revision
+selection, fast-forward, clean merge, conflict, already-merged state, unrelated
+histories, and a history-limit failure.
+
+The Web integration test selects the current and first revisions without
+JavaScript. It checks that the page shows comparison state and changed content.
+
+## Consequences
+
+Each request pays the object-read and merge-analysis cost, but it cannot read or
+return unbounded data. A later cache can store only these reproducible values.
+Review code can select an immutable revision and use its displayed paths and
+commit IDs as comment anchors.

@@ -1,0 +1,6 @@
+#!/bin/sh
+set -eu
+
+./scripts/check
+cargo test --locked --release --test pull_requests
+cargo test --locked --release --test public_routes runs_the_complete_issue_workflow_without_javascript

@@ -122,6 +122,24 @@
     pub(crate) hunks: Vec<u8>,
 }
 
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+pub(crate) enum Mergeability {
+    Unrelated,
+    AlreadyMerged,
+    FastForward,
+    Clean,
+    Conflicting,
+}
+
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub(crate) struct Comparison {
+    pub(crate) merge_base: Option<ObjectId>,
+    pub(crate) commits: Vec<CommitInfo>,
+    pub(crate) changed_paths: Vec<Vec<u8>>,
+    pub(crate) files: Vec<DiffFile>,
+    pub(crate) mergeability: Mergeability,
+}
+
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub(crate) struct BlameHunk {
     pub(crate) start_line: u32,
@@ -163,7 +181,10 @@
             return Err(ReadError::NotBare(path.to_owned()));
         }
         validate_limits(&limits)?;
-        Ok(Self { repository, limits })
+        Ok(Self {
+            repository: repository.with_object_memory(),
+            limits,
+        })
     }
 
     pub(crate) fn references(
@@ -347,10 +368,113 @@
         cancellation: &ReadCancellation,
     ) -> Result<Vec<DiffFile>, ReadError> {
         let budget = self.budget(cancellation);
-        let old = self.read_commit(old_commit, &budget)?;
-        let new = self.read_commit(new_commit, &budget)?;
-        let old_files = self.flatten_tree(old.tree, &budget)?;
-        let new_files = self.flatten_tree(new.tree, &budget)?;
+        self.diff_with_budget(old_commit, new_commit, &budget)
+    }
+
+    pub(crate) fn comparison(
+        &self,
+        base_commit: ObjectId,
+        head_commit: ObjectId,
+        cancellation: &ReadCancellation,
+    ) -> Result<Comparison, ReadError> {
+        let budget = self.budget(cancellation);
+        let base_history = self.history_with_budget(base_commit, &budget)?;
+        let head_history = self.history_with_budget(head_commit, &budget)?;
+        if base_history.len().saturating_add(head_history.len()) > self.limits.max_history_commits {
+            return Err(ReadError::Limit("comparison commits"));
+        }
+        let base_ids: HashSet<_> = base_history.iter().map(|commit| commit.id).collect();
+        let head_ids: HashSet<_> = head_history.iter().map(|commit| commit.id).collect();
+        let head_tree = head_history
+            .first()
+            .ok_or(ReadError::ObjectNotFound(head_commit))?
+            .tree;
+        budget.check()?;
+        let merge_base = match self.repository.merge_base(base_commit, head_commit) {
+            Ok(id) => Some(id.detach()),
+            Err(gix::repository::merge_base::Error::NotFound { .. }) => None,
+            Err(error) => return Err(ReadError::Git(error.to_string())),
+        };
+        let mut commit_bytes = 0_usize;
+        let mut commits = Vec::new();
+        for commit in head_history
+            .into_iter()
+            .filter(|commit| !base_ids.contains(&commit.id))
+        {
+            budget.check()?;
+            commit_bytes = checked_add(
+                commit_bytes,
+                commit.message.len(),
+                self.limits.max_diff_bytes,
+                "comparison output bytes",
+            )?;
+            commits.push(commit);
+        }
+        let files = match merge_base {
+            Some(merge_base) => self.diff_with_budget(merge_base, head_commit, &budget)?,
+            None => self.diff_trees_with_budget(
+                ObjectId::empty_tree(self.repository.object_hash()),
+                head_tree,
+                &budget,
+            )?,
+        };
+        let changed_paths = files.iter().map(|file| file.path.clone()).collect();
+        let mergeability = match merge_base {
+            None => Mergeability::Unrelated,
+            Some(_) if base_ids.contains(&head_commit) => Mergeability::AlreadyMerged,
+            Some(_) if head_ids.contains(&base_commit) => Mergeability::FastForward,
+            Some(merge_base) => {
+                budget.check()?;
+                self.diff_with_budget(merge_base, base_commit, &budget)?;
+                let options = self
+                    .repository
+                    .tree_merge_options()
+                    .map_err(|error| ReadError::Git(error.to_string()))?
+                    .with_rewrites(Some(Default::default()))
+                    .with_fail_on_conflict(Some(Default::default()));
+                let outcome = self
+                    .repository
+                    .merge_commits(base_commit, head_commit, Default::default(), options.into())
+                    .map_err(|error| ReadError::Git(error.to_string()))?;
+                if outcome
+                    .tree_merge
+                    .has_unresolved_conflicts(Default::default())
+                {
+                    Mergeability::Conflicting
+                } else {
+                    Mergeability::Clean
+                }
+            }
+        };
+        budget.check()?;
+        Ok(Comparison {
+            merge_base,
+            commits,
+            changed_paths,
+            files,
+            mergeability,
+        })
+    }
+
+    fn diff_with_budget(
+        &self,
+        old_commit: ObjectId,
+        new_commit: ObjectId,
+        budget: &ReadBudget<'_>,
+    ) -> Result<Vec<DiffFile>, ReadError> {
+        let old = self.read_commit(old_commit, budget)?;
+        let new = self.read_commit(new_commit, budget)?;
+        self.diff_trees_with_budget(old.tree, new.tree, budget)
+    }
+
+    fn diff_trees_with_budget(
+        &self,
+        old_tree: ObjectId,
+        new_tree: ObjectId,
+        budget: &ReadBudget<'_>,
+    ) -> Result<Vec<DiffFile>, ReadError> {
+        let old_files = self.flatten_tree(old_tree, budget)?;
+        let new_files = self.flatten_tree(new_tree, budget)?;
         let paths: BTreeMap<&[u8], ()> = old_files
             .keys()
             .chain(new_files.keys())
@@ -365,8 +489,8 @@
             if old == new {
                 continue;
             }
-            let old_data = self.diff_blob(old, &budget)?;
-            let new_data = self.diff_blob(new, &budget)?;
+            let old_data = self.diff_blob(old, budget)?;
+            let new_data = self.diff_blob(new, budget)?;
             bytes = checked_add(
                 bytes,
                 old_data.len(),

@@ -1,7 +1,7 @@
 use askama::Template;
 use axum::Router;
 use axum::body::Bytes;
-use axum::extract::{DefaultBodyLimit, Extension, Path, State};
+use axum::extract::{DefaultBodyLimit, Extension, Path, Query, State};
 use axum::http::{HeaderMap, StatusCode, header};
 use axum::response::Response;
 use axum::routing::{get, post};
@@ -84,6 +84,7 @@
     Extension(request_id): Extension<RequestId>,
     Extension(actor): Extension<RequestActor>,
     Path(path): Path<PullRequestPath>,
+    Query(query): Query<RevisionQuery>,
     headers: HeaderMap,
 ) -> Response {
     let Some(service) = state.pull_requests.clone() else {
@@ -92,12 +93,19 @@
     let owner = path.owner.clone();
     let repository = path.repository.clone();
     let result = job(state, move || {
-        service.get(&owner, &repository, path.number, actor.0.as_deref())
+        service.compare(
+            &owner,
+            &repository,
+            path.number,
+            query.revision,
+            actor.0.as_deref(),
+        )
     })
     .await;
     match result {
-        Ok(detail) => {
+        Ok(result) => {
             let csrf = cookie(&headers, CSRF_COOKIE).unwrap_or_default();
+            let detail = &result.detail;
             let pull_request = &detail.pull_request;
             render(
                 StatusCode::OK,
@@ -108,6 +116,8 @@
                     pull_request,
                     body_html: markdown::render(&pull_request.body),
                     revisions: &detail.revisions,
+                    selected_revision: result.revision.number,
+                    comparison: ComparisonView::from(&result.comparison),
                     csrf: &csrf,
                     can_revise: detail.can_revise && !csrf.is_empty(),
                 },
@@ -220,6 +230,7 @@
             "The pull request was not found.",
         ),
         PullRequestError::Number
+        | PullRequestError::Revision
         | PullRequestError::Auth(_)
         | PullRequestError::RepositoryName(_) => bad_request(request_id),
         _ => internal(request_id),
@@ -294,6 +305,11 @@
     number: i64,
 }
 
+#[derive(Clone, Default, Deserialize)]
+struct RevisionQuery {
+    revision: Option<i64>,
+}
+
 #[derive(Template)]
 #[template(path = "pull_requests.html")]
 struct PullRequestListTemplate<'a> {
@@ -322,6 +338,68 @@
     pull_request: &'a crate::store::PullRequestRecord,
     body_html: RenderedMarkdown,
     revisions: &'a [crate::store::PullRequestRevisionRecord],
+    selected_revision: i64,
+    comparison: ComparisonView,
     csrf: &'a str,
     can_revise: bool,
+}
+
+struct ComparisonView {
+    merge_base: String,
+    mergeability: &'static str,
+    commits: Vec<CommitView>,
+    changed_paths: Vec<String>,
+    files: Vec<DiffView>,
+}
+
+struct CommitView {
+    id: String,
+    message: String,
+}
+
+struct DiffView {
+    path: String,
+    binary: bool,
+    hunks: String,
+}
+
+impl From<&crate::git::read::Comparison> for ComparisonView {
+    fn from(comparison: &crate::git::read::Comparison) -> Self {
+        use crate::git::read::Mergeability;
+
+        Self {
+            merge_base: comparison
+                .merge_base
+                .map_or_else(|| "none".to_owned(), |id| id.to_string()),
+            mergeability: match comparison.mergeability {
+                Mergeability::Unrelated => "unrelated histories",
+                Mergeability::AlreadyMerged => "already merged",
+                Mergeability::FastForward => "fast-forward",
+                Mergeability::Clean => "clean merge",
+                Mergeability::Conflicting => "conflicts",
+            },
+            commits: comparison
+                .commits
+                .iter()
+                .map(|commit| CommitView {
+                    id: commit.id.to_string(),
+                    message: String::from_utf8_lossy(&commit.message).into_owned(),
+                })
+                .collect(),
+            changed_paths: comparison
+                .changed_paths
+                .iter()
+                .map(|path| String::from_utf8_lossy(path).into_owned())
+                .collect(),
+            files: comparison
+                .files
+                .iter()
+                .map(|file| DiffView {
+                    path: String::from_utf8_lossy(&file.path).into_owned(),
+                    binary: file.binary,
+                    hunks: String::from_utf8_lossy(&file.hunks).into_owned(),
+                })
+                .collect(),
+        }
+    }
 }

@@ -9,10 +9,13 @@
 
 use crate::auth::{AuthError, validate_username};
 use crate::domain::repository::{RepositoryNameError, validate_slug};
+use crate::git::read::{
+    Comparison, ReadCancellation, ReadError, ReadLimits, RepositoryReadService,
+};
 use crate::git::repository::{GitRepository, GitRepositoryError};
 use crate::store::{
     NewPullRequestRefIntent, PullRequestDetail, PullRequestRecord, PullRequestRefIntentRecord,
-    Store, StoreError,
+    PullRequestRevisionRecord, Store, StoreError,
 };
 
 pub(crate) const MAX_TITLE_BYTES: usize = 200;
@@ -23,6 +26,12 @@
     database: PathBuf,
     repositories: PathBuf,
     operations: Arc<Mutex<()>>,
+}
+
+pub(crate) struct PullRequestComparison {
+    pub(crate) detail: PullRequestDetail,
+    pub(crate) revision: PullRequestRevisionRecord,
+    pub(crate) comparison: Comparison,
 }
 
 impl PullRequestService {
@@ -153,6 +162,10 @@
             .pull_request)
     }
 
+    #[allow(
+        dead_code,
+        reason = "integration tests and later non-Web callers read pull requests without comparison"
+    )]
     pub(crate) fn get(
         &self,
         owner: &str,
@@ -176,6 +189,51 @@
         Store::open(&self.database)?
             .pull_request(owner, repository, number, actor)
             .map_err(Into::into)
+    }
+
+    pub(crate) fn compare(
+        &self,
+        owner: &str,
+        repository: &str,
+        number: i64,
+        revision: Option<i64>,
+        actor: Option<&str>,
+    ) -> Result<PullRequestComparison, PullRequestError> {
+        validate_username(owner)?;
+        validate_slug(repository)?;
+        if let Some(actor) = actor {
+            validate_username(actor)?;
+        }
+        if number < 1 || revision.is_some_and(|number| number < 1) {
+            return Err(PullRequestError::Number);
+        }
+        let _operation = self
+            .operations
+            .lock()
+            .unwrap_or_else(std::sync::PoisonError::into_inner);
+        self.recover_inner()?;
+        let detail = Store::open(&self.database)?.pull_request(owner, repository, number, actor)?;
+        let revision = match revision {
+            Some(number) => detail
+                .revisions
+                .iter()
+                .find(|revision| revision.number == number),
+            None => detail.revisions.last(),
+        }
+        .cloned()
+        .ok_or(PullRequestError::Revision)?;
+        let path = self.repository_path(&detail.repository.id)?;
+        let reader = RepositoryReadService::open(&path, ReadLimits::default())?;
+        let comparison = reader.comparison(
+            parse_id(&revision.base_object_id)?,
+            parse_id(&revision.head_object_id)?,
+            &ReadCancellation::default(),
+        )?;
+        Ok(PullRequestComparison {
+            detail,
+            revision,
+            comparison,
+        })
     }
 
     #[allow(
@@ -365,6 +423,8 @@
     Branch,
     #[error("pull-request number is not valid")]
     Number,
+    #[error("pull-request revision does not exist")]
+    Revision,
     #[error("pull-request refs have not changed")]
     Unchanged,
     #[error("stored pull-request object ID is not valid")]
@@ -375,6 +435,8 @@
     RepositoryPath,
     #[error("cannot access a pull-request repository: {0}")]
     Io(#[from] std::io::Error),
+    #[error(transparent)]
+    Read(#[from] ReadError),
     #[error("cannot create a random pull-request ID")]
     Random,
     #[error("the system clock is before the Unix epoch")]

@@ -25,9 +25,56 @@
     <h2 id="revisions-heading">Revisions</h2>
     <ol>
 {% for revision in revisions %}
-      <li value="{{ revision.number }}">{{ revision.author }} recorded <code>{{ revision.head_object_id }}</code> against <code>{{ revision.base_object_id }}</code> at <time>{{ revision.created_at }}</time>.</li>
+      <li value="{{ revision.number }}">
+        {{ revision.author }} recorded <code>{{ revision.head_object_id }}</code> against <code>{{ revision.base_object_id }}</code> at <time>{{ revision.created_at }}</time>.
+{% if revision.number == selected_revision %}
+        <strong>Selected</strong>
+{% else %}
+        <a href="/{{ owner }}/{{ repository }}/pulls/{{ pull_request.number }}?revision={{ revision.number }}">Compare this revision</a>
+{% endif %}
+      </li>
 {% endfor %}
     </ol>
+  </section>
+
+  <section aria-labelledby="comparison-heading">
+    <h2 id="comparison-heading">Comparison for revision {{ selected_revision }}</h2>
+    <p>Merge base: <code>{{ comparison.merge_base }}</code></p>
+    <p>Mergeability: {{ comparison.mergeability }}</p>
+
+    <h3>Commits</h3>
+{% if comparison.commits.is_empty() %}
+    <p>This revision has no commits that are not in the base.</p>
+{% else %}
+    <ol>
+{% for commit in comparison.commits %}
+      <li><code>{{ commit.id }}</code> <span class="preserve-whitespace">{{ commit.message }}</span></li>
+{% endfor %}
+    </ol>
+{% endif %}
+
+    <h3>Changed paths</h3>
+{% if comparison.changed_paths.is_empty() %}
+    <p>This revision does not change a path.</p>
+{% else %}
+    <ul>
+{% for path in comparison.changed_paths %}
+      <li><code>{{ path }}</code></li>
+{% endfor %}
+    </ul>
+{% endif %}
+
+    <h3>Diff</h3>
+{% for file in comparison.files %}
+    <article>
+      <h4><code>{{ file.path }}</code></h4>
+{% if file.binary %}
+      <p>This file has binary content.</p>
+{% else %}
+      <pre>{{ file.hunks }}</pre>
+{% endif %}
+    </article>
+{% endfor %}
   </section>
 
 {% if can_revise %}

@@ -781,6 +781,16 @@
             .text()
             .contains("git fetch origin refs/pull/1/head")
     );
+    assert!(
+        pull_request_page
+            .text()
+            .contains("Comparison for revision 1")
+    );
+    assert!(
+        pull_request_page
+            .text()
+            .contains("Mergeability: already merged")
+    );
     let worktree = fixture.instance.path().join("worktree");
     run(Command::new("git")
         .arg("-C")
@@ -818,6 +828,26 @@
             .count(),
         2
     );
+    assert!(
+        revised_pull_request_page
+            .text()
+            .contains("Comparison for revision 2")
+    );
+    assert!(
+        revised_pull_request_page
+            .text()
+            .contains("pull-request.txt")
+    );
+    let first_revision = request(
+        server.address(),
+        "GET",
+        "/alice/example/pulls/1?revision=1",
+        &[],
+        &[],
+    );
+    assert_eq!(first_revision.status, 200);
+    assert!(first_revision.text().contains("Comparison for revision 1"));
+    assert!(!first_revision.text().contains("pull-request.txt"));
     let search_page = request(server.address(), "GET", "/search", &[], &[]);
     assert_eq!(search_page.status, 200);
     assert!(

@@ -28,8 +28,11 @@
 use std::path::{Path, PathBuf};
 use std::process::Command;
 use std::sync::Arc;
+use std::time::Duration;
 
+use git::read::{Mergeability, ReadCancellation, ReadError, ReadLimits, RepositoryReadService};
 use git::repository::GitRepository;
+use gix::hash::ObjectId;
 use pull_request::{PullRequestError, PullRequestService};
 use rusqlite::params;
 use store::{NewPullRequestRefIntent, Store, StoreError};
@@ -53,6 +56,18 @@
             .expect("open a pull request");
         assert_eq!(opened.number, 1);
         assert_eq!(fixture.pull_ref(1), opened.head_object_id);
+        let comparison = service
+            .compare("alice", "project", 1, None, None)
+            .expect("compare the first revision");
+        assert_eq!(comparison.detail.pull_request.number, 1);
+        assert_eq!(comparison.revision.number, 1);
+        assert_eq!(
+            comparison.comparison.mergeability,
+            Mergeability::FastForward
+        );
+        assert_eq!(comparison.comparison.commits.len(), 1);
+        assert_eq!(comparison.comparison.changed_paths, [b"feature.txt"]);
+        assert_eq!(comparison.comparison.files.len(), 1);
         run(
             &fixture.worktree,
             Command::new("git")
@@ -95,6 +110,19 @@
         assert_eq!(second.revisions.len(), 2);
         assert_eq!(second.revisions[0].head_object_id, opened.head_object_id);
         assert_eq!(second.revisions[1].head_object_id, revised.head_object_id);
+        let original_comparison = service
+            .compare("alice", "project", 1, Some(1), Some("alice"))
+            .expect("compare the immutable first revision");
+        assert_eq!(
+            original_comparison.revision.head_object_id,
+            opened.head_object_id
+        );
+        assert_eq!(original_comparison.comparison.commits.len(), 1);
+        let current_comparison = service
+            .compare("alice", "project", 1, None, Some("alice"))
+            .expect("compare the current revision");
+        assert_eq!(current_comparison.revision.number, 2);
+        assert_eq!(current_comparison.comparison.commits.len(), 2);
 
         let git = GitRepository::open(&fixture.bare).expect("open the bare fixture");
         let base = git
@@ -220,6 +248,144 @@
     }
 }
 
+#[test]
+fn classifies_clean_conflicting_and_already_merged_revisions_for_both_hashes() {
+    for (index, object_format) in ["sha1", "sha256"].into_iter().enumerate() {
+        let fixture = Fixture::new(object_format, index + 10);
+        fixture.commit_on("main", "base.txt", "base side\n", "advance base");
+        fixture.commit_on(
+            "feature",
+            "feature-two.txt",
+            "feature side\n",
+            "advance feature",
+        );
+        let service = PullRequestService::new(&fixture.database, &fixture.repositories);
+        service
+            .open(
+                "alice",
+                "project",
+                "alice",
+                "Clean divergence",
+                "The branches change different paths.",
+                "refs/heads/main",
+                "refs/heads/feature",
+            )
+            .expect("open a clean divergent pull request");
+        let object_state = git_object_state(&fixture.bare);
+        let clean = service
+            .compare("alice", "project", 1, None, None)
+            .expect("compare clean divergence");
+        assert_eq!(clean.comparison.mergeability, Mergeability::Clean);
+        assert_eq!(clean.comparison.changed_paths.len(), 2);
+        assert_eq!(git_object_state(&fixture.bare), object_state);
+
+        fixture.commit_on("main", "README.md", "main content\n", "change base content");
+        fixture.commit_on(
+            "feature",
+            "README.md",
+            "feature content\n",
+            "change feature content",
+        );
+        service
+            .open(
+                "alice",
+                "project",
+                "alice",
+                "Conflicting divergence",
+                "The branches change the same line.",
+                "refs/heads/main",
+                "refs/heads/feature",
+            )
+            .expect("open a conflicting pull request");
+        let object_state = git_object_state(&fixture.bare);
+        let conflicting = service
+            .compare("alice", "project", 2, None, None)
+            .expect("compare conflicting divergence");
+        assert_eq!(
+            conflicting.comparison.mergeability,
+            Mergeability::Conflicting
+        );
+        assert_eq!(git_object_state(&fixture.bare), object_state);
+
+        fixture.merge_feature_into_main();
+        service
+            .open(
+                "alice",
+                "project",
+                "alice",
+                "Merged head",
+                "The head is already in the base.",
+                "refs/heads/main",
+                "refs/heads/feature",
+            )
+            .expect("open an already merged pull request");
+        let merged = service
+            .compare("alice", "project", 3, None, None)
+            .expect("compare an already merged head");
+        assert_eq!(merged.comparison.mergeability, Mergeability::AlreadyMerged);
+
+        fixture.create_unrelated_branch();
+        service
+            .open(
+                "alice",
+                "project",
+                "alice",
+                "Unrelated head",
+                "The branches do not have a common commit.",
+                "refs/heads/main",
+                "refs/heads/unrelated",
+            )
+            .expect("open an unrelated pull request");
+        let unrelated = service
+            .compare("alice", "project", 4, None, None)
+            .expect("compare unrelated histories");
+        assert_eq!(unrelated.comparison.mergeability, Mergeability::Unrelated);
+        assert_eq!(unrelated.comparison.merge_base, None);
+        assert_eq!(unrelated.comparison.changed_paths, [b"unrelated.txt"]);
+
+        let base = ObjectId::from_hex(merged.revision.base_object_id.as_bytes())
+            .expect("parse the base ID");
+        let head = ObjectId::from_hex(merged.revision.head_object_id.as_bytes())
+            .expect("parse the head ID");
+        let limits = ReadLimits {
+            max_history_commits: 1,
+            ..ReadLimits::default()
+        };
+        let reader = RepositoryReadService::open(&fixture.bare, limits)
+            .expect("open a limited repository reader");
+        assert!(matches!(
+            reader.comparison(base, head, &ReadCancellation::default()),
+            Err(ReadError::Limit("history commits" | "comparison commits"))
+        ));
+
+        let base = ObjectId::from_hex(clean.revision.base_object_id.as_bytes())
+            .expect("parse the clean base ID");
+        let head = ObjectId::from_hex(clean.revision.head_object_id.as_bytes())
+            .expect("parse the clean head ID");
+        let limits = ReadLimits {
+            max_diff_bytes: 1,
+            ..ReadLimits::default()
+        };
+        let reader = RepositoryReadService::open(&fixture.bare, limits)
+            .expect("open an output-limited repository reader");
+        assert!(matches!(
+            reader.comparison(base, head, &ReadCancellation::default()),
+            Err(ReadError::Limit("comparison output bytes" | "diff bytes"))
+        ));
+
+        let limits = ReadLimits {
+            max_duration: Duration::from_nanos(1),
+            ..ReadLimits::default()
+        };
+        let reader = RepositoryReadService::open(&fixture.bare, limits)
+            .expect("open a time-limited repository reader");
+        assert!(matches!(
+            reader.comparison(base, head, &ReadCancellation::default()),
+            Err(ReadError::Deadline)
+        ));
+    }
+}
+
 struct Fixture {
     _directory: TempDir,
     database: PathBuf,
@@ -320,6 +486,58 @@
         );
     }
 
+    fn commit_on(&self, branch: &str, path: &str, content: &str, message: &str) {
+        run(
+            &self.worktree,
+            Command::new("git").args(["switch", "-q", branch]),
+        );
+        fs::write(self.worktree.join(path), content).expect("write branch content");
+        git_commit(&self.worktree, message);
+        run(
+            &self.worktree,
+            Command::new("git")
+                .args(["push", "-q"])
+                .arg(&self.bare)
+                .arg(branch),
+        );
+    }
+
+    fn merge_feature_into_main(&self) {
+        run(
+            &self.worktree,
+            Command::new("git").args(["switch", "-q", "main"]),
+        );
+        run(
+            &self.worktree,
+            Command::new("git").args(["merge", "-q", "--no-commit", "-X", "ours", "feature"]),
+        );
+        git_commit(&self.worktree, "merge feature");
+        run(
+            &self.worktree,
+            Command::new("git")
+                .args(["push", "-q"])
+                .arg(&self.bare)
+                .arg("main"),
+        );
+    }
+
+    fn create_unrelated_branch(&self) {
+        run(
+            &self.worktree,
+            Command::new("git").args(["switch", "-q", "--orphan", "unrelated"]),
+        );
+        fs::write(self.worktree.join("unrelated.txt"), b"unrelated\n")
+            .expect("write unrelated content");
+        git_commit(&self.worktree, "unrelated root");
+        run(
+            &self.worktree,
+            Command::new("git")
+                .args(["push", "-q"])
+                .arg(&self.bare)
+                .arg("unrelated"),
+        );
+    }
+
     fn pull_ref(&self, number: i64) -> String {
         GitRepository::open(&self.bare)
             .expect("open the bare fixture")
@@ -356,6 +574,16 @@
         .expect("read a fixture object ID")
         .trim()
         .to_owned()
+}
+
+fn git_object_state(repository: &Path) -> String {
+    let output = Command::new("git")
+        .args(["count-objects", "-v"])
+        .current_dir(repository)
+        .output()
+        .expect("count fixture objects");
+    assert!(output.status.success(), "count fixture objects");
+    String::from_utf8(output.stdout).expect("read the object count")
 }
 
 fn run(directory: &Path, command: &mut Command) {