Skip to content

Consider new, untracked team config files during validate/generate#113

Open
dduugg wants to merge 1 commit into
mainfrom
fix-new-team-not-considered-when-untracked
Open

Consider new, untracked team config files during validate/generate#113
dduugg wants to merge 1 commit into
mainfrom
fix-new-team-not-considered-when-untracked

Conversation

@dduugg

@dduugg dduugg commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes rubyatscale/code_ownership#149 ("New teams aren't considered in validate of CLI calls").

Adding a new team was silently ignored until the new team.yml was git add-ed. The project walk deliberately excludes untracked files (#46/#74/#76 — so a developer can keep scratch files around locally without validate forcing them to assign an owner), but that exclusion also covered team config files, which define real ownership rules the moment they exist rather than being scratch work anyone would leave uncommitted on purpose.

Root cause, reproduced against a real build before touching any code: tracked_files::find_tracked_files runs git ls-files (staged/committed files only) and ProjectBuilder's WalkBuilder::filter_entry drops any file not in that set. When a new team file is added without staging it, it's invisible to the walk — so the team never gets loaded, and any already-tracked file that should newly become owned by it (e.g. an existing README.md) is reported as unowned instead. (Making the owned file new/untracked too, rather than already-tracked, masks the bug entirely — neither file is visible, so no mismatch is ever observed. That's why an early, broader version of this fix looked right in isolation but wasn't actually testing the reported scenario.)

Fix: exempt files matching team_file_glob from the tracked-files check specifically. Ordinary source files remain subject to it, unchanged.

I initially considered a broader fix (stop excluding any untracked-but-not-gitignored file), but cargo test caught that it directly regresses test_skip_untracked_files — an existing, intentional test from #74/#76 for exactly the scratch-file use case #46 describes. That's a real, legitimate use case I don't want to break, so this PR scopes the fix to team files only, per that discussion.

Test plan

  • Reproduced the exact bug against a real build first (confirmed failure, confirmed the fix resolves it, confirmed reverting reintroduces the failure)
  • Added tests/untracked_new_file_test.rs — reproduces #149 end to end (new untracked team + existing tracked file → validate succeeds)
  • Added tests/untracked_source_file_test.rs — pins that an untracked source file (non-team) is still correctly ignored, so this fix can't silently broaden later
  • Full cargo test (all 83+ unit tests + every integration test file, including the pre-existing test_skip_untracked_files), cargo clippy --all-targets, and cargo fmt --check all clean

rubyatscale/code_ownership#149: adding a new team was silently ignored
until the new team.yml was `git add`-ed. The project walk deliberately
excludes untracked files (codeowners-rs#46/#74/#76 - so a developer can
keep scratch files around locally without validate forcing them to
assign an owner), but that exclusion also covered team config files,
which define real ownership rules the moment they exist rather than
being scratch work anyone would leave uncommitted on purpose.

Exempt files matching `team_file_glob` from the tracked-files check
specifically, leaving ordinary source files subject to it as before.
Reproduced the exact bug against a real build first (a new team owning
an already-tracked file, e.g. README.md, was reported unowned because
the team file disappeared from the walk while README.md - already
committed - did not), confirmed a fix, then found and preserved a
directly conflicting existing test (test_skip_untracked_files) that
relies on the untracked-scratch-file behavior on purpose, which a
first, broader attempt at this fix would have silently regressed.

Added two tests: one reproducing #149 end to end (new untracked team
+ existing tracked file -> validate succeeds), and one pinning that an
untracked *source* file (non-team) is still correctly ignored, so the
scoped nature of this fix doesn't regress by accident later.

Verified: full `cargo test` suite (all 83+ tests, including the
pre-existing test_skip_untracked_files), cargo clippy --all-targets,
and cargo fmt --check all clean.
@dduugg
dduugg requested a review from a team as a code owner July 25, 2026 06:43
@github-project-automation github-project-automation Bot moved this to Triage in Modularity Jul 25, 2026
@dduugg
dduugg enabled auto-merge (squash) July 25, 2026 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

New teams aren't considered in validate of CLI calls

1 participant