feat(plugins): runtime, marketplace handler, public SDK + api k8s nil tolerance#272
feat(plugins): runtime, marketplace handler, public SDK + api k8s nil tolerance#272JoshuaAFerguson wants to merge 1 commit into
Conversation
Wraps up the in-flight plugin work that had been sitting in the working
tree across multiple session branches. Builds, vets, and tests pass
(the one failing -race test in internal/services.TestMultipleWorkers
is pre-existing on main and unrelated to this change).
Plugin runtime + registries (api/internal/plugins/):
- marketplace.go: +280 net lines (catalog browse/sync, install,
enable/disable, uninstall, configure)
- handlers/plugins.go: +256 net lines wiring the marketplace handler
alongside the existing id-based PluginHandler used by the UI
- runtime.go / runtime_v2.go: harden lifecycle handling and add v2
context plumbing
- api_registry.go, database.go, discovery.go, logger.go, scheduler.go,
ui_registry.go: bring registries up to the v2 plugin contract
- New tests: discovery_test.go, marketplace_test.go, runtime_v2_test.go
(small but real — exercise the public API surface)
Public SDK (api/pkg/pluginsdk/sdk.go):
- Re-exports the internal/plugins types as a stable public surface for
external plugin authors. Includes a flexible `Register()` that accepts
either a factory func or a concrete PluginHandler.
API hardening — k8s nil tolerance (api/cmd/main.go):
- The downstream handler signature already accepted nil k8sClient (see
comment near the constructor), but the init at the top hard-fataled
regardless. Now: only fatal when PLATFORM is unset or "kubernetes";
for "docker" / "vm" / "cloud" platforms, log the init failure and
continue with k8sClient=nil. This is what unblocks the docker-compose
dev loop on machines without a kubeconfig.
Plugin handler routes registered (verified via gin debug log on the
running api):
- UI-facing (id-based): /plugins, /plugins/:id, /plugins/catalog/:id
- External-facing (name-based): /plugins/marketplace/{install,enable,
disable,uninstall}/:name, /plugins/marketplace/{catalog,installed}
Known TODOs in plugin_marketplace.go (left as-is, pre-existing):
- EnablePlugin: only flips the DB flag, doesn't reload runtime
- UpdatePluginConfig: returns 200 but doesn't persist the config
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b391f4c79a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| go func() { | ||
| if err := h.downloadPluginFromRepository(catalogPlugin.Name, repoURL.String); err != nil { | ||
| if err := h.downloadPluginFromRepository(catalogPlugin.Name, repoURL.String, sourcePath.String); err != nil { | ||
| log.Printf("[PluginHandler] Warning: Failed to download plugin files for %s: %v", catalogPlugin.Name, err) | ||
| } else { | ||
| log.Printf("[PluginHandler] Plugin files downloaded to %s/%s", h.pluginDir, catalogPlugin.Name) |
There was a problem hiding this comment.
Wait for plugin download before runtime load
This spawns the repository download in a goroutine and then the same request path immediately calls runtime.LoadPluginWithConfig later in InstallPlugin, so runtime loading races the filesystem write and will often fail for newly installed remote plugins (returning only a warning while leaving the plugin marked installed/enabled). In practice this breaks the new “install and load immediately” flow whenever the plugin is not already present locally.
Useful? React with 👍 / 👎.
|
|
||
| for _, entry := range entrypoints { | ||
| entryURL := fmt.Sprintf("%s/%s/%s", baseURL, sourcePath, entry) | ||
| targetPath := filepath.Join(targetDir, filepath.Base(entry)) |
There was a problem hiding this comment.
Preserve entrypoint subpaths when downloading assets
Using filepath.Base(entry) strips directories from manifest entrypoints (for example bundle/plugin-runtime.so), so the downloaded file layout no longer matches the manifest path that discovery uses. Plugins with nested .so entrypoints will download successfully but then fail to load because runtime looks for .../bundle/plugin-runtime.so while only .../plugin-runtime.so exists.
Useful? React with 👍 / 👎.
| updated_at = NOW() | ||
| `, plugin.Name, plugin.Version, plugin.DisplayName, plugin.Description, | ||
| plugin.Category, plugin.Manifest.Type, plugin.IconURL, manifestJSON, plugin.Tags) | ||
| plugin.Path, plugin.Category, plugin.Manifest.Type, plugin.IconURL, manifestJSON, plugin.Tags) |
There was a problem hiding this comment.
Skip catalog upsert when manifest hydration fails
YAML catalog sync logs manifest fetch failures and continues, but this upsert still writes plugin.Manifest.Type and manifest for that plugin. When hydration fails transiently, those fields are empty/zero-value and overwrite previously valid catalog metadata in catalog_plugins, causing degraded plugin type/filter behavior until a later successful sync restores data.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| func (pd *PluginDiscovery) hasFile(path string) bool { | ||
| info, err := os.Stat(path) |
| } | ||
|
|
||
| func (pd *PluginDiscovery) findPluginFromManifest(manifestPath string) string { | ||
| manifestBytes, err := os.ReadFile(manifestPath) |
|
This pull request has been automatically marked as stale because it has not had recent activity. Action Required:
|
|
This pull request was automatically closed due to inactivity. If you believe this was closed in error, please reopen it. |
Summary
Wraps up the in-flight plugin work that had been sitting in the working tree across multiple branches. Single commit (`b391f4c`) covering:
18 files, 1055 insertions, 270 deletions.
Verification
Plugin handler routes (confirmed via gin debug)
Known TODOs (left in code; not introduced by this PR)
Two existing TODOs in `plugin_marketplace.go`:
These only affect the name-based external API path (the UI's id-based path goes through `PluginHandler` and works end-to-end).
Test plan