refactor: extract Trigger, EventEmitter, MetricsRecorder interfaces from module (#84) by intel352 · Pull Request #136 · GoCodeAlone/workflow

intel352 · 2026-02-23T12:21:50Z

Summary

Phase 4 engine decomposition — move interface types from module/ to interfaces/ to break the engine→module dependency:

interfaces/trigger.go: Trigger interface + TriggerRegistrar (satisfied by *module.TriggerRegistry)
interfaces/events.go: EventEmitter (satisfied by *module.WorkflowEventEmitter) + MetricsRecorder (satisfied by *module.MetricsCollector)
engine_module_bridge.go: Isolates remaining module/ imports into factory functions, with doc explaining Phase 5 blockers
engine.go: Fields now use interface types; RegisterTrigger takes interfaces.Trigger
module/trigger.go: type Trigger = interfaces.Trigger (type alias, backward compatible)
10 new tests for requires.plugins validation, mock GetService fixed for interface assignment

What's done

Trigger, EventEmitter, MetricsRecorder interfaces extracted to interfaces/
engine.go field types updated to interfaces
Bridge file isolates remaining module dependency
requires.plugins validation thoroughly tested

What remains (Phase 5)

StepRegistry/StepFactory/PipelineStep/Pipeline still in module/ (70+ files would need updating)

Closes #84 (partial)

Test plan

go test ./... — 88 packages pass
go test -race . — clean
golangci-lint run — 0 issues
All 38+ example configs load/validate/build

🤖 Generated with Claude Code

…rom module package (#84) Phase 4 engine decomposition progress: - Move Trigger interface to interfaces/ package (module.Trigger is now a type alias) - Add EventEmitter and MetricsRecorder interfaces in interfaces/ - Add TriggerRegistrar interface for registry abstraction - Create engine_module_bridge.go to isolate remaining module/ imports - Update engine.go fields to use interface types instead of concrete module types - Fix mock GetService to support interface assignment (matching real modular behavior) - Add 10 new tests for requires.plugins validation (15 test runs with subtests) Closes #84 (partial) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR continues the Phase 4 engine decomposition by moving key interface definitions out of module/ into interfaces/, reducing direct engine→module coupling while keeping backward compatibility via type aliases. It also introduces a small “bridge” file to concentrate remaining unavoidable module/ dependencies and adds tests around requires.plugins validation.

Changes:

Extracted Trigger, EventEmitter, and MetricsRecorder interfaces into interfaces/ (with module.Trigger kept as a type alias).
Updated engine.go to use the extracted interface types and moved remaining module-dependent helpers into engine_module_bridge.go.
Expanded engine_test.go with requires.plugins validation tests and adjusted the mock app’s GetService behavior.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
plugins/api/plugin.go	Minor constructor formatting adjustment (no functional change).
module/trigger.go	Replaced in-package `Trigger` interface with a type alias to `interfaces.Trigger`; clarified registry interface satisfaction.
module/query_handler_test.go	Removed trailing whitespace line.
module/command_handler_test.go	Removed trailing whitespace line.
interfaces/trigger.go	New canonical `Trigger` + `TriggerRegistrar` interfaces to decouple engine from module.
interfaces/events.go	New `EventEmitter` + `MetricsRecorder` interfaces for engine observability dependencies.
engine_module_bridge.go	New bridge helpers to isolate remaining `module/` imports and keep `engine.go` cleaner.
engine.go	Switched trigger/event/metrics-related fields and signatures to use `interfaces/` types; delegated plugin wiring/metrics logic to bridge helpers.
engine_test.go	Added `requires.plugins` validation tests; updated mock `GetService` reflection logic.

engine_test.go

engine_module_bridge.go

Copilot · 2026-02-23T12:27:26Z

engine_test.go

 	if out != nil {
 		// Get reflect values
 		outVal := reflect.ValueOf(out)
 		if outVal.Kind() != reflect.Pointer {
 			return fmt.Errorf("out parameter must be a pointer")
 		}

 		// Dereference the pointer
 		outVal = outVal.Elem()
 		if !outVal.CanSet() {
 			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()
+


mockApplication.GetService can panic in edge cases: if a service is registered with a nil value, reflect.ValueOf(svc) becomes invalid and svcVal.Type() will panic; and if the caller passes a nil pointer as out, outVal.Elem() will panic. Add guards for svc == nil and outVal.IsNil() (before Elem/Type) and return a clear error (or set a nil value) to keep this mock safe and closer to modular.StdApplication behavior.

engine_test.go

intel352 · 2026-02-23T14:13:19Z

@copilot open a new pull request to apply changes based on the comments in this thread

Copilot · 2026-02-23T14:13:28Z

@intel352 I've opened a new pull request, #148, to work on those changes. Once the pull request is ready, I'll request review from you.

… feedback (#148) * Initial plan * refactor: address code review feedback on engine_module_bridge and engine_test Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>

The function was closed with a mismatched `)` instead of `}` and had 2-space indented closing brace for the if block instead of tab-indented. This caused `expected statement, found ')'` errors, breaking all build/vet/test CI checks on any branch touching this file. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

interfaces/events.go

engine_module_bridge.go

admin/admin_test.go

intel352 · 2026-02-23T20:54:20Z

@copilot open a new pull request to apply changes based on the comments in this thread

and #136 (review)

Copilot · 2026-02-23T20:54:28Z

@intel352 I've opened a new pull request, #151, to work on those changes. Once the pull request is ready, I'll request review from you.

…ailure, remove duplicate admin tests (#151) * Initial plan * fix: address review comments - EventEmitter doc, registerPluginTrigger error handling, remove duplicate admin tests Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>

intel352 · 2026-02-24T00:27:29Z

@copilot Address all linter, build, and test issues regardless whether they pre-existed or not.

Copilot · 2026-02-24T00:27:36Z

@intel352 I've opened a new pull request, #153, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

…aulting Remove hardcoded default password fallback in the RDS driver. The Create method now returns an error if master_password is not provided, preventing insecure defaults from being used in production. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The flag was referenced but not declared, causing build failures. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…153) * Initial plan * fix: restore missing multiWorkflowAddr flag definition in cmd/server/main.go Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> Co-authored-by: Jonathan Langevin <codingsloth@pm.me>

Copilot AI review requested due to automatic review settings February 23, 2026 12:21

Copilot started reviewing on behalf of intel352 February 23, 2026 12:22 View session

Copilot AI reviewed Feb 23, 2026

View reviewed changes

Copilot AI mentioned this pull request Feb 23, 2026

fix: tighten engine_module_bridge and mockApplication based on review feedback #148

Merged

Copilot AI and others added 2 commits February 23, 2026 11:36

Copilot AI review requested due to automatic review settings February 23, 2026 20:45

Copilot started reviewing on behalf of intel352 February 23, 2026 20:46 View session

Copilot AI reviewed Feb 23, 2026

View reviewed changes

interfaces/events.go Outdated Show resolved Hide resolved

engine_module_bridge.go Outdated Show resolved Hide resolved

admin/admin_test.go Show resolved Hide resolved

admin/admin_test.go Show resolved Hide resolved

Copilot AI mentioned this pull request Feb 23, 2026

fix: EventEmitter doc accuracy, registerPluginTrigger deterministic failure, remove duplicate admin tests #151

Merged

Copilot AI and others added 2 commits February 23, 2026 19:18

Merge branch 'main' into feat/issue-84-engine-phase4-interfaces

cc28400

Copilot AI review requested due to automatic review settings February 24, 2026 00:24

Copilot started reviewing on behalf of intel352 February 24, 2026 00:24 View session

Copilot AI mentioned this pull request Feb 24, 2026

fix: restore missing multiWorkflowAddr flag in cmd/server/main.go #153

Merged

Copilot AI reviewed Feb 24, 2026

View reviewed changes

intel352 and others added 3 commits February 23, 2026 19:39

fix: restore missing multiWorkflowAddr flag declaration in cmd/server

00f03aa

The flag was referenced but not declared, causing build failures. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

intel352 merged commit 7d24c44 into main Feb 24, 2026
14 checks passed

intel352 deleted the feat/issue-84-engine-phase4-interfaces branch February 24, 2026 00:54

Conversation

intel352 commented Feb 23, 2026

Summary

What's done

What remains (Phase 5)

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

intel352 commented Feb 23, 2026

Uh oh!

Copilot AI commented Feb 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

intel352 commented Feb 23, 2026

Uh oh!

Copilot AI commented Feb 23, 2026

Uh oh!

intel352 commented Feb 24, 2026

Uh oh!

Copilot AI commented Feb 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants