Repository navigation
fix(diff): bound the snapshot walk and slow the new-file discovery cadence - #138
Conversation
…dence Follow-ups (1) and (2) from amirlehmam#133, left out of amirlehmam#135 as design calls. The 2000-file cap bounds how much the snapshot fallback READS but not how much it WALKS: on a home directory ~3,500 directories are enumerated before the cap bites, so which files you end up watching is an accident of traversal order. - SNAPSHOT_IGNORE gains platform directories (AppData, Library, Application Data, $RECYCLE.BIN, System Volume Information, OneDrive, Recovery) - MAX_SNAPSHOT_DIRS budget threaded through the walk, so traversal is bounded independently of the file cap - re-walk for new files now runs every 5th poll instead of every poll; the stat-based content check still runs every poll. Added files therefore surface within ~10s rather than ~2s; modifications and deletions are unaffected - discovered additions are remembered per-cwd so they do not flicker out of the list on the polls between re-walks - resolveWithin and the untracked-file branch of getFileDiff both used a startsWith prefix test that would accept C:\foobar as inside C:\foo; both now share an isWithin helper built on path.relative
|
Merged in 970f7b8. Verified locally first: 61 files / 667 tests, typecheck clean — exactly what you reported. Both halves are right, and (1) is the one I'd have got wrong on my own. #135 bounded how much we read; it left the walk itself unbounded, so the 2,000 files we ended up with were an accident of traversal order — a On (2): the flicker trap is the part of this PR I'm most glad you wrote down. Making discovery periodic while modification detection stayed per-poll creates two classes of file with different visibility rules, and the failure — present on poll 5, gone on 6–9, back on 10 — would have looked like a rendering bug for a long time before anyone suspected the poll cadence. ~10s to notice a new file is fine. Modifications and deletions are what people actually watch a diff pane for, and those are unchanged at one poll. The On (3) — I'm not going to offer the fallback for home-like trees. A diff pane pointed at Thank you — that's #133, #135 and now #138, and the standard hasn't dropped once. |
- agent-state declared blocked/working/idle panes (amirlehmam#128), consent-gated agent integration (amirlehmam#132), i18n audit + community locales (amirlehmam#136 amirlehmam#147), key remaps (amirlehmam#145), diff pane fixes (amirlehmam#133 amirlehmam#138 amirlehmam#141), orphaned PTY reaping (amirlehmam#139), V2 CLI caller scoping (amirlehmam#143), browser state in update backup (amirlehmam#146) - OMP fork kept: WMUX_AGENT_CMD=omp workers, OMP context/extension install (still unconditional at startup; Claude/OpenCode features now consent-gated), agent-label notifications via i18n keys, portable build scripts (em-dash -> --) - opencode-context: shared injectWmuxBlock + upstream stripWmuxBlock/remove paths
Follow-ups (1) and (2) from #133, which were deliberately left out of #135 as design calls rather than bug fixes. (3) — whether to offer the fallback at all for home-like trees — is still yours; happy to implement whatever you land on.
(1) Bound the walk, not just the read
SNAPSHOT_IGNOREgains the platform directories:AppData,Library,Application Data,$RECYCLE.BIN,System Volume Information,OneDrive,Recovery.More importantly,
MAX_SNAPSHOT_DIRS(500) is now threaded throughwalkDiras a mutable budget shared across the recursion, so traversal is bounded independently of the file cap. This is the half you identified as the important one: the 2,000-file cap bounds how much we read, but ~3,578 directories were still enumerated before it bit, and the resulting 2,000 files were an accident of traversal order.(2) Slower discovery cadence
The re-walk that finds new files now runs every 5th poll (
REWALK_EVERY_N_POLLS). The stat-based check for existing files still runs every poll.Behaviour change worth calling out explicitly: a newly added file now appears within ~10s instead of ~2s. Modifications and deletions are unchanged — they come from the snapshot map and are still detected on the very next poll. If you want a different number, it's a one-line constant.
There's a trap in this change that isn't obvious: because newly discovered files are not in the snapshot map, running discovery only every Nth poll makes them flicker — present on poll 5, absent on 6–9, present on 10. So discovered additions are now remembered per-cwd in
addedPathsand re-reported on every poll (and dropped when the file goes away).resetSnapshotclears that alongside the snapshot. Two tests pin this: one that an added file keeps appearing across 12 consecutive polls, one that it disappears once deleted.Also: the prefix check you noted
resolveWithinusedresolved.startsWith(path.resolve(cwd)), which acceptsC:\foobaras insideC:\foo. The untracked-file branch ofgetFileDiffhad the identical test three lines away. Both now go through oneisWithinhelper built onpath.relative.Neither was reachable — as you said, the paths come from a walk of
cwditself — so this is hygiene, not a security fix. Flagging it as slightly outside what you asked for; it seemed odd to fix one and leave its twin adjacent to it.Tests
tests/unit/diff-provider.test.tsgrows from 5 to 9 cases:AppData/**is never reported, across several pollsThe pre-existing "modified, added, deleted" case was split in two, because added-file detection is deliberately no longer immediate and the old case asserted that it was. Modified/deleted still assert next-poll detection.
Full suite: 61 files, 667 tests passing.
npm run typecheckandnpm run lintclean.