Skip to content

refactor(core): move plugin discovery and watching to the config side - #41618

Merged
kitlangton merged 2 commits into
v2from
refactor/plugin-supervisor-sources
Aug 11, 2026
Merged

refactor(core): move plugin discovery and watching to the config side#41618
kitlangton merged 2 commits into
v2from
refactor/plugin-supervisor-sources

Conversation

@kitlangton

@kitlangton kitlangton commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

What

  • Make PluginSupervisor consume config-produced plugin source operations and change notifications, leaving it responsible for module import, selection, lifecycle, and activation only.
  • Move plugin directory discovery, configured target resolution/stat acquisition, plugin-source classification, and external configured-file watches into ConfigPluginSource under src/config/plugin.
  • Give static runtimes a no-dependency ConfigPluginSource.empty node that supplies an empty source feed, so they activate only internal and host-registered SDK plugins with no plugin discovery or plugin-source watches.

Why

Core services should not know where configuration-backed sources live or how to watch them. The config side already owns filesystem source topology and its change feed, so plugin source acquisition belongs there rather than in the activation supervisor.

This removes direct FSUtil, Watcher, Config, and Location knowledge from the supervisor and follows the same config-feeds-core-services direction as the sibling skills refactor without depending on it.

How

  • Added config/plugin/source.ts as a config-side service rather than an internal plugin. Its fs-backed layer derives ordered operations from Config.entries(), scans the same {plugin,plugins}/*.{ts,js} paths, preserves configured-target resolution and mtime versioning, and owns the existing monotonic external-entrypoint watch set.
  • ConfigPluginSource.node owns the fs-backed layer and its Config/FSUtil/Watcher/Location dependencies. ConfigPluginSource.empty implements the same service tag with an empty operation feed, a never-ending change stream, and no dependencies.
  • PluginSupervisor.layer now requires ConfigPluginSource.Service; its default node depends on the fs-backed source node, while static profiles substitute the empty same-tag node through LayerNode composition. A future package export condition can select implementations at bundle level, and the empty implementation can move to its own condition-specific file without changing the supervisor.
  • The source service avoids a bootstrap cycle: making source acquisition an internal plugin would require the supervisor to activate the plugin before it could discover the generation to activate.
  • PluginInternal.requirements now owns the separate service bundle needed to construct built-in plugins, so the supervisor does not conflate those dependencies with plugin-source acquisition.

Testing

  • cd packages/core && bunx tsgo --noEmit
  • cd packages/core && OPENCODE_CONFIG_DIR=<empty-temp-dir> bun run test test/config/plugin.test.ts (13 pass)
  • cd packages/core && OPENCODE_CONFIG_DIR=<empty-temp-dir> bun run test test/location-layer.test.ts (18 pass)
  • Push hook: workspace bun turbo typecheck --concurrency=3 (33 tasks passed)
  • Targeted oxlint: 0 errors; two warnings remain in logic moved unchanged from the supervisor.

Findings

  1. Config.layer produces the entry feed: discover() builds ordered Document/Directory entries, entries() exposes the current snapshot, and changes() publishes raw watched-root updates. Before this PR, PluginSupervisor.scan turned that snapshot into operations and watchConfiguredSources extended the change feed. After this PR, ConfigPluginSource produces both the operation snapshot and plugin-source change stream. This creates no dependency cycle: the default graph is PluginSupervisor.node -> ConfigPluginSource.node -> Config/FSUtil/Watcher/Location, while static composition replaces the middle node with ConfigPluginSource.empty.
  2. Expected config-side acquisition imports remain in config.ts, config/variable.ts, and config/plugin/{agent,command,source}.ts.
  3. Other acquisition-oriented core modules still importing FSUtil or Watcher are instruction-discovery.ts; skill.ts and skill/discovery.ts; models-dev.ts; plugin/skill.ts (report diagnostics); project.ts; git.ts; vcs.ts and vcs/hg.ts; filesystem/location-watcher.ts; formatter.ts and formatter/builtins.ts; repository-cache.ts; and ripgrep/binary.ts. Tool-ground and mutation-only filesystem consumers are excluded.

@kitlangton
kitlangton force-pushed the refactor/plugin-supervisor-sources branch from 09d96f8 to d2b479b Compare August 10, 2026 23:09
@kitlangton

Copy link
Copy Markdown
Contributor Author

Investigated the repeated uses the active location when opened outside a session timeout. The failing line is the TUI harness waiting for its mocked /api/vcs/diff request; this PR has no diff from origin/v2 under packages/tui, packages/plugin, or packages/client.

  • PR (d2b479bcb7): cd packages/tui && bun run test test/cli/tui/diff-viewer.test.tsx, 3 independent runs, all 5/5 tests passed (also 30 --rerun-each passes and one full TUI suite pass).
  • origin/v2 (2580f880a8), clean detached worktree after bun install --frozen-lockfile: same command, 3 independent runs, all 5/5 tests passed (also 30 --rerun-each passes and one full TUI suite pass).

I cannot reproduce a revision-specific regression locally. I am rerunning the failed CI jobs to check the remaining CI-only timing/contention case.

@kitlangton

Copy link
Copy Markdown
Contributor Author

CI rerun update: the original diff-viewer test passed on both Linux and Windows, and Windows unit is fully green. Linux moved to an unrelated first-frame assertion in test/component/tab-pulse.test.tsx; that test passes 600/600 focused tests on the clean origin/v2 worktree, and the contemporaneous v2 Linux unit job also passed. Rerunning only the failed Linux job once more.

@kitlangton

Copy link
Copy Markdown
Contributor Author

Final CI result: all checks are green, including unit on Linux and Windows. The original diff-viewer failure was a CI-only timing flake in the unchanged TUI harness, not a plugin-source regression: focused and full-suite comparisons passed on both revisions, the first rerun cleared it on both platforms, and the final Linux rerun passed after an unrelated transient TUI frame assertion. No code change was needed.

@kitlangton
kitlangton marked this pull request as ready for review August 11, 2026 00:07
@kitlangton
kitlangton merged commit 71f5e41 into v2 Aug 11, 2026
15 of 18 checks passed
@kitlangton
kitlangton deleted the refactor/plugin-supervisor-sources branch August 11, 2026 00:15
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