Skip to content

[codex] Limit plugin directory broadcasts#106

Merged
den-sq merged 1 commit into
mainfrom
codex/issue51-limit-plugin-directory-broadcasts
Jul 6, 2026
Merged

[codex] Limit plugin directory broadcasts#106
den-sq merged 1 commit into
mainfrom
codex/issue51-limit-plugin-directory-broadcasts

Conversation

@den-sq

@den-sq den-sq commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Refs #102
Stacks on #105
Part of #51

Summary

  • stop broadcasting the full recursive nodes tree to plugin iframes on every directory state update
  • continue broadcasting selected directory path/name metadata through the existing message type
  • keep nodes in the payload as an empty object for shape compatibility while the fuller on-demand plugin API is designed

Root Cause

The iframe provider cloned and posted the full file explorer tree whenever directory context changed. For large folders this created another memory multiplier on top of watcher and renderer state.

Validation

  • npm run typecheck
    • passed

Follow-Up

This is a first slice of #102. Lazy-loading the file explorer tree and adding an explicit plugin request API are still tracked there.

@tavateva tavateva left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Draft Promotion Criteria PASS ✅

Refs #102 (not fixes — explicit partial slice). Stacks on #105. Part of #51. All 7 criteria hold.

  1. CI & mergeability — same stack pattern as #105: no checks fire because base is `codex/issue51-batch-folder-updates`. Once #104 and #105 merge, this PR rebases onto main and CI runs. `mergeable: MERGEABLE`, `mergeStateStatus: CLEAN`. ✓ (soft)

  2. Issue AC — this is the tricky one. #102's checklist has 5 items:

    • Lazy top-level entries first
    • Load child directories only on expand
    • No whole-tree plugin broadcasts (this PR)
    • Existing selection/drop workflows continue to work
    • `npm run typecheck` passes

    PR body correctly uses "Refs #102" (not "Fixes"), explicitly parks lazy-loading + on-demand plugin API as follow-up work in a Follow-Up section, and delivers only the one AC item it claims. This is a legitimate scoped-slice PR — evaluate criterion 2 against the claimed subset ("stop broadcasting the full recursive nodes tree"), which is satisfied:

    • Full tree no longer broadcast — `useContext(DirectoryContext)` destructures only `{ directoryPath, directoryName }`; broadcast payload sets `nodes: {}`. ✓
    • Shape compatibility retained — `nodes` field still present as empty object so existing plugins that read it don't crash. ✓
    • `npm run typecheck` passes — reported. ✓
  3. Test Plan checkboxes — N/A.

  4. Tests run — `npm run typecheck` → passed. ✓

  5. Scope — 1 file, +11/-10. Trivially tight. ✓

  6. Regressions & patterns — `broadcastDirectoryContext` wrapped in `useCallback` with correct deps (`broadcast`, `directoryName`, `directoryPath`); `useEffect` triggers on that callback. No longer depends on `data` (the full context object) so the effect no longer fires on every `nodes` change — that's the actual perf fix. Selection/drop workflows unaffected because they don't route through the iframe broadcast payload. ✓

  7. Judgement calls — three, all reasoned inline:

    • Partial-#102 slice, follow-up explicitly named — right way to ship an incremental change against a multi-AC issue.
    • Keep `nodes: {}` for shape compat — reasonable defensive choice while the on-demand plugin request API is designed.
    • No rename to `send-directory-metadata` — kept the same IPC message type. Preserves backwards-compat for existing plugins. Reasonable.

All seven criteria hold. Approving.

Stack note: rebase after #104 and #105 merge so CI fires against main before final merge.

Base automatically changed from codex/issue51-batch-folder-updates to main July 6, 2026 22:22
@den-sq den-sq marked this pull request as ready for review July 6, 2026 22:22
@den-sq den-sq merged commit 9d0dba4 into main Jul 6, 2026
@den-sq den-sq deleted the codex/issue51-limit-plugin-directory-broadcasts branch July 6, 2026 22:23
den-sq pushed a commit that referenced this pull request Jul 7, 2026
Adds a Reference section to the mkdocs documentation with a new
technical-constants.md page that documents the hardcoded file explorer
limits introduced by the #51 out-of-memory mitigation stack (#104, #105,
#106).

For each constant, records symbol name, current value, source file line,
rationale, user-visible behavior when the limit is reached, and guidance
on when maintainers should reconsider the value. Values are extracted
directly from src/main/event-handlers/filesystem.ts:

- FILE_EXPLORER_WATCH_DEPTH = 6
- FILE_EXPLORER_WATCH_LIMIT = 100_000
- FILE_EXPLORER_UPDATE_BATCH_LIMIT = 500
- FILE_EXPLORER_UPDATE_INTERVAL_MS = 100
- IGNORED_PATH_SEGMENTS = {node_modules, __pycache__, venv} plus any
  segment beginning with "."

Also documents the plugin directory-broadcast change from #106 (empty
nodes payload) and points plugin authors at the reference page.

Adds a short user-visible "Large Folder Behavior" section to the docs
index so users encountering the watcher limit warning have a place to
land, and wires the new page into mkdocs.yml.

Setup, development, and packaging docs were audited against main and
have no stale claims tied to the #104/#105/#106 changes, so no rewrites
in this pass; broader audit work is left as follow-up.
tavateva pushed a commit to tavateva/ouroboros that referenced this pull request Jul 7, 2026
…ntics (closes ChengLabResearch#111)

Adds `request-directory-contents` / `send-directory-contents-response`
message pair to the plugin iframe surface, so plugins can walk the
current directory root on demand after ChengLabResearch#106 stopped broadcasting the
full recursive tree.

Renderer:
- New valibot schemas in `src/renderer/src/schemas/iframe-message-schema.ts`:
  `RequestDirectoryContentsSchema` (plugin -> provider) and
  `SendDirectoryContentsResponseSchema` (provider -> requesting iframe).
  A recursive `PluginFileSystemNodeSchema` (via `lazy`) mirrors the
  renderer's `NodeChildren` shape so plugins get a drop-in replacement
  for the pre-ChengLabResearch#106 broadcast payload.
- `IFrameContext.tsx` gains `handleRequestDirectoryContentsRequest`
  wired alongside the existing `read-file`/`save-file`/`register-plugin`
  handlers. It reads the current root through a ref so the closure sees
  the latest `directoryPath` regardless of when the listener effect ran,
  applies a path-traversal guard (rejects `..` escapes and Windows
  drive-letter changes), and posts the response only to the requesting
  iframe. Root broadcast (`send-directory-contents { nodes: {} }`) is
  unchanged.
- Errors map to `denied` (traversal / no root / malformed),
  `not-found` (ENOENT / ENOTDIR), `limit` (enumeration truncated), or
  `internal` (anything else).

Main:
- `src/main/event-handlers/filesystem.ts` gains a `get-folder-contents`
  IPC handler backed by a top-level `getFolderContents` that does a
  one-shot `readdir` walk. Recursion is depth-first and bounded by the
  same `FILE_EXPLORER_WATCH_LIMIT` used by the watcher path; hitting
  the cap returns `{ nodes, truncated: true }` with the partial result.
- Existing watcher factory refactored to use a shared
  `shouldIgnorePathAgainstRoot` helper so both the watcher and the
  one-shot walk apply the same dotfile + `IGNORED_PATH_SEGMENTS` rule.
- Handler does not touch the watcher-manager state; enumeration is
  independent of expand/collapse.

Docs:
- `documentation/docs/guide/plugins.md` gets a "Requesting Directory
  Contents" section with request/response examples, the error-code
  taxonomy, and notes on the traversal guard, truncation semantics, and
  ignore-rule inheritance.
- `documentation/docs/reference/technical-constants.md` extends the
  `FILE_EXPLORER_WATCH_LIMIT` entry with its second role as the
  per-request enumeration cap, and points the "Plugin directory
  broadcast" section at the new on-demand API.

`src/preload/index.ts` documents the new IPC channel alongside the
existing `expand-folder` / `collapse-folder` entries.

Verification: `npm run typecheck` passes (both node and web tsconfigs).
`npm run lint` still fails with the pre-existing ESLint 9 config drift
called out in ChengLabResearch#109/ChengLabResearch#112, not introduced by this PR.

Path-traversal in the existing `read-file` / `save-file` handlers is
unrelated to ChengLabResearch#111 and left as a follow-up.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants