Skip to content

fix(tree): stop dropping entries whose names contain 'director'+'file' (output-mangling sweep) - #9

Merged
ryanmurf merged 2 commits into
masterfrom
fix/tree-summary-overmatch
May 30, 2026
Merged

ryanmurf merged 2 commits into
masterfrom
fix/tree-summary-overmatch

Conversation

@ryanmurf

Copy link
Copy Markdown

Output-mangling sweep — tree summary over-match (same class as #6, #8)

Systematic audit of the src/cmds/ output filters for the highest-severity bug class in rtk: a filter that parses/transforms command output and silently drops / reorders / truncates / corrupts content the model then reasons over. (Excludes grep #6 and wc #8, already fixed.)

Fixed: src/cmds/system/tree.rs — silent entry drop

The tree filter stripped tree's trailing summary line ("5 directories, 23 files") with a loose substring heuristic:

if line.contains("director") && line.contains("file") { continue; }

This silently dropped any legitimate tree entry whose name contains both substrings — e.g. director-files.txt, file_directory.json, files-and-directories.md, or a path segment like my directory with files. The model never saw those files existed. Exact same family as the grep BRE-pipe over-match (#6) and the wc space-filename drop (#8): an output transform that over-matches and strips real content.

Fix (mirrors #8's "anchor to the known shape, treat the rest verbatim"):

  • Replaced the substring check with a whole-line anchored regex matching only tree's real summary shape: ^\d+ director(y|ies)(?:, \d+ files?)?(?:, \d+ links?)?\.?$ (covers 0 directories, 1 directory, 0 files, 5 directories, 23 files, and the BSD/--du , N links variant).
  • Applied tail-only (tree's summary is always the last content line) so even a contrived mid-tree entry literally named like a summary survives.
  • 4 regression tests, all proven to fail against the old code (the three filename cases were dropped; the mid-tree case is the tail-only guard). Existing summary-stripping tests still pass.

Sweep verdicts on the other output-transforming filters

Filter Verdict Notes
system/ls.rs clean Already hardened (rtk-ai#948): date-regex anchor handles owner/group-with-spaces and space-containing filenames; year + symlink forms covered.
system/find_cmd.rs suspect (follow-up) Walks the FS itself (doesn't parse find output), so no token-split mangling. But dir-display truncation &dir[dir.len()-47..] is a raw byte-slice that will panic on a multibyte UTF-8 directory path when the boundary lands mid-codepoint. Real but separate (panic, not silent corruption) — left for a focused follow-up to keep this PR reviewable.
system/env_cmd.rs clean (by-design lossy) Uncategorized non-"interesting" vars are dropped when no filter is given, but the count is signaled (showing N relevant) and PATH-split/value-truncation are explicit. Intentional noise reduction, not silent corruption.
system/log_cmd.rs clean Dedup + normalize (timestamp/UUID/hex/num/path). take(10)/take(5) caps are signaled with +N more; char-based truncation is multibyte-safe (.chars().take(97), test present).
system/tree.rs FIXED see above.
git/diff_cmd.rs (condense_unified_diff) minor (not mangling) Shows all +/- lines in full per file (deliberately never truncates), then prints +N more when total>10 even though all are shown — confusingly worded but does not drop data; the existing tests bake this in. Not in the drop/corrupt class.

No other filter in the sweep silently drops/reorders/corrupts content. The one additional real defect (find_cmd byte-slice panic) is noted above for follow-up.

Gate (run locally, all green)

  • cargo fmt --all -- --check ✅
  • cargo clippy --all-targets -- -D unsafe_code ✅ (matches CI gate)
  • cargo test --all ✅ (1732 + 12 passed, 0 failed)
  • cargo build --release ✅

DeepSeek review: APPROVE — regex correct/comprehensive, regression tests genuine (fail against old code), no panic risk. Its one non-blocking suggestion (tail-only matching to kill the residual "file named like a summary" edge case) was adopted in this PR.

The tree filter stripped any line matching the loose substring heuristic
`line.contains("director") && line.contains("file")`, intended to drop
tree's trailing summary line (e.g. "5 directories, 23 files"). But this
silently dropped legitimate entries whose filenames contain both words,
e.g. `director-files.txt`, `file_directory.json`, or a path segment like
`my directory with files` — the model never saw those files existed.

Same bug class as the grep BRE-pipe over-match (#6) and the wc
space-filename drop (#8): an output transform that over-matches and
silently strips real content the model reasons over.

Fix: anchor the summary detector to tree's exact summary shape with a
whole-line regex (`^\d+ director(y|ies)(, N files)?(, N links)?$`)
instead of a substring contains-check. Regression tests prove the dropped
filenames now survive and the real summary line is still stripped,
including the `, N links` platform variation.
@github-actions

Copy link
Copy Markdown

👋 Thanks for the PR! It looks like this targets master, but all PRs should target the develop branch.

Please update the base branch:

  1. Click Edit at the top right of this PR
  2. Change the base branch from master to develop

See CONTRIBUTING.md for details.

@ryanmurf
ryanmurf enabled auto-merge (squash) May 30, 2026 01:28
@ryanmurf
ryanmurf merged commit d898926 into master May 30, 2026
16 checks passed
@ryanmurf
ryanmurf deleted the fix/tree-summary-overmatch branch May 30, 2026 01:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant