fix: bound, speed, and widen the probe's recursive file walk#145
Merged
Conversation
WalkFiles walked a foreign source tree with fs.WalkDir over os.Root.FS(), which had three defects the itd-95/itd-96 reviews raised: - iss-112: every directory was read with ReadDir(-1), so one directory of millions of entries ballooned memory before the file cap could apply. Each directory is now read with a bounded ReadDir, sharing maxDirEntries as the one canonical per-directory guard ListDir already uses. - iss-114: os.Root.FS() re-resolved every path from the containment root one component at a time, so a walk cost O(entries x depth) opens (measured 10.5s for a 60000-dir depth-30 tree). The walk now holds a sub-root per directory (os.Root.OpenRoot) and opens each child in O(1), so cost is O(entries) and independent of depth: a 48000-dir depth-30 tree walks in ~1.4s versus ~7s. The os.Root containment guarantee is unchanged - a symlink is still refused, never followed out of the tree, verified from a sub-root at depth. - iss-116: the skip set covered only Node and Go. It now also skips the common Python (.venv, venv, .tox, __pycache__), build/distribution (target, build, dist), and CocoaPods (Pods) trees, so a vendored TODO is not cited as the project's own open question and a large dot-prefixed dependency tree cannot exhaust the walk cap before the project's own src/ is reached. The depth-cap, directory-cap, file-cap, skip, and containment semantics are preserved; the pre-existing walk tests pin them unchanged. spc-12's skip-set and walk-cost language is amended in the same change. Assisted-by: Claude:claude-opus-4-8
The WalkFiles class fix lands the bounded per-directory read (iss-112), the O(1)-per-directory sub-root walk (iss-114), and the widened dependency-tree skip set (iss-116). Move all three from open/ to resolved/ with --impact fix. Assisted-by: Claude:claude-opus-4-8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the WalkFiles class — ledger issues iss-112 + iss-114 + iss-116 (all minor, all from the itd-95/96 reviews), one probe-walk overhaul in
internal/core/lifeboat/probe.goso the itd-88 coverage experiment's verdicts are trustworthy.Chained PR: base is
fix/iss-117-impact-write-paths(#136) — the ledger resolves use that branch's--impactverb. Sibling PR #144 (the marker-pattern fix) shares the same base; the two are disjoint files and were built by parallel worktree-isolated implementers.What changed
maxDirEntriesguard (theListDirbound — one canonical constant, not a second copy), so a single directory with millions of entries can no longer balloon memory before the file cap applies. Truncation stays loud through both consuming adapters.os.Root.OpenRoot), opening each child O(1) relative to its parent's already-open handle instead of re-resolving the whole path per component. Measured on a 48,000-directory depth-30 tree: 7.07s → 1.43s, and depth-independence proven (0.96s → 1.01s for depth 10 → 30, versus 1.57s → 4.34s before). The reviewer independently re-ran the benchmark (1.31s vs 6.50s baseline). The os.Root containment guarantee survives unchanged — verified empirically: every final-component symlink (escaping or in-root) is refused, no string re-resolution anywhere, fd peak O(depth)..git .tox .venv Pods __pycache__ build dist generated node_modules target vendor venv), so a vendored dependency's TODO is no longer cited as the project's own open question and a large dot-prefixed tree can no longer eat the file cap beforesrc/is reached — both consequences reproduced pre-fix and killed post-fix.Verification
fs.WalkDirsemantics — identical ordering, cap points, and truncation reporting. Two prose-precision notes are queued for capture once the id-allocator fix (fix: mint record ids across git refs and add a spec-id uniqueness lint #137) merges.make preflightexit 0,go test -race ./internal/core/lifeboat/exit 0,record-lintexit 0.impact: fix.