Skip to content

Commit a04ef70

Browse files
artfuldodgerclaude
andcommitted
fix: write CODEOWNERS annotations after glob-based sections
GitHub applies the last matching CODEOWNERS line, but the generator wrote the file-annotation section first. A file that a team glob excludes via unowned_globs and an annotation assigns to another team is valid, and for-file resolves it to the annotated team, yet the broader team glob came later in CODEOWNERS and won on GitHub (and in --from-codeowners lookups). Write the annotations section last. Resolution and validation keep their existing mapper order; only the generated file changes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent d22f8a8 commit a04ef70

15 files changed

Lines changed: 147 additions & 47 deletions

File tree

‎src/ownership.rs‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,9 @@ impl Ownership {
122122
let validator = Validator {
123123
project: self.project.clone(),
124124
mappers: self.mappers(),
125-
file_generator: FileGenerator { mappers: self.mappers() },
125+
file_generator: FileGenerator {
126+
mappers: self.codeowners_file_mappers(),
127+
},
126128
executable_name: self.project.executable_name.clone(),
127129
};
128130

@@ -166,7 +168,9 @@ impl Ownership {
166168
#[instrument(level = "debug", skip_all)]
167169
pub fn generate_file(&self) -> String {
168170
info!("generating codeowners file");
169-
let file_generator = FileGenerator { mappers: self.mappers() };
171+
let file_generator = FileGenerator {
172+
mappers: self.codeowners_file_mappers(),
173+
};
170174
file_generator.generate_file()
171175
}
172176

@@ -182,6 +186,20 @@ impl Ownership {
182186
Box::new(TeamGemMapper::build(self.project.clone())),
183187
]
184188
}
189+
190+
// GitHub applies the last matching CODEOWNERS line, so annotations are written last: an annotated
191+
// file can still match a broader team glob that excludes it through unowned_globs.
192+
fn codeowners_file_mappers(&self) -> Vec<Box<dyn Mapper>> {
193+
vec![
194+
Box::new(TeamGlobMapper::build(self.project.clone())),
195+
Box::new(DirectoryMapper::build(self.project.clone())),
196+
Box::new(RubyPackageMapper::build(self.project.clone())),
197+
Box::new(JavascriptPackageMapper::build(self.project.clone())),
198+
Box::new(TeamYmlMapper::build(self.project.clone())),
199+
Box::new(TeamGemMapper::build(self.project.clone())),
200+
Box::new(TeamFileMapper::build(self.project.clone())),
201+
]
202+
}
185203
}
186204

187205
#[cfg(test)]
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
use indoc::indoc;
2+
use predicates::prelude::*;
3+
use std::error::Error;
4+
5+
mod common;
6+
use common::OutputStream;
7+
use common::run_codeowners;
8+
9+
// Alpha owns ruby/app/**/* but excludes beta_owned.rb through unowned_globs, and Beta owns that file
10+
// through an annotation. GitHub applies the last matching CODEOWNERS line, so the annotation line must
11+
// come after Alpha's broader glob for GitHub to agree with for-file.
12+
const FIXTURE: &str = "annotation_in_unowned_team_glob";
13+
14+
#[test]
15+
fn test_validate_accepts_generated_order() -> Result<(), Box<dyn Error>> {
16+
run_codeowners(FIXTURE, &["validate"], true, OutputStream::Stdout, predicate::eq(""))?;
17+
Ok(())
18+
}
19+
20+
#[test]
21+
fn test_codeowners_file_agrees_with_for_file() -> Result<(), Box<dyn Error>> {
22+
run_codeowners(
23+
FIXTURE,
24+
&["crosscheck-owners"],
25+
true,
26+
OutputStream::Stdout,
27+
predicate::eq(indoc! {"
28+
Success! All files match between CODEOWNERS and for-file command.
29+
"}),
30+
)?;
31+
Ok(())
32+
}
33+
34+
#[test]
35+
fn test_for_file_from_codeowners_returns_annotated_owner() -> Result<(), Box<dyn Error>> {
36+
run_codeowners(
37+
FIXTURE,
38+
&["for-file", "--from-codeowners", "ruby/app/services/beta_owned.rb"],
39+
true,
40+
OutputStream::Stdout,
41+
predicate::str::contains("Team: Beta"),
42+
)?;
43+
Ok(())
44+
}

‎tests/executable_name_config_test.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,15 @@ fn test_custom_executable_name_full_error_message() -> Result<(), Box<dyn Error>
4545
-# Outdated content to trigger validation error
4646
-/app/old.rb @FooTeam
4747
+
48-
+# Annotations at the top of file
49-
+/app/foo.rb @FooTeam
50-
+
5148
+# Team-specific owned globs
5249
+/ruby/app/payments/** @PaymentTeam
5350
+
5451
+# Team YML ownership
5552
+/config/teams/foo.yml @FooTeam
5653
+/config/teams/payments.yml @PaymentTeam
54+
+
55+
+# Annotations at the top of file
56+
+/app/foo.rb @FooTeam
5757
5858
CODEOWNERS out of date. Run `bin/codeownership validate` to update the CODEOWNERS file
5959
@@ -74,11 +74,11 @@ fn test_default_executable_name_full_error_message() -> Result<(), Box<dyn Error
7474
The following changes are required (- current, + expected):
7575
-# Outdated content to trigger validation error
7676
-/app/old.rb @BarTeam
77-
+# Annotations at the top of file
78-
+/app/bar.rb @BarTeam
79-
+
8077
+# Team YML ownership
8178
+/config/teams/bar.yml @BarTeam
79+
+
80+
+# Annotations at the top of file
81+
+/app/bar.rb @BarTeam
8282
8383
CODEOWNERS out of date. Run `codeowners generate` to update the CODEOWNERS file
8484
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# STOP! - DO NOT EDIT THIS FILE MANUALLY
2+
# This file was automatically generated by "bin/codeownership validate".
3+
#
4+
# CODEOWNERS is used for GitHub to suggest code/file owners to various GitHub
5+
# teams. This is useful when developers create Pull Requests since the
6+
# code/file owner is notified. Reference GitHub docs for more details:
7+
# https://help.github.com/en/articles/about-code-owners
8+
9+
10+
# Team-specific owned globs
11+
/ruby/app/**/* @AlphaTeam
12+
13+
# Team YML ownership
14+
/config/teams/alpha.yml @AlphaTeam
15+
/config/teams/beta.yml @BetaTeam
16+
17+
# Annotations at the top of file
18+
/ruby/app/services/beta_owned.rb @BetaTeam
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
owned_globs:
2+
- "ruby/**/*.rb"
3+
team_file_glob:
4+
- config/teams/**/*.yml
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
name: Alpha
2+
github:
3+
team: '@AlphaTeam'
4+
owned_globs:
5+
- ruby/app/**/*
6+
unowned_globs:
7+
- ruby/app/services/beta_owned.rb
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
name: Beta
2+
github:
3+
team: '@BetaTeam'
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
class AlphaOwned
2+
end
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
# @team Beta
2+
3+
class BetaOwned
4+
end

‎tests/fixtures/custom_codeowners_path/docs/CODEOWNERS‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
# https://help.github.com/en/articles/about-code-owners
88

99

10-
# Annotations at the top of file
11-
/ruby/app/models/test.rb @TestTeam
12-
1310
# Team YML ownership
1411
/config/teams/test_team.yml @TestTeam
12+
13+
# Annotations at the top of file
14+
/ruby/app/models/test.rb @TestTeam

0 commit comments

Comments
 (0)