test(guards): fail when a source scan guards nothing

[worktrunk-bot]

Nightly sweep finding. The guard tests that scan src/ have two ways to pass while guarding nothing: their directory walks discard the read_dir error, and output_system_guard's two path allowlists are matched by string, so a rename leaves the exemption armed at the abandoned path. This lets the read error surface at every source-scanning walk, and adds a test that every allowlist entry still names a real file.

The stale-allowlist half

STDOUT_ALLOWED_PATHS and STD_STDERR_ALLOWED_PATHS exempt a path, and an entry exempts the whole file. When a file is renamed, nothing fails — the entry just stops matching anything. It stays armed, so a new file later occupying that path inherits a stdout permission nobody granted it, and the guard stays green while it writes. src/help.rs is the shape of the risk: a plausible name for a future refactor to reuse, currently exempt because --help-page and --version are genuinely the command's answer. git log --diff-filter=R -- src/ shows renames are routine here, so this is a rot-prevention pin rather than a fix for present drift — all 20 entries resolve today. It converts the rename into a failing test at the moment it happens, while the reviewer still knows whether the exemption should move with the file or go.

The vacuous-pass half

Every source-scanning guard walks with Err(_) => return, so a missing or unreadable scan root produces zero violations and the test reports success having read nothing. Rather than measure how much of the tree the walk reached, the walk now lets the error out:

let entries = fs::read_dir(dir)
    .unwrap_or_else(|e| panic!("{} unreadable during the src/ scan: {e}", dir.display()));

That is one line per walk, and the failure names the actual io error instead of a file count. It covers all five walks, including the two that carried a count-based floor of their own and would otherwise have set the convention: snapshot_formatting_guard.rs's seen > 500 and packaged_assets.rs's !assets.is_empty() are both removed in favour of the same treatment.

version_build.rs matters most of the five: that guard is what stands between a bare env!("VERGEN_…") and another #3123, where cargo install worktrunk failed to compile off the crates.io archive.

Verification

Each of the five walks was confirmed to panic on an injected fault (its scan root repointed at a nonexistent directory) and to pass on the real tree.

Injected-fault runs

test integration_tests::output_system_guard::check_no_unexpected_stdout_writes ... FAILED
test integration_tests::output_system_guard::check_stderr_macros_come_from_styling ... FAILED
test integration_tests::packaged_assets::embedded_assets_ship_in_package ... FAILED
test integration_tests::snapshot_formatting_guard::test_no_host_specific_paths_in_snapshots ... FAILED
test integration_tests::version_build::vergen_env_vars_are_read_optionally ... FAILED

Each panics naming the directory and the error:

thread '...::version_build::vergen_env_vars_are_read_optionally' panicked at tests/integration_tests/version_build.rs:50:29:
/home/runner/work/worktrunk/worktrunk/src-does-not-exist unreadable during the src/ scan: No such file or directory (os error 2)

All five passed silently before this change. Unmodified tree:

test result: ok. 14 passed; 0 failed; 1 ignored; 0 measured; 1988 filtered out

Also clean: cargo fmt --check, cargo clippy --test integration --all-features.

[worktrunk-bot]

Self-review. The mechanism is right, and allowlisted_paths_still_exist is the half with teeth — a path allowlist that stops matching anything is an armed exemption, not dead clutter. Two gaps, both of which I'm applying in a follow-up commit rather than leaving for a maintainer, since this PR is bot-authored.

version_build.rs has the identical hole and this PR doesn't cover it. vergen_env_vars_are_read_optionally walks src/ with the same let Ok(entries) = fs::read_dir(dir) else { return }; and then asserts violations.is_empty(), so a missing or relocated scan root passes it having read nothing — exactly the shape this PR pins. It matters more there than here: that guard is what stands between a bare env!("VERGEN_…") and another #3123, where cargo install worktrunk failed to compile off the crates.io archive. The other two source-scanning guards already carry floors — packaged_assets.rs's !assets.is_empty() and snapshot_formatting_guard.rs's seen > 500 — so as written this PR leaves version_build.rs as the only one without.

scanned > 0 is weaker than the precedent it follows. for_each_snapshot in snapshot_formatting_guard.rs asserts seen > 500 over its ~1200-file corpus for this same reason, under the same "would pass vacuously" comment. src/ holds 205 .rs files and the new docstring already says a real run "reaches hundreds of files", so a substantial floor is there for the taking — and it catches the partial walk the docstring itself names ("relocated src/"), which > 0 cannot: a workspace split that leaves a stub src/ behind clears zero while the guard inspects almost nothing.

Left alone deliberately, for a maintainer's call: four guard tests now hand-roll a near-identical recursive walk (output_system_guard, version_build, packaged_assets, and snapshot_formatting_guard's .snap variant), and this PR adds counting to two of them. A single counted walker in tests/common/ would make the floor structural instead of something each new guard has to remember — but it touches four files and is a separate concern from this one.

Verification of the follow-up commit

MIN_SCANNED_FILES = 100 in both files; src/ currently walks to 205, so the headroom is ~2x.

Injected fault — pointing the vergen scan at src-does-not-exist, which passed silently before:

thread '...::version_build::vergen_env_vars_are_read_optionally' panicked at tests/integration_tests/version_build.rs:46:5:
scanned only 0 .rs files under /home/runner/work/worktrunk/worktrunk/src-does-not-exist — the guard passed without inspecting the crate.
The walk swallows read errors, so this is a missing or unreadable scan root, not a clean result.

Unmodified tree:

test integration_tests::output_system_guard::allowlisted_paths_still_exist ... ok
test integration_tests::output_system_guard::check_no_unexpected_stdout_writes ... ok
test integration_tests::output_system_guard::check_stderr_macros_come_from_styling ... ok
test integration_tests::version_build::vergen_env_vars_are_read_optionally ... ok

Also clean: cargo fmt --check, cargo clippy --test integration --all-features.

[max-sixty]

Two changes here, and they're worth separating: allowlisted_paths_still_exist is the one with teeth, and the scanned-file floor is doing more work than the risk needs.

Keep the allowlist test. Both lists are matched by path string, so a rename leaves a dead entry armed at the old path — and git log --diff-filter=R -- src/ shows renames are routine here (commands/select/ → commands/picker/, copy_progress.rs → progress.rs, commands/external.rs → commands/custom.rs). Turning that into a red test at the moment it happens is worth 25 lines.

The floor is a different matter. The root cause it works around is that the walks discard the error:

let entries = match fs::read_dir(dir) {
    Ok(e) => e,
    Err(_) => return,
};

Rather than measuring how much the walk reached, let that error surface:

let entries = fs::read_dir(dir)
    .unwrap_or_else(|e| panic!("scan root {} unreadable: {e}", dir.display()));

One line per walk instead of a returned count threaded through three recursive functions, no MIN_SCANNED_FILES constant duplicated into version_build.rs, and the failure names the actual io error instead of a file count. The remaining case a floor would catch — a workspace split leaving a stub src/ behind — is speculative, and 100 is a number that needs revisiting as src/ grows.

snapshot_formatting_guard.rs (seen > 500) and packaged_assets.rs (!assets.is_empty()) have the same shape and are cited here as precedent. They're the reason this looked like the house pattern; they should move to the same treatment rather than set the convention.

This was written by Claude Code on behalf of max-sixty

[worktrunk-bot]

Self-review of the two commits since the last round. The requested direction is applied: the floors are gone from all four guards, MIN_SCANNED_FILES with them, and each of the five walks surfaces its read_dir error instead. All 20 allowlist entries resolve today, src/remove_dir.rs included, so allowlisted_paths_still_exist is green on the current tree.

One residual, raised as a decision rather than pushed as a commit. The walks now surface the error at the directory level, but the same swallow sits one level down, inside the very functions this diff touches:

• check_file in output_system_guard.rs — match fs::read_to_string(path) { … Err(_) => return }, in both the stdout scan and the stderr scan
• check_file in version_build.rs — let Ok(contents) = fs::read_to_string(path) else { return; };
• scan_file in packaged_assets.rs — the same let Ok(…) else form

A .rs file the walk reaches but can't read is skipped in silence, so the guard reports clean for content it never inspected. That is the shape the PR names, scoped to one file rather than the whole tree. visit_snap_files in snapshot_formatting_guard.rs already treats it as fatal (fs::read_to_string(&path).unwrap()), so as it stands the four guards now disagree about the file-level case while agreeing about the directory-level one. entries.flatten() is the same class again — it drops a mid-iteration DirEntry error in all five walks.

I stopped short of applying it because the last round's point was that this change should stay one line per walk, and pushing the treatment downward is a scope call worth making explicitly rather than assuming. If you want it, it's four one-line edits and I'll push them in a follow-up commit; if the file-level case is too remote to be worth the lines, the inconsistency with snapshot_formatting_guard.rs is the thing to settle, in whichever direction.

[worktrunk-bot]

Applied in 6091b13, and the PR description is rewritten to match. allowlisted_paths_still_exist is untouched; the floor is gone, MIN_SCANNED_FILES with it, and the walk lets the error out instead — including in snapshot_formatting_guard.rs (seen > 500) and packaged_assets.rs (!assets.is_empty()), so the count-based version doesn't survive anywhere as the pattern to copy.

Net effect on the two commits you were reading: output_system_guard.rs loses 25 lines rather than gaining them, and the other three each trade a swallowed error for a panic that names the directory and the io error.

Verified by repointing each of the five scan roots at a nonexistent directory — all five now fail, all five passed silently before:

/home/runner/work/worktrunk/worktrunk/src-does-not-exist unreadable during the src/ scan: No such file or directory (os error 2)

Checks are green on that head, codecov/patch included. The one thing I left as a question rather than a commit is in the review above — the same swallow one level down, in read_to_string — since it cuts against the one-line-per-walk shape you asked for.