From 5aa91a51af7f522e37b5b9dd909868465325efa6 Mon Sep 17 00:00:00 2001
From: David Crespo <david.crespo@oxidecomputer.com>
Date: Sat, 6 Dec 2025 10:10:38 -0600
Subject: [PATCH 1/9] add diff command

---
 crates/dropshot-api-manager/src/cmd/diff.rs   | 216 ++++++++++++++++++
 .../dropshot-api-manager/src/cmd/dispatch.rs  |  29 ++-
 crates/dropshot-api-manager/src/cmd/mod.rs    |   1 +
 3 files changed, 244 insertions(+), 2 deletions(-)
 create mode 100644 crates/dropshot-api-manager/src/cmd/diff.rs

diff --git a/crates/dropshot-api-manager/src/cmd/diff.rs b/crates/dropshot-api-manager/src/cmd/diff.rs
new file mode 100644
index 0000000..68cdceb
--- /dev/null
+++ b/crates/dropshot-api-manager/src/cmd/diff.rs
@@ -0,0 +1,216 @@
+// Copyright 2025 Oxide Computer Company
+
+use crate::{
+    apis::ManagedApis,
+    environment::{BlessedSource, ResolvedEnv},
+    output::{OutputOpts, Styles, display_load_problems, write_diff},
+    spec_files_blessed::BlessedApiSpecFile,
+    spec_files_generic::ApiFiles,
+    spec_files_local::LocalApiSpecFile,
+};
+use anyhow::Context;
+use camino::Utf8Path;
+use dropshot_api_manager_types::ApiIdent;
+use owo_colors::OwoColorize;
+use similar::TextDiff;
+use std::process::ExitCode;
+
+/// Compare local OpenAPI documents against blessed (upstream) versions.
+///
+/// For each API with differences, shows the diff between what's on disk locally
+/// and what's blessed in the upstream branch (typically origin/main).
+pub(crate) fn diff_impl(
+    apis: &ManagedApis,
+    env: &ResolvedEnv,
+    blessed_source: &BlessedSource,
+    output: &OutputOpts,
+) -> anyhow::Result<ExitCode> {
+    let mut styles = Styles::default();
+    if output.use_color(supports_color::Stream::Stdout) {
+        styles.colorize();
+    }
+
+    let (local_files, errors) = env.local_source.load(apis, &styles)?;
+    display_load_problems(&errors, &styles)?;
+
+    let (blessed_files, errors) =
+        blessed_source.load(&env.repo_root, apis, &styles)?;
+    display_load_problems(&errors, &styles)?;
+
+    let mut any_diff = false;
+
+    for api in apis.iter_apis() {
+        let ident = api.ident();
+        let local_api = local_files.get(ident);
+        let blessed_api = blessed_files.get(ident);
+
+        let has_diff = diff_api(ident, local_api, blessed_api, &styles)?;
+        any_diff = any_diff || has_diff;
+    }
+
+    if !any_diff {
+        eprintln!("No differences from blessed.");
+    }
+
+    Ok(ExitCode::SUCCESS)
+}
+
+fn diff_api(
+    ident: &ApiIdent,
+    local_api: Option<&ApiFiles<Vec<LocalApiSpecFile>>>,
+    blessed_api: Option<&ApiFiles<BlessedApiSpecFile>>,
+    styles: &Styles,
+) -> anyhow::Result<bool> {
+    // Collect all versions from both sources
+    let mut all_versions: Vec<semver::Version> = Vec::new();
+    if let Some(local) = local_api {
+        all_versions.extend(local.versions().keys().cloned());
+    }
+    if let Some(blessed) = blessed_api {
+        for v in blessed.versions().keys() {
+            if !all_versions.contains(v) {
+                all_versions.push(v.clone());
+            }
+        }
+    }
+    all_versions.sort();
+
+    if all_versions.is_empty() {
+        return Ok(false);
+    }
+
+    let mut has_diff = false;
+
+    for version in &all_versions {
+        let local_file = local_api
+            .and_then(|a| a.versions().get(version))
+            .and_then(|files| files.first());
+        let blessed_file = blessed_api.and_then(|a| a.versions().get(version));
+
+        match (blessed_file, local_file) {
+            (None, Some(local)) => {
+                // New version added locally. Diff against the previous blessed
+                // version to show what actually changed in the schema.
+                let prev_blessed = blessed_api.and_then(|api| {
+                    api.versions()
+                        .iter()
+                        .filter(|(v, _)| *v < version)
+                        .max_by_key(|(v, _)| *v)
+                        .map(|(_, file)| file)
+                });
+
+                let local_content = std::str::from_utf8(local.contents())
+                    .context("local file is not valid UTF-8")?;
+                let local_path = local.spec_file_name().path();
+
+                if let Some(prev) = prev_blessed {
+                    let base_content = std::str::from_utf8(prev.contents())
+                        .context("blessed file is not valid UTF-8")?;
+                    let base_path = prev.spec_file_name().path();
+
+                    // Skip if no actual diff (shouldn't happen, but be safe).
+                    if base_content == local_content {
+                        continue;
+                    }
+
+                    eprintln!(
+                        "\n{} v{}: {} (new locally)",
+                        ident.style(styles.filename),
+                        version,
+                        "added".style(styles.success_header),
+                    );
+                    let diff =
+                        TextDiff::from_lines(base_content, local_content);
+                    write_diff(
+                        &diff,
+                        base_path.as_ref(),  // old path
+                        local_path.as_ref(), // new path
+                        styles,
+                        3,    // context lines
+                        true, // show missing newline hint
+                        &mut std::io::stdout(),
+                    )?;
+                } else {
+                    // No previous version to compare against - show full file.
+                    eprintln!(
+                        "\n{} v{}: {} (new locally)",
+                        ident.style(styles.filename),
+                        version,
+                        "added".style(styles.success_header),
+                    );
+                    let diff = TextDiff::from_lines("", local_content);
+                    write_diff(
+                        &diff,
+                        Utf8Path::new("/dev/null"), // old path
+                        local_path.as_ref(),        // new path
+                        styles,
+                        3,    // context lines
+                        true, // show missing newline hint
+                        &mut std::io::stdout(),
+                    )?;
+                }
+                has_diff = true;
+            }
+            (Some(blessed), None) => {
+                // Version removed locally
+                eprintln!(
+                    "\n{} v{}: {} (removed locally)",
+                    ident.style(styles.filename),
+                    version,
+                    "removed".style(styles.failure_header),
+                );
+                let blessed_content =
+                    std::str::from_utf8(blessed.contents())
+                        .context("blessed file is not valid UTF-8")?;
+                let diff = TextDiff::from_lines(blessed_content, "");
+                let blessed_path = blessed.spec_file_name().path();
+                write_diff(
+                    &diff,
+                    blessed_path.as_ref(),      // old path
+                    Utf8Path::new("/dev/null"), // new path
+                    styles,
+                    3,    // context lines
+                    true, // show missing newline hint
+                    &mut std::io::stdout(),
+                )?;
+                has_diff = true;
+            }
+            (Some(blessed), Some(local)) => {
+                let blessed_content =
+                    std::str::from_utf8(blessed.contents())
+                        .context("blessed file is not valid UTF-8")?;
+                let local_content = std::str::from_utf8(local.contents())
+                    .context("local file is not valid UTF-8")?;
+
+                if blessed_content != local_content {
+                    eprintln!(
+                        "\n{} v{}: {}",
+                        ident.style(styles.filename),
+                        version,
+                        "modified".style(styles.warning_header),
+                    );
+                    let diff =
+                        TextDiff::from_lines(blessed_content, local_content);
+                    let blessed_path = blessed.spec_file_name().path();
+                    let local_path = local.spec_file_name().path();
+                    write_diff(
+                        &diff,
+                        blessed_path.as_ref(), // old path
+                        local_path.as_ref(),   // new path
+                        styles,
+                        3,    // context lines
+                        true, // show missing newline hint
+                        &mut std::io::stdout(),
+                    )?;
+                    has_diff = true;
+                }
+            }
+            (None, None) => {
+                // Shouldn't happen since we collected versions from both
+                unreachable!()
+            }
+        }
+    }
+
+    Ok(has_diff)
+}
diff --git a/crates/dropshot-api-manager/src/cmd/dispatch.rs b/crates/dropshot-api-manager/src/cmd/dispatch.rs
index 7421d7d..473f63a 100644
--- a/crates/dropshot-api-manager/src/cmd/dispatch.rs
+++ b/crates/dropshot-api-manager/src/cmd/dispatch.rs
@@ -3,8 +3,8 @@
 use crate::{
     apis::ManagedApis,
     cmd::{
-        check::check_impl, debug::debug_impl, generate::generate_impl,
-        list::list_impl,
+        check::check_impl, debug::debug_impl, diff::diff_impl,
+        generate::generate_impl, list::list_impl,
     },
     environment::{BlessedSource, Environment, GeneratedSource, ResolvedEnv},
     git::GitRevision,
@@ -33,6 +33,7 @@ impl App {
     pub fn exec(self, env: &Environment, apis: &ManagedApis) -> ExitCode {
         let result = match self.command {
             Command::Debug(args) => args.exec(env, apis, &self.output_opts),
+            Command::Diff(args) => args.exec(env, apis, &self.output_opts),
             Command::List(args) => args.exec(apis, &self.output_opts),
             Command::Generate(args) => args.exec(env, apis, &self.output_opts),
             Command::Check(args) => args.exec(env, apis, &self.output_opts),
@@ -53,6 +54,9 @@ pub enum Command {
     /// Dump debug information about everything the tool knows
     Debug(DebugArgs),
 
+    /// Show differences between local and blessed OpenAPI documents.
+    Diff(DiffArgs),
+
     /// List managed APIs.
     ///
     /// Returns information purely from code without consulting JSON files on
@@ -202,6 +206,27 @@ impl ListArgs {
     }
 }
 
+#[derive(Debug, Args)]
+pub struct DiffArgs {
+    #[clap(flatten)]
+    local: LocalSourceArgs,
+    #[clap(flatten)]
+    blessed: BlessedSourceArgs,
+}
+
+impl DiffArgs {
+    fn exec(
+        self,
+        env: &Environment,
+        apis: &ManagedApis,
+        output: &OutputOpts,
+    ) -> anyhow::Result<ExitCode> {
+        let env = env.resolve(self.local.dir)?;
+        let blessed_source = self.blessed.to_blessed_source(&env)?;
+        diff_impl(apis, &env, &blessed_source, output)
+    }
+}
+
 #[derive(Debug, Args)]
 pub struct GenerateArgs {
     #[clap(flatten)]
diff --git a/crates/dropshot-api-manager/src/cmd/mod.rs b/crates/dropshot-api-manager/src/cmd/mod.rs
index 455171a..dba8c4d 100644
--- a/crates/dropshot-api-manager/src/cmd/mod.rs
+++ b/crates/dropshot-api-manager/src/cmd/mod.rs
@@ -9,5 +9,6 @@ pub mod dispatch;
 // subcommands
 pub(crate) mod check;
 mod debug;
+pub(crate) mod diff;
 mod generate;
 mod list;

From fccefb153d862487881adc9faac0d1f3573ddb10 Mon Sep 17 00:00:00 2001
From: David Crespo <david.crespo@oxidecomputer.com>
Date: Sat, 6 Dec 2025 11:53:41 -0600
Subject: [PATCH 2/9] tests

---
 crates/dropshot-api-manager/src/cmd/diff.rs   |  23 +-
 crates/dropshot-api-manager/src/test_util.rs  |  33 ++-
 crates/integration-tests/src/environment.rs   |   8 +
 .../tests/integration/versioned.rs            | 233 ++++++++++++++++++
 4 files changed, 289 insertions(+), 8 deletions(-)

diff --git a/crates/dropshot-api-manager/src/cmd/diff.rs b/crates/dropshot-api-manager/src/cmd/diff.rs
index 68cdceb..7c99c3a 100644
--- a/crates/dropshot-api-manager/src/cmd/diff.rs
+++ b/crates/dropshot-api-manager/src/cmd/diff.rs
@@ -13,7 +13,7 @@ use camino::Utf8Path;
 use dropshot_api_manager_types::ApiIdent;
 use owo_colors::OwoColorize;
 use similar::TextDiff;
-use std::process::ExitCode;
+use std::{io::Write, process::ExitCode};
 
 /// Compare local OpenAPI documents against blessed (upstream) versions.
 ///
@@ -30,6 +30,8 @@ pub(crate) fn diff_impl(
         styles.colorize();
     }
 
+    // Load files and display any errors/warnings. We still proceed with the
+    // diff for files that loaded successfully.
     let (local_files, errors) = env.local_source.load(apis, &styles)?;
     display_load_problems(&errors, &styles)?;
 
@@ -44,7 +46,13 @@ pub(crate) fn diff_impl(
         let local_api = local_files.get(ident);
         let blessed_api = blessed_files.get(ident);
 
-        let has_diff = diff_api(ident, local_api, blessed_api, &styles)?;
+        let has_diff = diff_api(
+            ident,
+            local_api,
+            blessed_api,
+            &styles,
+            &mut std::io::stdout(),
+        )?;
         any_diff = any_diff || has_diff;
     }
 
@@ -55,11 +63,12 @@ pub(crate) fn diff_impl(
     Ok(ExitCode::SUCCESS)
 }
 
-fn diff_api(
+pub(crate) fn diff_api<W: Write>(
     ident: &ApiIdent,
     local_api: Option<&ApiFiles<Vec<LocalApiSpecFile>>>,
     blessed_api: Option<&ApiFiles<BlessedApiSpecFile>>,
     styles: &Styles,
+    writer: &mut W,
 ) -> anyhow::Result<bool> {
     // Collect all versions from both sources
     let mut all_versions: Vec<semver::Version> = Vec::new();
@@ -128,7 +137,7 @@ fn diff_api(
                         styles,
                         3,    // context lines
                         true, // show missing newline hint
-                        &mut std::io::stdout(),
+                        writer,
                     )?;
                 } else {
                     // No previous version to compare against - show full file.
@@ -146,7 +155,7 @@ fn diff_api(
                         styles,
                         3,    // context lines
                         true, // show missing newline hint
-                        &mut std::io::stdout(),
+                        writer,
                     )?;
                 }
                 has_diff = true;
@@ -171,7 +180,7 @@ fn diff_api(
                     styles,
                     3,    // context lines
                     true, // show missing newline hint
-                    &mut std::io::stdout(),
+                    writer,
                 )?;
                 has_diff = true;
             }
@@ -200,7 +209,7 @@ fn diff_api(
                         styles,
                         3,    // context lines
                         true, // show missing newline hint
-                        &mut std::io::stdout(),
+                        writer,
                     )?;
                     has_diff = true;
                 }
diff --git a/crates/dropshot-api-manager/src/test_util.rs b/crates/dropshot-api-manager/src/test_util.rs
index 9b2e683..d2182c9 100644
--- a/crates/dropshot-api-manager/src/test_util.rs
+++ b/crates/dropshot-api-manager/src/test_util.rs
@@ -7,10 +7,11 @@ use crate::{
     apis::ManagedApis,
     cmd::{
         check::check_impl,
+        diff::diff_api,
         dispatch::{BlessedSourceArgs, GeneratedSourceArgs},
     },
     environment::{Environment, GeneratedSource},
-    output::OutputOpts,
+    output::{OutputOpts, Styles},
 };
 
 /// Check that a set of APIs is up-to-date.
@@ -34,3 +35,33 @@ pub fn check_apis_up_to_date(
 
     check_impl(apis, &env, &blessed_source, &generated_source, &output)
 }
+
+/// Generate the diff output as a string. Used for testing.
+///
+/// Returns the diff output that would be written to stdout by the diff command.
+/// Load errors are ignored (files with errors are simply not included).
+pub fn get_diff_output(
+    env: &Environment,
+    apis: &ManagedApis,
+) -> Result<String, anyhow::Error> {
+    let env = env.resolve(None)?;
+
+    let blessed_source =
+        BlessedSourceArgs { blessed_from_git: None, blessed_from_dir: None }
+            .to_blessed_source(&env)?;
+
+    let styles = Styles::default();
+    let (local_files, _errors) = env.local_source.load(apis, &styles)?;
+    let (blessed_files, _errors) =
+        blessed_source.load(&env.repo_root, apis, &styles)?;
+
+    let mut output = Vec::new();
+    for api in apis.iter_apis() {
+        let ident = api.ident();
+        let local_api = local_files.get(ident);
+        let blessed_api = blessed_files.get(ident);
+        diff_api(ident, local_api, blessed_api, &styles, &mut output)?;
+    }
+
+    Ok(String::from_utf8(output)?)
+}
diff --git a/crates/integration-tests/src/environment.rs b/crates/integration-tests/src/environment.rs
index b5cd76d..9fa13e5 100644
--- a/crates/integration-tests/src/environment.rs
+++ b/crates/integration-tests/src/environment.rs
@@ -365,6 +365,14 @@ impl TestEnvironment {
         }
     }
 
+    /// Get the diff output as a string.
+    pub fn get_diff_output(&self, apis: &ManagedApis) -> Result<String> {
+        dropshot_api_manager::test_util::get_diff_output(
+            &self.environment,
+            apis,
+        )
+    }
+
     fn collect_files_recursive(
         &self,
         dir: &Utf8Path,
diff --git a/crates/integration-tests/tests/integration/versioned.rs b/crates/integration-tests/tests/integration/versioned.rs
index c7482ea..771f377 100644
--- a/crates/integration-tests/tests/integration/versioned.rs
+++ b/crates/integration-tests/tests/integration/versioned.rs
@@ -774,6 +774,239 @@ fn test_blessed_version_extra_local_spec() -> Result<()> {
     Ok(())
 }
 
+// ============================================================================
+// Diff command tests
+// ============================================================================
+
+/// Test that diff produces no output when local matches blessed.
+#[test]
+fn test_diff_empty_when_up_to_date() -> Result<()> {
+    let env = TestEnvironment::new()?;
+    let apis = versioned_health_apis()?;
+
+    // Generate and commit documents to make them blessed.
+    env.generate_documents(&apis)?;
+    env.commit_documents()?;
+
+    // Diff should produce no output.
+    let diff_output = env.get_diff_output(&apis)?;
+    assert!(diff_output.is_empty(), "diff should be empty when up-to-date");
+
+    Ok(())
+}
+
+/// Test that diff shows changes when a new version is added locally.
+///
+/// When a new version is added, the diff should compare against the previous
+/// blessed version to show what actually changed in the schema.
+#[test]
+fn test_diff_new_version_compares_to_previous() -> Result<()> {
+    let env = TestEnvironment::new()?;
+
+    // Start with reduced versions (1.0.0 and 2.0.0) and bless them.
+    let reduced_apis = versioned_health_reduced_apis()?;
+    env.generate_documents(&reduced_apis)?;
+    env.commit_documents()?;
+
+    // Generate with full APIs (adding version 3.0.0).
+    let full_apis = versioned_health_apis()?;
+    env.generate_documents(&full_apis)?;
+
+    // Get the diff output.
+    let diff_output = env.get_diff_output(&full_apis)?;
+
+    // Diff output should compare 2.0.0 to 3.0.0 and only show the new /metrics
+    // endpoint, not the full schema. If /health appeared as added, it would
+    // mean we're dumping the entire file instead of diffing properly.
+    assert!(
+        diff_output.contains("--- a/versioned-health/versioned-health-2.0.0-"),
+        "diff should show 2.0.0 as base:\n{}",
+        diff_output
+    );
+    assert!(
+        diff_output.contains("+++ b/versioned-health/versioned-health-3.0.0-"),
+        "diff should show 3.0.0 as target:\n{}",
+        diff_output
+    );
+    assert!(
+        diff_output.contains(
+            r#"-    "version": "2.0.0"
++    "description": "A versioned health API for testing version evolution",
++    "version": "3.0.0""#
+        ),
+        "diff should show version change:\n{}",
+        diff_output
+    );
+    assert!(
+        diff_output.contains(
+            r#"+    "/metrics": {
++      "get": {
++        "summary": "Get service metrics (v3+).","#
+        ),
+        "diff should show /metrics addition:\n{}",
+        diff_output
+    );
+
+    // The /health endpoint exists in both versions, so it should not appear as
+    // added content.
+    let health_added = diff_output
+        .lines()
+        .any(|line| line.starts_with('+') && line.contains("\"/health\""));
+    assert!(!health_added, "diff shows /health as added:\n{}", diff_output);
+
+    Ok(())
+}
+
+/// Test that diff shows removed content when a version is removed locally.
+#[test]
+fn test_diff_shows_removed_version() -> Result<()> {
+    let env = TestEnvironment::new()?;
+
+    // Start with full versions and bless them.
+    let full_apis = versioned_health_apis()?;
+    env.generate_documents(&full_apis)?;
+    env.commit_documents()?;
+
+    // Generate with reduced APIs (removing version 3.0.0).
+    let reduced_apis = versioned_health_reduced_apis()?;
+    env.generate_documents(&reduced_apis)?;
+
+    // Get the diff output.
+    let diff_output = env.get_diff_output(&reduced_apis)?;
+
+    // The diff should show the removed version's content with - prefix.
+    assert!(
+        diff_output.contains("versioned-health-3.0.0"),
+        "diff should reference removed version (3.0.0), got: {}",
+        diff_output
+    );
+    assert!(
+        diff_output.contains("/dev/null"),
+        "diff should show /dev/null as target (file removed), got: {}",
+        diff_output
+    );
+
+    Ok(())
+}
+
+/// Test that diff shows file as removed when content is modified in-place.
+///
+/// Versioned document filenames include a content hash. When a file is edited
+/// directly without regenerating, the hash no longer matches and the file is
+/// treated as removed. This test verifies that behavior.
+#[test]
+fn test_diff_shows_file_removed_when_hash_invalidated() -> Result<()> {
+    let env = TestEnvironment::new()?;
+
+    // Generate and bless the original documents.
+    let apis = versioned_health_apis()?;
+    env.generate_documents(&apis)?;
+    env.commit_documents()?;
+
+    // Manually modify a local file (this invalidates the filename hash).
+    let doc_path = env
+        .find_versioned_document_path("versioned-health", "1.0.0")?
+        .expect("should have v1.0.0 document");
+    let full_path = env.workspace_root().join(&doc_path);
+    let original_content = std::fs::read_to_string(&full_path)?;
+    let modified_content = original_content.replace(
+        "\"title\": \"Versioned Health API\"",
+        "\"title\": \"Modified Versioned Health API\"",
+    );
+    assert_ne!(
+        original_content, modified_content,
+        "replacement should have changed the content"
+    );
+    std::fs::write(&full_path, &modified_content)?;
+
+    // Get the diff output.
+    let diff_output = env.get_diff_output(&apis)?;
+
+    // Since the hash no longer matches, the file is treated as removed.
+    assert!(!diff_output.is_empty(), "diff should show content");
+    assert!(
+        diff_output.contains("/dev/null"),
+        "diff should show file as removed (hash invalidated), got: {}",
+        diff_output
+    );
+    // The blessed content should be shown as being removed.
+    assert!(
+        diff_output.contains('-'),
+        "diff should contain - lines for removed content, got: {}",
+        diff_output
+    );
+
+    Ok(())
+}
+
+/// Test that diff shows full content when no blessed documents exist.
+#[test]
+fn test_diff_shows_full_content_when_no_blessed() -> Result<()> {
+    let env = TestEnvironment::new()?;
+    let apis = versioned_health_apis()?;
+
+    // Generate documents but don't commit (no blessed documents exist).
+    env.generate_documents(&apis)?;
+
+    // Get the diff output.
+    let diff_output = env.get_diff_output(&apis)?;
+
+    // Should show full file content since there's nothing to compare against.
+    assert!(
+        diff_output.contains("/dev/null"),
+        "diff should show /dev/null as source when no blessed exists, got: {}",
+        diff_output
+    );
+    // Should contain OpenAPI content.
+    assert!(
+        diff_output.contains("openapi"),
+        "diff should show OpenAPI content, got: {}",
+        diff_output
+    );
+
+    Ok(())
+}
+
+/// Test that diff shows removed content when local documents are deleted.
+#[test]
+fn test_diff_shows_removed_when_local_deleted() -> Result<()> {
+    let env = TestEnvironment::new()?;
+    let apis = versioned_health_apis()?;
+
+    // Generate and commit to create blessed documents.
+    env.generate_documents(&apis)?;
+    env.commit_documents()?;
+
+    // Delete local documents.
+    let docs = env.list_versioned_documents("versioned-health")?;
+    for doc in docs {
+        let full_path = env.workspace_root().join(&doc);
+        std::fs::remove_file(&full_path)?;
+    }
+
+    // Get the diff output.
+    let diff_output = env.get_diff_output(&apis)?;
+
+    // Should show files being removed (blessed exists, local missing).
+    assert!(
+        diff_output.contains("/dev/null"),
+        "diff should show /dev/null as target (files removed), got: {}",
+        diff_output
+    );
+    // Should show content being removed (- lines).
+    assert!(
+        diff_output.contains('-'),
+        "diff should contain - lines for removed content, got: {}",
+        diff_output
+    );
+
+    Ok(())
+}
+
+// ============================================================================
+// Extra validation tests
+// ============================================================================
+
 struct VersionValidationPair {
     first: ValidationCall,
     second: ValidationCall,

From 39bec4d3c50c08557d59badd3391f89b06a060bd Mon Sep 17 00:00:00 2001
From: David Crespo <david.crespo@oxidecomputer.com>
Date: Sat, 6 Dec 2025 12:35:53 -0600
Subject: [PATCH 3/9] better test factoring, use diff_impl directly

---
 crates/dropshot-api-manager/src/cmd/diff.rs   | 25 +++++-----
 .../dropshot-api-manager/src/cmd/dispatch.rs  |  4 +-
 crates/dropshot-api-manager/src/test_util.rs  | 28 ++---------
 .../tests/integration/versioned.rs            | 50 -------------------
 4 files changed, 20 insertions(+), 87 deletions(-)

diff --git a/crates/dropshot-api-manager/src/cmd/diff.rs b/crates/dropshot-api-manager/src/cmd/diff.rs
index 7c99c3a..4c48536 100644
--- a/crates/dropshot-api-manager/src/cmd/diff.rs
+++ b/crates/dropshot-api-manager/src/cmd/diff.rs
@@ -13,25 +13,28 @@ use camino::Utf8Path;
 use dropshot_api_manager_types::ApiIdent;
 use owo_colors::OwoColorize;
 use similar::TextDiff;
-use std::{io::Write, process::ExitCode};
+use std::io::Write;
 
 /// Compare local OpenAPI documents against blessed (upstream) versions.
 ///
 /// For each API with differences, shows the diff between what's on disk locally
 /// and what's blessed in the upstream branch (typically origin/main).
+///
+/// Returns the diff output as a string. Headers describing each changed file
+/// are written to stderr.
 pub(crate) fn diff_impl(
     apis: &ManagedApis,
     env: &ResolvedEnv,
     blessed_source: &BlessedSource,
     output: &OutputOpts,
-) -> anyhow::Result<ExitCode> {
+) -> anyhow::Result<String> {
     let mut styles = Styles::default();
     if output.use_color(supports_color::Stream::Stdout) {
         styles.colorize();
     }
 
-    // Load files and display any errors/warnings. We still proceed with the
-    // diff for files that loaded successfully.
+    // Load files and display any errors/warnings. We proceed with the diff for
+    // files that loaded successfully (treating unloadable files as missing).
     let (local_files, errors) = env.local_source.load(apis, &styles)?;
     display_load_problems(&errors, &styles)?;
 
@@ -39,6 +42,7 @@ pub(crate) fn diff_impl(
         blessed_source.load(&env.repo_root, apis, &styles)?;
     display_load_problems(&errors, &styles)?;
 
+    let mut diff_output = Vec::new();
     let mut any_diff = false;
 
     for api in apis.iter_apis() {
@@ -46,13 +50,8 @@ pub(crate) fn diff_impl(
         let local_api = local_files.get(ident);
         let blessed_api = blessed_files.get(ident);
 
-        let has_diff = diff_api(
-            ident,
-            local_api,
-            blessed_api,
-            &styles,
-            &mut std::io::stdout(),
-        )?;
+        let has_diff =
+            diff_api(ident, local_api, blessed_api, &styles, &mut diff_output)?;
         any_diff = any_diff || has_diff;
     }
 
@@ -60,10 +59,10 @@ pub(crate) fn diff_impl(
         eprintln!("No differences from blessed.");
     }
 
-    Ok(ExitCode::SUCCESS)
+    String::from_utf8(diff_output).context("diff output is not valid UTF-8")
 }
 
-pub(crate) fn diff_api<W: Write>(
+fn diff_api<W: Write>(
     ident: &ApiIdent,
     local_api: Option<&ApiFiles<Vec<LocalApiSpecFile>>>,
     blessed_api: Option<&ApiFiles<BlessedApiSpecFile>>,
diff --git a/crates/dropshot-api-manager/src/cmd/dispatch.rs b/crates/dropshot-api-manager/src/cmd/dispatch.rs
index 473f63a..7ac9a7b 100644
--- a/crates/dropshot-api-manager/src/cmd/dispatch.rs
+++ b/crates/dropshot-api-manager/src/cmd/dispatch.rs
@@ -223,7 +223,9 @@ impl DiffArgs {
     ) -> anyhow::Result<ExitCode> {
         let env = env.resolve(self.local.dir)?;
         let blessed_source = self.blessed.to_blessed_source(&env)?;
-        diff_impl(apis, &env, &blessed_source, output)
+        let diff_output = diff_impl(apis, &env, &blessed_source, output)?;
+        print!("{}", diff_output);
+        Ok(ExitCode::SUCCESS)
     }
 }
 
diff --git a/crates/dropshot-api-manager/src/test_util.rs b/crates/dropshot-api-manager/src/test_util.rs
index d2182c9..65b3409 100644
--- a/crates/dropshot-api-manager/src/test_util.rs
+++ b/crates/dropshot-api-manager/src/test_util.rs
@@ -5,13 +5,9 @@
 pub use crate::output::CheckResult;
 use crate::{
     apis::ManagedApis,
-    cmd::{
-        check::check_impl,
-        diff::diff_api,
-        dispatch::{BlessedSourceArgs, GeneratedSourceArgs},
-    },
+    cmd::{check::check_impl, diff::diff_impl, dispatch::BlessedSourceArgs},
     environment::{Environment, GeneratedSource},
-    output::{OutputOpts, Styles},
+    output::OutputOpts,
 };
 
 /// Check that a set of APIs is up-to-date.
@@ -29,8 +25,7 @@ pub fn check_apis_up_to_date(
     let blessed_source =
         BlessedSourceArgs { blessed_from_git: None, blessed_from_dir: None }
             .to_blessed_source(&env)?;
-    let generated_source =
-        GeneratedSource::from(GeneratedSourceArgs { generated_from_dir: None });
+    let generated_source = GeneratedSource::Generated;
     let output = OutputOpts { color: clap::ColorChoice::Auto };
 
     check_impl(apis, &env, &blessed_source, &generated_source, &output)
@@ -39,7 +34,6 @@ pub fn check_apis_up_to_date(
 /// Generate the diff output as a string. Used for testing.
 ///
 /// Returns the diff output that would be written to stdout by the diff command.
-/// Load errors are ignored (files with errors are simply not included).
 pub fn get_diff_output(
     env: &Environment,
     apis: &ManagedApis,
@@ -49,19 +43,7 @@ pub fn get_diff_output(
     let blessed_source =
         BlessedSourceArgs { blessed_from_git: None, blessed_from_dir: None }
             .to_blessed_source(&env)?;
+    let output = OutputOpts { color: clap::ColorChoice::Auto };
 
-    let styles = Styles::default();
-    let (local_files, _errors) = env.local_source.load(apis, &styles)?;
-    let (blessed_files, _errors) =
-        blessed_source.load(&env.repo_root, apis, &styles)?;
-
-    let mut output = Vec::new();
-    for api in apis.iter_apis() {
-        let ident = api.ident();
-        let local_api = local_files.get(ident);
-        let blessed_api = blessed_files.get(ident);
-        diff_api(ident, local_api, blessed_api, &styles, &mut output)?;
-    }
-
-    Ok(String::from_utf8(output)?)
+    diff_impl(apis, &env, &blessed_source, &output)
 }
diff --git a/crates/integration-tests/tests/integration/versioned.rs b/crates/integration-tests/tests/integration/versioned.rs
index 771f377..2217d1d 100644
--- a/crates/integration-tests/tests/integration/versioned.rs
+++ b/crates/integration-tests/tests/integration/versioned.rs
@@ -889,56 +889,6 @@ fn test_diff_shows_removed_version() -> Result<()> {
     Ok(())
 }
 
-/// Test that diff shows file as removed when content is modified in-place.
-///
-/// Versioned document filenames include a content hash. When a file is edited
-/// directly without regenerating, the hash no longer matches and the file is
-/// treated as removed. This test verifies that behavior.
-#[test]
-fn test_diff_shows_file_removed_when_hash_invalidated() -> Result<()> {
-    let env = TestEnvironment::new()?;
-
-    // Generate and bless the original documents.
-    let apis = versioned_health_apis()?;
-    env.generate_documents(&apis)?;
-    env.commit_documents()?;
-
-    // Manually modify a local file (this invalidates the filename hash).
-    let doc_path = env
-        .find_versioned_document_path("versioned-health", "1.0.0")?
-        .expect("should have v1.0.0 document");
-    let full_path = env.workspace_root().join(&doc_path);
-    let original_content = std::fs::read_to_string(&full_path)?;
-    let modified_content = original_content.replace(
-        "\"title\": \"Versioned Health API\"",
-        "\"title\": \"Modified Versioned Health API\"",
-    );
-    assert_ne!(
-        original_content, modified_content,
-        "replacement should have changed the content"
-    );
-    std::fs::write(&full_path, &modified_content)?;
-
-    // Get the diff output.
-    let diff_output = env.get_diff_output(&apis)?;
-
-    // Since the hash no longer matches, the file is treated as removed.
-    assert!(!diff_output.is_empty(), "diff should show content");
-    assert!(
-        diff_output.contains("/dev/null"),
-        "diff should show file as removed (hash invalidated), got: {}",
-        diff_output
-    );
-    // The blessed content should be shown as being removed.
-    assert!(
-        diff_output.contains('-'),
-        "diff should contain - lines for removed content, got: {}",
-        diff_output
-    );
-
-    Ok(())
-}
-
 /// Test that diff shows full content when no blessed documents exist.
 #[test]
 fn test_diff_shows_full_content_when_no_blessed() -> Result<()> {

From 6a10d11ffe05f77892a13f26f18c30329e40380c Mon Sep 17 00:00:00 2001
From: David Crespo <david.crespo@oxidecomputer.com>
Date: Sat, 6 Dec 2025 12:46:38 -0600
Subject: [PATCH 4/9] try line ending normalization hack to see if that's it

---
 crates/integration-tests/tests/integration/versioned.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/crates/integration-tests/tests/integration/versioned.rs b/crates/integration-tests/tests/integration/versioned.rs
index 2217d1d..b084ed0 100644
--- a/crates/integration-tests/tests/integration/versioned.rs
+++ b/crates/integration-tests/tests/integration/versioned.rs
@@ -812,8 +812,8 @@ fn test_diff_new_version_compares_to_previous() -> Result<()> {
     let full_apis = versioned_health_apis()?;
     env.generate_documents(&full_apis)?;
 
-    // Get the diff output.
-    let diff_output = env.get_diff_output(&full_apis)?;
+    // Get the diff output (normalize line endings for cross-platform).
+    let diff_output = env.get_diff_output(&full_apis)?.replace("\r\n", "\n");
 
     // Diff output should compare 2.0.0 to 3.0.0 and only show the new /metrics
     // endpoint, not the full schema. If /health appeared as added, it would

From d684addb9aaeb271fe929d18d0886035997937a6 Mon Sep 17 00:00:00 2001
From: David Crespo <david.crespo@oxidecomputer.com>
Date: Sat, 6 Dec 2025 12:54:26 -0600
Subject: [PATCH 5/9] nope it was the backslashes

---
 crates/dropshot-api-manager/src/test_util.rs            | 2 ++
 crates/integration-tests/tests/integration/versioned.rs | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/crates/dropshot-api-manager/src/test_util.rs b/crates/dropshot-api-manager/src/test_util.rs
index 65b3409..fb1d20f 100644
--- a/crates/dropshot-api-manager/src/test_util.rs
+++ b/crates/dropshot-api-manager/src/test_util.rs
@@ -45,5 +45,7 @@ pub fn get_diff_output(
             .to_blessed_source(&env)?;
     let output = OutputOpts { color: clap::ColorChoice::Auto };
 
+    // Normalize path separators for cross-platform consistency in tests.
     diff_impl(apis, &env, &blessed_source, &output)
+        .map(|s| s.replace('\\', "/"))
 }
diff --git a/crates/integration-tests/tests/integration/versioned.rs b/crates/integration-tests/tests/integration/versioned.rs
index b084ed0..2217d1d 100644
--- a/crates/integration-tests/tests/integration/versioned.rs
+++ b/crates/integration-tests/tests/integration/versioned.rs
@@ -812,8 +812,8 @@ fn test_diff_new_version_compares_to_previous() -> Result<()> {
     let full_apis = versioned_health_apis()?;
     env.generate_documents(&full_apis)?;
 
-    // Get the diff output (normalize line endings for cross-platform).
-    let diff_output = env.get_diff_output(&full_apis)?.replace("\r\n", "\n");
+    // Get the diff output.
+    let diff_output = env.get_diff_output(&full_apis)?;
 
     // Diff output should compare 2.0.0 to 3.0.0 and only show the new /metrics
     // endpoint, not the full schema. If /health appeared as added, it would

From 6a9e41de6f7fd1a8c66058613e458bbcbdde6428 Mon Sep 17 00:00:00 2001
From: David Crespo <david.crespo@oxidecomputer.com>
Date: Sat, 6 Dec 2025 13:08:51 -0600
Subject: [PATCH 6/9] write to a writer to avoid interleaved output

---
 crates/dropshot-api-manager/src/cmd/diff.rs     | 16 ++++++++--------
 crates/dropshot-api-manager/src/cmd/dispatch.rs |  4 ++--
 crates/dropshot-api-manager/src/test_util.rs    |  9 +++++++--
 3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/crates/dropshot-api-manager/src/cmd/diff.rs b/crates/dropshot-api-manager/src/cmd/diff.rs
index 4c48536..e294b00 100644
--- a/crates/dropshot-api-manager/src/cmd/diff.rs
+++ b/crates/dropshot-api-manager/src/cmd/diff.rs
@@ -20,14 +20,15 @@ use std::io::Write;
 /// For each API with differences, shows the diff between what's on disk locally
 /// and what's blessed in the upstream branch (typically origin/main).
 ///
-/// Returns the diff output as a string. Headers describing each changed file
-/// are written to stderr.
-pub(crate) fn diff_impl(
+/// Diff output is written directly to `writer`. Headers describing each changed
+/// file are written to stderr.
+pub(crate) fn diff_impl<W: Write>(
     apis: &ManagedApis,
     env: &ResolvedEnv,
     blessed_source: &BlessedSource,
     output: &OutputOpts,
-) -> anyhow::Result<String> {
+    writer: &mut W,
+) -> anyhow::Result<()> {
     let mut styles = Styles::default();
     if output.use_color(supports_color::Stream::Stdout) {
         styles.colorize();
@@ -42,7 +43,6 @@ pub(crate) fn diff_impl(
         blessed_source.load(&env.repo_root, apis, &styles)?;
     display_load_problems(&errors, &styles)?;
 
-    let mut diff_output = Vec::new();
     let mut any_diff = false;
 
     for api in apis.iter_apis() {
@@ -51,15 +51,15 @@ pub(crate) fn diff_impl(
         let blessed_api = blessed_files.get(ident);
 
         let has_diff =
-            diff_api(ident, local_api, blessed_api, &styles, &mut diff_output)?;
-        any_diff = any_diff || has_diff;
+            diff_api(ident, local_api, blessed_api, &styles, writer)?;
+        any_diff |= has_diff;
     }
 
     if !any_diff {
         eprintln!("No differences from blessed.");
     }
 
-    String::from_utf8(diff_output).context("diff output is not valid UTF-8")
+    Ok(())
 }
 
 fn diff_api<W: Write>(
diff --git a/crates/dropshot-api-manager/src/cmd/dispatch.rs b/crates/dropshot-api-manager/src/cmd/dispatch.rs
index 7ac9a7b..f2d6d78 100644
--- a/crates/dropshot-api-manager/src/cmd/dispatch.rs
+++ b/crates/dropshot-api-manager/src/cmd/dispatch.rs
@@ -223,8 +223,8 @@ impl DiffArgs {
     ) -> anyhow::Result<ExitCode> {
         let env = env.resolve(self.local.dir)?;
         let blessed_source = self.blessed.to_blessed_source(&env)?;
-        let diff_output = diff_impl(apis, &env, &blessed_source, output)?;
-        print!("{}", diff_output);
+        let mut stdout = std::io::stdout();
+        diff_impl(apis, &env, &blessed_source, output, &mut stdout)?;
         Ok(ExitCode::SUCCESS)
     }
 }
diff --git a/crates/dropshot-api-manager/src/test_util.rs b/crates/dropshot-api-manager/src/test_util.rs
index fb1d20f..ddb2af3 100644
--- a/crates/dropshot-api-manager/src/test_util.rs
+++ b/crates/dropshot-api-manager/src/test_util.rs
@@ -45,7 +45,12 @@ pub fn get_diff_output(
             .to_blessed_source(&env)?;
     let output = OutputOpts { color: clap::ColorChoice::Auto };
 
+    let mut buffer = Vec::new();
+    diff_impl(apis, &env, &blessed_source, &output, &mut buffer)?;
+
     // Normalize path separators for cross-platform consistency in tests.
-    diff_impl(apis, &env, &blessed_source, &output)
-        .map(|s| s.replace('\\', "/"))
+    let result = String::from_utf8(buffer).map_err(|e| {
+        anyhow::anyhow!("diff output is not valid UTF-8: {}", e)
+    })?;
+    Ok(result.replace('\\', "/"))
 }

From 4f3aefdcfcae5bbe3adb140c402d40df03426f44 Mon Sep 17 00:00:00 2001
From: David Crespo <david.crespo@oxidecomputer.com>
Date: Mon, 8 Dec 2025 13:24:14 -0600
Subject: [PATCH 7/9] error out up front if any apis have multiple local specs

---
 crates/dropshot-api-manager/src/cmd/diff.rs   | 93 +++++++++++--------
 .../tests/integration/versioned.rs            | 60 ++++++++++++
 2 files changed, 116 insertions(+), 37 deletions(-)

diff --git a/crates/dropshot-api-manager/src/cmd/diff.rs b/crates/dropshot-api-manager/src/cmd/diff.rs
index e294b00..348a01f 100644
--- a/crates/dropshot-api-manager/src/cmd/diff.rs
+++ b/crates/dropshot-api-manager/src/cmd/diff.rs
@@ -5,15 +5,14 @@ use crate::{
     environment::{BlessedSource, ResolvedEnv},
     output::{OutputOpts, Styles, display_load_problems, write_diff},
     spec_files_blessed::BlessedApiSpecFile,
-    spec_files_generic::ApiFiles,
     spec_files_local::LocalApiSpecFile,
 };
-use anyhow::Context;
+use anyhow::{Context, bail};
 use camino::Utf8Path;
 use dropshot_api_manager_types::ApiIdent;
 use owo_colors::OwoColorize;
 use similar::TextDiff;
-use std::io::Write;
+use std::{collections::BTreeMap, io::Write};
 
 /// Compare local OpenAPI documents against blessed (upstream) versions.
 ///
@@ -39,6 +38,34 @@ pub(crate) fn diff_impl<W: Write>(
     let (local_files, errors) = env.local_source.load(apis, &styles)?;
     display_load_problems(&errors, &styles)?;
 
+    // Build maps from version to single file, validating that no version has
+    // multiple local files. Multiple files can happen if two developers create
+    // the same version with different content (resulting in different hashes).
+    let mut local_by_api: BTreeMap<&ApiIdent, BTreeMap<_, _>> = BTreeMap::new();
+    for (ident, api_files) in local_files.iter() {
+        let mut version_map = BTreeMap::new();
+        for (version, files) in api_files.versions() {
+            if files.len() > 1 {
+                let file_names: Vec<_> = files
+                    .iter()
+                    .map(|f| f.spec_file_name().path().to_string())
+                    .collect();
+                bail!(
+                    "{} v{}: found {} local files for the same version \
+                     ({}); run `generate` to resolve this conflict",
+                    ident,
+                    version,
+                    files.len(),
+                    file_names.join(", "),
+                );
+            }
+            if let Some(file) = files.first() {
+                version_map.insert(version, file);
+            }
+        }
+        local_by_api.insert(ident, version_map);
+    }
+
     let (blessed_files, errors) =
         blessed_source.load(&env.repo_root, apis, &styles)?;
     display_load_problems(&errors, &styles)?;
@@ -47,11 +74,21 @@ pub(crate) fn diff_impl<W: Write>(
 
     for api in apis.iter_apis() {
         let ident = api.ident();
-        let local_api = local_files.get(ident);
-        let blessed_api = blessed_files.get(ident);
-
-        let has_diff =
-            diff_api(ident, local_api, blessed_api, &styles, writer)?;
+        let empty = BTreeMap::new();
+        let local_versions = local_by_api.get(ident).unwrap_or(&empty);
+        let blessed_versions: BTreeMap<_, _> = blessed_files
+            .get(ident)
+            .into_iter()
+            .flat_map(|api| api.versions())
+            .collect();
+
+        let has_diff = diff_api(
+            ident,
+            local_versions,
+            &blessed_versions,
+            &styles,
+            writer,
+        )?;
         any_diff |= has_diff;
     }
 
@@ -64,48 +101,30 @@ pub(crate) fn diff_impl<W: Write>(
 
 fn diff_api<W: Write>(
     ident: &ApiIdent,
-    local_api: Option<&ApiFiles<Vec<LocalApiSpecFile>>>,
-    blessed_api: Option<&ApiFiles<BlessedApiSpecFile>>,
+    local_versions: &BTreeMap<&semver::Version, &LocalApiSpecFile>,
+    blessed_versions: &BTreeMap<&semver::Version, &BlessedApiSpecFile>,
     styles: &Styles,
     writer: &mut W,
 ) -> anyhow::Result<bool> {
-    // Collect all versions from both sources
-    let mut all_versions: Vec<semver::Version> = Vec::new();
-    if let Some(local) = local_api {
-        all_versions.extend(local.versions().keys().cloned());
-    }
-    if let Some(blessed) = blessed_api {
-        for v in blessed.versions().keys() {
-            if !all_versions.contains(v) {
-                all_versions.push(v.clone());
-            }
-        }
-    }
-    all_versions.sort();
-
-    if all_versions.is_empty() {
+    if local_versions.is_empty() && blessed_versions.is_empty() {
         return Ok(false);
     }
 
     let mut has_diff = false;
 
-    for version in &all_versions {
-        let local_file = local_api
-            .and_then(|a| a.versions().get(version))
-            .and_then(|files| files.first());
-        let blessed_file = blessed_api.and_then(|a| a.versions().get(version));
+    // Iterate all versions from both sources. BTreeMap keys are sorted.
+    for version in local_versions.keys().chain(blessed_versions.keys()) {
+        let local_file = local_versions.get(version).copied();
+        let blessed_file = blessed_versions.get(version).copied();
 
         match (blessed_file, local_file) {
             (None, Some(local)) => {
                 // New version added locally. Diff against the previous blessed
                 // version to show what actually changed in the schema.
-                let prev_blessed = blessed_api.and_then(|api| {
-                    api.versions()
-                        .iter()
-                        .filter(|(v, _)| *v < version)
-                        .max_by_key(|(v, _)| *v)
-                        .map(|(_, file)| file)
-                });
+                let prev_blessed = blessed_versions
+                    .range::<semver::Version, _>(..*version)
+                    .next_back()
+                    .map(|(_, file)| *file);
 
                 let local_content = std::str::from_utf8(local.contents())
                     .context("local file is not valid UTF-8")?;
diff --git a/crates/integration-tests/tests/integration/versioned.rs b/crates/integration-tests/tests/integration/versioned.rs
index 2217d1d..d4b456e 100644
--- a/crates/integration-tests/tests/integration/versioned.rs
+++ b/crates/integration-tests/tests/integration/versioned.rs
@@ -1035,6 +1035,66 @@ fn test_extra_validation_blessed_vs_non_blessed() -> Result<()> {
     Ok(())
 }
 
+/// Test that diff fails when there are multiple local files for the same
+/// version.
+///
+/// This can happen when two developers create the same API version in separate
+/// branches with different content, resulting in files with different hashes.
+#[test]
+fn test_diff_fails_with_duplicate_local_versions() -> Result<()> {
+    let env = TestEnvironment::new()?;
+    let apis = versioned_health_apis()?;
+
+    // Generate and commit initial documents to make them blessed.
+    env.generate_documents(&apis)?;
+    env.commit_documents()?;
+
+    // Generate with the incompatible APIs in a second environment. This
+    // produces files with different hashes for the same version.
+    let env2 = TestEnvironment::new()?;
+    let incompatible_apis = versioned_health_incompat_apis()?;
+    env2.generate_documents(&incompatible_apis)?;
+
+    // Ensure that the v3 documents are actually different between env and env2.
+    let env_path = env
+        .find_versioned_document_path("versioned-health", "3.0.0")?
+        .expect("should find v3.0.0 document");
+    let env2_path = env2
+        .find_versioned_document_path("versioned-health", "3.0.0")?
+        .expect("should find v3.0.0 document");
+    assert_ne!(
+        env_path, env2_path,
+        "incompatible APIs should lead to different hashes"
+    );
+
+    // Copy env2's document into env's documents directory. Now env has two
+    // files for version 3.0.0.
+    let src = env2.workspace_root().join(&env2_path);
+    let dst = env
+        .documents_dir()
+        .join("versioned-health")
+        .join(env2_path.file_name().unwrap());
+    std::fs::copy(&src, &dst)
+        .with_context(|| format!("failed to copy {} to {}", src, dst))?;
+
+    // Diff should fail because there are two local files for the same version.
+    let result = env.get_diff_output(&apis);
+    let err = result.expect_err("diff should fail with duplicate local files");
+    let err_msg = err.to_string();
+    assert!(
+        err_msg.contains("found 2 local files for the same version"),
+        "error should mention duplicate files, got: {}",
+        err_msg
+    );
+    assert!(
+        err_msg.contains("versioned-health") && err_msg.contains("3.0.0"),
+        "error should identify the API and version, got: {}",
+        err_msg
+    );
+
+    Ok(())
+}
+
 #[test]
 fn test_extra_validation_with_extra_file() -> Result<()> {
     let env = TestEnvironment::new()?;

From 5bf52741c0a43d9239011db2f4605a48ba7e2aa2 Mon Sep 17 00:00:00 2001
From: David Crespo <david.crespo@oxidecomputer.com>
Date: Mon, 8 Dec 2025 14:05:24 -0600
Subject: [PATCH 8/9] test for modified case showing why we need to dedupe the
 versions

---
 crates/dropshot-api-manager/src/cmd/diff.rs   | 15 ++++-
 .../tests/integration/versioned.rs            | 64 +++++++++++++++++++
 2 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/crates/dropshot-api-manager/src/cmd/diff.rs b/crates/dropshot-api-manager/src/cmd/diff.rs
index 348a01f..b0ccde3 100644
--- a/crates/dropshot-api-manager/src/cmd/diff.rs
+++ b/crates/dropshot-api-manager/src/cmd/diff.rs
@@ -12,7 +12,10 @@ use camino::Utf8Path;
 use dropshot_api_manager_types::ApiIdent;
 use owo_colors::OwoColorize;
 use similar::TextDiff;
-use std::{collections::BTreeMap, io::Write};
+use std::{
+    collections::{BTreeMap, BTreeSet},
+    io::Write,
+};
 
 /// Compare local OpenAPI documents against blessed (upstream) versions.
 ///
@@ -112,8 +115,14 @@ fn diff_api<W: Write>(
 
     let mut has_diff = false;
 
-    // Iterate all versions from both sources. BTreeMap keys are sorted.
-    for version in local_versions.keys().chain(blessed_versions.keys()) {
+    // Collect unique versions from both sources to handle all combinations:
+    // local only (added), blessed only (removed), or both (potentially
+    // modified). We need a set because chaining the iterators would visit
+    // versions present in both maps twice.
+    let all_versions: BTreeSet<_> =
+        local_versions.keys().chain(blessed_versions.keys()).collect();
+
+    for version in all_versions {
         let local_file = local_versions.get(version).copied();
         let blessed_file = blessed_versions.get(version).copied();
 
diff --git a/crates/integration-tests/tests/integration/versioned.rs b/crates/integration-tests/tests/integration/versioned.rs
index d4b456e..981b425 100644
--- a/crates/integration-tests/tests/integration/versioned.rs
+++ b/crates/integration-tests/tests/integration/versioned.rs
@@ -953,6 +953,70 @@ fn test_diff_shows_removed_when_local_deleted() -> Result<()> {
     Ok(())
 }
 
+/// Test that diff shows changes when a blessed version is modified locally.
+#[test]
+fn test_diff_shows_modified_version() -> Result<()> {
+    let env = TestEnvironment::new()?;
+    let apis = versioned_health_apis()?;
+
+    // Generate and commit documents to make them blessed.
+    env.generate_documents(&apis)?;
+    env.commit_documents()?;
+
+    // Generate different content for the same version in a second environment.
+    // The incompat API has /health/detailed instead of /status for v2.
+    let env2 = TestEnvironment::new()?;
+    let incompat_apis = versioned_health_incompat_apis()?;
+    env2.generate_documents(&incompat_apis)?;
+
+    // Replace env's v2 file with env2's v2 file (different content, same
+    // version).
+    let env_path = env
+        .find_versioned_document_path("versioned-health", "2.0.0")?
+        .expect("should find v2.0.0 document in env");
+    let env2_path = env2
+        .find_versioned_document_path("versioned-health", "2.0.0")?
+        .expect("should find v2.0.0 document in env2");
+
+    let src = env2.workspace_root().join(&env2_path);
+    let old_dst = env.workspace_root().join(&env_path);
+    let new_dst = env
+        .documents_dir()
+        .join("versioned-health")
+        .join(env2_path.file_name().unwrap());
+
+    std::fs::remove_file(&old_dst)?;
+    std::fs::copy(&src, &new_dst)?;
+
+    // Get the diff output.
+    let diff_output = env.get_diff_output(&apis)?;
+
+    // Should show the modification (description differs between the APIs).
+    assert!(
+        diff_output.contains("-    \"description\": \"A versioned health API for testing version evolution\""),
+        "diff should show removed description, got: {}",
+        diff_output
+    );
+    assert!(
+        diff_output.contains("+    \"description\": \"A versioned health API with incompatible changes\""),
+        "diff should show added description, got: {}",
+        diff_output
+    );
+
+    // The v2 diff should only appear once (regression test for duplicate
+    // iteration bug where versions in both maps were visited twice).
+    let diff_header_count = diff_output
+        .matches("--- a/versioned-health/versioned-health-2.0.0")
+        .count();
+    assert_eq!(
+        diff_header_count, 1,
+        "v2.0.0 diff should appear exactly once, got: {}",
+        diff_output
+    );
+
+    Ok(())
+}
+
 // ============================================================================
 // Extra validation tests
 // ============================================================================

From d0b762151009d698647204a9da9239c5d7a44a17 Mon Sep 17 00:00:00 2001
From: David Crespo <david.crespo@oxidecomputer.com>
Date: Mon, 8 Dec 2025 15:25:18 -0600
Subject: [PATCH 9/9] move test to be next to its brothers and sisters

---
 .../tests/integration/versioned.rs            | 120 +++++++++---------
 1 file changed, 60 insertions(+), 60 deletions(-)

diff --git a/crates/integration-tests/tests/integration/versioned.rs b/crates/integration-tests/tests/integration/versioned.rs
index 981b425..46c8c1c 100644
--- a/crates/integration-tests/tests/integration/versioned.rs
+++ b/crates/integration-tests/tests/integration/versioned.rs
@@ -1017,6 +1017,66 @@ fn test_diff_shows_modified_version() -> Result<()> {
     Ok(())
 }
 
+/// Test that diff fails when there are multiple local files for the same
+/// version.
+///
+/// This can happen when two developers create the same API version in separate
+/// branches with different content, resulting in files with different hashes.
+#[test]
+fn test_diff_fails_with_duplicate_local_versions() -> Result<()> {
+    let env = TestEnvironment::new()?;
+    let apis = versioned_health_apis()?;
+
+    // Generate and commit initial documents to make them blessed.
+    env.generate_documents(&apis)?;
+    env.commit_documents()?;
+
+    // Generate with the incompatible APIs in a second environment. This
+    // produces files with different hashes for the same version.
+    let env2 = TestEnvironment::new()?;
+    let incompatible_apis = versioned_health_incompat_apis()?;
+    env2.generate_documents(&incompatible_apis)?;
+
+    // Ensure that the v3 documents are actually different between env and env2.
+    let env_path = env
+        .find_versioned_document_path("versioned-health", "3.0.0")?
+        .expect("should find v3.0.0 document");
+    let env2_path = env2
+        .find_versioned_document_path("versioned-health", "3.0.0")?
+        .expect("should find v3.0.0 document");
+    assert_ne!(
+        env_path, env2_path,
+        "incompatible APIs should lead to different hashes"
+    );
+
+    // Copy env2's document into env's documents directory. Now env has two
+    // files for version 3.0.0.
+    let src = env2.workspace_root().join(&env2_path);
+    let dst = env
+        .documents_dir()
+        .join("versioned-health")
+        .join(env2_path.file_name().unwrap());
+    std::fs::copy(&src, &dst)
+        .with_context(|| format!("failed to copy {} to {}", src, dst))?;
+
+    // Diff should fail because there are two local files for the same version.
+    let result = env.get_diff_output(&apis);
+    let err = result.expect_err("diff should fail with duplicate local files");
+    let err_msg = err.to_string();
+    assert!(
+        err_msg.contains("found 2 local files for the same version"),
+        "error should mention duplicate files, got: {}",
+        err_msg
+    );
+    assert!(
+        err_msg.contains("versioned-health") && err_msg.contains("3.0.0"),
+        "error should identify the API and version, got: {}",
+        err_msg
+    );
+
+    Ok(())
+}
+
 // ============================================================================
 // Extra validation tests
 // ============================================================================
@@ -1099,66 +1159,6 @@ fn test_extra_validation_blessed_vs_non_blessed() -> Result<()> {
     Ok(())
 }
 
-/// Test that diff fails when there are multiple local files for the same
-/// version.
-///
-/// This can happen when two developers create the same API version in separate
-/// branches with different content, resulting in files with different hashes.
-#[test]
-fn test_diff_fails_with_duplicate_local_versions() -> Result<()> {
-    let env = TestEnvironment::new()?;
-    let apis = versioned_health_apis()?;
-
-    // Generate and commit initial documents to make them blessed.
-    env.generate_documents(&apis)?;
-    env.commit_documents()?;
-
-    // Generate with the incompatible APIs in a second environment. This
-    // produces files with different hashes for the same version.
-    let env2 = TestEnvironment::new()?;
-    let incompatible_apis = versioned_health_incompat_apis()?;
-    env2.generate_documents(&incompatible_apis)?;
-
-    // Ensure that the v3 documents are actually different between env and env2.
-    let env_path = env
-        .find_versioned_document_path("versioned-health", "3.0.0")?
-        .expect("should find v3.0.0 document");
-    let env2_path = env2
-        .find_versioned_document_path("versioned-health", "3.0.0")?
-        .expect("should find v3.0.0 document");
-    assert_ne!(
-        env_path, env2_path,
-        "incompatible APIs should lead to different hashes"
-    );
-
-    // Copy env2's document into env's documents directory. Now env has two
-    // files for version 3.0.0.
-    let src = env2.workspace_root().join(&env2_path);
-    let dst = env
-        .documents_dir()
-        .join("versioned-health")
-        .join(env2_path.file_name().unwrap());
-    std::fs::copy(&src, &dst)
-        .with_context(|| format!("failed to copy {} to {}", src, dst))?;
-
-    // Diff should fail because there are two local files for the same version.
-    let result = env.get_diff_output(&apis);
-    let err = result.expect_err("diff should fail with duplicate local files");
-    let err_msg = err.to_string();
-    assert!(
-        err_msg.contains("found 2 local files for the same version"),
-        "error should mention duplicate files, got: {}",
-        err_msg
-    );
-    assert!(
-        err_msg.contains("versioned-health") && err_msg.contains("3.0.0"),
-        "error should identify the API and version, got: {}",
-        err_msg
-    );
-
-    Ok(())
-}
-
 #[test]
 fn test_extra_validation_with_extra_file() -> Result<()> {
     let env = TestEnvironment::new()?;