[codex] Bound file explorer watcher#104
Conversation
tavateva
left a comment
There was a problem hiding this comment.
Draft Promotion Criteria PASS ✅
Fixes #101 (part of #51 stack). All 7 criteria hold.
-
CI & mergeability — coverage check green (52s); `mergeable: MERGEABLE`, `mergeStateStatus: CLEAN`. ✓
-
Issue #101 AC — 5 items, all addressed in the diff:
- Ignore hidden + high-churn folders before traversal — `shouldIgnorePath` at filesystem.ts:56 is passed to the watcher's `ignore` callback, running before the watcher walks the subtree. Segment set: `{node_modules, pycache, venv}` plus dotfiles. ✓
- Explicit `depth` and `limit` — `FILE_EXPLORER_WATCH_DEPTH = 6`, `FILE_EXPLORER_WATCH_LIMIT = 100_000` constants at top of file, both wired into the watcher options. ✓
- Watcher errors → renderer via IPC event — new `folder-contents-error` IPC channel; `sendFSError(...)` invoked from three sites: the watcher `error` event, the limit-warning helper when `visibleAddCount` crosses the limit, and a try/catch around the per-event handler. Renderer registers a listener in `DirectoryProvider` that logs to console + surfaces through `addAlert(msg, 'warning')`. ✓
- Normal small-folder behavior unchanged — no functional path change on small folders (limit=100k means the warning only fires on genuinely large trees; ignore list is a strict superset of the previous inline dotfile check). ✓
- `npm run typecheck` passes — PR body reports passed. ✓
-
Test Plan checkboxes — N/A.
-
Tests run — `npm run typecheck` → passed. Issue AC lists typecheck as the only test requirement; concrete result reported. ✓
-
Scope — 3 files: `filesystem.ts` (main-process watcher), `DirectoryContext.tsx` (renderer listener), `env.d.ts` (React 19 JSX namespace compat). The `env.d.ts` change is a small side-effect of running typecheck against React 19, explained inline in Summary ("add a React 19 JSX namespace compatibility declaration so `npm run typecheck` can validate the renderer"). Within scope. ✓
-
Regressions & patterns — try/catch guard around the per-event handler prevents a single bad `stat()` from tearing down the watcher subscription (nice hardening). `getMainWindow().isDestroyed()` check in `sendFSError` avoids a use-after-free crash on window teardown. `clearFolderErrorListener` correctly returned from the effect cleanup. ✓
-
Judgement calls — depth=6 and limit=100k are reasonable but not explicitly reasoned inline (they're specific numeric decisions worth a one-line justification). Ignore-segment list is hardcoded (`node_modules, pycache, venv`) — reasonable for this codebase's Node + Python worlds, but not extensible. Both are documentation nits, not blockers. ✓
All seven criteria hold. Approving.
Fixes #101
Part of #51
Summary
folder-contents-errornpm run typecheckcan validate the rendererRoot Cause
The file explorer opened folders by starting an unbounded recursive watcher and mirroring every discovered path into renderer state. Very large folders can therefore consume large amounts of main-process watcher memory and renderer heap.
Validation
npm run typecheck