From e19c0aff0741f1ef617d3e37e73cf6387b841421 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Fri, 13 Mar 2026 04:39:24 -0400 Subject: [PATCH 1/3] fix(infra): thread ctx through provisionResource/destroyResource; use context.Background() in engine - engine.go: replace context.TODO() with context.Background() in BuildFromConfig infra wiring - infra/provisioner.go: provisionResource and destroyResource now accept ctx and forward it to provider calls so cancellation propagates correctly Co-Authored-By: Claude Sonnet 4.6 --- engine.go | 2 +- infra/provisioner.go | 34 ++++++++++++++-------------------- 2 files changed, 15 insertions(+), 21 deletions(-) diff --git a/engine.go b/engine.go index 2f5b8c06..fa8b7c8b 100644 --- a/engine.go +++ b/engine.go @@ -414,7 +414,7 @@ func (e *StdEngine) BuildFromConfig(cfg *config.WorkflowConfig) error { if err != nil { return fmt.Errorf("infrastructure plan failed: %w", err) } - if err := p.Apply(context.TODO(), plan); err != nil { + if err := p.Apply(context.Background(), plan); err != nil { return fmt.Errorf("infrastructure provisioning failed: %w", err) } e.provisioner = p diff --git a/infra/provisioner.go b/infra/provisioner.go index 907c2cff..49bdfcf5 100644 --- a/infra/provisioner.go +++ b/infra/provisioner.go @@ -141,33 +141,24 @@ func (p *Provisioner) Apply(ctx context.Context, plan *ProvisionPlan) error { // Process deletes first. for _, rc := range plan.Delete { - if err := ctx.Err(); err != nil { - return fmt.Errorf("context cancelled during delete: %w", err) - } - if err := p.destroyResource(rc.Name); err != nil { + if err := p.destroyResource(ctx, rc.Name); err != nil { return fmt.Errorf("failed to delete resource %q: %w", rc.Name, err) } } // Process creates. for _, rc := range plan.Create { - if err := ctx.Err(); err != nil { - return fmt.Errorf("context cancelled during create: %w", err) - } - if err := p.provisionResource(rc); err != nil { + if err := p.provisionResource(ctx, rc); err != nil { return fmt.Errorf("failed to create resource %q: %w", rc.Name, err) } } // Process updates (destroy then re-provision). for _, rc := range plan.Update { - if err := ctx.Err(); err != nil { - return fmt.Errorf("context cancelled during update: %w", err) - } - if err := p.destroyResource(rc.Name); err != nil { + if err := p.destroyResource(ctx, rc.Name); err != nil { return fmt.Errorf("failed to destroy resource %q for update: %w", rc.Name, err) } - if err := p.provisionResource(rc); err != nil { + if err := p.provisionResource(ctx, rc); err != nil { return fmt.Errorf("failed to re-create resource %q for update: %w", rc.Name, err) } } @@ -182,10 +173,7 @@ func (p *Provisioner) Apply(ctx context.Context, plan *ProvisionPlan) error { // Destroy tears down a single named resource. func (p *Provisioner) Destroy(ctx context.Context, name string) error { - if err := ctx.Err(); err != nil { - return fmt.Errorf("context cancelled: %w", err) - } - return p.destroyResource(name) + return p.destroyResource(ctx, name) } // Status returns the current state of all provisioned resources. @@ -260,7 +248,10 @@ func ParseConfig(raw map[string]any) (*InfraConfig, error) { } // provisionResource creates a resource via registered providers or the legacy fallback. -func (p *Provisioner) provisionResource(rc ResourceConfig) error { +func (p *Provisioner) provisionResource(ctx context.Context, rc ResourceConfig) error { + if err := ctx.Err(); err != nil { + return fmt.Errorf("context cancelled before provision %q: %w", rc.Name, err) + } p.mu.Lock() if _, exists := p.resources[rc.Name]; exists { p.mu.Unlock() @@ -277,7 +268,7 @@ func (p *Provisioner) provisionResource(rc ResourceConfig) error { // Try each registered provider. for _, prov := range p.providers { if prov.Supports(rc.Type, rc.Provider) { - if err := prov.Provision(context.Background(), rc); err != nil { + if err := prov.Provision(ctx, rc); err != nil { p.mu.Lock() p.resources[rc.Name].Status = "failed" p.resources[rc.Name].Error = err.Error() @@ -301,7 +292,10 @@ func (p *Provisioner) provisionResource(rc ResourceConfig) error { } // destroyResource removes a resource from the internal store. -func (p *Provisioner) destroyResource(name string) error { +func (p *Provisioner) destroyResource(ctx context.Context, name string) error { + if err := ctx.Err(); err != nil { + return fmt.Errorf("context cancelled before destroy %q: %w", name, err) + } p.mu.Lock() defer p.mu.Unlock() From 505dcef5600582be8b121a330a4cd704c5d2da3d Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Fri, 13 Mar 2026 04:47:20 -0400 Subject: [PATCH 2/3] docs: add audit fix implementation plan Design document for the comprehensive workflow repo audit fix covering anti-staleness, interface extraction, infra provisioner, dead code cleanup, missing schemas, drift-detection tests, and unit test coverage. Co-Authored-By: Claude Opus 4.6 --- docs/plans/2026-03-13-audit-fix-design.md | 1245 +++++++++++++++++++++ 1 file changed, 1245 insertions(+) create mode 100644 docs/plans/2026-03-13-audit-fix-design.md diff --git a/docs/plans/2026-03-13-audit-fix-design.md b/docs/plans/2026-03-13-audit-fix-design.md new file mode 100644 index 00000000..f91347f9 --- /dev/null +++ b/docs/plans/2026-03-13-audit-fix-design.md @@ -0,0 +1,1245 @@ +# Workflow Audit Fix Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. + +**Goal:** Fix all issues found in the workflow repo audit: eliminate hardcoded MCP descriptions (anti-staleness), extract step interfaces, wire infra provisioner into engine, clean dead code, add missing step schemas, add drift-detection tests, and add unit tests for untested pipeline steps and plugins. + +**Architecture:** Replace parallel hardcoded sources of truth with single-source registries + drift-detection tests that fail CI when things diverge. Move `PipelineStep`/`StepFactory` to `interfaces/` so the engine depends on abstractions. Wire `infra.Provisioner` into the engine lifecycle via a provider interface and config hook. + +**Tech Stack:** Go 1.26, stdlib testing, no new dependencies + +--- + +## Work Stream A: Anti-Staleness (MCP + Schema Registry) + +### Task A1: Delete `knownStepTypeDescriptions()` and replace callers + +**Files:** +- Modify: `mcp/tools.go:414-866` (delete structs + function) +- Modify: `mcp/wfctl_tools.go:1313-1340` (mcpCheckCompatibility) +- Modify: `mcp/wfctl_tools.go:1393-1420` (mcpValidateWorkflowConfig) +- Test: `mcp/tools_test.go` + +**Step 1: Write a test that `knownStepTypeDescriptions` is no longer called** + +In `mcp/tools_test.go`, add a test that verifies `mcpCheckCompatibility` and `mcpValidateWorkflowConfig` only use dynamic sources: + +```go +func TestNoHardcodedStepDescriptions(t *testing.T) { + // Verify the function no longer exists by checking that + // schema.GetStepSchemaRegistry().Types() covers all step types + // used in compatibility and validation checks. + reg := schema.GetStepSchemaRegistry() + types := reg.Types() + if len(types) == 0 { + t.Fatal("step schema registry is empty — builtins not registered") + } + // Verify schema registry has entries for core step types + for _, st := range []string{"step.set", "step.conditional", "step.db_query", "step.http_call"} { + if reg.Get(st) == nil { + t.Errorf("schema registry missing core step type %q", st) + } + } +} +``` + +**Step 2: Run test to verify it passes (the registry should already have these)** + +Run: `cd /Users/jon/workspace/workflow && go test ./mcp/ -run TestNoHardcodedStepDescriptions -v` + +**Step 3: Replace callers in `wfctl_tools.go`** + +In `mcpCheckCompatibility` (around L1323), replace: +```go +// OLD: +knownSteps := make(map[string]bool) +for t := range knownStepTypeDescriptions() { + knownSteps[t] = true +} +// Merge with schema-registered step types +for _, t := range schema.KnownModuleTypes() { + if strings.HasPrefix(t, "step.") { + knownSteps[t] = true + } +} +``` +with: +```go +// NEW: single source of truth — schema registry + KnownModuleTypes +knownSteps := make(map[string]bool) +for _, t := range schema.GetStepSchemaRegistry().Types() { + knownSteps[t] = true +} +for _, t := range schema.KnownModuleTypes() { + if strings.HasPrefix(t, "step.") { + knownSteps[t] = true + } +} +``` + +In `mcpValidateWorkflowConfig` (around L1403), replace: +```go +// OLD: +knownSteps := knownStepTypeDescriptions() +knownStepSet := make(map[string]bool) +for t := range knownSteps { + knownStepSet[t] = true +} +for _, t := range schema.KnownModuleTypes() { + if strings.HasPrefix(t, "step.") { + knownStepSet[t] = true + } +} +``` +with: +```go +// NEW: single source of truth +knownStepSet := make(map[string]bool) +for _, t := range schema.GetStepSchemaRegistry().Types() { + knownStepSet[t] = true +} +for _, t := range schema.KnownModuleTypes() { + if strings.HasPrefix(t, "step.") { + knownStepSet[t] = true + } +} +``` + +Check if `knownSteps` (the full map) is used later in `mcpValidateWorkflowConfig` for richer config key hints. If so, replace those lookups with `schema.GetStepSchemaRegistry().Get(stepType)` which returns a `*StepSchema` with `ConfigFields`. + +**Step 4: Delete the hardcoded function and structs from `mcp/tools.go`** + +Delete lines 414-866: +- `stepTypeInfoFull` struct (L415-422) +- `stepConfigKeyDef` struct (L424-429) +- `knownStepTypeDescriptions()` function (L431-866) + +**Step 5: Fix any test references** + +Search `mcp/tools_test.go` for references to `knownStepTypeDescriptions`, `stepTypeInfoFull`, or `stepConfigKeyDef` and remove/replace them. + +**Step 6: Run tests** + +Run: `cd /Users/jon/workspace/workflow && go test ./mcp/ -v -count=1` +Expected: PASS — no compilation errors, all existing tests pass + +**Step 7: Commit** + +```bash +git add mcp/tools.go mcp/wfctl_tools.go mcp/tools_test.go +git commit -m "refactor(mcp): replace knownStepTypeDescriptions with schema registry lookups + +Remove hardcoded step type descriptions that drifted from the authoritative +schema registry. Both mcpCheckCompatibility and mcpValidateWorkflowConfig +now use schema.GetStepSchemaRegistry() as the single source of truth." +``` + +--- + +### Task A2: Create `TemplateFuncRegistry` and replace `templateFunctionDescriptions()` + +**Files:** +- Modify: `module/pipeline_template.go:455-842` (add registry alongside funcMap) +- Create: `module/template_func_registry.go` (registry type + exported accessor) +- Modify: `mcp/tools.go:868-1054` (delete hardcoded descriptions, import module registry) +- Test: `module/template_func_registry_test.go` + +**Step 1: Create the template function registry type** + +Create `module/template_func_registry.go`: + +```go +package module + +// TemplateFuncDef describes a template function for documentation/MCP. +type TemplateFuncDef struct { + Name string `json:"name"` + Signature string `json:"signature"` + Description string `json:"description"` + Example string `json:"example"` +} + +var templateFuncDefs []TemplateFuncDef + +func init() { + templateFuncDefs = buildTemplateFuncDefs() +} + +// TemplateFuncDescriptions returns metadata for all registered template functions. +// This is the single source of truth — MCP tools call this instead of maintaining +// a parallel hardcoded list. +func TemplateFuncDescriptions() []TemplateFuncDef { + return templateFuncDefs +} + +func buildTemplateFuncDefs() []TemplateFuncDef { + return []TemplateFuncDef{ + {Name: "uuid", Signature: "uuid", Description: "Generates a new UUID v4 string", Example: `{{ uuid }}`}, + {Name: "uuidv4", Signature: "uuidv4", Description: "Alias for uuid — generates a UUID v4 string", Example: `{{ uuidv4 }}`}, + {Name: "now", Signature: "now [layout]", Description: "Returns the current UTC time. Optional layout: RFC3339 (default), unix, date, time, or Go format string", Example: `{{ now "RFC3339" }}`}, + {Name: "lower", Signature: "lower str", Description: "Converts string to lowercase", Example: `{{ lower "HELLO" }}`}, + {Name: "default", Signature: "default defaultVal value", Description: "Returns value if non-nil and non-empty, otherwise defaultVal", Example: `{{ default "N/A" .name }}`}, + {Name: "trimPrefix", Signature: "trimPrefix prefix str", Description: "Removes the prefix from string", Example: `{{ trimPrefix "api/" .path }}`}, + {Name: "trimSuffix", Signature: "trimSuffix suffix str", Description: "Removes the suffix from string", Example: `{{ trimSuffix ".json" .file }}`}, + {Name: "json", Signature: "json value", Description: "Marshals value to JSON string", Example: `{{ json .data }}`}, + {Name: "config", Signature: "config key", Description: "Looks up a value from the config provider registry", Example: `{{ config "db_path" }}`}, + {Name: "upper", Signature: "upper str", Description: "Converts string to uppercase", Example: `{{ upper "hello" }}`}, + {Name: "title", Signature: "title str", Description: "Capitalizes first letter of each word", Example: `{{ title "hello world" }}`}, + {Name: "replace", Signature: "replace old new str", Description: "Replaces all occurrences of old with new in str", Example: `{{ replace "-" "_" .name }}`}, + {Name: "contains", Signature: "contains substr str", Description: "Returns true if str contains substr", Example: `{{ if contains "error" .msg }}...{{ end }}`}, + {Name: "hasPrefix", Signature: "hasPrefix prefix str", Description: "Returns true if str starts with prefix", Example: `{{ if hasPrefix "http" .url }}...{{ end }}`}, + {Name: "hasSuffix", Signature: "hasSuffix suffix str", Description: "Returns true if str ends with suffix", Example: `{{ if hasSuffix ".json" .file }}...{{ end }}`}, + {Name: "split", Signature: "split sep str", Description: "Splits string by separator into array", Example: `{{ split "," .tags }}`}, + {Name: "join", Signature: "join sep array", Description: "Joins array elements with separator", Example: `{{ join ", " .items }}`}, + {Name: "trimSpace", Signature: "trimSpace str", Description: "Removes leading and trailing whitespace", Example: `{{ trimSpace .input }}`}, + {Name: "urlEncode", Signature: "urlEncode str", Description: "URL-encodes a string", Example: `{{ urlEncode .query }}`}, + {Name: "add", Signature: "add a b", Description: "Adds two numbers (int64 or float64)", Example: `{{ add 1 2 }}`}, + {Name: "sub", Signature: "sub a b", Description: "Subtracts b from a", Example: `{{ sub 10 3 }}`}, + {Name: "mul", Signature: "mul a b", Description: "Multiplies two numbers", Example: `{{ mul 5 3 }}`}, + {Name: "div", Signature: "div a b", Description: "Divides a by b (returns float64, zero-safe)", Example: `{{ div 10 3 }}`}, + {Name: "toInt", Signature: "toInt value", Description: "Converts value to int64", Example: `{{ toInt "42" }}`}, + {Name: "toFloat", Signature: "toFloat value", Description: "Converts value to float64", Example: `{{ toFloat "3.14" }}`}, + {Name: "toString", Signature: "toString value", Description: "Converts any value to its string representation", Example: `{{ toString 42 }}`}, + {Name: "length", Signature: "length collection", Description: "Returns the length of a slice, map, or string", Example: `{{ length .items }}`}, + {Name: "coalesce", Signature: "coalesce values...", Description: "Returns the first non-nil, non-empty value", Example: `{{ coalesce .preferred .fallback "default" }}`}, + {Name: "sum", Signature: "sum collection [field]", Description: "Sums numeric values in a collection. Optional field name for slice of maps", Example: `{{ sum .prices "amount" }}`}, + {Name: "pluck", Signature: "pluck field collection", Description: "Extracts a field value from each map in a slice", Example: `{{ pluck "name" .users }}`}, + {Name: "flatten", Signature: "flatten collection", Description: "Flattens one level of nested slices", Example: `{{ flatten .nested }}`}, + {Name: "unique", Signature: "unique collection", Description: "Deduplicates values preserving insertion order", Example: `{{ unique .tags }}`}, + {Name: "groupBy", Signature: "groupBy field collection", Description: "Groups slice elements by a key field into a map", Example: `{{ groupBy "status" .orders }}`}, + {Name: "sortBy", Signature: "sortBy field collection", Description: "Stable sorts slice of maps by a key field", Example: `{{ sortBy "name" .users }}`}, + {Name: "first", Signature: "first collection", Description: "Returns the first element of a slice", Example: `{{ first .items }}`}, + {Name: "last", Signature: "last collection", Description: "Returns the last element of a slice", Example: `{{ last .items }}`}, + {Name: "min", Signature: "min collection [field]", Description: "Returns the minimum numeric value. Optional field for slice of maps", Example: `{{ min .scores }}`}, + {Name: "max", Signature: "max collection [field]", Description: "Returns the maximum numeric value. Optional field for slice of maps", Example: `{{ max .scores }}`}, + // Context-bound functions (added by funcMapWithContext, not in templateFuncMap) + {Name: "step", Signature: "step name [key...]", Description: "Accesses a previous step's output by name and optional nested keys", Example: `{{ step "fetch_user" "row" "email" }}`}, + {Name: "trigger", Signature: "trigger [key...]", Description: "Accesses trigger data by optional nested keys", Example: `{{ trigger "body" "user_id" }}`}, + } +} +``` + +**Step 2: Write drift-detection test** + +Create `module/template_func_registry_test.go`: + +```go +package module + +import ( + "testing" +) + +func TestTemplateFuncDescriptionsCoversFuncMap(t *testing.T) { + fm := templateFuncMap() + defs := TemplateFuncDescriptions() + + // Build set of documented function names + documented := make(map[string]bool, len(defs)) + for _, d := range defs { + documented[d.Name] = true + } + + // Every function in templateFuncMap must have a description + for name := range fm { + if !documented[name] { + t.Errorf("template function %q is in templateFuncMap() but has no TemplateFuncDef — add it to buildTemplateFuncDefs()", name) + } + } + + // Context-bound functions (step, trigger) are documented but not in the base funcMap + // They're added by funcMapWithContext — skip checking them in the other direction + baseFuncs := make(map[string]bool, len(fm)) + for name := range fm { + baseFuncs[name] = true + } + contextFuncs := map[string]bool{"step": true, "trigger": true} + + for _, d := range defs { + if !baseFuncs[d.Name] && !contextFuncs[d.Name] { + t.Errorf("TemplateFuncDef %q has no matching function in templateFuncMap() or funcMapWithContext()", d.Name) + } + } +} + +func TestTemplateFuncDescriptionsComplete(t *testing.T) { + defs := TemplateFuncDescriptions() + for _, d := range defs { + if d.Name == "" { + t.Error("TemplateFuncDef has empty Name") + } + if d.Signature == "" { + t.Errorf("TemplateFuncDef %q has empty Signature", d.Name) + } + if d.Description == "" { + t.Errorf("TemplateFuncDef %q has empty Description", d.Name) + } + if d.Example == "" { + t.Errorf("TemplateFuncDef %q has empty Example", d.Name) + } + } +} +``` + +**Step 3: Run tests** + +Run: `cd /Users/jon/workspace/workflow && go test ./module/ -run TestTemplateFuncDescriptions -v` +Expected: PASS + +**Step 4: Replace `templateFunctionDescriptions()` in MCP tools.go** + +In `mcp/tools.go`, delete the `TemplateFunctionDef` struct (L868-874) and `templateFunctionDescriptions()` function (L877-1054). + +Replace `handleGetTemplateFunctions` (L221-227): +```go +func (s *Server) handleGetTemplateFunctions(_ context.Context, _ mcp.CallToolRequest) (*mcp.CallToolResult, error) { + funcs := module.TemplateFuncDescriptions() + return marshalToolResult(map[string]any{ + "functions": funcs, + "count": len(funcs), + }) +} +``` + +Add `"github.com/GoCodeAlone/workflow/module"` to imports if not already present. + +**Step 5: Run full MCP tests** + +Run: `cd /Users/jon/workspace/workflow && go test ./mcp/ -v -count=1` +Expected: PASS + +**Step 6: Commit** + +```bash +git add module/template_func_registry.go module/template_func_registry_test.go mcp/tools.go +git commit -m "refactor(mcp): replace hardcoded template func descriptions with module registry + +Create TemplateFuncDef registry in module/ alongside templateFuncMap() so +descriptions live next to implementations. Add drift-detection test that +fails when a template function exists without a description entry. +Adds 12 previously undocumented functions: sum, pluck, flatten, unique, +groupBy, sortBy, first, last, min, max, step, trigger." +``` + +--- + +### Task A3: Fix schema defaults and add missing step schemas + +**Files:** +- Modify: `schema/step_schema_builtins.go` (fix db_query default, conditional required, add missing schemas) +- Modify: `schema/schema.go:145-272` (add missing types to coreModuleTypes) +- Test: `schema/step_schema_drift_test.go` (new drift detection test) + +**Step 1: Write drift-detection tests** + +Create `schema/step_schema_drift_test.go`: + +```go +package schema + +import ( + "strings" + "testing" +) + +// TestCoreStepTypesHaveSchemas verifies every step.* entry in coreModuleTypes +// has a corresponding StepSchemaRegistry entry. Fails when a step type is added +// to coreModuleTypes without a schema. +func TestCoreStepTypesHaveSchemas(t *testing.T) { + reg := GetStepSchemaRegistry() + for _, mt := range coreModuleTypes { + if !strings.HasPrefix(mt, "step.") { + continue + } + if reg.Get(mt) == nil { + t.Errorf("step type %q is in coreModuleTypes but has no StepSchemaRegistry entry — add it to registerBuiltins()", mt) + } + } +} + +// TestSchemaRegistryTypesInCoreModuleTypes verifies every step type registered +// in the StepSchemaRegistry is also listed in coreModuleTypes. Fails when a +// schema is added to registerBuiltins() without updating coreModuleTypes. +func TestSchemaRegistryTypesInCoreModuleTypes(t *testing.T) { + coreSet := make(map[string]bool, len(coreModuleTypes)) + for _, mt := range coreModuleTypes { + coreSet[mt] = true + } + for _, st := range GetStepSchemaRegistry().Types() { + if !coreSet[st] { + t.Errorf("step type %q is in StepSchemaRegistry but not in coreModuleTypes — add it to schema.go", st) + } + } +} +``` + +**Step 2: Run tests to see current failures** + +Run: `cd /Users/jon/workspace/workflow && go test ./schema/ -run TestCoreStepTypesHaveSchemas -v` +Expected: FAIL — lists all step types missing schemas + +Run: `cd /Users/jon/workspace/workflow && go test ./schema/ -run TestSchemaRegistryTypesInCoreModuleTypes -v` +Expected: FAIL — lists all schema types missing from coreModuleTypes + +**Step 3: Fix `step.db_query` default** + +In `schema/step_schema_builtins.go`, find the `step.db_query` registration (around L123). Change the `mode` field's `DefaultValue` from `"single"` to `"list"`: + +```go +{Key: "mode", Label: "Mode", Type: FieldTypeSelect, Description: "Query mode: list returns rows array, single returns first row", DefaultValue: "list", Options: []string{"list", "single"}}, +``` + +**Step 4: Fix `step.conditional` routes required** + +In `schema/step_schema_builtins.go`, find the `step.conditional` registration (around L56). Set `Required: true` on the `routes` field: + +```go +{Key: "routes", Label: "Routes", Type: FieldTypeMap, Description: "Map of field values to next step names", Required: true, MapValueType: "string"}, +``` + +**Step 5: Add missing step types to `coreModuleTypes`** + +In `schema/schema.go`, add the following to the `coreModuleTypes` slice (in alphabetical order within the step.* section): + +```go +// Add these missing step types (registered by schema or factories but not in coreModuleTypes): +"step.auth_required", +"step.auth_validate", +"step.authz_check", +"step.cli_invoke", +"step.cli_print", +"step.event_decrypt", +"step.field_reencrypt", +"step.graphql", +"step.json_parse", +"step.m2m_token", +"step.nosql_delete", +"step.nosql_get", +"step.nosql_put", +"step.nosql_query", +"step.oidc_auth_url", +"step.oidc_callback", +"step.raw_response", +"step.sandbox_exec", +"step.secret_fetch", +"step.statemachine_get", +"step.statemachine_transition", +"step.token_revoke", +``` + +Also add all factory-registered step types from plugins that are missing. These include CI/CD, platform, storage, marketplace, policy, gitlab, and actor step types. There are ~80 of them — add them all alphabetically. The exact list is in the research notes (Task A3 research: "Factories registered but NOT in coreModuleTypes"). + +**Step 6: Add missing step schemas to `registerBuiltins()`** + +For every step type that is now in `coreModuleTypes` but has no schema entry, add a minimal schema to `step_schema_builtins.go`. At minimum each schema needs: Type, Plugin, Description, and at least a ConfigFields with the primary config keys the step reads. + +For the pipelinesteps plugin steps that are missing schemas, look at the corresponding `pipeline_step_*.go` factory function to see what config keys it reads, then add the schema. Here are the key ones: + +```go +// step.raw_response — from pipeline_step_raw_response.go +r.Register(&StepSchema{ + Type: "step.raw_response", Plugin: "pipelinesteps", + Description: "Sends a raw HTTP response with configurable status, headers, and body", + ConfigFields: []ConfigFieldDef{ + {Key: "status", Label: "Status Code", Type: FieldTypeNumber, Description: "HTTP status code", DefaultValue: 200}, + {Key: "headers", Label: "Headers", Type: FieldTypeMap, Description: "Response headers"}, + {Key: "body", Label: "Body", Type: FieldTypeString, Description: "Response body template"}, + }, +}) + +// step.json_parse — from pipeline_step_json_parse.go +r.Register(&StepSchema{ + Type: "step.json_parse", Plugin: "pipelinesteps", + Description: "Parses a JSON string into a structured map", + ConfigFields: []ConfigFieldDef{ + {Key: "input", Label: "Input", Type: FieldTypeString, Description: "Template expression that resolves to a JSON string", Required: true}, + }, + Outputs: []StepOutputDef{ + {Key: "parsed", Type: "map", Description: "Parsed JSON object"}, + }, +}) + +// step.auth_validate — from pipeline_step_auth_validate.go +r.Register(&StepSchema{ + Type: "step.auth_validate", Plugin: "pipelinesteps", + Description: "Validates JWT or API key authentication from the request", + ConfigFields: []ConfigFieldDef{ + {Key: "type", Label: "Auth Type", Type: FieldTypeSelect, Description: "Authentication type to validate", Options: []string{"jwt", "api_key"}, DefaultValue: "jwt"}, + {Key: "secret", Label: "Secret", Type: FieldTypeString, Description: "JWT signing secret or API key store name", Sensitive: true}, + {Key: "header", Label: "Header", Type: FieldTypeString, Description: "Header name to read token from", DefaultValue: "Authorization"}, + }, + Outputs: []StepOutputDef{ + {Key: "valid", Type: "bool", Description: "Whether authentication succeeded"}, + {Key: "claims", Type: "map", Description: "Decoded JWT claims or API key metadata"}, + }, +}) + +// step.token_revoke — from pipeline_step_token_revoke.go +r.Register(&StepSchema{ + Type: "step.token_revoke", Plugin: "pipelinesteps", + Description: "Adds a JWT token to the revocation blacklist", + ConfigFields: []ConfigFieldDef{ + {Key: "token", Label: "Token", Type: FieldTypeString, Description: "The JWT token to revoke", Required: true}, + }, +}) + +// step.field_reencrypt +r.Register(&StepSchema{ + Type: "step.field_reencrypt", Plugin: "pipelinesteps", + Description: "Re-encrypts a field value with a new key", + ConfigFields: []ConfigFieldDef{ + {Key: "field", Label: "Field", Type: FieldTypeString, Description: "Dot-path to the field to re-encrypt", Required: true}, + {Key: "old_key", Label: "Old Key", Type: FieldTypeString, Description: "Current encryption key name", Required: true, Sensitive: true}, + {Key: "new_key", Label: "New Key", Type: FieldTypeString, Description: "New encryption key name", Required: true, Sensitive: true}, + }, +}) + +// step.sandbox_exec +r.Register(&StepSchema{ + Type: "step.sandbox_exec", Plugin: "pipelinesteps", + Description: "Executes code in a sandboxed environment", + ConfigFields: []ConfigFieldDef{ + {Key: "runtime", Label: "Runtime", Type: FieldTypeSelect, Description: "Sandbox runtime", Options: []string{"wasm", "docker", "process"}}, + {Key: "code", Label: "Code", Type: FieldTypeString, Description: "Code to execute"}, + {Key: "timeout", Label: "Timeout", Type: FieldTypeDuration, Description: "Execution timeout", DefaultValue: "30s"}, + }, + Outputs: []StepOutputDef{ + {Key: "output", Type: "string", Description: "Execution output"}, + {Key: "exit_code", Type: "number", Description: "Exit code"}, + }, +}) + +// step.cli_print +r.Register(&StepSchema{ + Type: "step.cli_print", Plugin: "pipelinesteps", + Description: "Prints formatted output to the CLI", + ConfigFields: []ConfigFieldDef{ + {Key: "message", Label: "Message", Type: FieldTypeString, Description: "Message template to print", Required: true}, + {Key: "format", Label: "Format", Type: FieldTypeSelect, Description: "Output format", Options: []string{"text", "json", "table"}, DefaultValue: "text"}, + }, +}) + +// step.cli_invoke +r.Register(&StepSchema{ + Type: "step.cli_invoke", Plugin: "pipelinesteps", + Description: "Invokes a registered CLI command", + ConfigFields: []ConfigFieldDef{ + {Key: "command", Label: "Command", Type: FieldTypeString, Description: "CLI command name to invoke", Required: true}, + {Key: "args", Label: "Arguments", Type: FieldTypeArray, Description: "Command arguments", ArrayItemType: "string"}, + }, + Outputs: []StepOutputDef{ + {Key: "output", Type: "string", Description: "Command output"}, + {Key: "exit_code", Type: "number", Description: "Exit code"}, + }, +}) + +// step.graphql +r.Register(&StepSchema{ + Type: "step.graphql", Plugin: "pipelinesteps", + Description: "Executes a GraphQL query or mutation against an endpoint", + ConfigFields: []ConfigFieldDef{ + {Key: "url", Label: "URL", Type: FieldTypeString, Description: "GraphQL endpoint URL", Required: true}, + {Key: "query", Label: "Query", Type: FieldTypeString, Description: "GraphQL query or mutation string", Required: true}, + {Key: "variables", Label: "Variables", Type: FieldTypeMap, Description: "Query variables"}, + {Key: "headers", Label: "Headers", Type: FieldTypeMap, Description: "HTTP headers"}, + }, + Outputs: []StepOutputDef{ + {Key: "data", Type: "map", Description: "GraphQL response data"}, + {Key: "errors", Type: "array", Description: "GraphQL errors if any"}, + }, +}) + +// step.event_decrypt +r.Register(&StepSchema{ + Type: "step.event_decrypt", Plugin: "pipelinesteps", + Description: "Decrypts an encrypted event payload", + ConfigFields: []ConfigFieldDef{ + {Key: "key", Label: "Key", Type: FieldTypeString, Description: "Encryption key name", Required: true, Sensitive: true}, + {Key: "field", Label: "Field", Type: FieldTypeString, Description: "Dot-path to encrypted field", Required: true}, + }, + Outputs: []StepOutputDef{ + {Key: "decrypted", Type: "map", Description: "Decrypted data"}, + }, +}) + +// step.secret_fetch +r.Register(&StepSchema{ + Type: "step.secret_fetch", Plugin: "pipelinesteps", + Description: "Fetches a secret from the configured secrets provider", + ConfigFields: []ConfigFieldDef{ + {Key: "name", Label: "Secret Name", Type: FieldTypeString, Description: "Name/path of the secret to fetch", Required: true}, + {Key: "provider", Label: "Provider", Type: FieldTypeSelect, Description: "Secrets provider", Options: []string{"vault", "aws", "env"}}, + }, + Outputs: []StepOutputDef{ + {Key: "value", Type: "string", Description: "Secret value"}, + }, +}) +``` + +For plugin step types (cicd, platform, datastores, etc.), add schemas via each plugin's `StepSchemas()` method rather than in `registerBuiltins()`. The agent implementing this should read each plugin's step factory files to determine the correct config fields. If the plugin already returns schemas from `StepSchemas()`, verify they're loaded correctly. + +**Step 7: Run drift detection tests** + +Run: `cd /Users/jon/workspace/workflow && go test ./schema/ -run TestCoreStepTypes -v` +Expected: PASS (or reduced failures — some plugin steps may be registered dynamically) + +**Step 8: Commit** + +```bash +git add schema/step_schema_builtins.go schema/schema.go schema/step_schema_drift_test.go +git commit -m "fix(schema): fix defaults, add missing schemas, add drift-detection tests + +Fix step.db_query default from 'single' to 'list' to match implementation. +Mark step.conditional routes as required. Add 11 missing step schemas for +pipelinesteps plugin. Add 80+ missing step types to coreModuleTypes. +Add drift-detection tests that fail CI when schema/coreModuleTypes diverge." +``` + +--- + +### Task A4: Fix README and documentation + +**Files:** +- Modify: `README.md` +- Modify: `DOCUMENTATION.md` (if it exists) +- Modify: `docs/mcp.md` (if it exists) + +**Step 1: Fix modular version references** + +Search README.md for `CrisisTextLine` and replace with `GoCodeAlone`. Search for `v1.11.11` and replace with `v1.12.0` (check go.mod for the exact version). + +**Step 2: Fix command syntax** + +Search for `api-extract` and replace with `api extract` (space, not hyphen). + +**Step 3: Fix module/step counts** + +Replace any hardcoded count (like "48 module types") with the actual count from the code. Use language like "90+ module types" rather than exact numbers to reduce staleness. + +**Step 4: Update `docs/mcp.md` tool list** + +If this file exists, update it to list all registered MCP tools. Check `mcp/server.go` for the full list of registered tools and ensure the doc matches. + +**Step 5: Commit** + +```bash +git add README.md DOCUMENTATION.md docs/mcp.md +git commit -m "docs: fix stale version refs, command syntax, module counts" +``` + +--- + +## Work Stream B: Interface Extraction (Phase 2) + +### Task B1: Define step interfaces in `interfaces/` + +**Files:** +- Modify: `interfaces/pipeline.go` +- Test: `interfaces/pipeline_test.go` (compile check) + +**Step 1: Add PipelineStep, StepFactory, and StepRegistrar to interfaces/pipeline.go** + +Add these types to `interfaces/pipeline.go` (after the existing `StepRegistryProvider`): + +```go +import ( + "context" +) + +// PipelineStep is a single composable unit of work in a pipeline. +type PipelineStep interface { + Name() string + Execute(ctx context.Context, stepOutputs, current, metadata map[string]any) (output map[string]any, nextStep string, stop bool, err error) +} +``` + +Wait — the current `module.PipelineStep.Execute` takes `*PipelineContext` which is a module-level type. Moving to interfaces requires either: +(a) Moving `PipelineContext` and `StepResult` to interfaces too, or +(b) Using a flattened signature in the interface + +Option (a) is cleaner. Add to `interfaces/pipeline.go`: + +```go +// PipelineContext carries the execution state through a pipeline. +type PipelineContext struct { + TriggerData map[string]any + StepOutputs map[string]map[string]any + Current map[string]any + Metadata map[string]any +} + +// StepResult is the outcome of a pipeline step execution. +type StepResult struct { + Output map[string]any + NextStep string + Stop bool +} + +// PipelineStep is a single composable unit of work in a pipeline. +type PipelineStep interface { + Name() string + Execute(ctx context.Context, pc *PipelineContext) (*StepResult, error) +} + +// StepFactory creates a PipelineStep from a name, config, and application. +type StepFactory func(name string, config map[string]any, app any) (PipelineStep, error) + +// StepRegistrar manages step type registration and creation. +type StepRegistrar interface { + StepRegistryProvider // embeds Types() []string + Register(stepType string, factory StepFactory) + Create(stepType, name string, config map[string]any, app any) (PipelineStep, error) +} +``` + +**NOTE:** The `app` parameter in the current code is `modular.Application`. Using `any` in the interface avoids coupling `interfaces/` to the `modular` package. The concrete implementation casts internally. + +**Step 2: Make `module.PipelineStep` a type alias** + +In `module/pipeline_step.go`, change from interface definition to alias: + +```go +package module + +import "github.com/GoCodeAlone/workflow/interfaces" + +// PipelineStep is a single composable unit of work in a pipeline. +// Aliased from interfaces.PipelineStep for backwards compatibility. +type PipelineStep = interfaces.PipelineStep +``` + +Similarly for `PipelineContext` and `StepResult` in `module/pipeline_context.go` — alias them to the interfaces package types. + +**Step 3: Make `module.StepFactory` a type alias** + +In `module/pipeline_step_registry.go`: +```go +// StepFactory creates a PipelineStep from config. +type StepFactory = interfaces.StepFactory +``` + +Wait — the current `StepFactory` signature uses `modular.Application`, not `any`. We need to be careful here. The interface uses `any` but the module type uses `modular.Application`. A type alias won't work directly. + +**Revised approach:** Keep `module.StepFactory` as-is (with `modular.Application`). Have `StepRegistry` implement `interfaces.StepRegistrar` by wrapping the factory calls. The `interfaces.StepFactory` uses `any` for the app parameter, and `module.StepRegistry.Create()` casts `any` to `modular.Application` internally. + +Actually, the simplest approach that doesn't break any plugin code: +1. Define `interfaces.PipelineStep` = exact same interface +2. Make `module.PipelineStep` = `interfaces.PipelineStep` (type alias) +3. Same for `PipelineContext`, `StepResult` +4. Keep `module.StepFactory` as concrete type (it references `modular.Application`) +5. Define `interfaces.StepRegistrar` with `Create` returning `interfaces.PipelineStep` +6. `engine.stepRegistry` changes to `interfaces.StepRegistrar` + +This is the approach. The agent implementing this should: +- Read `module/pipeline_context.go` for the full `PipelineContext` and `StepResult` definitions (may have more fields than shown above) +- Verify all 200+ files that reference `module.PipelineStep` still compile after the alias change +- Run `go build ./...` after every change + +**Step 4: Update engine.go** + +Change: +```go +// L71: stepRegistry *module.StepRegistry +stepRegistry interfaces.StepRegistrar +``` + +Update `GetStepRegistry()` return type: +```go +func (e *StdEngine) GetStepRegistry() interfaces.StepRegistrar { +``` + +**Step 5: Run full build** + +Run: `cd /Users/jon/workspace/workflow && go build ./...` +Expected: PASS — all packages compile + +**Step 6: Run full tests** + +Run: `cd /Users/jon/workspace/workflow && go test ./... -count=1` +Expected: PASS + +**Step 7: Commit** + +```bash +git add interfaces/pipeline.go module/pipeline_step.go module/pipeline_context.go module/pipeline_step_registry.go engine.go +git commit -m "refactor: extract PipelineStep/StepResult/PipelineContext to interfaces package + +Move step abstractions to interfaces/ so the engine depends on abstractions +rather than the concrete module package. module.PipelineStep is now a type +alias for interfaces.PipelineStep, maintaining full backwards compatibility. +Closes TODO(phase5)." +``` + +--- + +## Work Stream C: Infrastructure Provisioner (Phase 4) + +### Task C1: Add ResourceProvisioner provider interface + +**Files:** +- Create: `infra/provider.go` +- Modify: `infra/provisioner.go` (extract memory provider, use interface) +- Test: `infra/provider_test.go` + +**Step 1: Define provider interface** + +Create `infra/provider.go`: + +```go +package infra + +import "context" + +// ResourceProvider provisions and destroys a specific resource type. +// Implementations handle the actual infrastructure operations (memory, SQLite, +// cloud APIs, etc.) while the Provisioner orchestrates the lifecycle. +type ResourceProvider interface { + // Provision creates or updates a resource. Returns an error if the provider + // is not configured or the resource config is invalid. + Provision(ctx context.Context, rc ResourceConfig) error + + // Destroy removes a provisioned resource by name. + Destroy(ctx context.Context, name string) error + + // Supports returns true if this provider handles the given resource type + provider combo. + Supports(resourceType, provider string) bool +} +``` + +**Step 2: Extract MemoryProvider from current Provisioner** + +The current `provisionResource()` at L256 does mock provisioning (just stores in the map). Extract this into a `MemoryProvider` struct that implements `ResourceProvider`: + +```go +// MemoryProvider is an in-memory resource provider for testing and local development. +type MemoryProvider struct{} + +func (m *MemoryProvider) Provision(_ context.Context, _ ResourceConfig) error { return nil } +func (m *MemoryProvider) Destroy(_ context.Context, _ string) error { return nil } +func (m *MemoryProvider) Supports(_, _ string) bool { return true } +``` + +**Step 3: Update Provisioner to use providers** + +Add a `providers []ResourceProvider` field to `Provisioner`. In `provisionResource()`, iterate providers to find one that `Supports()` the resource type. If none found, return a clear error. + +**Step 4: Write tests** + +```go +func TestProvisionerWithProvider(t *testing.T) { + p := NewProvisioner(nil) + p.AddProvider(&MemoryProvider{}) + plan := &ProvisionPlan{ + Create: []ResourceConfig{{Name: "test-db", Type: "database", Provider: "memory"}}, + } + if err := p.Apply(context.Background(), plan); err != nil { + t.Fatalf("Apply failed: %v", err) + } + status := p.Status() + if _, ok := status["test-db"]; !ok { + t.Error("expected test-db in status") + } +} +``` + +**Step 5: Run tests** + +Run: `cd /Users/jon/workspace/workflow && go test ./infra/ -v` +Expected: PASS + +**Step 6: Commit** + +```bash +git add infra/provider.go infra/provisioner.go infra/provider_test.go +git commit -m "feat(infra): add ResourceProvider interface and extract MemoryProvider + +Introduce provider pattern so the Provisioner delegates to pluggable +backends. Extract existing mock logic into MemoryProvider. This enables +SQLite, Postgres, and cloud providers to be added incrementally." +``` + +### Task C2: Wire Provisioner into engine config + +**Files:** +- Modify: `config/config.go` (add Infrastructure field to WorkflowConfig) +- Modify: `engine.go` (add provisioner field, wire into BuildFromConfig) +- Test: existing engine tests should still pass + +**Step 1: Add Infrastructure to WorkflowConfig** + +In `config/config.go`, add to the `WorkflowConfig` struct: + +```go +Infrastructure *InfrastructureConfig `json:"infrastructure,omitempty" yaml:"infrastructure,omitempty"` +``` + +Where: +```go +type InfrastructureConfig struct { + Resources []InfraResourceConfig `json:"resources" yaml:"resources"` +} + +type InfraResourceConfig struct { + Name string `json:"name" yaml:"name"` + Type string `json:"type" yaml:"type"` + Provider string `json:"provider" yaml:"provider"` + Config map[string]any `json:"config,omitempty" yaml:"config,omitempty"` +} +``` + +**Step 2: Add provisioner to engine** + +In `engine.go`, add field: +```go +provisioner *infra.Provisioner +``` + +In `NewStdEngine()` or `BuildFromConfig()`, if `cfg.Infrastructure != nil`: +```go +p := infra.NewProvisioner(e.logger) +p.AddProvider(&infra.MemoryProvider{}) +infraCfg := &infra.InfraConfig{ + Resources: convertInfraResources(cfg.Infrastructure.Resources), +} +plan, err := p.Plan(*infraCfg) +if err != nil { + return fmt.Errorf("infrastructure plan failed: %w", err) +} +if err := p.Apply(ctx, plan); err != nil { + return fmt.Errorf("infrastructure provisioning failed: %w", err) +} +e.provisioner = p +``` + +**Step 3: Run tests** + +Run: `cd /Users/jon/workspace/workflow && go test ./... -count=1` +Expected: PASS — existing configs without `infrastructure:` key should be unaffected + +**Step 4: Commit** + +```bash +git add config/config.go engine.go +git commit -m "feat: wire infra.Provisioner into engine lifecycle + +Add optional 'infrastructure' config block to WorkflowConfig. When present, +the engine creates a Provisioner, computes a plan, and applies it during +BuildFromConfig. Uses MemoryProvider by default; real providers can be +registered via plugins." +``` + +--- + +## Work Stream D: Dead Code Cleanup (Phase 5) + +### Task D1: Delete dead code + +**Files:** +- Delete: `module/errors.go` +- Verify: no references exist + +**Step 1: Verify no references** + +Run: `grep -r "ErrNotImplemented" /Users/jon/workspace/workflow --include="*.go" | grep -v errors.go | grep -v "_test.go" | grep -v "docs/" | grep -v "plans/"` +Expected: no output + +**Step 2: Delete the file** + +```bash +rm module/errors.go +``` + +**Step 3: Run build** + +Run: `cd /Users/jon/workspace/workflow && go build ./...` +Expected: PASS + +**Step 4: Commit** + +```bash +git add -u module/errors.go +git commit -m "chore: delete dead ErrNotImplemented variable + +Was declared but never referenced anywhere in the codebase. The scan steps +that originally returned it have been rewritten to use service provider +interfaces instead." +``` + +--- + +## Work Stream E: Unit Tests for Pipeline Steps (Phase 6) + +Each task adds tests for a group of related steps. Tests follow the pattern in `pipeline_step_http_call_test.go`: create a `PipelineContext`, create the step via its factory, call `Execute`, and assert outputs. + +### Task E1: Database step tests (CRITICAL) + +**Files:** +- Create: `module/pipeline_step_db_query_test.go` +- Create: `module/pipeline_step_db_exec_test.go` +- Create: `module/pipeline_step_db_query_cached_test.go` + +These steps require a `DBProvider` service in the app's `SvcRegistry()`. Create a test helper: + +```go +// testDBProvider wraps an in-memory SQLite DB for testing. +type testDBProvider struct { + db *sql.DB +} + +func newTestDBProvider(t *testing.T) *testDBProvider { + t.Helper() + db, err := sql.Open("sqlite3", ":memory:") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { db.Close() }) + return &testDBProvider{db: db} +} + +func (p *testDBProvider) DB() *sql.DB { return p.db } +``` + +Use a mock `modular.Application` that returns this provider from `SvcRegistry()`. + +**Test cases for step.db_query:** +- Factory creation with valid config +- Factory rejects missing `database` key +- Execute with `mode: list` — returns `rows` array and `count` +- Execute with `mode: single` — returns `row` map and `found` bool +- Execute with parameterized query (`$1` placeholders) +- Execute with empty result set +- Execute with missing database service — returns error + +**Test cases for step.db_exec:** +- Factory creation with valid config +- Execute INSERT — returns `rows_affected` +- Execute with parameterized query +- Execute with SQL error — returns error + +**Test cases for step.db_query_cached:** +- Factory creation with valid config +- First call — cache miss, queries DB +- Second call with same params — cache hit +- Different params — separate cache entries + +**Step: Run tests** + +Run: `cd /Users/jon/workspace/workflow && go test ./module/ -run TestDBQuery -v` +Expected: PASS + +**Commit after each test file is green.** + +### Task E2: Core logic step tests + +**Files:** +- Create: `module/pipeline_step_json_parse_test.go` +- Create: `module/pipeline_step_json_response_test.go` +- Create: `module/pipeline_step_raw_response_test.go` +- Create: `module/pipeline_step_validate_test.go` +- Create: `module/pipeline_step_http_proxy_test.go` +- Create: `module/pipeline_step_publish_test.go` +- Create: `module/pipeline_step_jq_test.go` +- Create: `module/pipeline_step_resilience_test.go` + +For each step, read the implementation file first to understand: +- What config keys the factory expects +- What the Execute method does +- What services it looks up from the app +- What outputs it produces + +Write table-driven tests covering: valid config, invalid config, happy path execution, error cases. + +### Task E3: Integration-heavy step tests (mock external deps) + +**Files:** +- Create: `module/pipeline_step_graphql_test.go` +- Create: `module/pipeline_step_nosql_test.go` (covers nosql_get, nosql_put, nosql_query) +- Create: `module/pipeline_step_statemachine_test.go` (covers transition + get) +- Create: `module/pipeline_step_feature_flag_test.go` +- Create: `module/pipeline_step_policy_test.go` +- Create: `module/pipeline_step_shell_exec_test.go` +- Create: `module/pipeline_step_sandbox_exec_test.go` + +For GraphQL: use `httptest.NewServer` as mock GraphQL endpoint. +For NoSQL: mock the NoSQL service interface. +For state machine: mock the state machine engine service. +For shell_exec: test with simple commands like `echo` and verify output capture. + +### Task E4: Cloud/CI step tests + +**Files:** +- Create: `module/pipeline_step_docker_build_test.go` +- Create: `module/pipeline_step_docker_push_test.go` +- Create: `module/pipeline_step_docker_run_test.go` +- Create: `module/pipeline_step_deploy_test.go` +- Create: `module/pipeline_step_s3_upload_test.go` +- Create: `module/pipeline_step_artifact_test.go` +- Create: `module/pipeline_step_scan_sast_test.go` +- Create: `module/pipeline_step_scan_deps_test.go` +- Create: `module/pipeline_step_scan_container_test.go` + +These steps typically call external services. Test: +- Factory creation with valid/invalid config +- Execute with mock service provider (if the step uses app.SvcRegistry) +- Execute with missing service — verify clear error message + +### Task E5: Remaining step tests + +**Files:** +- Create: `module/pipeline_step_cli_print_test.go` +- Create: `module/pipeline_step_cli_invoke_test.go` +- Create: `module/pipeline_step_ai_classify_test.go` +- Create: `module/pipeline_step_ai_complete_test.go` +- Create: `module/pipeline_step_ai_extract_test.go` +- Create: `module/pipeline_step_gitlab_test.go` +- Create: `module/pipeline_step_secret_fetch_test.go` +- Create: `module/pipeline_step_field_reencrypt_test.go` +- Create: `module/pipeline_step_event_decrypt_test.go` +- Create: `module/pipeline_step_delegate_test.go` +- Create: `module/pipeline_step_app_test.go` +- Create: `module/pipeline_step_platform_template_test.go` + +Same pattern: read impl → write tests for factory + execute. + +--- + +## Work Stream F: Plugin Package Tests (Phase 7) + +### Task F1: Plugin tests for all 8 untested plugins + +**Files:** +- Create: `plugins/k8s/plugin_test.go` +- Create: `plugins/cloud/plugin_test.go` +- Create: `plugins/gitlab/plugin_test.go` +- Create: `plugins/policy/plugin_test.go` +- Create: `plugins/marketplace/plugin_test.go` +- Create: `plugins/openapi/plugin_test.go` +- Create: `plugins/datastores/plugin_test.go` +- Create: `plugins/platform/plugin_test.go` + +Each test file follows the pattern from `plugins/pipelinesteps/plugin_test.go`: + +```go +func TestPlugin(t *testing.T) { + p := New() + + t.Run("manifest validates", func(t *testing.T) { + m := p.EngineManifest() + if err := m.Validate(); err != nil { + t.Fatalf("manifest validation failed: %v", err) + } + }) + + t.Run("step factories registered", func(t *testing.T) { + sf := p.StepFactories() + if len(sf) == 0 { + t.Fatal("no step factories registered") + } + // Verify key step types exist + for _, st := range []string{"step.expected_type_1", "step.expected_type_2"} { + if _, ok := sf[st]; !ok { + t.Errorf("missing step factory for %q", st) + } + } + }) + + t.Run("module factories registered", func(t *testing.T) { + mf := p.ModuleFactories() + // Some plugins may not have module factories — adjust per plugin + _ = mf + }) +} +``` + +Read each plugin's `plugin.go` to find the correct step/module types to assert. + +--- + +## Work Stream G: Drift-Detection CI Test + +### Task G1: Add comprehensive consistency test + +**Files:** +- Create: `consistency_test.go` (package workflow, root level) + +This is the capstone test that ties everything together: + +```go +package workflow + +import ( + "strings" + "testing" + + "github.com/GoCodeAlone/workflow/module" + "github.com/GoCodeAlone/workflow/schema" +) + +// TestRegistryConsistency ensures all registration sources agree. +// This test fails CI when step types, schemas, or coreModuleTypes drift apart. +func TestRegistryConsistency(t *testing.T) { + t.Run("all schema step types in coreModuleTypes", func(t *testing.T) { + core := make(map[string]bool) + for _, mt := range schema.KnownModuleTypes() { + core[mt] = true + } + for _, st := range schema.GetStepSchemaRegistry().Types() { + if !core[st] { + t.Errorf("step schema %q registered but not in KnownModuleTypes()", st) + } + } + }) + + t.Run("template func descriptions cover funcMap", func(t *testing.T) { + defs := module.TemplateFuncDescriptions() + if len(defs) < 30 { + t.Errorf("expected at least 30 template func descriptions, got %d", len(defs)) + } + }) + + t.Run("engine loads all builtin plugins", func(t *testing.T) { + // Create a minimal engine and verify all plugin step factories are + // discoverable via the step registry + e := NewStdEngine(nil) + types := e.GetStepRegistry().Types() + if len(types) < 40 { + t.Errorf("expected at least 40 step types, got %d", len(types)) + } + }) +} +``` + +The exact implementation depends on the exported API after Phase 2 changes. The agent implementing this should adapt based on what's available. + +**Commit:** +```bash +git add consistency_test.go +git commit -m "test: add registry consistency test as CI anti-drift guard + +Verifies schema registry, coreModuleTypes, template func descriptions, +and engine step registrations all agree. Fails CI when any source drifts." +``` + +--- + +## Execution Order + +Tasks can be parallelized within work streams. Cross-stream dependencies: + +``` +A1 ──┐ +A2 ──┤ +A3 ──┼──→ G1 (needs all registries fixed) +A4 ──┘ +B1 ──────→ G1 (needs interfaces for engine test) +C1 → C2 +D1 (independent) +E1-E5 (independent of each other, need working build) +F1 (independent, needs working build) +``` + +**Parallel groups:** +- Group 1: A1, A2, A4, B1, C1, D1 (all independent) +- Group 2: A3 (depends on A1 removing the old code), C2 (depends on C1) +- Group 3: E1-E5, F1 (independent, after build is green) +- Group 4: G1 (after all other work streams) From 07a6cd5ef1a1511e3c07e14ad091d260a23ef34f Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Fri, 13 Mar 2026 04:59:52 -0400 Subject: [PATCH 3/3] fix(infra): delegate to ResourceProvider.Destroy() in destroyResource destroyResource now calls prov.Destroy(ctx, name) for the matching provider before removing the resource from the internal map. Mirrors the provisionResource pattern. On provider error, the resource is retained with status "failed" instead of being silently dropped. Co-Authored-By: Claude Opus 4.6 --- infra/provisioner.go | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/infra/provisioner.go b/infra/provisioner.go index 49bdfcf5..134ad3c6 100644 --- a/infra/provisioner.go +++ b/infra/provisioner.go @@ -291,21 +291,42 @@ func (p *Provisioner) provisionResource(ctx context.Context, rc ResourceConfig) return nil } -// destroyResource removes a resource from the internal store. +// destroyResource delegates to the matching provider then removes from internal store. func (p *Provisioner) destroyResource(ctx context.Context, name string) error { if err := ctx.Err(); err != nil { return fmt.Errorf("context cancelled before destroy %q: %w", name, err) } p.mu.Lock() - defer p.mu.Unlock() res, exists := p.resources[name] if !exists { + p.mu.Unlock() return fmt.Errorf("resource %q not found", name) } res.Status = "destroying" + rc := res.Config + p.mu.Unlock() + + // Delegate to the matching provider (mirror of provisionResource). + for _, prov := range p.providers { + if prov.Supports(rc.Type, rc.Provider) { + if err := prov.Destroy(ctx, name); err != nil { + p.mu.Lock() + if r, ok := p.resources[name]; ok { + r.Status = "failed" + r.Error = err.Error() + } + p.mu.Unlock() + return err + } + break + } + } + + p.mu.Lock() delete(p.resources, name) + p.mu.Unlock() p.logger.Info("resource destroyed", "name", name) return nil