From a323710a6f1ad593605280c038b979faac1f0847 Mon Sep 17 00:00:00 2001 From: mirivlad Date: Fri, 10 Jul 2026 04:03:08 +0800 Subject: [PATCH] fix: resolve plugin lifecycle dependencies deterministically --- docs/PLUGIN_RUNTIME.md | 37 +++++---- internal/api/app.go | 46 ++++------- internal/core/capability/registry.go | 8 ++ internal/core/capability/registry_test.go | 21 +++++ internal/core/plugin/lifecycle.go | 75 +++++++++++++++++ internal/core/plugin/lifecycle_test.go | 99 +++++++++++++++++++++++ main.go | 53 ++---------- 7 files changed, 247 insertions(+), 92 deletions(-) create mode 100644 internal/core/capability/registry_test.go create mode 100644 internal/core/plugin/lifecycle.go diff --git a/docs/PLUGIN_RUNTIME.md b/docs/PLUGIN_RUNTIME.md index e48d491..7128882 100644 --- a/docs/PLUGIN_RUNTIME.md +++ b/docs/PLUGIN_RUNTIME.md @@ -112,17 +112,25 @@ capRegistry.Register("verstak-desktop", []string{"verstak/core/vault/v1"}) ### Plugin capability resolution ``` -foreach plugin: - 1. зарегистрировать plugin.provides в capability registry - 2. проверить plugin.requires — если есть missing → missing-required-capability - 3. проверить plugin.optionalRequires — если есть missing → degraded - 4. иначе → loaded - 5. зарегистрировать plugin.contributes в contribution registry +1. Исключить disabled plugins. +2. Пока есть неразрешённые plugins: + - plugin с уже доступными `requires` атомарно регистрирует `provides`; + - при конфликте `provides` plugin получает `failed`; + - если за проход ничего не разрешилось, остальные получают + `missing-required-capability`. +3. После разрешения всех обязательных зависимостей проверить + `optionalRequires`: отсутствующие переводят plugin в `degraded`. +4. Зарегистрировать `contributes` только для `loaded` и `degraded` plugins. ``` +Таким образом, порядок discovery не влияет на обычные зависимости между +plugins. При конкурирующих providers одного capability порядок discovery служит +tie-breaker. `provides` плагина с неразрешённым `requires` или ошибкой +регистрации не попадают в registry. + ### Capability конфликт -Два plugin не могут предоставлять один и тот же capability. При попытке повторной регистрации — ошибка и статус `failed` для второго плагина. +Два plugin не могут предоставлять один и тот же capability. При попытке повторной регистрации второй plugin получает `failed`; регистрация атомарна, поэтому ни один capability такого plugin не остаётся в registry. ## Plugin Manifest Format @@ -251,7 +259,7 @@ Icon fields use shell icon names rendered through the bundled Lucide SVG wrapper 1. Plugin `Register(pluginID, contributions)` — все contributions регистрируются 2. `Unregister(pluginID)` — удаляет все contributions указанного plugin -3. Reload: `Unregister → Register` (предотвращает дублирование) +3. Reload сначала удаляет contributions всех plugins предыдущего discovery, затем регистрирует только `loaded`/`degraded` plugins 4. Disable plugin → `Unregister` (contributions исчезают из UI) 5. Enable plugin → `Register` при следующем Reload 6. Registry idempotent: Register удаляет старые записи перед добавлением новых @@ -619,13 +627,12 @@ bundled runtime. Это реальный runtime contract для cooperative bun `ReloadPlugins()` в `internal/api/app.go` позволяет перезагрузить plugins без перезапуска приложения: -1. Unregister all non-core capabilities. -2. Re-register core capabilities + vault + workspace (если открыт). -3. Re-scan discovery directories. -4. For each plugin: re-run capability resolution. -5. **Unregister contributions** before re-registering (предотвращает дубли). -6. Register contributions for loaded/degraded plugins (disabled/failed — не регистрируются). -7. Update plugins list. +1. Unregister contributions всех plugins предыдущего discovery. +2. Unregister all non-core capabilities. +3. Re-register core capabilities + vault + workspace (если открыт). +4. Re-scan discovery directories и повторить capability resolution. +5. Register contributions для loaded/degraded plugins (disabled/failed — не регистрируются). +6. Update plugins list. Frontend вызывает это при нажатии "Reload" в Plugin Manager. diff --git a/internal/api/app.go b/internal/api/app.go index cf3b163..2974ec0 100644 --- a/internal/api/app.go +++ b/internal/api/app.go @@ -572,6 +572,13 @@ func (a *App) ReloadPlugins() (int, string) { discoveryDirs := plugin.DefaultDiscoveryDirs() log.Printf("[api] ReloadPlugins: scanning dirs: %v", discoveryDirs) + // Remove entries for plugins that may no longer be discovered. + if a.contribRegistry != nil { + for _, existing := range a.plugins { + a.contribRegistry.Unregister(existing.Manifest.ID) + } + } + // Unregister all non-core capabilities a.capRegistry.UnregisterAll() @@ -605,44 +612,23 @@ func (a *App) ReloadPlugins() (int, string) { plugins, errs := plugin.DiscoverPlugins(discoveryDirs) - // Plugin lifecycle: register capabilities + contributions + plugin.ResolveLifecycle(plugins, a.capRegistry, func(pluginID string) bool { + return a.pluginState != nil && a.pluginState.IsDisabled(pluginID) + }) + + // Register contributions for plugins with a resolved lifecycle. for i := range plugins { p := &plugins[i] - // Skip disabled plugins - if a.pluginState != nil && a.pluginState.IsDisabled(p.Manifest.ID) { - log.Printf("[plugin] %s: disabled in vault plugin state — skipping", p.Manifest.ID) - p.Status = plugin.StatusDisabled - p.Enabled = false - continue - } - - if len(p.Manifest.Provides) > 0 { - if err := a.capRegistry.Register(p.Manifest.ID, p.Manifest.Provides); err != nil { - log.Printf("[plugin] %s: capability registration failed: %v", p.Manifest.ID, err) - p.Status = plugin.StatusFailed - p.Error = err.Error() - continue + if p.Status != plugin.StatusLoaded && p.Status != plugin.StatusDegraded { + if p.Error != "" { + log.Printf("[plugin] %s: status=%s: %s", p.Manifest.ID, p.Status, p.Error) } - } - - missingRequired := a.capRegistry.CheckRequired(p.Manifest.Requires) - if len(missingRequired) > 0 { - p.Status = plugin.StatusMissingRequiredCapability - p.Error = fmt.Sprintf("missing required: %s", strings.Join(missingRequired, ", ")) continue } - missingOptional := a.capRegistry.CheckRequired(p.Manifest.OptionalRequires) - if len(missingOptional) > 0 { - p.Status = plugin.StatusDegraded - } else { - p.Status = plugin.StatusLoaded - } - - // Register contributions (unregister first to prevent duplicates) + // Register contributions after old discovery entries were removed. if p.Manifest.Contributes != nil { - a.contribRegistry.Unregister(p.Manifest.ID) a.contribRegistry.Register(p.Manifest.ID, p.Manifest.Contributes) } diff --git a/internal/core/capability/registry.go b/internal/core/capability/registry.go index 0d00b5c..f5e3a4b 100644 --- a/internal/core/capability/registry.go +++ b/internal/core/capability/registry.go @@ -33,10 +33,18 @@ func (r *Registry) Register(pluginID string, capabilities []string) error { r.mu.Lock() defer r.mu.Unlock() + pending := make(map[string]struct{}, len(capabilities)) for _, name := range capabilities { if existing, ok := r.capabilities[name]; ok { return fmt.Errorf("capability %q already registered by plugin %q", name, existing.PluginID) } + if _, ok := pending[name]; ok { + return fmt.Errorf("capability %q is registered more than once by plugin %q", name, pluginID) + } + pending[name] = struct{}{} + } + + for _, name := range capabilities { r.capabilities[name] = &Entry{ Name: name, PluginID: pluginID, diff --git a/internal/core/capability/registry_test.go b/internal/core/capability/registry_test.go new file mode 100644 index 0000000..8d816e7 --- /dev/null +++ b/internal/core/capability/registry_test.go @@ -0,0 +1,21 @@ +package capability + +import "testing" + +func TestRegisterDoesNotPartiallyRegisterCapabilitiesOnConflict(t *testing.T) { + registry := NewRegistry() + if err := registry.Register("existing.plugin", []string{"shared.capability"}); err != nil { + t.Fatalf("register existing capability: %v", err) + } + + if err := registry.Register("failed.plugin", []string{"new.capability", "shared.capability"}); err == nil { + t.Fatal("Register returned nil for a duplicate capability") + } + if registry.Has("new.capability") { + t.Fatal("Register leaked a capability from the failed registration") + } + entry := registry.Get("shared.capability") + if entry == nil || entry.PluginID != "existing.plugin" { + t.Fatalf("shared capability entry = %#v, want existing.plugin", entry) + } +} diff --git a/internal/core/plugin/lifecycle.go b/internal/core/plugin/lifecycle.go new file mode 100644 index 0000000..285fa12 --- /dev/null +++ b/internal/core/plugin/lifecycle.go @@ -0,0 +1,75 @@ +package plugin + +import ( + "fmt" + "strings" + + "github.com/verstak/verstak-desktop/internal/core/capability" +) + +// ResolveLifecycle registers only viable plugin capabilities and assigns final +// lifecycle statuses after all required dependencies have had a chance to load. +func ResolveLifecycle(plugins []Plugin, registry *capability.Registry, isDisabled func(string) bool) { + if registry == nil { + for i := range plugins { + plugins[i].Status = StatusFailed + plugins[i].Error = "capability registry is unavailable" + } + return + } + + pending := make(map[int]struct{}, len(plugins)) + for i := range plugins { + p := &plugins[i] + p.Error = "" + if !p.Enabled || (isDisabled != nil && isDisabled(p.Manifest.ID)) { + p.Enabled = false + p.Status = StatusDisabled + continue + } + pending[i] = struct{}{} + } + + for len(pending) > 0 { + progressed := false + for i := range plugins { + if _, ok := pending[i]; !ok { + continue + } + p := &plugins[i] + if len(registry.CheckRequired(p.Manifest.Requires)) > 0 { + continue + } + if err := registry.Register(p.Manifest.ID, p.Manifest.Provides); err != nil { + p.Status = StatusFailed + p.Error = err.Error() + } else { + p.Status = StatusLoaded + } + delete(pending, i) + progressed = true + } + if progressed { + continue + } + + for i := range pending { + p := &plugins[i] + missing := registry.CheckRequired(p.Manifest.Requires) + p.Status = StatusMissingRequiredCapability + p.Error = fmt.Sprintf("missing required: %s", strings.Join(missing, ", ")) + } + break + } + + for i := range plugins { + p := &plugins[i] + if p.Status != StatusLoaded { + continue + } + if missing := registry.CheckRequired(p.Manifest.OptionalRequires); len(missing) > 0 { + p.Status = StatusDegraded + p.Error = fmt.Sprintf("missing optional: %s", strings.Join(missing, ", ")) + } + } +} diff --git a/internal/core/plugin/lifecycle_test.go b/internal/core/plugin/lifecycle_test.go index f49c636..68b4b08 100644 --- a/internal/core/plugin/lifecycle_test.go +++ b/internal/core/plugin/lifecycle_test.go @@ -336,3 +336,102 @@ func TestLifecycle_DisabledPlugin(t *testing.T) { t.Errorf("expected plugin ID 'test.lifecycle.disabled', got %q", plugins[0].Manifest.ID) } } + +func TestResolveLifecycleResolvesRequiredCapabilitiesRegardlessOfDiscoveryOrder(t *testing.T) { + reg := capability.NewRegistry() + plugins := []Plugin{ + { + Manifest: Manifest{ + ID: "consumer.plugin", + Provides: []string{"consumer.capability"}, + Requires: []string{"provider.capability"}, + Permissions: []string{"vault.read"}, + }, + Enabled: true, + }, + { + Manifest: Manifest{ + ID: "provider.plugin", + Provides: []string{"provider.capability"}, + Permissions: []string{"vault.read"}, + }, + Enabled: true, + }, + } + + ResolveLifecycle(plugins, reg, nil) + + if plugins[0].Status != StatusLoaded { + t.Fatalf("consumer status = %q, want %q; error=%q", plugins[0].Status, StatusLoaded, plugins[0].Error) + } + if plugins[1].Status != StatusLoaded { + t.Fatalf("provider status = %q, want %q; error=%q", plugins[1].Status, StatusLoaded, plugins[1].Error) + } + if !reg.Has("consumer.capability") || !reg.Has("provider.capability") { + t.Fatalf("resolved capabilities = %#v, want provider and consumer", reg.Available()) + } +} + +func TestResolveLifecycleDoesNotExposeCapabilitiesFromUnresolvedPlugin(t *testing.T) { + reg := capability.NewRegistry() + plugins := []Plugin{ + { + Manifest: Manifest{ + ID: "dependent.plugin", + Provides: []string{"dependent.capability"}, + Requires: []string{"unresolved.capability"}, + Permissions: []string{"vault.read"}, + }, + Enabled: true, + }, + { + Manifest: Manifest{ + ID: "unresolved.plugin", + Provides: []string{"unresolved.capability"}, + Requires: []string{"missing.capability"}, + Permissions: []string{"vault.read"}, + }, + Enabled: true, + }, + } + + ResolveLifecycle(plugins, reg, nil) + + for _, p := range plugins { + if p.Status != StatusMissingRequiredCapability { + t.Fatalf("%s status = %q, want %q; error=%q", p.Manifest.ID, p.Status, StatusMissingRequiredCapability, p.Error) + } + } + if reg.Has("unresolved.capability") || reg.Has("dependent.capability") { + t.Fatalf("unresolved capabilities leaked into registry: %#v", reg.Available()) + } +} + +func TestResolveLifecycleEvaluatesOptionalCapabilitiesAfterProvidersLoad(t *testing.T) { + reg := capability.NewRegistry() + plugins := []Plugin{ + { + Manifest: Manifest{ + ID: "optional.consumer", + Provides: []string{"consumer.capability"}, + OptionalRequires: []string{"optional.capability"}, + Permissions: []string{"vault.read"}, + }, + Enabled: true, + }, + { + Manifest: Manifest{ + ID: "optional.provider", + Provides: []string{"optional.capability"}, + Permissions: []string{"vault.read"}, + }, + Enabled: true, + }, + } + + ResolveLifecycle(plugins, reg, nil) + + if plugins[0].Status != StatusLoaded { + t.Fatalf("optional consumer status = %q, want %q; error=%q", plugins[0].Status, StatusLoaded, plugins[0].Error) + } +} diff --git a/main.go b/main.go index 646f02b..8bb4e8d 100644 --- a/main.go +++ b/main.go @@ -2,10 +2,8 @@ package main import ( "embed" - "fmt" "log" "os" - "strings" "github.com/wailsapp/wails/v2" "github.com/wailsapp/wails/v2/pkg/options" @@ -140,6 +138,9 @@ func main() { if debugEnabled { debug.Logf("[main] starting plugin lifecycle for %d plugins", len(plugins)) } + plugin.ResolveLifecycle(plugins, capRegistry, func(pluginID string) bool { + return pluginStateMgr != nil && pluginStateMgr.IsDisabled(pluginID) + }) for i := range plugins { p := &plugins[i] @@ -147,55 +148,13 @@ func main() { debug.Logf("[main] lifecycle[%d]: id=%s status=%s enabled=%v", i, p.Manifest.ID, p.Status, p.Enabled) } - // Check if plugin is disabled in vault plugin state - if pluginStateMgr != nil && pluginStateMgr.IsDisabled(p.Manifest.ID) { - log.Printf("[plugin] %s: disabled in vault plugin state — skipping", p.Manifest.ID) - if debugEnabled { - debug.Logf("[main] lifecycle: %s disabled in vault state, skipping", p.Manifest.ID) + if p.Status != plugin.StatusLoaded && p.Status != plugin.StatusDegraded { + if p.Error != "" { + log.Printf("[plugin] %s: status=%s: %s", p.Manifest.ID, p.Status, p.Error) } - p.Status = plugin.StatusDisabled - p.Enabled = false continue } - // Register provided capabilities - if len(p.Manifest.Provides) > 0 { - if err := capRegistry.Register(p.Manifest.ID, p.Manifest.Provides); err != nil { - log.Printf("[plugin] %s: capability registration failed: %v", p.Manifest.ID, err) - if debugEnabled { - debug.Logf("[main] lifecycle: %s capability registration failed: %v", p.Manifest.ID, err) - } - p.Status = plugin.StatusFailed - p.Error = err.Error() - continue - } - log.Printf("[plugin] %s: registered %d capabilities", p.Manifest.ID, len(p.Manifest.Provides)) - } - - // Resolve required capabilities - missingRequired := capRegistry.CheckRequired(p.Manifest.Requires) - if len(missingRequired) > 0 { - log.Printf("[plugin] %s: missing required capabilities: %v", p.Manifest.ID, missingRequired) - if debugEnabled { - debug.Logf("[main] lifecycle: %s missing required: %v", p.Manifest.ID, missingRequired) - } - p.Status = plugin.StatusMissingRequiredCapability - p.Error = fmt.Sprintf("missing required: %s", strings.Join(missingRequired, ", ")) - continue - } - - // Check optional capabilities for degraded mode - missingOptional := capRegistry.CheckRequired(p.Manifest.OptionalRequires) - if len(missingOptional) > 0 { - log.Printf("[plugin] %s: missing optional capabilities (degraded): %v", p.Manifest.ID, missingOptional) - if debugEnabled { - debug.Logf("[main] lifecycle: %s missing optional (degraded): %v", p.Manifest.ID, missingOptional) - } - p.Status = plugin.StatusDegraded - } else { - p.Status = plugin.StatusLoaded - } - // Register contributions if p.Manifest.Contributes != nil { contribRegistry.Register(p.Manifest.ID, p.Manifest.Contributes)