fix: write CODEOWNERS annotations after glob-based sections

src/ownership.rs

@@ -122,7 +122,9 @@ impl Ownership {
         let validator = Validator {
             project: self.project.clone(),
             mappers: self.mappers(),
-            file_generator: FileGenerator { mappers: self.mappers() },
+            file_generator: FileGenerator {
+                mappers: self.codeowners_file_mappers(),
+            },
             executable_name: self.project.executable_name.clone(),
         };
 
@@ -166,7 +168,9 @@ impl Ownership {
     #[instrument(level = "debug", skip_all)]
     pub fn generate_file(&self) -> String {
         info!("generating codeowners file");
-        let file_generator = FileGenerator { mappers: self.mappers() };
+        let file_generator = FileGenerator {
+            mappers: self.codeowners_file_mappers(),
+        };
         file_generator.generate_file()
     }
 
@@ -182,6 +186,20 @@ impl Ownership {
             Box::new(TeamGemMapper::build(self.project.clone())),
         ]
     }
+
+    // GitHub applies the last matching CODEOWNERS line, so annotations are written last: an annotated
+    // file can still match a broader team glob that excludes it through unowned_globs.
+    fn codeowners_file_mappers(&self) -> Vec<Box<dyn Mapper>> {
+        vec![
+            Box::new(TeamGlobMapper::build(self.project.clone())),
+            Box::new(DirectoryMapper::build(self.project.clone())),
+            Box::new(RubyPackageMapper::build(self.project.clone())),
+            Box::new(JavascriptPackageMapper::build(self.project.clone())),
+            Box::new(TeamYmlMapper::build(self.project.clone())),
+            Box::new(TeamGemMapper::build(self.project.clone())),
+            Box::new(TeamFileMapper::build(self.project.clone())),
+        ]
+    }
 }
 
 #[cfg(test)]

tests/annotation_in_unowned_team_glob_test.rs

@@ -0,0 +1,64 @@
+use assert_cmd::prelude::*;
+use indoc::indoc;
+use predicates::prelude::*;
+use std::error::Error;
+use std::path::Path;
+use std::process::Command;
+use tempfile::TempDir;
+
+mod common;
+use common::OutputStream;
+use common::git_add_all_files;
+use common::run_codeowners;
+use common::setup_fixture_repo;
+
+// Alpha owns ruby/app/**/* but excludes beta_owned.rb through unowned_globs, and Beta owns that file
+// through an annotation. GitHub applies the last matching CODEOWNERS line, so the annotation line must
+// come after Alpha's broader glob for GitHub to agree with for-file.
+const FIXTURE: &str = "annotation_in_unowned_team_glob";
+
+// Copies the fixture and regenerates its CODEOWNERS, so assertions exercise the generator rather than
+// the committed file.
+fn fixture_with_generated_codeowners() -> Result<TempDir, Box<dyn Error>> {
+    let temp_dir = setup_fixture_repo(&Path::new("tests/fixtures").join(FIXTURE));
+    git_add_all_files(temp_dir.path());
+    codeowners(temp_dir.path(), &["generate"])?.assert().success();
+    Ok(temp_dir)
+}
+
+fn codeowners(project_root: &Path, args: &[&str]) -> Result<Command, Box<dyn Error>> {
+    let mut cmd = Command::cargo_bin("codeowners")?;
+    cmd.arg("--project-root").arg(project_root).arg("--no-cache").args(args);
+    Ok(cmd)
+}
+
+#[test]
+fn test_validate_accepts_generated_order() -> Result<(), Box<dyn Error>> {
+    run_codeowners(FIXTURE, &["validate"], true, OutputStream::Stdout, predicate::eq(""))?;
+    Ok(())
+}
+
+#[test]
+fn test_generated_codeowners_agrees_with_for_file() -> Result<(), Box<dyn Error>> {
+    let temp_dir = fixture_with_generated_codeowners()?;
+    codeowners(temp_dir.path(), &["crosscheck-owners"])?
+        .assert()
+        .success()
+        .stdout(predicate::eq(indoc! {"
+            Success! All files match between CODEOWNERS and for-file command.
+        "}));
+    Ok(())
+}
+
+#[test]
+fn test_for_file_from_generated_codeowners_returns_annotated_owner() -> Result<(), Box<dyn Error>> {
+    let temp_dir = fixture_with_generated_codeowners()?;
+    codeowners(
+        temp_dir.path(),
+        &["for-file", "--from-codeowners", "ruby/app/services/beta_owned.rb"],
+    )?
+    .assert()
+    .success()
+    .stdout(predicate::str::contains("Team: Beta"));
+    Ok(())
+}

tests/executable_name_config_test.rs

@@ -45,15 +45,15 @@ fn test_custom_executable_name_full_error_message() -> Result<(), Box<dyn Error>
     -# Outdated content to trigger validation error
     -/app/old.rb @FooTeam
     +
-    +# Annotations at the top of file
-    +/app/foo.rb @FooTeam
-    +
     +# Team-specific owned globs
     +/ruby/app/payments/** @PaymentTeam
     +
     +# Team YML ownership
     +/config/teams/foo.yml @FooTeam
     +/config/teams/payments.yml @PaymentTeam
+    +
+    +# Annotations at the top of file
+    +/app/foo.rb @FooTeam
 
     CODEOWNERS out of date. Run `bin/codeownership validate` to update the CODEOWNERS file
 
@@ -74,11 +74,11 @@ fn test_default_executable_name_full_error_message() -> Result<(), Box<dyn Error
     The following changes are required (- current, + expected):
     -# Outdated content to trigger validation error
     -/app/old.rb @BarTeam
-    +# Annotations at the top of file
-    +/app/bar.rb @BarTeam
-    +
     +# Team YML ownership
     +/config/teams/bar.yml @BarTeam
+    +
+    +# Annotations at the top of file
+    +/app/bar.rb @BarTeam
 
     CODEOWNERS out of date. Run `codeowners generate` to update the CODEOWNERS file
 

tests/fixtures/annotation_in_unowned_team_glob/.github/CODEOWNERS

@@ -0,0 +1,18 @@
+# STOP! - DO NOT EDIT THIS FILE MANUALLY
+# This file was automatically generated by "bin/codeownership validate".
+#
+# CODEOWNERS is used for GitHub to suggest code/file owners to various GitHub
+# teams. This is useful when developers create Pull Requests since the
+# code/file owner is notified. Reference GitHub docs for more details:
+# https://help.github.com/en/articles/about-code-owners
+
+
+# Team-specific owned globs
+/ruby/app/**/* @AlphaTeam
+
+# Team YML ownership
+/config/teams/alpha.yml @AlphaTeam
+/config/teams/beta.yml @BetaTeam
+
+# Annotations at the top of file
+/ruby/app/services/beta_owned.rb @BetaTeam

tests/fixtures/annotation_in_unowned_team_glob/config/code_ownership.yml

@@ -0,0 +1,4 @@
+owned_globs:
+  - "ruby/**/*.rb"
+team_file_glob:
+  - config/teams/**/*.yml

tests/fixtures/annotation_in_unowned_team_glob/config/teams/alpha.yml

@@ -0,0 +1,7 @@
+name: Alpha
+github:
+  team: '@AlphaTeam'
+owned_globs:
+  - ruby/app/**/*
+unowned_globs:
+  - ruby/app/services/beta_owned.rb

tests/fixtures/annotation_in_unowned_team_glob/config/teams/beta.yml

@@ -0,0 +1,3 @@
+name: Beta
+github:
+  team: '@BetaTeam'

tests/fixtures/annotation_in_unowned_team_glob/ruby/app/services/alpha_owned.rb

@@ -0,0 +1,2 @@
+class AlphaOwned
+end

tests/fixtures/annotation_in_unowned_team_glob/ruby/app/services/beta_owned.rb

@@ -0,0 +1,4 @@
+# @team Beta
+
+class BetaOwned
+end

tests/fixtures/custom_codeowners_path/docs/CODEOWNERS

@@ -7,8 +7,8 @@
 # https://help.github.com/en/articles/about-code-owners
 
 
-# Annotations at the top of file
-/ruby/app/models/test.rb @TestTeam
-
 # Team YML ownership
 /config/teams/test_team.yml @TestTeam
+
+# Annotations at the top of file
+/ruby/app/models/test.rb @TestTeam

tests/fixtures/custom_codeowners_path/expected/CODEOWNERS

@@ -7,8 +7,8 @@
 # https://help.github.com/en/articles/about-code-owners
 
 
-# Annotations at the top of file
-/ruby/app/models/test.rb @TestTeam
-
 # Team YML ownership
 /config/teams/test_team.yml @TestTeam
+
+# Annotations at the top of file
+/ruby/app/models/test.rb @TestTeam

tests/fixtures/valid_project/.github/CODEOWNERS

@@ -7,15 +7,6 @@
 # https://help.github.com/en/articles/about-code-owners
 
 
-# Annotations at the top of file
-/javascript/packages/PayrollFlow/index.tsx @PayrollTeam
-/javascript/packages/list/page-admin.tsx @PaymentsTeam
-/ruby/app/models/bank_account.rb @PaymentsTeam
-/ruby/app/models/payroll.rb @PayrollTeam
-/ruby/app/views/foos/edit.erb @PayrollTeam
-/ruby/app/views/foos/index.html.erb @UX
-/ruby/app/views/foos/new.html.erb @PayrollTeam
-
 # Team-specific owned globs
 /ruby/app/payments/**/* @PaymentsTeam
 
@@ -39,3 +30,12 @@
 # Team owned gems
 /gems/payroll_calculator/**/** @PayrollTeam
 /gems/pets/**/** @UX
+
+# Annotations at the top of file
+/javascript/packages/PayrollFlow/index.tsx @PayrollTeam
+/javascript/packages/list/page-admin.tsx @PaymentsTeam
+/ruby/app/models/bank_account.rb @PaymentsTeam
+/ruby/app/models/payroll.rb @PayrollTeam
+/ruby/app/views/foos/edit.erb @PayrollTeam
+/ruby/app/views/foos/index.html.erb @UX
+/ruby/app/views/foos/new.html.erb @PayrollTeam

tests/fixtures/valid_project_with_overrides/.github/CODEOWNERS

@@ -7,18 +7,6 @@
 # https://help.github.com/en/articles/about-code-owners
 
 
-# Annotations at the top of file
-/frontend/packages/components/datepicker/src/picks/dp.tsx @RockiesTeam
-/frontend/packages/components/list/src/item.tsx @BrewersTeam
-/frontend/packages/components/textfield/src/field.tsx @GiantsTeam
-/frontend/packages/components/textfield/src/fields/small.tsx @GiantsTeam
-/gems/apollo/lib/apollo.rb @GiantsTeam
-/gems/ivy/lib/ivy.rb @CubsTeam
-/gems/lager/lib/lager.rb @BrewersTeam
-/gems/summit/lib/summit.rb @RockiesTeam
-/ruby/app/cubs/services/models/db/price.rb @BrewersTeam
-/ruby/app/cubs/services/play.rb @CubsTeam
-
 # Team-specific owned globs
 /frontend/packages/components/** @BrewersTeam
 /ruby/app/brewers/**/* @BrewersTeam
@@ -51,3 +39,15 @@
 /gems/ivy/**/** @CubsTeam
 /gems/lager/**/** @BrewersTeam
 /gems/summit/**/** @RockiesTeam
+
+# Annotations at the top of file
+/frontend/packages/components/datepicker/src/picks/dp.tsx @RockiesTeam
+/frontend/packages/components/list/src/item.tsx @BrewersTeam
+/frontend/packages/components/textfield/src/field.tsx @GiantsTeam
+/frontend/packages/components/textfield/src/fields/small.tsx @GiantsTeam
+/gems/apollo/lib/apollo.rb @GiantsTeam
+/gems/ivy/lib/ivy.rb @CubsTeam
+/gems/lager/lib/lager.rb @BrewersTeam
+/gems/summit/lib/summit.rb @RockiesTeam
+/ruby/app/cubs/services/models/db/price.rb @BrewersTeam
+/ruby/app/cubs/services/play.rb @CubsTeam

tests/invalid_project_test.rs

@@ -23,12 +23,6 @@ fn test_validate() -> Result<(), Box<dyn Error>> {
     +# code/file owner is notified. Reference GitHub docs for more details:
     +# https://help.github.com/en/articles/about-code-owners
     +
-    +# Annotations at the top of file
-    +/gems/payroll_calculator/calculator.rb @PaymentTeam
-    +/ruby/app/models/bank_account.rb @PaymentTeam
-    +/ruby/app/models/payroll.rb @PayrollTeam
-    +/ruby/app/services/multi_owned.rb @PaymentTeam
-    +
     +# Team-specific owned globs
     +/ruby/app/payments/**/* @PaymentTeam
     +
@@ -44,6 +38,12 @@ fn test_validate() -> Result<(), Box<dyn Error>> {
     +
     +# Team owned gems
     +/gems/payroll_calculator/**/** @PayrollTeam
+    +
+    +# Annotations at the top of file
+    +/gems/payroll_calculator/calculator.rb @PaymentTeam
+    +/ruby/app/models/bank_account.rb @PaymentTeam
+    +/ruby/app/models/payroll.rb @PayrollTeam
+    +/ruby/app/services/multi_owned.rb @PaymentTeam
 
     CODEOWNERS out of date. Run `codeowners generate` to update the CODEOWNERS file
 

tests/valid_project_test.rs

@@ -299,12 +299,6 @@ fn test_for_team() -> Result<(), Box<dyn Error>> {
         predicate::eq(indoc! {"
             # Code Ownership Report for `Payroll` Team
 
-            ## Annotations at the top of file
-            /javascript/packages/PayrollFlow/index.tsx
-            /ruby/app/models/payroll.rb
-            /ruby/app/views/foos/edit.erb
-            /ruby/app/views/foos/new.html.erb
-
             ## Team-specific owned globs
             This team owns nothing in this category.
 
@@ -324,6 +318,12 @@ fn test_for_team() -> Result<(), Box<dyn Error>> {
 
             ## Team owned gems
             /gems/payroll_calculator/**/**
+
+            ## Annotations at the top of file
+            /javascript/packages/PayrollFlow/index.tsx
+            /ruby/app/models/payroll.rb
+            /ruby/app/views/foos/edit.erb
+            /ruby/app/views/foos/new.html.erb
         "}),
     )?;