diff --git a/cmd/verstak-gui/bindings_plugins.go b/cmd/verstak-gui/bindings_plugins.go index 31b1286..56c28bf 100644 --- a/cmd/verstak-gui/bindings_plugins.go +++ b/cmd/verstak-gui/bindings_plugins.go @@ -115,14 +115,16 @@ func (a *App) SetPluginEnabled(name string, enabled bool) error { } // Activate runtime — if this fails, do NOT persist to config if err := a.plugins.ActivatePlugin(name); err != nil { - // Rollback: deactivate and un-enable + // Rollback: deactivate runtime AND un-enable in-memory state a.plugins.DeactivatePlugin(name) + a.plugins.Disable(name) return fmt.Errorf("activate %q: %w", name, err) } // Only persist to config after successful activation if err := a.saveEnabledPlugin(name); err != nil { // Config save failed — rollback runtime too a.plugins.DeactivatePlugin(name) + a.plugins.Disable(name) return fmt.Errorf("save config for %q: %w", name, err) } } else { diff --git a/cmd/verstak-gui/plugins_lifecycle_test.go b/cmd/verstak-gui/plugins_lifecycle_test.go new file mode 100644 index 0000000..a41775b --- /dev/null +++ b/cmd/verstak-gui/plugins_lifecycle_test.go @@ -0,0 +1,123 @@ +package main + +import ( + "os" + "path/filepath" + "testing" + + "verstak/internal/core/config" + "verstak/internal/core/plugins" +) + +// TestSetPluginEnabled_BrokenPlugin_Rollback verifies that when activation fails, +// the plugin is fully rolled back: not Active, not Enabled, and not in config. +func TestSetPluginEnabled_BrokenPlugin_Rollback(t *testing.T) { + // Isolate global config to a temp dir so we don't pollute the user's real config + tmpCfgDir := filepath.Join(t.TempDir(), "config") + if err := os.MkdirAll(tmpCfgDir, 0o750); err != nil { + t.Fatalf("mkdir config dir: %v", err) + } + t.Setenv("XDG_CONFIG_HOME", tmpCfgDir) + + vaultRoot := t.TempDir() + + // Create .verstak/plugins/ structure + pluginsDir := filepath.Join(vaultRoot, ".verstak", "plugins") + if err := os.MkdirAll(pluginsDir, 0o750); err != nil { + t.Fatalf("mkdir plugins: %v", err) + } + + // Create a broken plugin: valid Lua but invalid background task interval + brokenDir := filepath.Join(pluginsDir, "broken") + if err := os.MkdirAll(brokenDir, 0o750); err != nil { + t.Fatalf("mkdir broken plugin: %v", err) + } + + // plugin.json with an invalid background task interval ("not-a-duration") + pluginJSON := `{ + "name": "broken", + "version": "0.1.0", + "hooks": { + "on_install": "on_install", + "on_init": "on_init" + }, + "background_tasks": [ + {"id": "bad-task", "interval": "not-a-duration", "script": "bad.lua"} + ] + }` + if err := os.WriteFile(filepath.Join(brokenDir, "plugin.json"), []byte(pluginJSON), 0o644); err != nil { + t.Fatalf("write plugin.json: %v", err) + } + + // main.lua — on_install is a no-op (tables not needed for this test), + // on_init is harmless. Activation will fail on the invalid background task interval. + mainLua := ` +function on_install() + -- no-op +end + +function on_init() + -- harmless +end +` + if err := os.WriteFile(filepath.Join(brokenDir, "main.lua"), []byte(mainLua), 0o644); err != nil { + t.Fatalf("write main.lua: %v", err) + } + + // Create App with real plugin manager + app := &App{ + plugins: plugins.NewManager(vaultRoot), + vault: vaultRoot, + vaultOpen: true, + } + + // Discover the plugin + app.plugins.Discover() + + // Step 1: Install the plugin (creates tables, marks installed in config) + if err := app.plugins.Install("broken"); err != nil { + t.Fatalf("install broken plugin: %v", err) + } + + // Verify plugin is installed + if !app.plugins.IsInstalled("broken") { + t.Fatal("expected plugin to be installed") + } + + // Step 2: Try to enable — should fail because background task has invalid interval + err := app.SetPluginEnabled("broken", true) + if err == nil { + t.Fatal("expected SetPluginEnabled to fail for broken plugin, got nil") + } + t.Logf("SetPluginEnabled returned expected error: %v", err) + + // Step 3: Verify plugin is NOT Active and NOT Enabled (in-memory rollback) + found := false + for _, p := range app.plugins.Plugins() { + if p.Meta.Name == "broken" { + found = true + if p.Active { + t.Error("expected plugin to NOT be Active after failed activation") + } + if p.Enabled { + t.Error("expected plugin to NOT be Enabled after failed activation (in-memory rollback)") + } + } + } + if !found { + t.Fatal("plugin 'broken' not found in manager") + } + + // Step 4: Verify plugin is NOT in EnabledPlugins config + appCfg, err := config.LoadAppConfig() + if err != nil { + t.Fatalf("load app config: %v", err) + } + if appCfg != nil { + for _, name := range appCfg.EnabledPlugins { + if name == "broken" { + t.Error("expected 'broken' to NOT be in EnabledPlugins config after failed activation") + } + } + } +} diff --git a/internal/core/plugins/manager_lifecycle.go b/internal/core/plugins/manager_lifecycle.go index bef96cf..e0f887e 100644 --- a/internal/core/plugins/manager_lifecycle.go +++ b/internal/core/plugins/manager_lifecycle.go @@ -12,6 +12,7 @@ import ( // ActivatePlugin fully activates a plugin: creates Lua VM, loads main.lua, starts scheduler. // Only works if plugin is installed and enabled but not yet active. // Returns error if VM creation, script loading, scheduler setup, or on_init hook fails. +// on_init failure is fatal — the plugin cannot run without proper initialization. func (m *Manager) ActivatePlugin(name string) error { for i := range m.plugins { p := &m.plugins[i] @@ -56,8 +57,12 @@ func (m *Manager) ActivatePlugin(name string) error { if hookName, ok := p.Meta.Hooks["on_init"]; ok { if err := vm.CallHook(hookName); err != nil { - // on_init failure is non-fatal for activation — log but continue - log.Printf("[plugins] %s: on_init error: %v", name, err) + // on_init failure is fatal — rollback activation + p.scheduler.Stop() + p.scheduler = nil + p.vm.Close() + p.vm = nil + return fmt.Errorf("on_init for %q: %w", name, err) } }