refactor(core): move plugin discovery and watching to the config side - #41618
Conversation
…discovery and watching
09d96f8 to
d2b479b
Compare
|
Investigated the repeated
I cannot reproduce a revision-specific regression locally. I am rerunning the failed CI jobs to check the remaining CI-only timing/contention case. |
|
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 |
|
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. |
What
PluginSupervisorconsume config-produced plugin source operations and change notifications, leaving it responsible for module import, selection, lifecycle, and activation only.ConfigPluginSourceundersrc/config/plugin.ConfigPluginSource.emptynode 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, andLocationknowledge from the supervisor and follows the same config-feeds-core-services direction as the sibling skills refactor without depending on it.How
config/plugin/source.tsas a config-side service rather than an internal plugin. Its fs-backed layer derives ordered operations fromConfig.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.nodeowns the fs-backed layer and itsConfig/FSUtil/Watcher/Locationdependencies.ConfigPluginSource.emptyimplements the same service tag with an empty operation feed, a never-ending change stream, and no dependencies.PluginSupervisor.layernow requiresConfigPluginSource.Service; its default node depends on the fs-backed source node, while static profiles substitute the empty same-tag node throughLayerNodecomposition. 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.PluginInternal.requirementsnow 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 --noEmitcd 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)bun turbo typecheck --concurrency=3(33 tasks passed)Findings
Config.layerproduces the entry feed:discover()builds orderedDocument/Directoryentries,entries()exposes the current snapshot, andchanges()publishes raw watched-root updates. Before this PR,PluginSupervisor.scanturned that snapshot into operations andwatchConfiguredSourcesextended the change feed. After this PR,ConfigPluginSourceproduces both the operation snapshot and plugin-source change stream. This creates no dependency cycle: the default graph isPluginSupervisor.node -> ConfigPluginSource.node -> Config/FSUtil/Watcher/Location, while static composition replaces the middle node withConfigPluginSource.empty.config.ts,config/variable.ts, andconfig/plugin/{agent,command,source}.ts.FSUtilorWatcherareinstruction-discovery.ts;skill.tsandskill/discovery.ts;models-dev.ts;plugin/skill.ts(report diagnostics);project.ts;git.ts;vcs.tsandvcs/hg.ts;filesystem/location-watcher.ts;formatter.tsandformatter/builtins.ts;repository-cache.ts; andripgrep/binary.ts. Tool-ground and mutation-only filesystem consumers are excluded.