fix(bundle): collect each junit report once, not once per path to it

[dfrankland]

Problem

scan_from_glob relies on the glob crate, whose ** deliberately resolves symlinked directories:

// glob 0.3.3, PathWrapper::from_dir_entry
if file_type.is_symlink() { None } else { Some(file_type.is_dir()) }
// "We need to use fs::metadata to resolve the actual path if it's a symlink."
.or_else(|| fs::metadata(&path).map(|m| m.is_dir()).ok())

A workspace that links its packages into each other's dependency directories makes every package reachable through every dependency edge. A glob such as packages/**/test-results/*.xml then yields one match per route through the dependency graph rather than one per file on disk, and the same report is bundled once per route.

The reported bundle was 3.9MB compressed and expanded to 880MB, of which internal.bin was 410MB:

report paths collected	1,658
distinct files on disk	19
paths reached via a symlinked directory	1,639 (98.9%)
test_case_runs	1,717,605
distinct tests	9,471
redundant runs	99.4%

84% of that 410MB is repeated name/classname/file strings.

The duplicates are one file reached many ways, not many files that happen to match. last_modified_epoch_ns is a perfect function of the terminal package — 878 paths sharing one nanosecond-identical mtime and one content hash, with zero violations — and every package directory is byte-and-nanosecond identical to its alias under another package's dependency directory.

Contents are physically duplicated, not linked: bundler.rs does File::open(path) + tar.append_file(...), so every duplicate route ships its own full copy of the bytes.

Downstream consequence. The CLI leaves attempt_number unset, so the services side derives it from a per-test positional counter. Duplicate copies of one test are therefore recorded as retries of it — in this bundle, up to 35,616 attempts of a test that ran once.

This is not a recent regression: the glob code is unchanged since #276 (2025-01-10), and the reporting org's maximum bundle size has been flat for three months. A dependency graph grew into a latent bug.

Fix

Split discovery from grouping.

collect_files_per_glob expands every glob and returns the files each one owns, keyed by canonical path, so a file is bundled once no matter how many globs reach it or how many routes lead to it. That also fixes double-counting when two globs match the same file, which the previous per-glob structure could not express.

Canonicalizing is what makes this simple: the canonical path of a symlinked alias is the real file's path, so there is no ranking of candidate routes to do — the filesystem answers it directly.

RepoRoot holds the invariant that made this subtle. Reported paths are canonical, so the root they are made relative to must be canonical too; otherwise strip_prefix misses (/var vs /private/var) and every path silently falls back to an absolute one that codeowners cannot match.

Ownership is first glob to match, which respects the order the caller listed and matches how the services side already resolves a file's test runner report (first match wins).

Symlink following is unchanged

Deduplicating is orthogonal to following. A report reachable only through a symlink — a single hop that is a literal component of the glob rather than part of a ** — is still collected. Two cases are covered explicitly: a symlinked artifacts directory inside the repo is reported where the reports actually live, and one that leaves the repo keeps its repo-relative route, since resolving it would produce an absolute path outside the tree that codeowners could not match.

Note on same_file::Handle

It is the obvious primitive for file identity and is already in the lock file, but it retains an open descriptor for the handle's lifetime (file: Option<File>). A set over a large glob exhausts RLIMIT_NOFILE — 1,658 handles is past the 1024 Linux default and far past macOS's 256. Canonical paths avoid the problem entirely and need no platform split, unlike (dev, ino), whose Windows equivalent (file_index()) is still unstable in std.

Verification

Four e2e tests in the upload suite, driving a real trunk upload --dry-run and asserting on the produced meta.json and internal.bin together:

• reports are collected once per package, with internal.bin holding one run per test rather than one per route, and no synthesized attempt numbers
• two overlapping globs do not each contribute a copy, and the glob that owns nothing keeps its empty file set
• a symlinked artifacts directory inside the repo is reported at the real location
• a symlink that leaves the repo keeps its repo-relative route

The fixture builds a five-package workspace whose packages are symlinked into each other's dependency directories, which a ** glob reaches by 31 routes. Reverting bundle/src/files.rs fails the first test 31 vs 5, and the failing assertion is the bundled file count — the user-visible outcome, not an internal detail.

All 58 upload tests pass. cargo check --workspace --all-targets is clean, with no new clippy warnings. All fixtures are unix-gated, since creating symlinks on Windows needs elevation; the canonical-path logic itself is cross-platform.

Behavior changes to be aware of

• num_files / num_tests now count distinct files and runs. For the reported bundle that is 1,658 → 19 and 1.7M → ~9.5k. Correct, but a visible discontinuity in anything trending them.
• A FileSet can now be empty if an earlier glob claimed everything it matched. Kept rather than dropped, so the meta still records that the glob was specified.
• A symlinked directory inside the repo now reports the real location of its files rather than the route used to reach them.

The meta.json wire format is unchanged: Vec<FileSet> still expresses this, just as a partition rather than overlapping bags. No new BundleMetaV0_* variant.

Follow-up, not in this PR

defect.rs reconstructs a test_runner_report from file_sets by matching test_case_runs[].file (a repo-relative source file, empty when the runner omits it) against files[].original_path (an absolute path to a report file). Those are different categories of value, so the lookup always yields None.

Its only consumer is junit_validate, and none of the file-set runner report's fields (resolved_status, resolved_start_time_epoch_ms, resolved_end_time_epoch_ms) are read anywhere on the services side — label, timing and status all come from internal.bin now. So no test data is affected; the impact is confined to validation warnings that never fire. It looks like a straggler from the migration that moved label parsing off file_sets, and the likely fix is to delete it. Worth confirming against a BEP bundle first.

🤖 Generated with Claude Code

[trunk-io]

😎 Merged successfully - details.