diff --git a/cmd/server/main.go b/cmd/server/main.go index 04858923..cc8062f2 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -83,6 +83,7 @@ var ( anthropicModel = flag.String("anthropic-model", "", "Anthropic model name") // Multi-workflow mode flags + multiWorkflowAddr = flag.String("multi-workflow-addr", ":8080", "Listen address for the multi-workflow API server") databaseDSN = flag.String("database-dsn", "", "PostgreSQL connection string for multi-workflow mode") jwtSecret = flag.String("jwt-secret", "", "JWT signing secret for API authentication") adminEmail = flag.String("admin-email", "", "Initial admin user email (first-run bootstrap)") diff --git a/engine.go b/engine.go index 6c8e9f61..d5dda5d5 100644 --- a/engine.go +++ b/engine.go @@ -57,15 +57,19 @@ type StdEngine struct { moduleFactories map[string]ModuleFactory logger modular.Logger modules []modular.Module - triggers []module.Trigger - triggerRegistry *module.TriggerRegistry + triggers []interfaces.Trigger + triggerRegistry interfaces.TriggerRegistrar dynamicRegistry *dynamic.ComponentRegistry dynamicLoader *dynamic.Loader - eventEmitter *module.WorkflowEventEmitter + eventEmitter interfaces.EventEmitter secretsResolver *secrets.MultiResolver - stepRegistry *module.StepRegistry - pluginInstaller *plugin.PluginInstaller - configDir string // directory of the config file, for resolving relative paths + // stepRegistry is a concrete *module.StepRegistry because StepFactory and + // PipelineStep are module-level types not yet abstracted in interfaces. + // TODO(phase5): move StepFactory/PipelineStep to interfaces and change this + // field to interfaces.StepRegistrar. + stepRegistry *module.StepRegistry + pluginInstaller *plugin.PluginInstaller + configDir string // directory of the config file, for resolving relative paths // triggerTypeMap maps trigger config type keys (e.g., "http", "schedule") // to trigger names (e.g., "trigger.http", "trigger.schedule"). Populated @@ -129,10 +133,10 @@ func NewStdEngine(app modular.Application, logger modular.Logger) *StdEngine { moduleFactories: make(map[string]ModuleFactory), logger: logger, modules: make([]modular.Module, 0), - triggers: make([]module.Trigger, 0), - triggerRegistry: module.NewTriggerRegistry(), + triggers: make([]interfaces.Trigger, 0), + triggerRegistry: newTriggerRegistrar(), // bridge: returns *module.TriggerRegistry secretsResolver: secrets.NewMultiResolver(), - stepRegistry: module.NewStepRegistry(), + stepRegistry: newStepRegistry(), // bridge: returns *module.StepRegistry triggerTypeMap: make(map[string]string), triggerConfigWrappers: make(map[string]plugin.TriggerConfigWrapperFunc), pipelineRegistry: make(map[string]*module.Pipeline), @@ -159,7 +163,7 @@ func (e *StdEngine) RegisterWorkflowHandler(handler WorkflowHandler) { } // RegisterTrigger registers a trigger with the engine -func (e *StdEngine) RegisterTrigger(trigger module.Trigger) { +func (e *StdEngine) RegisterTrigger(trigger interfaces.Trigger) { e.triggers = append(e.triggers, trigger) e.triggerRegistry.RegisterTrigger(trigger) } @@ -220,26 +224,17 @@ func (e *StdEngine) LoadPlugin(p plugin.EnginePlugin) error { schema.RegisterModuleType(typeName) } for typeName, factory := range p.StepFactories() { - stepFactory := factory - capturedType := typeName - e.stepRegistry.Register(typeName, func(name string, cfg map[string]any, app modular.Application) (module.PipelineStep, error) { - result, err := stepFactory(name, cfg, app) - if err != nil { - return nil, err - } - if step, ok := result.(module.PipelineStep); ok { - return step, nil - } - return nil, fmt.Errorf("step factory for %q returned non-PipelineStep type", capturedType) - }) + // Delegate to the bridge helper which type-asserts to module.PipelineStep + // so that engine.go need not reference that concrete type directly. + e.registerPluginSteps(typeName, factory) } // Register triggers from plugin. The factory map key is the trigger // config type (e.g., "http", "schedule") used in YAML configs. for triggerType, factory := range p.TriggerFactories() { - result := factory() - if trigger, ok := result.(module.Trigger); ok { - e.triggerTypeMap[triggerType] = trigger.Name() - e.RegisterTrigger(trigger) + // Delegate to the bridge helper; triggers are interfaces.Trigger values + // (module.Trigger is a type alias for interfaces.Trigger). + if err := e.registerPluginTrigger(triggerType, factory); err != nil { + return fmt.Errorf("load plugin: %w", err) } } @@ -404,8 +399,8 @@ func (e *StdEngine) BuildFromConfig(cfg *config.WorkflowConfig) error { e.logger.Debug("Loaded service: " + name) } - // Initialize the workflow event emitter - e.eventEmitter = module.NewWorkflowEventEmitter(e.app) + // Initialize the workflow event emitter via bridge (avoids direct module dep). + e.eventEmitter = newEventEmitter(e.app) // Register config section for workflow e.app.RegisterConfigSection("workflow", modular.NewStdConfigProvider(cfg)) @@ -590,14 +585,9 @@ func (e *StdEngine) TriggerWorkflow(ctx context.Context, workflowType string, ac return fmt.Errorf("no handler found for workflow type: %s", workflowType) } -// recordWorkflowMetrics records workflow execution metrics if the metrics collector is available. -func (e *StdEngine) recordWorkflowMetrics(workflowType, action, status string, duration time.Duration) { - var mc *module.MetricsCollector - if err := e.app.GetService("metrics.collector", &mc); err == nil && mc != nil { - mc.RecordWorkflowExecution(workflowType, action, status) - mc.RecordWorkflowDuration(workflowType, action, duration) - } -} +// recordWorkflowMetrics is defined in engine_module_bridge.go. +// It records execution metrics via interfaces.MetricsRecorder so that engine.go +// need not reference the concrete *module.MetricsCollector type. // configureTriggers sets up all triggers from configuration func (e *StdEngine) configureTriggers(triggerConfigs map[string]any) error { @@ -638,7 +628,7 @@ func (e *StdEngine) configureTriggers(triggerConfigs map[string]any) error { // by looking up the trigger type in the engine's registry. Falls back to // matching the trigger name directly (e.g., trigger type "mock" matches // trigger name "mock.trigger" via "trigger." convention). -func (e *StdEngine) canHandleTrigger(trigger module.Trigger, triggerType string) bool { +func (e *StdEngine) canHandleTrigger(trigger interfaces.Trigger, triggerType string) bool { // Check the trigger type registry first (populated by LoadPlugin and RegisterTriggerType) if expectedName, ok := e.triggerTypeMap[triggerType]; ok { return trigger.Name() == expectedName @@ -927,7 +917,7 @@ func (e *StdEngine) LoadedPlugins() []plugin.EnginePlugin { type Engine interface { RegisterWorkflowHandler(handler WorkflowHandler) - RegisterTrigger(trigger module.Trigger) + RegisterTrigger(trigger interfaces.Trigger) AddModuleType(moduleType string, factory ModuleFactory) BuildFromConfig(cfg *config.WorkflowConfig) error Start(ctx context.Context) error diff --git a/engine_module_bridge.go b/engine_module_bridge.go new file mode 100644 index 00000000..c1b2ca7e --- /dev/null +++ b/engine_module_bridge.go @@ -0,0 +1,100 @@ +package workflow + +// engine_module_bridge.go bridges the engine core (engine.go) with concrete +// module implementations that cannot yet be abstracted away without a larger +// refactor. This file intentionally imports the module package so that +// engine.go itself need not import it for these specific operations. +// +// Remaining blockers preventing full engine.go ↔ module decoupling: +// +// 1. module.StepRegistry / module.StepFactory / module.PipelineStep — the +// StepFactory signature returns (PipelineStep, error), and PipelineStep.Execute +// takes *PipelineContext, both of which are concrete types in the module package. +// Moving them to interfaces would require updating 70+ files in module/. Deferred. +// +// 2. module.Pipeline struct construction (configurePipelines, +// configureRoutePipelines, buildPipelineSteps) — depends on module.PipelineStep +// slices and module.ErrorStrategy constants. These functions live in engine.go +// and are candidates for moving here once StepFactory is abstracted. +// +// What HAS been cleaned up in this phase: +// - module.Trigger → interfaces.Trigger (type alias in module; engine uses interfaces) +// - module.WorkflowEventEmitter → interfaces.EventEmitter (engine field + bridge ctor) +// - module.TriggerRegistry → interfaces.TriggerRegistrar (engine field + bridge ctor) +// - recordWorkflowMetrics → uses interfaces.MetricsRecorder (no concrete *MetricsCollector) +// - Plugin step/trigger wiring → bridge helpers (registerPluginSteps, registerPluginTrigger) + +import ( + "fmt" + "time" + + "github.com/CrisisTextLine/modular" + "github.com/GoCodeAlone/workflow/interfaces" + "github.com/GoCodeAlone/workflow/module" +) + +// newTriggerRegistrar creates the default concrete trigger registry. +// Called from NewStdEngine so that engine.go need not import module. +func newTriggerRegistrar() interfaces.TriggerRegistrar { + return module.NewTriggerRegistry() +} + +// newStepRegistry creates the default concrete step registry. +// *module.StepRegistry satisfies interfaces.StepRegistryProvider and is used +// as a concrete type in engine.go until StepFactory is fully abstracted. +func newStepRegistry() *module.StepRegistry { + return module.NewStepRegistry() +} + +// newEventEmitter creates a WorkflowEventEmitter from the application service +// registry. Called from BuildFromConfig after app.Init(). +func newEventEmitter(app modular.Application) interfaces.EventEmitter { + return module.NewWorkflowEventEmitter(app) +} + +// recordWorkflowMetrics records workflow execution metrics if the metrics +// collector service is available. Uses interfaces.MetricsRecorder so that +// engine.go need not hold a concrete *module.MetricsCollector pointer. +func (e *StdEngine) recordWorkflowMetrics(workflowType, action, status string, duration time.Duration) { + var mr interfaces.MetricsRecorder + if err := e.app.GetService("metrics.collector", &mr); err == nil && mr != nil { + mr.RecordWorkflowExecution(workflowType, action, status) + mr.RecordWorkflowDuration(workflowType, action, duration) + } +} + +// registerPluginSteps wires step factories from a plugin into the engine's +// step registry. Lives here (instead of LoadPlugin in engine.go) because it +// type-asserts the factory result to module.PipelineStep. +func (e *StdEngine) registerPluginSteps(typeName string, stepFactory func(name string, cfg map[string]any, app modular.Application) (any, error)) { + capturedType := typeName + e.stepRegistry.Register(typeName, func(name string, cfg map[string]any, app modular.Application) (module.PipelineStep, error) { + result, err := stepFactory(name, cfg, app) + if err != nil { + return nil, err + } + if step, ok := result.(module.PipelineStep); ok { + return step, nil + } + return nil, fmt.Errorf("step factory for %q returned non-PipelineStep type", capturedType) + }) +} + +// registerPluginTrigger wires a trigger from a plugin into the engine. +// Lives here to avoid a direct module.Trigger type assertion in engine.go. +// Since module.Trigger is now an alias for interfaces.Trigger, the assertion +// uses the canonical interface type. +// Returns an error when the factory returns a value that does not satisfy +// interfaces.Trigger, so LoadPlugin can fail deterministically instead of +// silently skipping the trigger and surfacing a confusing "no handler found" +// error later at runtime. +func (e *StdEngine) registerPluginTrigger(triggerType string, factory func() any) error { + result := factory() + trigger, ok := result.(interfaces.Trigger) + if !ok { + return fmt.Errorf("workflow: plugin trigger factory for %q returned non-Trigger type %T", triggerType, result) + } + e.triggerTypeMap[triggerType] = trigger.Name() + e.RegisterTrigger(trigger) + return nil +} diff --git a/engine_test.go b/engine_test.go index 96740c64..c9d499ae 100644 --- a/engine_test.go +++ b/engine_test.go @@ -17,6 +17,7 @@ import ( "github.com/GoCodeAlone/workflow/handlers" "github.com/GoCodeAlone/workflow/mock" "github.com/GoCodeAlone/workflow/module" + "github.com/GoCodeAlone/workflow/plugin" ) // setupEngineTest creates an isolated test environment for engine tests @@ -269,7 +270,9 @@ func (a *mockApplication) Init() error { return nil } -// GetService retrieves a service by name and populates the out parameter if provided +// GetService retrieves a service by name and populates the out parameter if provided. +// Supports both direct type assignment and interface satisfaction checks (matching +// the behaviour of the real modular.StdApplication.GetService). func (a *mockApplication) GetService(name string, out any) error { svc, ok := a.services[name] if !ok { @@ -278,10 +281,15 @@ func (a *mockApplication) GetService(name string, out any) error { // If out is provided, try to assign the service to it using reflection if out != nil { + // Guard: nil service value would make reflect.ValueOf(svc).Type() panic. + if svc == nil { + return fmt.Errorf("service %s has a nil value", name) + } + // Get reflect values outVal := reflect.ValueOf(out) - if outVal.Kind() != reflect.Pointer { - return fmt.Errorf("out parameter must be a pointer") + if outVal.Kind() != reflect.Pointer || outVal.IsNil() { + return fmt.Errorf("out parameter must be a non-nil pointer") } // Dereference the pointer @@ -290,14 +298,24 @@ func (a *mockApplication) GetService(name string, out any) error { return fmt.Errorf("out parameter cannot be set") } - // Set the value if compatible svcVal := reflect.ValueOf(svc) - if !svcVal.Type().AssignableTo(outVal.Type()) { - return fmt.Errorf("service type %s not assignable to out parameter type %s", - svcVal.Type(), outVal.Type()) + svcType := svcVal.Type() + targetType := outVal.Type() + + // Case 1: target is an interface that the service implements + if targetType.Kind() == reflect.Interface && svcType.Implements(targetType) { + outVal.Set(svcVal) + return nil } - outVal.Set(svcVal) + // Case 2: direct type assignment + if svcType.AssignableTo(targetType) { + outVal.Set(svcVal) + return nil + } + + return fmt.Errorf("service type %s not assignable to out parameter type %s", + svcType, targetType) } return nil @@ -1249,3 +1267,328 @@ func TestCanHandleTrigger_EventBus(t *testing.T) { t.Errorf("canHandleTrigger(%q, %q) = false, want true", module.EventBusTriggerName, "eventbus") } } + +// ============================================================================ +// Tests for requires.plugins validation (Phase 4 - Engine Decomposition) +// ============================================================================ + +// minimalPlugin is a test-only EnginePlugin with a controllable name/version. +type minimalPlugin struct { + plugin.BaseEnginePlugin +} + +func newMinimalPlugin(name, version string) *minimalPlugin { + return &minimalPlugin{ + BaseEnginePlugin: plugin.BaseEnginePlugin{ + BaseNativePlugin: plugin.BaseNativePlugin{ + PluginName: name, + PluginVersion: version, + }, + Manifest: plugin.PluginManifest{ + Name: name, + Version: version, + Author: "test", + Description: "test plugin for requires validation", + }, + }, + } +} + +// TestEngine_BuildFromConfig_RequiresPlugins_NilRequires verifies that a +// config with no requires section is accepted without error. +func TestEngine_BuildFromConfig_RequiresPlugins_NilRequires(t *testing.T) { + app := newMockApplication() + engine := NewStdEngine(app, app.Logger()) + + cfg := &config.WorkflowConfig{ + Modules: []config.ModuleConfig{}, + Workflows: map[string]any{}, + Triggers: map[string]any{}, + Requires: nil, + } + + if err := engine.BuildFromConfig(cfg); err != nil { + t.Fatalf("expected no error for nil Requires, got: %v", err) + } +} + +// TestEngine_BuildFromConfig_RequiresPlugins_PluginLoaded verifies that a +// required plugin that is loaded passes validation. +func TestEngine_BuildFromConfig_RequiresPlugins_PluginLoaded(t *testing.T) { + app := newMockApplication() + engine := NewStdEngine(app, app.Logger()) + + // Load a minimal plugin with a known name. + p := newMinimalPlugin("my-plugin", "1.2.3") + if err := engine.LoadPlugin(p); err != nil { + t.Fatalf("LoadPlugin failed: %v", err) + } + + cfg := &config.WorkflowConfig{ + Modules: []config.ModuleConfig{}, + Workflows: map[string]any{}, + Triggers: map[string]any{}, + Requires: &config.RequiresConfig{ + Plugins: []config.PluginRequirement{ + {Name: "my-plugin"}, + }, + }, + } + + if err := engine.BuildFromConfig(cfg); err != nil { + t.Fatalf("expected no error when required plugin is loaded, got: %v", err) + } +} + +// TestEngine_BuildFromConfig_RequiresPlugins_PluginNotLoaded verifies that a +// required plugin that is NOT loaded produces a clear error. +func TestEngine_BuildFromConfig_RequiresPlugins_PluginNotLoaded(t *testing.T) { + app := newMockApplication() + engine := NewStdEngine(app, app.Logger()) + // Do NOT load "missing-plugin" — only load an unrelated one. + p := newMinimalPlugin("other-plugin", "1.0.0") + if err := engine.LoadPlugin(p); err != nil { + t.Fatalf("LoadPlugin failed: %v", err) + } + + cfg := &config.WorkflowConfig{ + Modules: []config.ModuleConfig{}, + Workflows: map[string]any{}, + Triggers: map[string]any{}, + Requires: &config.RequiresConfig{ + Plugins: []config.PluginRequirement{ + {Name: "missing-plugin"}, + }, + }, + } + + err := engine.BuildFromConfig(cfg) + if err == nil { + t.Fatal("expected error when required plugin is not loaded") + } + if !strings.Contains(err.Error(), "missing-plugin") { + t.Errorf("expected error to mention missing plugin name, got: %v", err) + } +} + +// TestEngine_BuildFromConfig_RequiresPlugins_VersionSatisfied verifies that +// a semver constraint that IS satisfied passes validation. +func TestEngine_BuildFromConfig_RequiresPlugins_VersionSatisfied(t *testing.T) { + app := newMockApplication() + engine := NewStdEngine(app, app.Logger()) + + p := newMinimalPlugin("versioned-plugin", "2.5.1") + if err := engine.LoadPlugin(p); err != nil { + t.Fatalf("LoadPlugin failed: %v", err) + } + + cfg := &config.WorkflowConfig{ + Modules: []config.ModuleConfig{}, + Workflows: map[string]any{}, + Triggers: map[string]any{}, + Requires: &config.RequiresConfig{ + Plugins: []config.PluginRequirement{ + {Name: "versioned-plugin", Version: ">=2.0.0"}, + }, + }, + } + + if err := engine.BuildFromConfig(cfg); err != nil { + t.Fatalf("expected no error for satisfied version constraint, got: %v", err) + } +} + +// TestEngine_BuildFromConfig_RequiresPlugins_VersionNotSatisfied verifies that +// a semver constraint that is NOT satisfied returns a meaningful error. +func TestEngine_BuildFromConfig_RequiresPlugins_VersionNotSatisfied(t *testing.T) { + app := newMockApplication() + engine := NewStdEngine(app, app.Logger()) + + p := newMinimalPlugin("versioned-plugin", "1.0.0") + if err := engine.LoadPlugin(p); err != nil { + t.Fatalf("LoadPlugin failed: %v", err) + } + + cfg := &config.WorkflowConfig{ + Modules: []config.ModuleConfig{}, + Workflows: map[string]any{}, + Triggers: map[string]any{}, + Requires: &config.RequiresConfig{ + Plugins: []config.PluginRequirement{ + {Name: "versioned-plugin", Version: ">=2.0.0"}, + }, + }, + } + + err := engine.BuildFromConfig(cfg) + if err == nil { + t.Fatal("expected error for unsatisfied version constraint") + } + if !strings.Contains(err.Error(), "versioned-plugin") { + t.Errorf("expected error to mention plugin name, got: %v", err) + } + if !strings.Contains(err.Error(), ">=2.0.0") { + t.Errorf("expected error to mention version constraint, got: %v", err) + } +} + +// TestEngine_BuildFromConfig_RequiresPlugins_NoVersionConstraint verifies that +// a plugin requirement with no version constraint matches any loaded version. +func TestEngine_BuildFromConfig_RequiresPlugins_NoVersionConstraint(t *testing.T) { + app := newMockApplication() + engine := NewStdEngine(app, app.Logger()) + + p := newMinimalPlugin("any-version-plugin", "99.0.0") + if err := engine.LoadPlugin(p); err != nil { + t.Fatalf("LoadPlugin failed: %v", err) + } + + cfg := &config.WorkflowConfig{ + Modules: []config.ModuleConfig{}, + Workflows: map[string]any{}, + Triggers: map[string]any{}, + Requires: &config.RequiresConfig{ + Plugins: []config.PluginRequirement{ + {Name: "any-version-plugin"}, // no Version constraint + }, + }, + } + + if err := engine.BuildFromConfig(cfg); err != nil { + t.Fatalf("expected no error when no version constraint is set, got: %v", err) + } +} + +// TestEngine_BuildFromConfig_RequiresPlugins_MultiplePlugins verifies that all +// plugins in the requires list must be loaded. +func TestEngine_BuildFromConfig_RequiresPlugins_MultiplePlugins(t *testing.T) { + app := newMockApplication() + engine := NewStdEngine(app, app.Logger()) + + // Load two of three required plugins. + for _, name := range []string{"plugin-alpha", "plugin-beta"} { + if err := engine.LoadPlugin(newMinimalPlugin(name, "1.0.0")); err != nil { + t.Fatalf("LoadPlugin(%s) failed: %v", name, err) + } + } + // "plugin-gamma" is intentionally NOT loaded. + + cfg := &config.WorkflowConfig{ + Modules: []config.ModuleConfig{}, + Workflows: map[string]any{}, + Triggers: map[string]any{}, + Requires: &config.RequiresConfig{ + Plugins: []config.PluginRequirement{ + {Name: "plugin-alpha"}, + {Name: "plugin-beta"}, + {Name: "plugin-gamma"}, + }, + }, + } + + err := engine.BuildFromConfig(cfg) + if err == nil { + t.Fatal("expected error when one of multiple required plugins is missing") + } + if !strings.Contains(err.Error(), "plugin-gamma") { + t.Errorf("expected error to mention the missing plugin 'plugin-gamma', got: %v", err) + } +} + +// TestEngine_BuildFromConfig_RequiresPlugins_NoPluginLoaderFails verifies that +// if the plugin loader is nil but plugins are required, the validation is skipped +// (not a hard failure — engine must be usable without a plugin loader). +func TestEngine_BuildFromConfig_RequiresPlugins_NoPluginLoader(t *testing.T) { + app := newMockApplication() + engine := NewStdEngine(app, app.Logger()) + // Explicitly do not set a plugin loader (pluginLoader stays nil). + + cfg := &config.WorkflowConfig{ + Modules: []config.ModuleConfig{}, + Workflows: map[string]any{}, + Triggers: map[string]any{}, + Requires: &config.RequiresConfig{ + Plugins: []config.PluginRequirement{ + {Name: "some-plugin"}, + }, + }, + } + + // When no plugin loader is configured, requires.plugins validation is + // skipped (pluginLoader == nil guard in validateRequirements). + // This is intentional: simple engines that don't use LoadPlugin() should + // not be broken by a requires section. + if err := engine.BuildFromConfig(cfg); err != nil { + t.Fatalf("expected no error when plugin loader is nil, got: %v", err) + } +} + +// TestEngine_BuildFromConfig_RequiresPlugins_EmptyPluginsList verifies that +// an empty plugins list in requires is handled gracefully. +func TestEngine_BuildFromConfig_RequiresPlugins_EmptyPluginsList(t *testing.T) { + app := newMockApplication() + engine := NewStdEngine(app, app.Logger()) + if err := engine.LoadPlugin(newMinimalPlugin("some-plugin", "1.0.0")); err != nil { + t.Fatalf("LoadPlugin failed: %v", err) + } + + cfg := &config.WorkflowConfig{ + Modules: []config.ModuleConfig{}, + Workflows: map[string]any{}, + Triggers: map[string]any{}, + Requires: &config.RequiresConfig{ + Plugins: []config.PluginRequirement{}, // empty list + }, + } + + if err := engine.BuildFromConfig(cfg); err != nil { + t.Fatalf("expected no error for empty plugins list, got: %v", err) + } +} + +// TestEngine_BuildFromConfig_RequiresPlugins_ExactVersionMatch verifies +// exact version matching (=1.2.3 constraint). +func TestEngine_BuildFromConfig_RequiresPlugins_ExactVersionMatch(t *testing.T) { + tests := []struct { + constraint string + wantOK bool + }{ + {"=1.2.3", true}, + {"=1.2.4", false}, + {">=1.0.0", true}, + {">=2.0.0", false}, + {"<2.0.0", true}, + {"<1.0.0", false}, + } + + for _, tt := range tests { + t.Run(tt.constraint, func(t *testing.T) { + cfg := &config.WorkflowConfig{ + Modules: []config.ModuleConfig{}, + Workflows: map[string]any{}, + Triggers: map[string]any{}, + Requires: &config.RequiresConfig{ + Plugins: []config.PluginRequirement{ + {Name: "exact-plugin", Version: tt.constraint}, + }, + }, + } + + // Use a fresh app and engine (with a freshly loaded plugin) for each + // sub-test to avoid state contamination. + subApp := newMockApplication() + subEngine := NewStdEngine(subApp, subApp.Logger()) + subP := newMinimalPlugin("exact-plugin", "1.2.3") + if err := subEngine.LoadPlugin(subP); err != nil { + t.Fatalf("LoadPlugin failed: %v", err) + } + + err := subEngine.BuildFromConfig(cfg) + if tt.wantOK && err != nil { + t.Errorf("expected no error for constraint %q, got: %v", tt.constraint, err) + } else if !tt.wantOK && err == nil { + t.Errorf("expected error for constraint %q but got none", tt.constraint) + } + }) + } +} diff --git a/interfaces/events.go b/interfaces/events.go new file mode 100644 index 00000000..74f52057 --- /dev/null +++ b/interfaces/events.go @@ -0,0 +1,23 @@ +package interfaces + +import ( + "context" + "time" +) + +// EventEmitter publishes workflow lifecycle events. +// *module.WorkflowEventEmitter satisfies this interface. +// All methods must be safe to call when no event bus is configured (no-ops). +type EventEmitter interface { + EmitWorkflowStarted(ctx context.Context, workflowType, action string, data map[string]any) + EmitWorkflowCompleted(ctx context.Context, workflowType, action string, duration time.Duration, results map[string]any) + EmitWorkflowFailed(ctx context.Context, workflowType, action string, duration time.Duration, err error) +} + +// MetricsRecorder records workflow execution metrics. +// *module.MetricsCollector satisfies this interface. +// All methods must be safe to call when no metrics backend is configured (no-ops). +type MetricsRecorder interface { + RecordWorkflowExecution(workflowType, action, status string) + RecordWorkflowDuration(workflowType, action string, duration time.Duration) +} diff --git a/interfaces/trigger.go b/interfaces/trigger.go new file mode 100644 index 00000000..997878df --- /dev/null +++ b/interfaces/trigger.go @@ -0,0 +1,25 @@ +package interfaces + +import "github.com/CrisisTextLine/modular" + +// Trigger defines what can start a workflow execution. +// Moving this interface here breaks the engine→module import dependency while +// preserving backward compatibility via the type alias in the module package. +// +// *module.HTTPTrigger, *module.ScheduleTrigger, and other concrete trigger +// implementations all satisfy this interface. +type Trigger interface { + modular.Module + modular.Startable + modular.Stoppable + + // Configure sets up the trigger from configuration. + Configure(app modular.Application, triggerConfig any) error +} + +// TriggerRegistrar manages registered triggers. +// *module.TriggerRegistry satisfies this interface. +type TriggerRegistrar interface { + // RegisterTrigger adds a trigger to the registry. + RegisterTrigger(trigger Trigger) +} diff --git a/module/trigger.go b/module/trigger.go index 663a8789..8262f091 100644 --- a/module/trigger.go +++ b/module/trigger.go @@ -1,20 +1,17 @@ package module import ( - "github.com/CrisisTextLine/modular" + "github.com/GoCodeAlone/workflow/interfaces" ) -// Trigger defines what can start a workflow execution -type Trigger interface { - modular.Module - modular.Startable - modular.Stoppable +// Trigger is a type alias for interfaces.Trigger. +// The canonical definition lives in the interfaces package so that the engine +// and other packages can reference it without importing this module package. +// All existing code using module.Trigger is unaffected by this alias. +type Trigger = interfaces.Trigger - // Configure sets up the trigger from configuration - Configure(app modular.Application, triggerConfig any) error -} - -// TriggerRegistry manages registered triggers and allows finding them by name +// TriggerRegistry manages registered triggers and allows finding them by name. +// It satisfies interfaces.TriggerRegistrar. type TriggerRegistry struct { triggers map[string]Trigger } diff --git a/platform/providers/aws/drivers/rds.go b/platform/providers/aws/drivers/rds.go index 52546027..4abfb9aa 100644 --- a/platform/providers/aws/drivers/rds.go +++ b/platform/providers/aws/drivers/rds.go @@ -56,7 +56,7 @@ func (d *RDSDriver) Create(ctx context.Context, name string, properties map[stri } masterPass, _ := properties["master_password"].(string) if masterPass == "" { - masterPass = "changeme123!" // Would be from secrets in production + return nil, fmt.Errorf("rds: create %q: master_password is required", name) } input := &rds.CreateDBInstanceInput{ diff --git a/platform/providers/aws/drivers/rds_test.go b/platform/providers/aws/drivers/rds_test.go index 1532939c..640ee5ed 100644 --- a/platform/providers/aws/drivers/rds_test.go +++ b/platform/providers/aws/drivers/rds_test.go @@ -96,6 +96,7 @@ func TestRDSDriver_Create(t *testing.T) { "instance_class": "db.r5.large", "allocated_storage": 50, "multi_az": true, + "master_password": "s3cureP@ss!", }) if err != nil { t.Fatalf("Create() error: %v", err) @@ -111,6 +112,18 @@ func TestRDSDriver_Create(t *testing.T) { } } +func TestRDSDriver_CreateMissingPassword(t *testing.T) { + d := NewRDSDriverWithClient(&mockRDSClient{}) + ctx := context.Background() + + _, err := d.Create(ctx, "test-db", map[string]any{ + "engine": "postgres", + }) + if err == nil { + t.Fatal("expected error for missing master_password") + } +} + func TestRDSDriver_Read(t *testing.T) { d := NewRDSDriverWithClient(&mockRDSClient{}) ctx := context.Background()