From c7f9cddd6b1e60e5653d82d57100120d9a8d7ceb Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Fri, 29 May 2026 09:32:27 -0400 Subject: [PATCH] style: gofmt import sort after plugins/ path change --- DOCUMENTATION.md | 4 +- cmd/server/main.go | 6 +- docs/DEFERRED_ISSUES.md | 2 +- ...6-05-29-plugin-dir-consolidation-design.md | 145 ++++++++++++++++++ {plugin => plugins}/admincore/plugin.go | 0 {plugin => plugins}/admincore/plugin_test.go | 0 {plugin => plugins}/docmanager/handler.go | 0 .../docmanager/handler_test.go | 0 {plugin => plugins}/docmanager/plugin.go | 0 {plugin => plugins}/storebrowser/handler.go | 0 .../storebrowser/handler_test.go | 0 {plugin => plugins}/storebrowser/plugin.go | 0 12 files changed, 151 insertions(+), 6 deletions(-) create mode 100644 docs/plans/2026-05-29-plugin-dir-consolidation-design.md rename {plugin => plugins}/admincore/plugin.go (100%) rename {plugin => plugins}/admincore/plugin_test.go (100%) rename {plugin => plugins}/docmanager/handler.go (100%) rename {plugin => plugins}/docmanager/handler_test.go (100%) rename {plugin => plugins}/docmanager/plugin.go (100%) rename {plugin => plugins}/storebrowser/handler.go (100%) rename {plugin => plugins}/storebrowser/handler_test.go (100%) rename {plugin => plugins}/storebrowser/plugin.go (100%) diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index 059feeaa..7b997e8a 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -1791,7 +1791,7 @@ Runs LSP diagnostics on YAML content directly from a pipeline step. --- -### Admin Core Plugin (`plugin/admincore/`) +### Admin Core Plugin (`plugins/admincore/`) The `admincore` plugin is a NativePlugin that registers the built-in admin UI page definitions. It declares no HTTP routes -- all views are rendered entirely in the React frontend. Registering this plugin ensures navigation is driven by the plugin system with no static fallbacks. @@ -1811,7 +1811,7 @@ The `admincore` plugin is a NativePlugin that registers the built-in admin UI pa Global pages appear in the main navigation. Workflow-scoped pages (`executions`, `logs`, `events`) are only shown when a workflow is open. -The plugin is auto-registered via `init()` in `plugin/admincore/plugin.go`. No YAML configuration is required. +The plugin is auto-registered via `init()` in `plugins/admincore/plugin.go`. No YAML configuration is required. --- diff --git a/cmd/server/main.go b/cmd/server/main.go index f6d3ba2b..9729221f 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -37,12 +37,12 @@ import ( "github.com/GoCodeAlone/workflow/observability" "github.com/GoCodeAlone/workflow/observability/tracing" "github.com/GoCodeAlone/workflow/plugin" - _ "github.com/GoCodeAlone/workflow/plugin/admincore" - _ "github.com/GoCodeAlone/workflow/plugin/docmanager" pluginexternal "github.com/GoCodeAlone/workflow/plugin/external" - _ "github.com/GoCodeAlone/workflow/plugin/storebrowser" + _ "github.com/GoCodeAlone/workflow/plugins/admincore" allplugins "github.com/GoCodeAlone/workflow/plugins/all" + _ "github.com/GoCodeAlone/workflow/plugins/docmanager" pluginpipeline "github.com/GoCodeAlone/workflow/plugins/pipelinesteps" + _ "github.com/GoCodeAlone/workflow/plugins/storebrowser" "github.com/GoCodeAlone/workflow/provider" _ "github.com/GoCodeAlone/workflow/provider/azure" _ "github.com/GoCodeAlone/workflow/provider/digitalocean" diff --git a/docs/DEFERRED_ISSUES.md b/docs/DEFERRED_ISSUES.md index e2a486ec..bd010736 100644 --- a/docs/DEFERRED_ISSUES.md +++ b/docs/DEFERRED_ISSUES.md @@ -115,7 +115,7 @@ These are the most significant issues but carry the highest refactoring risk and - `step.ai_complete`, `step.ai_classify`, `step.ai_extract` pipeline steps - CI/CD steps: `step.docker_build`, `step.docker_push`, `step.docker_run`, `step.scan_sast`, `step.scan_container`, `step.scan_deps`, `step.artifact_push`, `step.artifact_pull` - `step.jq` — JSON query transformations - - `plugin/admincore/` — admin platform plugin + - `plugins/admincore/` — admin platform plugin - `observability.otel` — no example YAML or configuration guide ### Example Count Exaggeration diff --git a/docs/plans/2026-05-29-plugin-dir-consolidation-design.md b/docs/plans/2026-05-29-plugin-dir-consolidation-design.md new file mode 100644 index 00000000..18bd0692 --- /dev/null +++ b/docs/plans/2026-05-29-plugin-dir-consolidation-design.md @@ -0,0 +1,145 @@ +# Design: plugin/ ↔ plugins/ consolidation + registry tracking + +Date: 2026-05-29 +Repo: GoCodeAlone/workflow (+ GoCodeAlone/workflow-registry) +Status: Draft → adversarial-design-review + +## Intent (user) + +`plugin/` (singular) = general plugin-supporting FRAMEWORK (loader, manager, +manifest, sdk, builder, registry, installer, cosign, native/external support). +`plugins/` (plural) = built-in plugin IMPLEMENTATIONS, each `New() *Plugin`, +wired via `plugins/all/all.go`, and announced to workflow-registry. Some +implementations are misfiled under `plugin/` (e.g. `plugin/storebrowser`). +Consolidate; ensure all built-in plugins tracked in workflow-registry. Cut a +release when done. + +## Findings (from classification investigation) + +Two registration patterns: **A** EnginePlugin `New()` + `plugins/all` import; +**B** NativePlugin `init()`→`plugin.RegisterNativePluginFactory`, activated by +blank import in `cmd/server/main.go` + invoked via `plugin.BuiltinNativePlugins`. + +`plugin/` subdir classification: + +| subdir | type | action | +|---|---|---| +| admincore | IMPL (NativePlugin, pattern B; main.go:40) | MOVE → plugins/admincore | +| docmanager | IMPL (NativePlugin, pattern B; main.go:41) | MOVE → plugins/docmanager | +| storebrowser | IMPL (NativePlugin, pattern B; main.go:43) | MOVE → plugins/storebrowser | +| builder | FRAMEWORK (Builder iface+registry; used by plugins/builder-*, wfctl) | STAY | +| registry | FRAMEWORK (RegistryProvider iface+registry; used by plugins/registry-*) | STAY | +| external | FRAMEWORK (gRPC/go-plugin loader + author SDK) | STAY | +| sdk | FRAMEWORK (manifest schema, scaffolder, iaclint used by IaC plugin repos) | STAY | +| community | orphaned dead code (0 importers) | DELETE | +| rbac | orphaned dead code (0 importers) | DELETE | +| ai/{anthropic,openai,generic} | `ai.AIProvider` impls (NOT plugins; 0 importers) | MOVE → ai/providers/ | + +Registry: `workflow-registry/plugins//manifest.json` +(`{name,version,author,source,type:builtin|external,tier,path,capabilities}`). +admincore/docmanager/storebrowser NOT tracked → add. `plugins/builder-*` and +`plugins/registry-*` are wfctl-internal (implement builder/registry ifaces, not +engine capabilities) → intentionally untracked; document, do not add. + +## Decisions + +### D1 — Move the 3 native-plugin impls to plugins/ (core) +`git mv plugin/{admincore,docmanager,storebrowser} plugins/`. Each keeps +importing `…/workflow/plugin` (framework) — no circular risk (plugins/ already +depends on plugin/; plugin/ never imports plugins/). Update the 3 blank imports +in `cmd/server/main.go` from `…/plugin/X` → `…/plugins/X`. Zero other importers. +No registration-pattern conversion (pattern B mechanism lives in +`plugin/builtins.go`, which stays). + +### D2 — Wiring location for moved NativePlugins +They are NativePlugins, not EnginePlugins; `plugins/all` aggregates +EnginePlugins only. Keep their activation as blank imports in +`cmd/server/main.go` (unchanged behavior), NOT in `plugins/all`. Rationale: +`plugins/all.DefaultPlugins()` returns `[]EnginePlugin`; forcing NativePlugins +in would conflate two contracts. (Adversarial-review question: is a +`plugins/all` native-plugin aggregator worth adding? Default: no — minimal change.) + +### D3 — DEFER orphaned dead code (do NOT delete this pass) +`plugin/community` (submission-validation tooling) + `plugin/rbac` +(auth bridge `BuiltinProvider`, has a 226-line test suite) are orphaned +(0 production importers, not wired, not registered). Per adversarial review: +deleting exported packages the user didn't ask to remove is scope-creep, and +`plugin/rbac` is plausibly intended-but-unwired. **Leave them untouched this +pass**; file a follow-up to delete-or-wire with an explicit decision. (Rev: +was "delete"; downgraded to defer to avoid removing possibly-intended +functionality without confirmation.) NOTE the stale `scripts/audit-cloud-symbols.sh:131` +comment references `plugin/rbac/aws.go` which never existed — leave for the +follow-up, not this pass. + +### D4 — Relocate misfiled AI providers (MOVE-ONLY, no wiring) +`git mv plugin/ai/{anthropic,openai,generic} ai/providers/`. These implement +`ai.AIProvider` (sibling to `ai/llm`, `ai/copilot`), not the plugin contracts — +they do not belong in `plugins/`. **Move only; do NOT wire.** (Rev: adversarial +review found the proposed `initAIService` wiring is architecturally impossible — +`initAIService` registers `ai.WorkflowGenerator` impls, but these are +`ai.AIProvider` impls consumed by `ai.AIModelRegistry.RegisterProvider`, which +is owned by `plugins/ai/plugin.go`, not initAIService. Wiring there would also +be a behavior change — activating currently-dead providers + a new +ANTHROPIC_API_KEY startup dependency — which is out of scope for a structural +cleanup.) Add a doc comment in each provider's `New()` pointing to +`plugins/ai/plugin.go`'s `aiRegistry.RegisterProvider(...)` as the future +wiring site, and file a follow-up issue for provider activation. They remain +unreferenced after the move (same as today) — but now correctly located. + +### D5 — Registry reconciliation +Add `workflow-registry/plugins/{admincore,docmanager,storebrowser}/manifest.json` +(type:builtin, source:github.com/GoCodeAlone/workflow, path:plugins/X, tier per +capability, capabilities from each plugin's pages/routes/steps). Audit all +`plugins/*` engine packages vs registry. +**Audit result (verified):** the only unregistered `plugins/*` dirs are `all` +(aggregator, not a plugin) + `builder-{custom,go,nodejs}` + +`registry-{aws,azure,do,gcp,github,gitlab}` (wfctl-internal builder/registry- +provider impls — no engine capabilities). **0 additional engine-plugin gaps** +beyond the 3 moved ones → D5 adds exactly 3 manifests; the 10 internal dirs stay +intentionally untracked. Follow the existing manifest.json schema +(`workflow-registry/schema/registry-schema.json`) + regen `v1/index.json` via +the registry build/script if required (verify tests/ pass). + +## Scope / phases (PR grouping) + +- **PR-1 (workflow):** D1 move 3 native plugins (admincore/docmanager/storebrowser + → plugins/) + D2 wiring (blank imports stay in cmd/server/main.go) + fix admincore + path refs: `DOCUMENTATION.md:1794,1814`, `docs/DEFERRED_ISSUES.md:118`. Self-contained; + build+test green. (D3 community/rbac NOT touched — deferred.) +- **PR-2 (workflow):** D4 AI provider relocation, MOVE-ONLY (no wiring). Separable. +- **PR-3 (workflow-registry):** D5 — exactly 3 manifests (admincore/docmanager/ + storebrowser) + index regen. +- Then: cut workflow release **v0.65.0** (minor bump — D1 changes public import + paths `plugin/X`→`plugins/X`; D4 moves public packages; pre-1.0 semver policy + per docs/API_STABILITY.md treats public-package moves as minor). + +Out of scope: renaming/restructuring the `plugin/` framework itself; changing +the EnginePlugin/NativePlugin contracts; external-plugin (`plugin/external`) SDK. + +## Verification + +- `go build ./...` + `go test ./...` green (esp. cmd/server, plugins/*, ai/*). +- `cmd/server` boots; native plugins still register (admincore pages, + storebrowser/docmanager routes present) — runtime-launch check. +- workflow-registry schema-validates (scripts/ + tests/). +- `golangci-lint run`; gofmt. +- No dangling `plugin/{admincore,docmanager,storebrowser,community,rbac,ai}` + references anywhere (grep go + non-go). + +## Risks + +- Import-path churn is tiny (3 lines main.go) — low. +- AI provider wiring (D4) could change runtime AI behavior → gate behind config; + separable PR-2. +- Registry manifest schema mismatch → validate against schema + existing entries. +- `plugin/ai/*` move target (`ai/providers/`) is a judgment call (not `plugins/`) + — flagged for adversarial review. +- community/rbac: DEFERRED (not deleted/moved this pass) — removes that risk. + +## Rollback + +Each PR is a single revertable commit, no data migration. No compat aliases +needed: moved packages have no external direct-type callers; activation is via +internal blank imports (cmd/server) / aggregator. Reverting PR-1 restores the +plugin/ paths. Downstream local worktrees with stale `plugin/{admincore,...}` +blank imports rebase onto the new paths (acceptable; internal). diff --git a/plugin/admincore/plugin.go b/plugins/admincore/plugin.go similarity index 100% rename from plugin/admincore/plugin.go rename to plugins/admincore/plugin.go diff --git a/plugin/admincore/plugin_test.go b/plugins/admincore/plugin_test.go similarity index 100% rename from plugin/admincore/plugin_test.go rename to plugins/admincore/plugin_test.go diff --git a/plugin/docmanager/handler.go b/plugins/docmanager/handler.go similarity index 100% rename from plugin/docmanager/handler.go rename to plugins/docmanager/handler.go diff --git a/plugin/docmanager/handler_test.go b/plugins/docmanager/handler_test.go similarity index 100% rename from plugin/docmanager/handler_test.go rename to plugins/docmanager/handler_test.go diff --git a/plugin/docmanager/plugin.go b/plugins/docmanager/plugin.go similarity index 100% rename from plugin/docmanager/plugin.go rename to plugins/docmanager/plugin.go diff --git a/plugin/storebrowser/handler.go b/plugins/storebrowser/handler.go similarity index 100% rename from plugin/storebrowser/handler.go rename to plugins/storebrowser/handler.go diff --git a/plugin/storebrowser/handler_test.go b/plugins/storebrowser/handler_test.go similarity index 100% rename from plugin/storebrowser/handler_test.go rename to plugins/storebrowser/handler_test.go diff --git a/plugin/storebrowser/plugin.go b/plugins/storebrowser/plugin.go similarity index 100% rename from plugin/storebrowser/plugin.go rename to plugins/storebrowser/plugin.go