PR #360 follow-ups: UserPathGuardTests self-check is vacuous; scan picks up nested checkouts nf-1fan ← Beads

closed priority 2 task unassigned activesession:a

Twinned from https://github.com/cfsh/ninefold/issues/372 by tools/beads/import_github.py's reconcile pass.

Suggestions from ninefold-reviewer's review of #360. Both are in the new shared/tests/UserPathGuardTests.cs (added in that PR as F2). Neither blocks anything — #360 converged.

1. TheGuardActuallyMatchesSomething is vacuous — it counts its own source

shared/tests/UserPathGuardTests.cs counts every line in the tree containing the literal UserPaths.Resolve and asserts routed >= 3. But the counted set includes the test sources themselves, and the guard file alone contains that literal on four lines (two assert-message strings, the l.Contains("UserPaths.Resolve", …) predicate itself, and the self-check's own message), plus two more in UserPathsTests.ResolveUsesTheAmbientRedirect. So the count is >= 6 before any production code is considered.

Net effect: delete all three production call sites (PersistenceDriver, TonapseOptions, AudioBuses) and this assertion still passes. The stated purpose — "a scanner that silently matches nothing can't pass forever" — isn't delivered.

Fix: count only non-test files (or exclude shared/tests/ from the enumeration) before comparing against 3. Also note the self-check omits the bin exclusion the main scan has.

2. The scan walks the whole tree, so nested checkouts and scratch files become offenders

EveryUserSchemePathGoesThroughUserPaths enumerates *.cs under the repo root, skipping only .godot, obj, and bin. Anything else that happens to sit in the working tree is treated as authoritative source — including a full second copy of the repo.

Demonstrated by running the guard's exact logic over a real ninefold checkout: 11 offenders, 7 of them spurious

``` .claude\worktrees\<name>\games\tonapse\PersistenceDriver.cs:22 .claude\worktrees\<name>\games\tonapse\TonapseOptions.cs:15 .claude\worktrees\<name>\shared\AudioBuses.cs:38 .claude\worktrees\<name>\shared\VisualProbe.cs:65 head7_shared_VisualProbe.cs:65 head9_shared_VisualProbe.cs:65 head_VisualProbe_p5.cs:65 ```

.claude/worktrees/ is where Claude Code's worktree isolation puts full repo copies, and the head*.cs files are untracked scratch copies fetched during review — neither is gitignored, and both are ordinary in these sessions. A clone with either goes red on ./run_tests.sh for reasons unrelated to the committed tree, with a failure message that points at a pre-#358 copy of VisualProbe.cs and accuses it of cross-clone corruption.

Fix: drive the scan from git ls-files "*.cs", or skip .claude/ and any directory containing its own .git.

Dropped as sub-floor (recorded, not asked for)

Dependencies

None.

Comments

No comments.

Add a comment