Skip to content

refactor(core): tidy plugin host and config reload wiring - #61176

Merged
rekram1-node merged 2 commits into
devfrom
plugin-host-tidy
Nov 2, 2026
Merged

rekram1-node merged 2 commits into
devfrom
plugin-host-tidy

Conversation

@rekram1-node

Copy link
Copy Markdown
Collaborator

Problem

The plugin host and a couple of config plugins had the same code copied in a few places, so a fix to one copy could easily miss the others.

Change

I pulled each repeat into one place. Behavior should be the same.

 config/plugin/entry-observer.ts
+  observeSources()       # load, watch, debounce, reload (agent + command used to copy this)
+  isDirectorySource()    # replaces isAgentSource / isCommandSource / isPluginSource
 plugin/host.ts
+  atLocation(ref, effect)   # remote agent list and remote mcp.list
+  sessionPermission(input)  # permission.get and permission.reply
+  connectionInfo(conn)      # the 3 Credential.ID brandings
 plugin.ts
+  snapshot()             # the 3 inventory rebuilds
 plugin/internal.ts
+  built-ins come back with revision "internal" and source builtin (was done in supervisor)
 plugin/module.ts
+  source(target, version?)  # supervisor's pluginSource now uses it
 plugin/supervisor.ts
-  inline Failure type    # uses Plugin.Failure

observeSources keeps the old order: it subscribes to config changes and config.updated before the first scan.

Out of scope

  • methodImplementation in host.ts: merging the command and external branches, or the env branch with the fallback, doesn't typecheck without casts, so I left them separate.
  • The builtin / server: true defaults for duplicate-ID failures in supervisor.ts: there are only two short copies in different modules, and sharing them would need a new export.

Verification

  • bun typecheck in packages/core
  • bun test test/config test/plugin test/plugin.test.ts test/plugin-failure.test.ts test/model-plugins.test.ts in packages/core: 837 pass
  • bun run check from the root

@opencodejr opencodejr Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good. It's a behaviour-preserving refactor as described. I checked each extraction against the old code: snapshot() reads discovered, which equals failures at both rebuilds that used it, and PluginModule.source matches both old call sites. Built-ins tagged in PluginInternal.list reach the supervisor unchanged. observeSources keeps the subscribe-before-scan order. tsc -b passes, the config and plugin tests pass (836 pass, 1 skip), and a TUI smoke test with a project-local agent and command behaves the same as on the base.

Public API: unchanged (all changes are internal to packages/core).

One small fix: a comment ended up above the wrong function in module.ts (inline).

Comment thread packages/core/src/plugin/module.ts Outdated
@@ -60,6 +60,12 @@ export const make = Effect.fn("PluginModule.make")(function* () {

// A missing dependency may have missing parents too. Watch the nearest existing

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The "A missing dependency may have missing parents too…" comment describes watchTarget, but it now sits above source(), so it reads as documentation for the wrong function. Could you move those two lines back above watchTarget?

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