fix: rollback Enabled on activation failure + fatal on_init + rollback test
1. SetPluginEnabled(true): after DeactivatePlugin, also call Disable(name) to rollback in-memory Enabled state (not just config). 2. on_init failure is now fatal for ActivatePlugin — returns error and rolls back scheduler + VM (was incorrectly non-fatal). 3. TestSetPluginEnabled_BrokenPlugin_Rollback: end-to-end test with broken plugin (invalid interval), verifies error + not Active + not Enabled + not in config.
This commit is contained in:
parent
45cfe1b0a6
commit
3b79754f45
|
|
@ -115,14 +115,16 @@ func (a *App) SetPluginEnabled(name string, enabled bool) error {
|
||||||
}
|
}
|
||||||
// Activate runtime — if this fails, do NOT persist to config
|
// Activate runtime — if this fails, do NOT persist to config
|
||||||
if err := a.plugins.ActivatePlugin(name); err != nil {
|
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.DeactivatePlugin(name)
|
||||||
|
a.plugins.Disable(name)
|
||||||
return fmt.Errorf("activate %q: %w", name, err)
|
return fmt.Errorf("activate %q: %w", name, err)
|
||||||
}
|
}
|
||||||
// Only persist to config after successful activation
|
// Only persist to config after successful activation
|
||||||
if err := a.saveEnabledPlugin(name); err != nil {
|
if err := a.saveEnabledPlugin(name); err != nil {
|
||||||
// Config save failed — rollback runtime too
|
// Config save failed — rollback runtime too
|
||||||
a.plugins.DeactivatePlugin(name)
|
a.plugins.DeactivatePlugin(name)
|
||||||
|
a.plugins.Disable(name)
|
||||||
return fmt.Errorf("save config for %q: %w", name, err)
|
return fmt.Errorf("save config for %q: %w", name, err)
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
|
|
|
||||||
|
|
@ -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")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -12,6 +12,7 @@ import (
|
||||||
// ActivatePlugin fully activates a plugin: creates Lua VM, loads main.lua, starts scheduler.
|
// 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.
|
// 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.
|
// 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 {
|
func (m *Manager) ActivatePlugin(name string) error {
|
||||||
for i := range m.plugins {
|
for i := range m.plugins {
|
||||||
p := &m.plugins[i]
|
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 hookName, ok := p.Meta.Hooks["on_init"]; ok {
|
||||||
if err := vm.CallHook(hookName); err != nil {
|
if err := vm.CallHook(hookName); err != nil {
|
||||||
// on_init failure is non-fatal for activation — log but continue
|
// on_init failure is fatal — rollback activation
|
||||||
log.Printf("[plugins] %s: on_init error: %v", name, err)
|
p.scheduler.Stop()
|
||||||
|
p.scheduler = nil
|
||||||
|
p.vm.Close()
|
||||||
|
p.vm = nil
|
||||||
|
return fmt.Errorf("on_init for %q: %w", name, err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue